Re: [PATCH v3 2/3] xsm: consolidate loading the policy buffer

["Daniel P. Smith" <dpsmith@apertussolutions.com>]

On 5/31/22 12:05, Jan Beulich wrote:
> On 31.05.2022 17:08, Daniel P. Smith wrote:
>> Previously, initializing the policy buffer was split between two functions,
>> xsm_{multiboot,dt}_policy_init() and xsm_core_init(). The latter for loading
>> the policy from boot modules and the former for falling back to built-in policy.
>>
>> This patch moves all policy buffer initialization logic under the
>> xsm_{multiboot,dt}_policy_init() functions. It then ensures that an error
>> message is printed for every error condition that may occur in the functions.
>> With all policy buffer init contained and only called when the policy buffer
>> must be populated, the respective xsm_{mb,dt}_init() functions will panic if an
>> error occurs attempting to populate the policy buffer.
> 
> "flask=late" is also a mode where, afaict, no policy is required. I can't,
> however, see how you're taking care of that (but maybe I'm overlooking
> something); inspecting flask_bootparam in generic XSM code would actually
> be a layering violation.

Good point, flask=late is meant to be enforcing with a late loading of a
policy file. I will address it.

>> --- a/xen/include/xsm/xsm.h
>> +++ b/xen/include/xsm/xsm.h
>> @@ -775,7 +775,7 @@ int xsm_multiboot_init(
>>      unsigned long *module_map, const multiboot_info_t *mbi);
>>  int xsm_multiboot_policy_init(
>>      unsigned long *module_map, const multiboot_info_t *mbi,
>> -    void **policy_buffer, size_t *policy_size);
>> +    const unsigned char *policy_buffer[], size_t *policy_size);
> 
> I don't think we're dealing with an array here, so const unsigned char **
> would seem the more correct representation to me.
> 
> Also - what about the DT counterpart function?

Ack.

v/r
dps