Re: [PATCH v3 2/5] xen/arm: pci: plumb xen_arch_domainconfig with pci info

[Julien Grall <julien@xxxxxxx>]

On 09/10/2023 20:57, Stewart Hildebrand wrote:
> Add a flag to struct xen_arch_domainconfig to allow specifying at domain
> creation time whether the domain uses vPCI.
>
> Add a corresponding flag to struct arch_domain to indicate vPCI and set it
> accordingly.
The new boolean you are adding doesn't seem very arch specific. So could 
we use a bit in xen_domctl_createdomain.flags?
> Bump XEN_DOMCTL_INTERFACE_VERSION since we're modifying struct
> xen_arch_domainconfig.
>
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
> ---
> v2->v3:
> * new patch
> ---
>   xen/arch/arm/domain.c             | 10 ++++++++++
>   xen/arch/arm/include/asm/domain.h |  2 ++
>   xen/include/public/arch-arm.h     |  4 ++++
>   xen/include/public/domctl.h       |  2 +-
>   4 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 28e3aaa5e482..9470c28595da 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -687,6 +687,13 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>           return -EINVAL;
>       }
>   
> +    if ( (config->arch.pci_flags & XEN_DOMCTL_CONFIG_PCI_VPCI) &&
> +         !IS_ENABLED(CONFIG_HAS_VPCI) )
> +    {
> +        dprintk(XENLOG_INFO, "vPCI support not enabled\n");
> +        return -EINVAL;
> +    }
> +
>       return 0;
>   }
>   
> @@ -737,6 +744,9 @@ int arch_domain_create(struct domain *d,
>           BUG();
>       }
>   
> +    if ( config->arch.pci_flags & XEN_DOMCTL_CONFIG_PCI_VPCI )
> +        d->arch.has_vpci = true;
> +
>       if ( (rc = domain_vgic_register(d, &count)) != 0 )
>           goto fail;
>   
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index 99e798ffff68..0a66ed09a617 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -119,6 +119,8 @@ struct arch_domain
>       void *tee;
>   #endif
>   
> +    bool has_vpci;
> +
>   }  __cacheline_aligned;
>   
>   struct arch_vcpu
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 6a4467e8f5d1..5c8424341aad 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -300,6 +300,8 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
>   #define XEN_DOMCTL_CONFIG_TEE_OPTEE     1
>   #define XEN_DOMCTL_CONFIG_TEE_FFA       2
>   
> +#define XEN_DOMCTL_CONFIG_PCI_VPCI      (1U << 0)
> +
>   struct xen_arch_domainconfig {
>       /* IN/OUT */
>       uint8_t gic_version;
> @@ -323,6 +325,8 @@ struct xen_arch_domainconfig {
>        *
>        */
>       uint32_t clock_frequency;
> +    /* IN */
> +    uint8_t pci_flags;
Regardless what I wrote above. Do you have any plan for adding more PCI 
specific flags? If not, then if you want to keep the flag Arm specific, 
then I think this should be named pci_flags.
As you add a new field, I believe, you also want to modoify the various 
language specific bindings (e.g. go, ocaml).
>   };
>   #endif /* __XEN__ || __XEN_TOOLS__ */
>   
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index a33f9ec32b08..dcd42b3d603d 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -21,7 +21,7 @@
>   #include "hvm/save.h"
>   #include "memory.h"
>   
> -#define XEN_DOMCTL_INTERFACE_VERSION 0x00000016
> +#define XEN_DOMCTL_INTERFACE_VERSION 0x00000017
>   
>   /*
>    * NB. xen_domctl.domain is an IN/OUT parameter for this operation.
Cheers,

--
Julien Grall