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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.08.17 at 12:10, <julien.grall@xxxxxxx> wrote:
> CC "THE REST" maintainers to get an opinion on the public headers.

Please be more specific as to what you expect - the only public
header affected here is ARM-specific.

> 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.

>> +#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.

>> +        ((d0) << 24 | (d1) << 16 | (d2) << 8 | (d3) << 0),              \
>> +        ((d4) << 24 | (d5) << 16 | (d6) << 8 | (d7) << 0)}})
>> +
>> +/*
>> + * Hypervisor Service version.
>> + *
>> + * We can't use XEN version here, because of SMCCC requirements:
>> + * Major revision should change every time SMC/HVC function is removed.
>> + * Minor revision should change every time SMC/HVC function is added.
>> + * So, it is SMCCC protocol revision code, not XEN version.

I don't understand this explanation - how is the situation here
different from some arbitrary, non-toolstack-only hypercall?

>> + * Those values are subjected to change, when interface will be extended.
>> + * They should not be stored in public/asm-arm/smc.h because they should
>> + * be queried by guest using SMC/HVC interface.
>> + */
>> +#define XEN_SMCCC_MAJOR_REVISION 0
>> +#define XEN_SMCCC_MINOR_REVISION 1

The comment says the values shouldn't be put here, but then
they're still being put here?

>> +/* Hypervisor Service UID. Randomly generated with 3rd party tool.  */
>> +#define XEN_SMCCC_UID XEN_ARM_SMCCC_UID(0xa71812dc, 0xc698, 0x4369, \
>> +                                        0x9a, 0xcf, 0x79, 0xd1, \
>> +                                        0x8d, 0xde, 0xe6, 0x67)

Why is the right side using _ARM_ in its name, but the macro being
defined isn't?

Jan


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