Re: [PATCH v3 5/7] xsm: decouple xsm header inclusion selection

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

On 8/30/21 9:24 AM, Jan Beulich wrote:
> On 27.08.2021 16:06, Daniel P. Smith wrote:
>> On 8/26/21 4:13 AM, Jan Beulich wrote:
>>> On 05.08.2021 16:06, Daniel P. Smith wrote:
>>>> --- /dev/null
>>>> +++ b/xen/include/xsm/xsm-core.h
>>>> @@ -0,0 +1,273 @@
>>>> +/*
>>>> + *  This file contains the XSM hook definitions for Xen.
>>>> + *
>>>> + *  This work is based on the LSM implementation in Linux 2.6.13.4.
>>>> + *
>>>> + *  Author:  George Coker, <gscoker@xxxxxxxxxxxxxx>
>>>> + *
>>>> + *  Contributors: Michael LeMay, <mdlemay@xxxxxxxxxxxxxx>
>>>> + *
>>>> + *  This program is free software; you can redistribute it and/or modify
>>>> + *  it under the terms of the GNU General Public License version 2,
>>>> + *  as published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#ifndef __XSM_CORE_H__
>>>> +#define __XSM_CORE_H__
>>>> +
>>>> +#include <xen/sched.h>
>>>> +#include <xen/multiboot.h>
>>>
>>> I was going to ask to invert the order (as we try to arrange #include-s
>>> alphabetically), but it looks like multiboot.h isn't fit for this.
>>
>> So my understanding is to leave this as is.
> 
> Yes, unfortunately.
> 
>>>> +typedef void xsm_op_t;
>>>> +DEFINE_XEN_GUEST_HANDLE(xsm_op_t);
>>>
>>> Just FTR - I consider this dubious. If void is meant, I don't see why
>>> a void handle can't be used.
>>
>> Unless I am misunderstanding what you are calling for, I am afraid this
>> will trickle further that what intended to be addressed in this patch
>> set. If disagree and would like to provide me a suggest that stays
>> bounded, I would gladly incorporate.
> 
> All I'm asking is to remove this pointless typedef and handle definition,
> seeing that you're doing a major rework anyway. I'm afraid I don't see
> how this would collide with the purpose of the overall series (albeit I
> may also have misunderstood your reply, as the 2nd half of the first
> sentence makes me struggle some with trying to parse it).
> 
> Jan
> 

If I drop the typedef and start changing everywhere xsm_op_t is
referenced to void, this now adds hypercall.h to the files I am now
touching.

In the end it is not about whether the change is big or small, but that
more and more unrelated small changes/clean ups keep getting requested.
There has to be a cut-off point to limit the scope of changes down to
the purpose of the patch set, which is to unravel and simplify the XSM
hooks. And this is being done so, so that the the XSM-Roles work can be
introduced to bring a more solid definition to the the default access
control system, which itself is needed to bring in hyperlaunch.

v/r,
dps