Re: [XEN][PATCH v3] x86: make Viridian support optional

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.09.2025 15:41, Grygorii Strashko wrote:
> --- a/xen/arch/x86/hvm/Kconfig
> +++ b/xen/arch/x86/hvm/Kconfig
> @@ -62,6 +62,16 @@ config ALTP2M
>  
>         If unsure, stay with defaults.
>  
> +config HVM_VIRIDIAN

I may have said so already on v1: I'm not quite convinced of the need
or usefulness of the HVM_ part here. Viridian necessarily means HVM,
aiui.

> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -1116,14 +1116,14 @@ static int cf_check viridian_save_domain_ctxt(
>  {
>      const struct domain *d = v->domain;
>      const struct viridian_domain *vd = d->arch.hvm.viridian;
> -    struct hvm_viridian_domain_context ctxt = {
> -        .hypercall_gpa = vd->hypercall_gpa.raw,
> -        .guest_os_id = vd->guest_os_id.raw,
> -    };
> +    struct hvm_viridian_domain_context ctxt = {};
>  
>      if ( !is_viridian_domain(d) )
>          return 0;

This check doesn't check for vd being non-NULL, so this still feels a little
fragile, even if it looks correct now.

> +    ctxt.hypercall_gpa = vd->hypercall_gpa.raw;
> +    ctxt.guest_os_id = vd->guest_os_id.raw,
> +
>      viridian_time_save_domain_ctxt(d, &ctxt);
>      viridian_synic_save_domain_ctxt(d, &ctxt);
>  

Just below here we have viridian_load_domain_ctxt(), which I'm pretty sure
now also needs to gain some check: Save records coming from user space, we
can't really rely on there being none of this type for a non-Viridian domain.

Jan