[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi,

On 10/08/17 08:30, Jan Beulich wrote:
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?

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.