On 06.09.21 17:11, Jan Beulich wrote:
> On 03.09.2021 12:08, Oleksandr Andrushchenko wrote:
>> @@ -593,6 +625,36 @@ static int init_bars(struct pci_dev *pdev)
>> }
>> REGISTER_VPCI_INIT(init_bars, VPCI_PRIORITY_MIDDLE);
>>
>> +int vpci_bar_add_handlers(const struct domain *d, struct pci_dev *pdev)
>> +{
>> + int rc;
>> +
>> + /* Remove previously added registers. */
>> + vpci_remove_device_registers(pdev);
>> +
>> + /* It only makes sense to add registers for hwdom or guest domain. */
>> + if ( d->domain_id >= DOMID_FIRST_RESERVED )
>> + return 0;
>> +
>> + if ( is_hardware_domain(d) )
>> + rc = add_bar_handlers(pdev, true);
>> + else
>> + rc = add_bar_handlers(pdev, false);
> rc = add_bar_handlers(pdev, is_hardware_domain(d));
Indeed, thank you ;)
>
>> + if ( rc )
>> + gprintk(XENLOG_ERR,
>> + "%pp: failed to add BAR handlers for dom%d\n", &pdev->sbdf,
>> + d->domain_id);
> Please use %pd and correct indentation. Logging the error code might
> also help some in diagnosing issues.
Sure, I'll change it to %pd
> Further I'm not sure this is a
> message we want in release builds
Why not?
> - perhaps gdprintk()?
I'll change if we decide so
>
>> + return rc;
>> +}
>> +
>> +int vpci_bar_remove_handlers(const struct domain *d, struct pci_dev *pdev)
>> +{
>> + /* Remove previously added registers. */
>> + vpci_remove_device_registers(pdev);
>> + return 0;
>> +}
> Also - in how far is the goal of your work to also make vPCI work for
> x86 DomU-s? If that's not a goal
It is not, unfortunately. The goal is not to break x86 and to enable Arm
> , I'd like to ask that you limit the
> introduction of code that ends up dead there.
What's wrong with this function even if it is a one-liner?
This way we have a pair vpci_bar_add_handlers/vpci_bar_remove_handlers
and if I understood correctly you suggest
vpci_bar_add_handlers/vpci_remove_device_registers?
What would we gain from that, but yet another secret knowledge that in order
to remove BAR handlers one needs to call vpci_remove_device_registers
while I would personally expect to call vpci_bar_add_handlers' counterpart,
vpci_remove_device_registers namely.
> Jan
>
Thank you,
Oleksandr
