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

[Julien Grall <julien.grall@arm.com>]

Hi,

On 10/08/17 08:30, Jan Beulich wrote:
>>>> On 09.08.17 at 23:39, <volodymyr_babchuk@epam.com> wrote:
>> On 09.08.17 14:58, Jan Beulich wrote:
>>>>>> On 09.08.17 at 12:10, <julien.grall@arm.com> 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?

Because the version depends on the interface implemented which is Xen 
specific.

You have to match the UID (Xen Specific) and the version to know what is 
actually supported.

Cheers,

-- 
Julien Grall

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