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

Re: [PATCH v5 7/9] xen/arm: Fix mapping for PCI bridge mmio region


  • To: Julien Grall <julien@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Date: Mon, 6 Nov 2023 15:24:38 -0500
  • 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=K8iGUZkNGBL474ZwZDGSTdsDCXJ/VRYsDvP6g0IjKuM=; b=h3m+T+VFOflmVs0YI0XjiwYy1XLC+ME+Gnxd3Pkfu2FU6erhYm74BkIlGi0naxxF7+gl97tAOkbAgO7zHVHRnj3CpwCBeayggB2xoPngJGsuYhPx80WxB9Jl+WFGxkaNla65Kw5d944lX5QJrpLO6IzS9WDTKIvDvwLFNEFIy79cI4XkBtsA9/mrge/n8XNRyu8BDtwybiT+7FiJ+baXZDjpsoROD7VJLNVdc2iDlyWzp6tGi85I/oYeUp8U32zMjvbIIsvuba7i7yXjE9jSWAK045D2nvWXmARIuF8wZVVO7ak+tPEsjBRLmW7aRFoNnvFiOqawgOG6XuAX5lNg5g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NYnhZk1ABtS9bOI/QjOtzqDXGMlMQ4J+mfGKfg/fIhJu126LOYHsPQtR39EX+dXXlRVw8aDKVFrffMNAvIM9ITAQfuYuiIFGRVI8OaHef9uvM0p21fu+bFeVdK+eO+vJ7d1wLatFHEAyiBDQ2LKq3EXrl1EbHIDH48saYgNHRt8vkEHhZQ0ddK3Nj9GxzdRfG47/qKVZHSwghS33tDVI1gkS/HjVtE1ZMoj9yDdNjU9/MsaLqHJ+dycrx/i/hLjMnwQpRDBY7kM6hHemodXycKxtjmzYbVBr8vSrcSVffXiPKeZkxsT64R6XgasxdcV5x3YVpHyOERoH3I38krQdXA==
  • Cc: Rahul Singh <rahul.singh@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 06 Nov 2023 20:25:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 10/20/23 14:17, Julien Grall wrote:
> Hi Stewart,
> 
> On 04/10/2023 15:55, Stewart Hildebrand wrote:
>> From: Rahul Singh <rahul.singh@xxxxxxx>
>>
>> Current code skip the mapping for PCI bridge MMIO region to dom0 when
>> pci_passthrough_enabled flag is set. Mapping should be skip when
>> has_vpci(d) is enabled for the domain, as we need to skip the mapping
>> only when VPCI handler are registered for ECAM.
>>
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
>> ---
>> v4->v5:
>> * new patch
> 
> I am a bit lost. How is this a new patch but...
> 
>> * rebase on top of "dynamic node programming using overlay dtbo" series
>> * replace !is_pci_passthrough_enabled() check with 
>> !IS_ENABLED(CONFIG_HAS_PCI)
>>    instead of removing
> 
> ... there is a changelog. Did you get the patch from somewhere?

Yes, it was picked up from [1]. The changelog is describing what changed since 
getting the patch from [1]. I'll clarify this here in the next rev.

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00483.html

> 
>> ---
>>   xen/arch/arm/device.c       | 2 +-
>>   xen/arch/arm/domain_build.c | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
>> index 1f631d327441..4d69c298858d 100644
>> --- a/xen/arch/arm/device.c
>> +++ b/xen/arch/arm/device.c
>> @@ -330,7 +330,7 @@ int handle_device(struct domain *d, struct 
>> dt_device_node *dev, p2m_type_t p2mt,
>>           .d = d,
>>           .p2mt = p2mt,
>>           .skip_mapping = !own_device ||
>> -                        (is_pci_passthrough_enabled() &&
>> +                        (has_vpci(d) &&
>>                           (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
>>           .iomem_ranges = iomem_ranges,
>>           .irq_ranges = irq_ranges
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 7da254709d17..2c55528a62d4 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1064,7 +1064,7 @@ static void __init assign_static_memory_11(struct 
>> domain *d,
>>   #endif
>>
>>   /*
>> - * When PCI passthrough is available we want to keep the
>> + * When HAS_PCI is enabled we want to keep the
>>    * "linux,pci-domain" in sync for every host bridge.
>>    *
>>    * Xen may not have a driver for all the host bridges. So we have
>> @@ -1080,7 +1080,7 @@ static int __init handle_linux_pci_domain(struct 
>> kernel_info *kinfo,
>>       uint16_t segment;
>>       int res;
>>
>> -    if ( !is_pci_passthrough_enabled() )
>> +    if ( !IS_ENABLED(CONFIG_HAS_PCI) )
> 
> I don't understand why this wants to be HAS_PCI rather than has_vcpi()?
> This also doesn't seem to be mentioned in the commit message.

This particular change was essentially preparation for reverting the 
pci-passthrough option to ensure we didn't break the ability to bisect. Since 
we will not revert the pci-passthrough option after all, I will drop this 
change in domain_build.c for the next rev.

> 
>>           return 0;
>>
>>       if ( !dt_device_type_is_equal(node, "pci") )
> 
> Cheers,
> 
> -- 
> Julien Grall



 


Rackspace

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