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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.08.17 at 23:39, <volodymyr_babchuk@xxxxxxxx> wrote:
> On 09.08.17 14:58, Jan Beulich wrote:
>>>>> On 09.08.17 at 12:10, <julien.grall@xxxxxxx> 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.
> Just to be clear: you are proposing to store UID in such struct
> struct uuid_t {
>      unsigned32  time_low;
>      unsigned16  time_mid;
>      unsigned16  time_hi_and_version;
>      unsigned8   clock_seq_hi_and_reserved;
>      unsigned8   clock_seq_low;
>      byte        node[6];
> };
> right?

Type-wise yes; the names of the fields look uncommon to me.

>>>> +#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.
> Oops, sorry. Didn't knew that XEN should be C89 compatible.
> Is there any guide for novices? I didn't found anything useful in docs/ 
> (not even coding style document). On wiki I have found only 
> "Submitting_Xen_Project_Patches" page, which is very helpful, but it 
> does not cover topics like which C standard to use.

The public headers are required to the C89 compatible; Xen
code in general is fine to use extensions.

>>>> +        ((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?
> Because this is generic interface that should be supported by all 
> hypervisors, including XEN. Think about this as a way for a guest to 
> determine under which hypervisor it operates, and which functions it 
> provides.

In which case - why the XEN_ prefixes?

Jan


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