Re: [PATCH v4 1/2] xsm: create idle domain privileged and demote after setup

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 4/26/22 02:35, Jan Beulich wrote:
> On 25.04.2022 19:22, Daniel P. Smith wrote:
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -621,6 +621,9 @@ static void noreturn init_done(void)
> >       void *va;
> >       unsigned long start, end;
> >   
> > +    if ( xsm_set_system_active() != 0 )
> > +        panic("xsm: unable to set hypervisor to SYSTEM_ACTIVE privilege\n");
> Roger did request that the panic() either also report the error
> code, or that the function be returning bool. You did neither,
> and your earlier verbal reply also didn't really respond to this
> part of Roger's comments.
Opps, my apologies. I meant to add his suggestion of adding the error to 
the panic message.
> > --- a/xen/xsm/flask/hooks.c
> > +++ b/xen/xsm/flask/hooks.c
> > @@ -186,6 +186,26 @@ static int cf_check flask_domain_alloc_security(struct 
> > domain *d)
> >       return 0;
> >   }
> >   
> > +static int cf_check flask_set_system_active(void)
> > +{
> > +    struct domain *d = current->domain;
> > +
> > +    if ( d->domain_id != DOMID_IDLE )
> > +    {
> > +        printk("xsm_set_system_active should only be called by idle domain\n");
> > +        return -EPERM;
> > +    }
> > +
> > +    /*
> > +     * While is_privileged has no significant meaning under flask, set to false
> > +     * as there are times in hypervisor code privilege checks check this
> > +     * directly instead of going through XSM.
> > +     */
> It feels as if there is "which" missing between "checks" and "check",
> or something else (better fitting "as there are times"), without which
> the sentence is a little hard to follow.
You are correct, will fix.

v/r
dps