Re: [PATCH v2 6/7] flask: drop dead compat translation code

[Jan Beulich <jbeulich@suse.com>]

On 15.07.2020 10:41, Roger Pau Monné wrote:
> On Wed, Jul 15, 2020 at 08:42:44AM +0200, Jan Beulich wrote:
>> On 14.07.2020 16:58, Roger Pau Monné wrote:
>>> On Wed, Jul 01, 2020 at 12:28:07PM +0200, Jan Beulich wrote:
>>>> Translation macros aren't needed at all (or else a devicetree_label
>>>> entry would have been missing), and userlist has been removed quite some
>>>> time ago.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>
>>>> --- a/xen/include/xlat.lst
>>>> +++ b/xen/include/xlat.lst
>>>> @@ -148,14 +148,11 @@
>>>>  ?	xenoprof_init			xenoprof.h
>>>>  ?	xenoprof_passive		xenoprof.h
>>>>  ?	flask_access			xsm/flask_op.h
>>>> -!	flask_boolean			xsm/flask_op.h
>>>>  ?	flask_cache_stats		xsm/flask_op.h
>>>>  ?	flask_hash_stats		xsm/flask_op.h
>>>> -!	flask_load			xsm/flask_op.h
>>>>  ?	flask_ocontext			xsm/flask_op.h
>>>>  ?	flask_peersid			xsm/flask_op.h
>>>>  ?	flask_relabel			xsm/flask_op.h
>>>>  ?	flask_setavc_threshold		xsm/flask_op.h
>>>>  ?	flask_setenforce		xsm/flask_op.h
>>>> -!	flask_sid_context		xsm/flask_op.h
>>>>  ?	flask_transition		xsm/flask_op.h
>>>
>>> Shouldn't those become checks then?
>>
>> No, checking will never succeed for structures containing
>> XEN_GUEST_HANDLE(). But there's no point in generating xlat macros
>> when they're never used. There are two fundamentally different
>> strategies for handling the compat hypercalls: One is to wrap a
>> translation layer around the native hypercall. That's where the
>> xlat macros come into play. The other, used here, is to compile
>> the entire hypercall function a second time, arranging for the
>> compat structures to get used in place of the native ones. There
>> are no xlat macros involved here, all that's needed are correctly
>> translated structures. (For completeness, x86's MCA hypercall
>> uses yet another, quite adhoc strategy for handling, but also not
>> involving any xlat macro use. Hence the consideration there to
>> possibly drop the respective lines from the file here.)
> 
> Thanks, I think this explanation is helpful and I wonder whether it
> would be possible to have something along this lines in a file or as a
> comment somewhere, maybe at the top of xlat.lst?

To be honest - I'm not sure: Such a comment may indeed be helpful
to have, but I don't think I can see any single good place for it
to live. For people editing xlat.lst (a file the existence of which
many aren't even aware of), this would be a good place. But how
would others have any chance of running into this comment?

> Also could you add a line to the commit message noting that flask code
> doesn't use any of the translation macros because it follows a
> different approach to compat handling?

I've made the sentence start "Translation macros aren't used (and
hence needed) at all ..." - is that enough of an adjustment?

> For the patch:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

Jan