Re: [Xen-devel] [PATCH 4/7] arm: smccc: handle SMCs according to SMCCC

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.17 at 23:41, <volodymyr_babchuk@xxxxxxxx> wrote:
> Hello Jan,
> 
> On Wed, Aug 09, 2017 at 05:58:19AM -0600, Jan Beulich wrote:
> 
>> 
>> > On 08/08/17 21:08, Volodymyr Babchuk wrote:
>> >> +#ifndef __XEN_PUBLIC_ARCH_ARM_SMC_H__
>> >> +#define __XEN_PUBLIC_ARCH_ARM_SMC_H__
>> >> +
>> >> +typedef struct {
>> >> +    uint32_t a[4];
>> >> +} xen_arm_smccc_uid;
>> 
>> This is not the normal way of encoding a UID type.
> I thought about this. According to RFC 4122, UUID should be defined like 
> this:
> struct xen_uuid_rfc_4122 {
>     u32     time_low;                  /* low part of timestamp */
>     u16     time_mid;                  /* mid part of timestamp */
>     u16     time_hi_and_version;       /* high part of timestamp and version 
> */
>     u8      clock_seq_hi_and_reserved; /* clock seq hi and variant */
>     u8      clock_seq_low;             /* clock seq low */
>     u8      node[6];                   /* nodes */
> };
> 
> This resembles structure of RFC, but it is highly inconvenient to use. The 
> most
> used operation for UUIDs is comparison, I think. Next popular operations are
> serialization and deserialization. All those are very trivial, if you are 
> using
> array instead of separate fields. I just checked Linux kernel, it uses array
> of 16 u8s. I used array of four u32s because this is how it is represented in
> SMC convention.

In our tree, see e.g. struct xenpf_efi_guid and EFI_GUID plus the
"conversion" function between them (cast_guid()).

> Now I'm going to create separate public header for UUIDs.

I don't see the need for a separate header.

>And I'm not sure
> that RFC 4122 approach is the best. Serialization code for that structure
> will require some fiddling with binary shifts. Personally I stick to the
> Linux way (uint8_t data[16]).
> So, I'm interested in maintainers opinion.
> 
>> >> +#define XEN_ARM_SMCCC_UID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)      \
>> >> +    ((xen_arm_smccc_uid) {{(a), ((b) << 16 | (c) ),                     \
>> 
>> This is not C89 compatible.
> 
> I'm sorry, but I not quite sure why this is not C89 compatible. According to 
> [1]
> C89 supports initializer lists.

It's the (<type>){<initializers>} style which C89 doesn't support
afaik, and ...

>> >> +        ((d0) << 24 | (d1) << 16 | (d2) << 8 | (d3) << 0),              \
>> >> +        ((d4) << 24 | (d5) << 16 | (d6) << 8 | (d7) << 0)}})
>> >> +
> 
> [1] http://port70.net/~nsz/c/c89/c89-draft.html#3.5.7 

... this also doesn't have anything like that.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel