| Message ID | 20230620152924.23622-2-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/2] xen/arm: pci: introduce PCI_PASSTHROUGH Kconfig option | expand |
Hi, On 20/06/2023 16:29, Stewart Hildebrand wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > VPCI is disabled on ARM. Make it depend on CONFIG_HAS_VPCI to test the PCI > passthrough support. > > While here, remove the comment on the preceding line. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Signed-off-by: Rahul Singh <rahul.singh@arm.com> > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> > --- > There are two downstreams [1] [2] that have independently made a version this > change, each with different Signed-off-by's. I simply picked one at random for > the Author: field, and added both Signed-off-by lines. Please let me know if > there are any objections. > > downstream->v1: > * change to IS_ENABLED(CONFIG_HAS_VPCI) instead of hardcoded to true > * remove the comment on the preceding line > > [1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/27be1729ce8128dbe37275ce7946b2fbd2e5a382 > [2] https://github.com/xen-troops/xen/commit/bf12185e6fb2e31db0d8e6ea9ccd8a02abadec17 > --- > xen/arch/arm/include/asm/domain.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h > index 99e798ffff68..6e016b00bae1 100644 > --- a/xen/arch/arm/include/asm/domain.h > +++ b/xen/arch/arm/include/asm/domain.h > @@ -298,8 +298,7 @@ static inline void arch_vcpu_block(struct vcpu *v) {} > > #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag) > > -/* vPCI is not available on Arm */ > -#define has_vpci(d) ({ (void)(d); false; }) > +#define has_vpci(d) ({ (void)(d); IS_ENABLED(CONFIG_HAS_VPCI); }) This will enable vPCI for all the domains. However, in the cover letter, you seemed to suggest that guest support is not there. So shouldn't this be "is_harware_domain(d)"? Or d->arch.has_vcpi? Cheers,
On 6/25/23 08:45, Julien Grall wrote: > Hi, > > On 20/06/2023 16:29, Stewart Hildebrand wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> VPCI is disabled on ARM. Make it depend on CONFIG_HAS_VPCI to test the PCI >> passthrough support. >> >> While here, remove the comment on the preceding line. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> Signed-off-by: Rahul Singh <rahul.singh@arm.com> >> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com> >> --- >> There are two downstreams [1] [2] that have independently made a version this >> change, each with different Signed-off-by's. I simply picked one at random for >> the Author: field, and added both Signed-off-by lines. Please let me know if >> there are any objections. >> >> downstream->v1: >> * change to IS_ENABLED(CONFIG_HAS_VPCI) instead of hardcoded to true >> * remove the comment on the preceding line >> >> [1] https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc/-/commit/27be1729ce8128dbe37275ce7946b2fbd2e5a382 >> [2] https://github.com/xen-troops/xen/commit/bf12185e6fb2e31db0d8e6ea9ccd8a02abadec17 >> --- >> xen/arch/arm/include/asm/domain.h | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h >> index 99e798ffff68..6e016b00bae1 100644 >> --- a/xen/arch/arm/include/asm/domain.h >> +++ b/xen/arch/arm/include/asm/domain.h >> @@ -298,8 +298,7 @@ static inline void arch_vcpu_block(struct vcpu *v) {} >> >> #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag) >> >> -/* vPCI is not available on Arm */ >> -#define has_vpci(d) ({ (void)(d); false; }) >> +#define has_vpci(d) ({ (void)(d); IS_ENABLED(CONFIG_HAS_VPCI); }) > > This will enable vPCI for all the domains. However, in the cover letter, > you seemed to suggest that guest support is not there. So shouldn't this > be "is_harware_domain(d)"? Or d->arch.has_vcpi? Right, I mentioned in the SMMU series discussion [3] that it will only work in dom0 / hardware domain (unless additional vPCI series [4] is applied). So, making it depend on is_hardware_domain makes sense for now: #define has_vpci(d) ({ IS_ENABLED(CONFIG_HAS_VPCI) && is_hardware_domain(d); }) However, the is_hardware_domain check should be removed when the vPCI series [4] is merged. [3] https://lists.xenproject.org/archives/html/xen-devel/2023-06/msg01135.html [4] https://lists.xenproject.org/archives/html/xen-devel/2023-06/msg00863.html
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h index 99e798ffff68..6e016b00bae1 100644 --- a/xen/arch/arm/include/asm/domain.h +++ b/xen/arch/arm/include/asm/domain.h @@ -298,8 +298,7 @@ static inline void arch_vcpu_block(struct vcpu *v) {} #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag) -/* vPCI is not available on Arm */ -#define has_vpci(d) ({ (void)(d); false; }) +#define has_vpci(d) ({ (void)(d); IS_ENABLED(CONFIG_HAS_VPCI); }) struct arch_vcpu_io { struct instr_details dabt_instr; /* when the instruction is decoded */