Re: [PATCH v3 3/7] xsm: refactor xsm_ops handling

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

On 8/25/21 11:16 AM, Jan Beulich wrote:
> On 05.08.2021 16:06, Daniel P. Smith wrote:
>> @@ -747,16 +747,16 @@ extern int xsm_dt_policy_init(void **policy_buffer, 
>> size_t *policy_size);
>>  extern bool has_xsm_magic(paddr_t);
>>  #endif
>>  
>> -extern int register_xsm(struct xsm_operations *ops);
>> -
>> -extern struct xsm_operations dummy_xsm_ops;
>> -extern void xsm_fixup_ops(struct xsm_operations *ops);
>> +extern void xsm_fixup_ops(struct xsm_ops *ops);
>>  
>>  #ifdef CONFIG_XSM_FLASK
>> -extern void flask_init(const void *policy_buffer, size_t policy_size);
>> +extern const struct xsm_ops *flask_init(const void *policy_buffer,
>> +                                        size_t policy_size);
>>  #else
>> -static inline void flask_init(const void *policy_buffer, size_t policy_size)
>> +static inline struct xsm_ops *flask_init(const void *policy_buffer,
> 
> Please use const consistently between real function and stub.

Ack.

>> @@ -766,9 +766,12 @@ extern const unsigned int xsm_flask_init_policy_size;
>>  #endif
>>  
>>  #ifdef CONFIG_XSM_SILO
>> -extern void silo_init(void);
>> +extern const struct xsm_ops *silo_init(void);
>>  #else
>> -static inline void silo_init(void) {}
>> +static inline struct xsm_ops *silo_init(void)
> 
> Same here.

Ack.

>> --- a/xen/xsm/flask/hooks.c
>> +++ b/xen/xsm/flask/hooks.c
>> @@ -1745,7 +1745,7 @@ static int flask_argo_send(const struct domain *d, 
>> const struct domain *t)
>>  long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
>>  int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
>>  
>> -static struct xsm_operations flask_ops = {
>> +static const struct xsm_ops __initconst flask_ops = {
> 
> __initconstrel please (I thought I had pointed out the difference
> already during earlier discussion). Same for SILO then of course.

Apologies, yes you did and that is my fault as I had only recorded the
__initconst suggestion in my checklist and did not update with your
__initconstrel suggestion.

>> --- a/xen/xsm/xsm_core.c
>> +++ b/xen/xsm/xsm_core.c
>> @@ -28,9 +28,17 @@
>>  #include <asm/setup.h>
>>  #endif
>>  
>> -#define XSM_FRAMEWORK_VERSION    "1.0.0"
>> +#define XSM_FRAMEWORK_VERSION    "1.0.1"
>>  
>> -struct xsm_operations *xsm_ops;
>> +struct xsm_ops xsm_ops;
> 
> __read_mostly?

Ack.

v/r,
dps