On 7/7/23 04:55, Julien Grall wrote: > Hi, > > On 07/07/2023 02:47, Stewart Hildebrand wrote: >> From: Oleksandr Andrushchenko <[email protected]> >> >> VPCI is disabled on ARM. Make it depend on CONFIG_HAS_VPCI to test the PCI >> passthrough support. Also make it depend on is_hardware_domain for now. The >> is_hardware_domain check should be removed when vPCI is upstreamed. >> >> While here, remove the comment on the preceding line. >> >> Signed-off-by: Oleksandr Andrushchenko <[email protected]> >> Signed-off-by: Rahul Singh <[email protected]> >> Signed-off-by: Stewart Hildebrand <[email protected]> >> --- >> 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. >> >> v1->v2: >> * add is_hardware_domain check. This will need to be removed after the vPCI >> series [3] is merged. >> >> 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 >> [3] >> https://lists.xenproject.org/archives/html/xen-devel/2023-06/msg00863.html >> --- >> 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..1a13965a26b8 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) ({ IS_ENABLED(CONFIG_HAS_VPCI) && >> is_hardware_domain(d); }) > > So in v1, I asked whether we should use is_hardware_domain() or > d->arch.pci. I see you went with the former, but wouldn't this mean that > the vPCI is always enabled for dom0 when CONFIG_HAS_VPCI=y?
Yes... > Shouldn't this instead be conditional to pci_passthrough_enabled? That could be a possibility, however, in v5 of the PCI ARM SMMU series [4], we propose removing the pci_passthrough_enabled flag (as a way to use PCI devices in dom0 without pci-passthrough=yes). If pci_passthrough_enabled is gone, the conditions under which vpci should be enabled for dom0 aren't entirely clear to me (other than CONFIG_HAS_VPCI=y). [4] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html > So you could return d->arch.pci in has_vcpi(). The field would be set by > domain_create() based on the flags passed by the caller. I would > properly plumb to xen_domctl_createdomain and has a check in > arch_sanitise_domain_config() to confirm the flag can be set. I'll plumb this for v3 of this series. > > Cheers, > > -- > Julien Grall
