[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs


  • To: Julien Grall <julien@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>
  • From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Date: Tue, 31 Oct 2023 10:15:38 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=xen.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • 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=r5YoK7P1dGjRWJBXzcEWlU5MbijPE5FJf86kXvhu1m4=; b=mz0eRzbOECWOYIzbFcKvpLeqdJ3jfUWAzF5XvlCM3bP7ciUrq9yTH9cNVTzUa5xSdW/kpPRQSfksG2sCHMR/C2lJMy7Sm1cr+3jP+UO9MyQNH+Gj6I14TQ+zIXWda15XApoIkwYfxdfS8xYdtlAaWjcGhSpZ0nQ/xICNPtNlPOkErDE/XRUZSIRq+sOjFwTvgdOn+PRZJqs6sM5P5ZGf7XVtbOboUe4lW+WGL+ZXMscCarXEMCjEpNjfOp8W+bF3zUmzMqtSmX2+z9DQ4I93XKod4u61j9uc82J7LB9vJqkBjzlfeNOaWPaawVGRfmGV1vkBmnU/FWO2FL3CY/IdeA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WcCjNDtOtPcyhDIAkOU5zot/sV8Ju2VzZyPXWA5z/mABT75A2CKbRJAgzXzoeRIi1DPmqyGXAvVCXG5ysN3bFBtJA2OrSxPhhxPu6rDLUh8pQ9vpeqgXWK/5jD56MtJ14OdBZJVXp/TP7qhy4wCEylWNMKiOTup41ZqR5OAp/oK2SI8J3YnS3WDD7W06X+80uVmPAMA6M+V2hkKrtxD2v3GKNdrzOR93X/e/cidmr85r1sm8uI6nY7/ttFRNJ228FS1QwRX8wBpaYItLVmI5xKBXS8x5m+8hD/YL/qjUcDswQ5kNixUX0qbTcxOcU86wanjwQoDsx7GECk2PMSILug==
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 31 Oct 2023 14:15:52 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 10/31/23 09:17, Julien Grall wrote:
> Hi,
> 
> On 31/10/2023 11:03, Jan Beulich wrote:
>> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>>> --- a/xen/drivers/passthrough/pci.c
>>> +++ b/xen/drivers/passthrough/pci.c
>>> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>>>           bus = PCI_BUS(machine_sbdf);
>>>           devfn = PCI_DEVFN(machine_sbdf);
>>>   +        if ( IS_ENABLED(CONFIG_ARM) &&
>>> +             !is_hardware_domain(d) &&
>>> +             !is_system_domain(d) &&
>>> +             (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )
>>
>> I don't think you need the explicit ARM check; that's redundant with
>> checking !HAS_VPCI_GUEST_SUPPORT.

Currently that is true. However, this is allowing for the possibility that we 
eventually may want to enable PCI passthrough for PVH domU using vPCI (e.g. 
hyperlaunch, or eliminating qemu backend), in which case we may want to enable 
CONFIG_HAS_VPCI_GUEST_SUPPORT=y on x86.

>> It's also not really clear why you
>> need to check for the system domain here.

xl pci-assignable-add will assign the device to domIO, which doesn't have vPCI, 
but it is still a valid assignment. Perhaps an in code comment would be helpful 
for clarity?

> 
> I might be missing but I wouldn't expect the domain to have vPCI enabled if 
> CONFIG_HAVE_VPCI_GUEST_SUPPORT=n. So why can't this simply be:
> 
> if ( !has_vcpi(d) )
> {
>    ...
> }

Right, the CONFIG_HAVE_VPCI_GUEST_SUPPORT check here is not strictly needed 
because this case is already caught by the other half of this patch in 
xen/arch/arm/vpci.c. This simplifies it to:

    if ( IS_ENABLED(CONFIG_ARM) &&
         !is_hardware_domain(d) &&
         !is_system_domain(d) /* !domIO */ &&
         !has_vpci(d) )

On x86, unless I misunderstood something, I think it's valid to assign PCI 
devices to a domU without has_vpci().

BTW, it's valid for has_vpci() to be true and CONFIG_HAVE_VPCI_GUEST_SUPPORT=n 
for dom0.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.