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

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

On 4/21/22 05:20, Jan Beulich wrote:
> On 21.04.2022 00:28, Daniel P. Smith wrote:
>> There are now instances where internal hypervisor logic needs to make 
>> resource
>> allocation calls that are protectd by XSM checks. The internal hypervisor 
>> logic
>> is represented a number of system domains which by designed are represented 
>> by
>> non-privileged struct domain instances. To enable these logic blocks to
>> function correctly but in a controlled manner, this commit changes the idle
>> domain to be created as a privileged domain under the default policy, which 
>> is
>> inherited by the SILO policy, and demoted before transitioning to running. A
>> new XSM hook, xsm_transition_running, is introduced to allow each XSM policy
>> type to demote the idle domain appropriately for that policy type.
>>
>> For flask a stub is added to ensure that flask policy system will function
>> correctly with this patch until flask is extended with support for starting 
>> the
>> idle domain privileged and properly demoting it on the call to
>> xsm_transtion_running.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> 
> Looks okay to me, but I'm not sure in how far agreement was reached on
> taking this route. Just one nit:

Thank you. As for the approach, Jason suggested it and Roger stated that
if this approach was adopted it would resolve his concerns over __init.
If you have a concern with this approach, please let me know so I can
ensure it is addressed to the best of my ability.

>> --- a/xen/include/xsm/dummy.h
>> +++ b/xen/include/xsm/dummy.h
>> @@ -101,6 +101,18 @@ static always_inline int xsm_default_action(
>>      }
>>  }
>>  
>> +static XSM_INLINE void cf_check xsm_transition_running(void)
>> +{
>> +    struct domain *d = current->domain;
>> +
>> +    if ( d->domain_id != DOMID_IDLE )
>> +        panic("xsm_transition_running should only be called by idle 
>> domain\n");
>> +
>> +    d->is_privileged = false;
>> +
>> +    return;
>> +}
> 
> Please omit such return statements.

Ack.

v/r,
dps