Re: [Xen-devel] [PATCH v3 1/3] xen/flask: Drop the gen-policy.py script

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.12.2019 18:01, Andrew Cooper wrote:
> On 09/12/2019 13:38, Jan Beulich wrote:
>> On 07.12.2019 22:16, Andrew Cooper wrote:
>>> --- /dev/null
>>> +++ b/xen/xsm/flask/flask-policy.S
>>> @@ -0,0 +1,20 @@
>>> +        .section .init.rodata, "a", %progbits
>>> +
>>> +/* const unsigned char xsm_flask_init_policy[] __initconst */
>>> +        .align 4
>> I'm afraid .align is not universal enough to be used here - iirc
>> some architectures have it alias .p2align rather than (how e.g.
>> x86 behaves) .balign. Looks like .p2align is available in all
>> binutils versions we claim to be able to be built with. (I'm
>> not sure the one here is needed anyway, but the one below we
>> surely want.)
> 
> I can switch to p2align, but...
> 
>>
>>> +        .global xsm_flask_init_policy
>>> +xsm_flask_init_policy:
>>> +        .incbin "policy.bin"
>>> +.Lend:
>>> +
>>> +        .type xsm_flask_init_policy, %object
>>> +        .size xsm_flask_init_policy, . - xsm_flask_init_policy
>>> +
>>> +/* const unsigned int __initconst xsm_flask_init_policy_size */
>>> +        .align 4
>>> +        .global xsm_flask_init_policy_size
>>> +xsm_flask_init_policy_size:
>>> +        .long .Lend - xsm_flask_init_policy
>> Similarly .long isn't really universal (various arches override
>> it in gas). Aiui .dc.l is intended to be portable (despite still
>> carrying the 'l' in its name, and despite even this one getting
>> overridden by two arches). But perhaps best to ask on the
>> binutils list.
> 
> ... this is not a clear or obvious way to go, not least because it makes
> a different expectation that int will never change from being 32 bits. 
> At least .long will work even if it becomes longer in a future toolchain.

There are a few targets where .long (and .int) appear to produce 2-byte
values (at the first glance, i.e. without checking very closely).

> What is used here doesn't need to be universal - it only needs to work
> for the architectures we support.

But it also would better not break silently for some future port. How
about an equivalent to Linux'es _ASM_PTR() (say ASM_WORD()), which each
architecture has to supply explicitly?

> If hand writing an asm file isn't considered good enough, then the other
> options are a C file with inline asm incbin, or `objdump
> --rename-section`.  The latter one would require a few changes elsewhere
> in the code, but only for linkage purposes.

I'm entirely fine with an assembler source here, it just needs a little
more polishing imo.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel