Re: [PATCH v6 2/2] x86/xen: Allow per-domain usage of hardware virtualized APIC

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 08, 2022 at 05:36:43PM +0000, Jane Malalane wrote:
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 39fdca1b49..ba5b8f433f 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -1384,8 +1384,9 @@ void 
> libxl__arch_domain_create_info_setdefault(libxl__gc *gc,
>      }
>  }
>  
> -void libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> -                                              libxl_domain_build_info 
> *b_info)
> +int libxl__arch_domain_build_info_setdefault(libxl__gc *gc,
> +                                             libxl_domain_build_info *b_info,
> +                                             const libxl_physinfo *physinfo)
>  {
>      /* ACPI is disabled by default */
>      libxl_defbool_setdefault(&b_info->acpi, false);
> @@ -1399,6 +1400,8 @@ void libxl__arch_domain_build_info_setdefault(libxl__gc 
> *gc,
>      memset(&b_info->u, '\0', sizeof(b_info->u));
>      b_info->type = LIBXL_DOMAIN_TYPE_INVALID;
>      libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
> +
> +    return 0;

There's a void return above the memset (out of context in the diff)
that you also need to patch to 'return 0;'.


> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 77ce0b2121..7adb043ab7 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1157,6 +1157,10 @@ static int construct_vmcs(struct vcpu *v)
>          __vmwrite(PLE_WINDOW, ple_window);
>      }
>  
> +    if ( !has_assisted_xapic(v->domain) )

Nit: you already have a local 'd' variable here that's v->domain, so
please use that.

> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index 5b7ec0cf69..65a978f670 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -373,6 +373,12 @@ int hvm_get_param(struct domain *d, uint32_t index, 
> uint64_t *value);
>  #define hvm_tsc_scaling_ratio(d) \
>      ((d)->arch.hvm.tsc_scaling_ratio)
>  
> +#define has_assisted_xapic(d) \
> +    ((d)->arch.hvm.assisted_xapic)
> +
> +#define has_assisted_x2apic(d) \
> +    ((d)->arch.hvm.assisted_x2apic)

Nit: I think there's no need to split those into two lines, ie:

#define has_assisted_xapic(d) ((d)->arch.hvm.assisted_xapic)

Is well below the 80 column limit.

Thanks, Roger.