[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
 
    
     |