On Tue, May 21, 2024 at 12:30:32PM +0200, Jan Beulich wrote:
> On 17.05.2024 15:33, Roger Pau Monne wrote:
> > Enabling it using an HVM param is fragile, and complicates the logic when
> > deciding whether options that interact with altp2m can also be enabled.
> >
> > Leave the HVM param value for consumption by the guest, but prevent it from
> > being set. Enabling is now done using and additional altp2m specific field
> > in
> > xen_domctl_createdomain.
> >
> > Note that albeit only currently implemented in x86, altp2m could be
> > implemented
> > in other architectures, hence why the field is added to
> > xen_domctl_createdomain
> > instead of xen_arch_domainconfig.
> >
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
>
> Reviewed-by: Jan Beulich <jbeul...@suse.com> # hypervisor
> albeit with one question:
>
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -637,6 +637,8 @@ int arch_sanitise_domain_config(struct
> > xen_domctl_createdomain *config)
> > bool hap = config->flags & XEN_DOMCTL_CDF_hap;
> > bool nested_virt = config->flags & XEN_DOMCTL_CDF_nested_virt;
> > unsigned int max_vcpus;
> > + unsigned int altp2m_mode = MASK_EXTR(config->altp2m_opts,
> > + XEN_DOMCTL_ALTP2M_mode_mask);
> >
> > if ( hvm ? !hvm_enabled : !IS_ENABLED(CONFIG_PV) )
> > {
> > @@ -715,6 +717,26 @@ int arch_sanitise_domain_config(struct
> > xen_domctl_createdomain *config)
> > return -EINVAL;
> > }
> >
> > + if ( config->altp2m_opts & ~XEN_DOMCTL_ALTP2M_mode_mask )
> > + {
> > + dprintk(XENLOG_INFO, "Invalid altp2m options selected: %#x\n",
> > + config->flags);
> > + return -EINVAL;
> > + }
> > +
> > + if ( altp2m_mode && nested_virt )
> > + {
> > + dprintk(XENLOG_INFO,
> > + "Nested virt and altp2m are not supported together\n");
> > + return -EINVAL;
> > + }
> > +
> > + if ( altp2m_mode && !hap )
> > + {
> > + dprintk(XENLOG_INFO, "altp2m is only supported with HAP\n");
> > + return -EINVAL;
> > + }
>
> Should this last one perhaps be further extended to permit altp2m with EPT
> only?
Hm, yes, that would be more accurate as:
if ( altp2m_mode && (!hap || !hvm_altp2m_supported()) )
Would you be fine adjusting at commit, or would you prefer me to send
an updated version?
Thanks, Roger.
