On 8/30/21 9:46 AM, Jan Beulich wrote: > 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 >
I think we can agree that XSM has not been receiving appropriate care and maintenance. It can't all be fixed in one go and I am trying to step up to get it back into shape. Since this is really undoing a masking of void, I can add this and the risk is very low of breaking something else. Which this is my biggest concern as I see new files getting brought in with each cleanup requested. v/r, dps
