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

Re: [PATCH v2 2/3] xen/arm: make has_vpci depend on CONFIG_HAS_VPCI



Hi,

On 07/07/2023 02:47, Stewart Hildebrand wrote:
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

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 <oleksandr_andrushchenko@xxxxxxxx>
Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
---
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?

Shouldn't this instead be conditional to pci_passthrough_enabled?

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.

Cheers,

--
Julien Grall



 


Rackspace

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