Re: [PATCH v2 3/3] xen: Expose the PMU to the guests

[Jan Beulich <jbeulich@suse.com>]

On 06.10.2021 12:58, Michal Orzel wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
>          return -EINVAL;
>      }
>  
> +    if ( config->flags & XEN_DOMCTL_CDF_vpmu )
> +    {
> +        dprintk(XENLOG_INFO, "vpmu support not ready yet\n");
> +        return -EINVAL;
> +    }

I consider this message potentially misleading (as x86 does have vPMU
support, it merely doesn't get enabled this way). But isn't this redundant
with ...

> @@ -534,6 +535,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>          return -EINVAL;
>      }
>  
> +    if ( vpmu && !vpmu_is_available )
> +    {
> +        dprintk(XENLOG_INFO, "vpmu requested but not available\n");
> +        return -EINVAL;
> +    }

... this? (This message is again potentially misleading.)

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -70,9 +70,12 @@ struct xen_domctl_createdomain {
>  #define XEN_DOMCTL_CDF_iommu          (1U<<_XEN_DOMCTL_CDF_iommu)
>  #define _XEN_DOMCTL_CDF_nested_virt   6
>  #define XEN_DOMCTL_CDF_nested_virt    (1U << _XEN_DOMCTL_CDF_nested_virt)
> +/* Should we expose the vPMU to the guest? */
> +#define _XEN_DOMCTL_CDF_vpmu           7
> +#define XEN_DOMCTL_CDF_vpmu            (1U << _XEN_DOMCTL_CDF_vpmu)

Like for the earlier patch, I think we should stop with the bad habit of
defining both the bit position and the mask separately.

Jan