Re: [PATCH v7 09/12] xen: add runtime parameter access support to hypfs

[Jürgen Groß <jgross@xxxxxxxx>]

On 03.04.20 16:51, Jan Beulich wrote:
> On 02.04.2020 17:46, Juergen Gross wrote:
> > V7:
> > - fine tune some parameter initializations (Jan Beulich)
> > - call custom_runtime_set_var() after updating the value
> > - modify alignment in Arm linker script to 4 (Jan Beulich)
> I didn't ask for this to be unilaterally 4 - I don't think this
> would work on Arm64, seeing that there are pointers inside the
> struct. This wants to be pointer size, i.e. 4 for Arm32 but 8
> for Arm64.
Oh, how silly of me. Should be POINTER_ALIGN, of course.

> > --- a/docs/misc/hypfs-paths.pandoc
> > +++ b/docs/misc/hypfs-paths.pandoc
> > @@ -152,3 +152,12 @@ The major version of Xen.
> >   #### /buildinfo/version/minor = INTEGER
> >   
> >   The minor version of Xen.
> > +
> > +#### /params/
> > +
> > +A directory of runtime parameters.
> > +
> > +#### /params/*
> > +
> > +The individual parameters. The description of the different parameters can be
> > +found in `docs/misc/xen-command-line.pandoc`.
> Is .pandoc a useful specification here, or do such extensions get
> converted when rendering into different formats?
I looked into xenstore-paths.pandoc and found references to other docs
like pvcalls.pandoc. So I assumed it is fine this way.

> > --- a/xen/arch/x86/hvm/vmx/vmcs.c
> > +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> > @@ -70,6 +70,30 @@ integer_param("ple_window", ple_window);
> >   static bool __read_mostly opt_ept_pml = true;
> >   static s8 __read_mostly opt_ept_ad = -1;
> >   int8_t __read_mostly opt_ept_exec_sp = -1;
> > +static char opt_ept_setting[24];
> > +
> > +static void update_ept_param_append(const char *str, int val)
> > +{
> > +    char *pos = opt_ept_setting + strlen(opt_ept_setting);
> > +
> > +    snprintf(pos, sizeof(opt_ept_setting) - (pos - opt_ept_setting),
> > +             ",%s=%d", str, val);
> > +}
> > +
> > +static void update_ept_param(void)
> > +{
> > +    snprintf(opt_ept_setting, sizeof(opt_ept_setting), "pml=%d", opt_ept_pml);
> > +    if ( opt_ept_ad >= 0 )
> > +        update_ept_param_append("ad", opt_ept_ad);
> With the new patch 1, is the if() here really still needed? Then
> again, only "exec-sp" is a runtime sub-parameter anyway afaict,
> and hence I'd expect only that part of the option should be
> displayed (I'm sorry for not having paid attention to this
> earlier).
Oh, you are right. Will change it.


Juergen