Re: [PATCH] x86: Conditionalise init_dom0_cpu_policy()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Jul 25, 2025 at 12:02:18PM +0200, Alejandro Vallejo wrote:
> On Wed Jul 23, 2025 at 9:18 AM CEST, Roger Pau Monné wrote:
> > On Thu, Jul 17, 2025 at 07:58:24PM +0200, Alejandro Vallejo wrote:
> >> Later patches will keep refactoring create_dom0()
> >> until it can create arbitrary domains. This is one
> >> small step in that direction.
> >> 
> >> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> >> ---
> >>  xen/arch/x86/setup.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> >> index c6890669b9..6943ffba79 100644
> >> --- a/xen/arch/x86/setup.c
> >> +++ b/xen/arch/x86/setup.c
> >> @@ -1054,7 +1054,8 @@ static struct domain *__init create_dom0(struct boot_info *bi)
> >>      if ( IS_ERR(d) )
> >>          panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
> >>  
> >> -    init_dom0_cpuid_policy(d);
> >> +    if ( pv_shim || d->cdf & (CDF_privileged | CDF_hardware) )
> >
> > You possibly want this to be:
> >
> > (d->cdf & (CDF_privileged | CDF_hardware)) == (CDF_privileged | CDF_hardware)
> >
> > To ensure the contents of dom0_cpuid_cmdline is only applied to dom0,
> > and not to the hardware or control domains.  I assume it should be
> > possible to pass a different set of cpuid options for the hardware vs
> > the control domains.
> >
> > Thanks, Roger.
> 
> Why only a hwdom+ctldom, surely a single hwdom should get it too.

hm, not really I think: a late hardware domain would get any custom
cpuid options from the toolstack that created it, or in the
hyperlaunch case from the provided configuration, but not from the
dom0-cpuid command line option I would expect.  Otherwise you have two
different sources of cpuid options, the inheritance from dom0-cpuid,
plus whatever is provided from the hardware domain configuration.

> I can see
> the argument for a ctldom not getting it. For our use case having dom0
> disaggregated is of the essence, so what happens with a hwdom that is not a
> ctldom is fairly important.
> 
> That said, I'm thinking moving in a different direction and have a generic
> init_cpuid_policy() that internally checks for hw or control, or leave the
> default policy or something else.

Right, so instead of introducing init_{hwdom,ctrldom}_cpuid_policy()
equivalents you would rather generalize init_dom0_cpuid_policy() so
it's used by all domains.

> This would remove the conditional and allow much finer selection. e.g: a domain
> brought up through a "nomigrate" DTB node (TBD: nonexisting binding atm) would
> get itsc reported, just as dom0 does today.

We might want to enforce such no migration attribute at the hypervisor
level (by adding a new domain field to signal it possibly?), as this
is all toolstack knowledge ATM.

On a related tangent: domains brought up using hyperlaunch will have a
config file, capable of expressing options like cpuid features I
expect, at which point ITSC could be set in the config file and Xen
won't need to do any guessing?

Thanks, Roger.