Re: [Xen-devel] [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 02 September 2019 13:34
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
> <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Tim 
> (Xen.org) <tim@xxxxxxx>; Wei Liu
> <wl@xxxxxxx>
> Subject: Re: [PATCH v7 1/6] x86/domain: remove the 'oos_off' flag
> 
> On 30.08.2019 10:29, Paul Durrant wrote:
> > The flag is not needed since the domain 'options' can now be tested
> > directly.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> > Cc: Tim Deegan <tim@xxxxxxx>
> > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Wei Liu <wl@xxxxxxx>
> > Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> >
> > v3:
> >  - Force 'oos_off' to be set for PV guests (to avoid call to
> >    is_hvm_domain() except in ASSERT)
> >  - Dropped Tim's A-b because of the change
> 
> I've been debating with myself whether to not wait any longer for
> Tim to re-instate his ack, but now that I've looked again ...
> 
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -313,11 +313,19 @@ static int sanitise_domain_config(struct 
> > xen_domctl_createdomain *config)
> >          return -EINVAL;
> >      }
> >
> > -    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) &&
> > -         (config->flags & XEN_DOMCTL_CDF_hap) )
> > +    if ( !(config->flags & XEN_DOMCTL_CDF_hvm_guest) )
> >      {
> > -        dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> > -        return -EINVAL;
> > +        if ( config->flags & XEN_DOMCTL_CDF_hap )
> > +        {
> > +            dprintk(XENLOG_INFO, "HAP requested for non-HVM guest\n");
> > +            return -EINVAL;
> > +        }
> > +
> > +        /*
> > +         * It is only meaningful for XEN_DOMCTL_CDF_oos_off to be clear
> > +         * for HVM guests.
> > +         */
> > +        config->flags |= XEN_DOMCTL_CDF_oos_off;
> 
> ... I wonder whether this last part wouldn't better belong into
> x86's arch_sanitise_domain_config(). Arm, to the contrary, should
> force/require the bit to be uniformly off.
> 

I'm sure I had a reason for doing it like this but it's sufficiently long ago 
now that I've forgotten what it was*. Would it be ok to take the code as-is and 
I'll investigate whether this can be tidied up?

[ * I suspect it was concern over breaking existing tool-stacks by requiring 
them to set a flag that previously they did not need to, but I'm not sure that 
was the only reason ]

  Paul




> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel