On 30.08.2021 15:41, Daniel P. Smith wrote: > 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, <[email protected]> >>>>> + * >>>>> + * Contributors: Michael LeMay, <[email protected]> >>>>> + * >>>>> + * 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). > > 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.
Well, yes, you effectively suffer from XSM not having been actively maintained for a number of years. As said in the original reply, I'd prefer my ack to cover all the suggested changes, but I didn't mean to insist. If this particular one goes too far for your taste, so be it. Jan
