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

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-10-28 16:17, Grygorii Strashko wrote:
> Hi Jason,
>
> On 24.10.25 21:55, Jason Andryuk wrote:
> > On 2025-10-23 11:18, Grygorii Strashko wrote:
> > > From: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> > >
> > > Add config option VIRIDIAN that covers viridian code within HVM.
> > > Calls to viridian functions guarded by is_viridian_domain() and 
> > > related macros.
> > > Having this option may be beneficial by reducing code footprint for 
> > > systems
> > > that are not using Hyper-V.
> > >
> > > [grygorii_strashko@xxxxxxxx: fixed NULL pointer deref in
> > > viridian_save_domain_ctxt(); stub viridian_vcpu/domain_init/deinit()]
> > > Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> > > Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> > > ---
> > > diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/ 
> > > viridian/viridian.c
> > > index f79cffcb3767..b935803700fd 100644
> > > --- a/xen/arch/x86/hvm/viridian/viridian.c
> > > +++ b/xen/arch/x86/hvm/viridian/viridian.c
> > > @@ -1153,6 +1156,9 @@ static int cf_check viridian_load_vcpu_ctxt(
> > >       struct vcpu *v;
> > >       struct hvm_viridian_vcpu_context ctxt;
> > > +    if ( !is_viridian_domain(d) )
> > > +        return -EILSEQ;
> > Given:
> >
> >   #define is_viridian_domain(d) \
> >      (IS_ENABLED(CONFIG_VIRIDIAN) && \
> >       is_hvm_domain(d) && (viridian_feature_mask(d) & HVMPV_base_freq))
> >
> > CONFIG_VIRIDIAN=n is okay because of the IS_ENABLED.
> >
> > For CONFIG_VIRIDIAN=y && a viridian domain, is HVM_PARAM_VIRIDIAN 
> > guaranteed to be loaded before viridian_load_vcpu_ctxt() is called, so 
> > that HVMPV_base_freq can be checked properly?  I don't know, but it 
> > seems a little fragile if this relies on implicit ordering.  Maybe 
> > just do:
> > if ( !IS_ENABLED(CONFIG_VIRIDIAN) )
> >      return -EILSEQ;
> >
> > ?
> Should it be done the same way for viridian_load_domain_ctxt() also?
Yes, I think so.  Thanks for catching that.

Regards,
Jason