Re: [PATCH v9 04/16] vpci: add hooks for PCI device assign/de-assign

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 13, 2023 at 11:53:56PM +0000, Volodymyr Babchuk wrote:
> 
> Hi,
> 
> Jan Beulich <jbeulich@xxxxxxxx> writes:
> 
> > On 13.09.2023 01:41, Volodymyr Babchuk wrote:
> >> Jan Beulich <jbeulich@xxxxxxxx> writes:
> >>> On 30.08.2023 01:19, Volodymyr Babchuk wrote:
> >>>> @@ -1481,6 +1488,13 @@ static int assign_device(struct domain *d, u16 
> >>>> seg, u8 bus, u8 devfn, u32 flag)
> >>>>      if ( pdev->broken && d != hardware_domain && d != dom_io )
> >>>>          goto done;
> >>>>  
> >>>> +    if ( IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) )
> >>>> +    {
> >>>> +        write_lock(&pdev->domain->pci_lock);
> >>>> +        vpci_deassign_device(pdev);
> >>>> +        write_unlock(&pdev->domain->pci_lock);
> >>>> +    }
> >>>
> >>> Why is the DomIO special case ...
> >> 
> >> vpci_deassign_device() does nothing if vPCI was initialized for a
> >> domain. So it not wrong to call this function even if pdev belongs to 
> >> dom_io.
> >
> > Well, okay, but then you acquire a lock just to do nothing (apart
> > from the apparent asymmetry).
> 
> Yes, I agree. I'll add the same check as below. Thanks for the review.

Hm, no, I would rather rely on the has_vpci check inside of
vpci_{,de}assign_device() than open coding dom_io checks elsewhere.

This is not a hot path, the extra lock taking is likely negligible
compared to the cost of assign operations.

Thanks, Roger.