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

[Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>]

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.
Now I'm going to create separate public header for UUIDs. 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.

> 
> >> +        ((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

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