[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 02/11] vpci: cancel pending map/unmap on vpci removal
- To: Jan Beulich <jbeulich@xxxxxxxx>, "roger.pau@xxxxxxxxxx" <roger.pau@xxxxxxxxxx>
- From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
- Date: Tue, 16 Nov 2021 13:41:29 +0000
- Accept-language: en-US
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=3TO7qUF6d0KnACzIH4mdxDQsOkzpgR0lYLPc5a6UECY=; b=NvuvTtOHHM8bK+B+nVzMtrU92pc+MkBf4zRNf9hFz0Hfz/Rkfip9QYQFdDtJ152mA+EBmocQyVGD5el3elXsOP3mdnlsOaOZyzhORKWuSye6Qe+NMS1ENmUmyYdw+ABD/ZqjyweJ2ksdh3/gp23DvpmLFKFAaLpSIgsNzAJZsn/voLWZK7S5del7sgiivVQcEGDieGwwkFbUmZ6DkPKNBaTFCLWY7ETDnjRgwXRhdr3uvDktTjAs4EoVHCEs2cHIyVl1o/5hCe+zFh+4eHFcBCtnM30/72VQg3gJE+DxKZU2HkgKggPRvuxt5mDL1ORmZl9q7rf5NlTh8CWIES0bgw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LJ5mOTlnzYYi7HhVK9972iTJYRe20EfwP13o9qZWH8TDm2uErYt4sXQMAQSAiQFXTlSiF1atuNWJB9SdJboWIgFv5enIXym3XRKFs12D4j+sjdhr8YI8H9UB/ktCi0l/xviu7lCENQTUSEfciAeTq8X8wFreWJs4gNX1lUTNsZ0PwHnesGLU3HH75X3/mIkob6ltNWHJxqymJrrCUKh9FXmiB0EnBY4p/zBX6a9MPdkSZP7lkBr85Ig1ycbB615IokJYqLhKGXGrlM5hmwhWZOn9xIHglcC92d30l8mPZUbuJaYH9tWsFN4Z5EswaSivi5o+OR6b0ZbumPyOYzXi8w==
- Cc: "julien@xxxxxxx" <julien@xxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>, "andrew.cooper3@xxxxxxxxxx" <andrew.cooper3@xxxxxxxxxx>, "george.dunlap@xxxxxxxxxx" <george.dunlap@xxxxxxxxxx>, "paul@xxxxxxx" <paul@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
- Delivery-date: Tue, 16 Nov 2021 13:41:51 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Thread-index: AQHX0hJIYALl/D9fL0OD6N0XGDJh2awE30EAgAD09oCAAAgGAIAABgyAgABY44A=
- Thread-topic: [PATCH v4 02/11] vpci: cancel pending map/unmap on vpci removal
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_andrushchenko@xxxxxxxx>
>>>>>
>>>>> 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
|