Re: [PATCH] pci: fix locking around vPCI removal in pci_remove_device()

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 2/29/24 08:23, Jan Beulich wrote:
> On 29.02.2024 14:15, Roger Pau Monne wrote:
>> Currently vpci_deassign_device() is called without holding the per-domain
>> pci_lock in pci_remove_device(), which leads to:
>>
>> Assertion 'rw_is_write_locked(&pdev->domain->pci_lock)' failed at 
>> ../drivers/vpci/vpci.c:47
>> [...]
>> Xen call trace:
>>    [<ffff82d040260eac>] R vpci_deassign_device+0x10d/0x1b9
>>    [<ffff82d04027932f>] S pci_remove_device+0x2b1/0x380
>>    [<ffff82d040260bd0>] F pci_physdev_op+0x197/0x19e
>>    [<ffff82d04032272d>] F do_physdev_op+0x342/0x12aa
>>    [<ffff82d0402f067a>] F pv_hypercall+0x58e/0x62b
>>    [<ffff82d0402012ba>] F lstar_enter+0x13a/0x140
>>
>> Move the existing block that removes the device from the domain pdev_list 
>> ahead
>> and also issue the call to vpci_deassign_device() there.  It's fine to remove
>> the device from the domain list of assigned devices, as further functions 
>> only
>> care that the pdev domain field is correctly set to the owner of the device
>> about to be removed.
>>
>> Moving the vpci_deassign_device() past the pci_cleanup_msi() call can be
>> dangerous, as doing the MSI cleanup ahead of having removed the vPCI handlers
>> could lead to stale data in vPCI MSI(-X) internal structures.
>>
>> Fixes: 4f78438b45e2 ('vpci: use per-domain PCI lock to protect vpci 
>> structure')
>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>

Thanks