On 16.11.21 10:23, Oleksandr Andrushchenko wrote:
>
> On 16.11.21 10:01, Jan Beulich wrote:
>> On 16.11.2021 08:32, Oleksandr Andrushchenko wrote:
>>> On 15.11.21 18:56, Jan Beulich wrote:
>>>> On 05.11.2021 07:56, Oleksandr Andrushchenko wrote:
>>>>> From: Oleksandr Andrushchenko <oleksandr_andrushche...@epam.com>
>>>>>
>>>>> When a vPCI is removed for a PCI device it is possible that we have
>>>>> scheduled a delayed work for map/unmap operations for that device.
>>>>> For example, the following scenario can illustrate the problem:
>>>>>
>>>>> pci_physdev_op
>>>>> pci_add_device
>>>>> init_bars -> modify_bars -> defer_map ->
>>>>> raise_softirq(SCHEDULE_SOFTIRQ)
>>>>> iommu_add_device <- FAILS
>>>>> vpci_remove_device -> xfree(pdev->vpci)
>>>>>
>>>>> leave_hypervisor_to_guest
>>>>> vpci_process_pending: v->vpci.mem != NULL; v->vpci.pdev->vpci ==
>>>>> NULL
>>>>>
>>>>> For the hardware domain we continue execution as the worse that
>>>>> could happen is that MMIO mappings are left in place when the
>>>>> device has been deassigned
>>>> Is continuing safe in this case? I.e. isn't there the risk of a NULL
>>>> deref?
>>> I think it is safe to continue
>> And why do you think so? I.e. why is there no race for Dom0 when there
>> is one for DomU?
> Well, then we need to use a lock to synchronize the two.
> I guess this needs to be pci devs lock unfortunately
The parties involved in deferred work and its cancellation:
MMIO trap -> vpci_write -> vpci_cmd_write -> modify_bars -> defer_map
Arm: leave_hypervisor_to_guest -> check_for_vcpu_work -> vpci_process_pending
x86: two places -> hvm_do_resume -> vpci_process_pending
So, both defer_map and vpci_process_pending need to be synchronized with
pcidevs_{lock|unlock).
As both functions existed before the code I introduce I would prefer this to be
a dedicated patch for v5 of the series.
Thank you,
Oleksandr
