Re: [PATCH v5 05/14] vpci: add hooks for PCI device assign/de-assign

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Roger!

On 13.01.22 13:40, Roger Pau Monné wrote:
> On Thu, Nov 25, 2021 at 01:02:42PM +0200, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>> +#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
>> +/* Notify vPCI that device is assigned to guest. */
>> +int vpci_assign_device(struct domain *d, struct pci_dev *pdev)
>> +{
>> +    int rc;
>> +
>> +    /* It only makes sense to assign for hwdom or guest domain. */
>> +    if ( is_system_domain(d) || !has_vpci(d) )
>> +        return 0;
>> +
>> +    spin_lock(&pdev->vpci_lock);
>> +    rc = run_vpci_init(pdev);
> Following my comment below, this will likely need to call
> vpci_add_handlers in order to allocate the pdev->vpci field.
>
> It's not OK to carry the contents of pdev->vpci across domain
> assignations, as the device should be reset, and thus the content of
> pdev->vpci would be stale.
>
>> +    spin_unlock(&pdev->vpci_lock);
>> +    if ( rc )
>> +        vpci_deassign_device(d, pdev);
>> +
>> +    return rc;
>> +}
>> +
>> +/* Notify vPCI that device is de-assigned from guest. */
>> +int vpci_deassign_device(struct domain *d, struct pci_dev *pdev)
>> +{
>> +    /* It only makes sense to de-assign from hwdom or guest domain. */
>> +    if ( is_system_domain(d) || !has_vpci(d) )
>> +        return 0;
>> +
>> +    spin_lock(&pdev->vpci_lock);
>> +    vpci_remove_device_handlers_locked(pdev);
> You need to free the pdev->vpci structure on deassign. I would expect
> the device to be reset on deassign, so keeping the pdev->vpci contents
> would be wrong.
Sure, I will re-allocate pdev->vpci then
>
> Thanks, Roger.
Thank you,
Oleksandr