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

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>
>>>>
>>>> --- 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@xxxxxxxxxx>

Thanks.

Jan