| Message ID | 20231004145604.1085358-8-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SMMU handling for PCIe Passthrough on ARM | expand |
Hi Stewart, On 04/10/2023 15:55, Stewart Hildebrand wrote: > From: Rahul Singh <rahul.singh@arm.com> > > 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@arm.com> > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> > --- > 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? > --- > 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. > return 0; > > if ( !dt_device_type_is_equal(node, "pci") ) Cheers,
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@arm.com> >> >> 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@arm.com> >> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> >> --- >> 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
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) ) return 0; if ( !dt_device_type_is_equal(node, "pci") )