diff mbox series

[v1,02/57] xen/riscv: add public arch-riscv.h

Message ID d9721f72f4a51b1240ba180e33193c551b987251.1692181079.git.oleksii.kurochko@gmail.com (mailing list archive)
State New, archived
Headers show
Series Enable build of full Xen for RISC-V | expand

Commit Message

Oleksii Kurochko Aug. 16, 2023, 10:19 a.m. UTC 
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/include/public/arch-riscv.h | 90 +++++++++++++++++++++++++++++++++
 xen/include/public/xen.h        |  2 +
 2 files changed, 92 insertions(+)
 create mode 100644 xen/include/public/arch-riscv.h

Comments

Jan Beulich Aug. 17, 2023, 3 p.m. UTC | #1 
On 16.08.2023 12:19, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/include/public/arch-riscv.h
> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Guest OS interface to RISC-V Xen.
> + * Initially based on the ARM implementation.
> + */
> +
> +#ifndef __XEN_PUBLIC_ARCH_RISCV_H__
> +#define __XEN_PUBLIC_ARCH_RISCV_H__
> +
> +#define  int64_aligned_t  int64_t __attribute__((__aligned__(8)))
> +#define uint64_aligned_t uint64_t __attribute__((__aligned__(8)))
> +
> +#ifndef __ASSEMBLY__
> +#define ___DEFINE_XEN_GUEST_HANDLE(name, type)                  \
> +    typedef union { type *p; unsigned long q; }                 \
> +        __guest_handle_ ## name;                                \
> +    typedef union { type *p; uint64_aligned_t q; }              \
> +        __guest_handle_64_ ## name
> +
> +/*
> + * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
> + * in a struct in memory. On RISCV is always 8 bytes sizes and 8 bytes
> + * aligned.
> + * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed as an
> + * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on aarch64.

Nit: aarch{32,64}?

> + */
> +#define __DEFINE_XEN_GUEST_HANDLE(name, type) \
> +    ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
> +    ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
> +#define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, name)
> +#define __XEN_GUEST_HANDLE(name)        __guest_handle_64_ ## name
> +#define XEN_GUEST_HANDLE(name)          __XEN_GUEST_HANDLE(name)
> +#define XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_ ## name
> +#define set_xen_guest_handle_raw(hnd, val)                  \
> +    do {                                                    \
> +        typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
> +        _sxghr_tmp->q = 0;                                  \
> +        _sxghr_tmp->p = (val);                              \
> +    } while ( 0 )

While I realize you simply took this from Arm, in new code I think it
would be helpful to avoid name space violations from the beginning.
Hence no leading underscore please for macro local variables. Trailing
underscores is what we mean to use instead.

It's also not really valid to use a gcc extension here, but I guess
that's hard to avoid.

> +#define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val)
> +
> +typedef uint64_t xen_pfn_t;
> +#define PRI_xen_pfn PRIx64
> +#define PRIu_xen_pfn PRIu64
> +
> +typedef uint64_t xen_ulong_t;
> +#define PRI_xen_ulong PRIx64
> +
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> +
> +struct vcpu_guest_context {
> +};
> +typedef struct vcpu_guest_context vcpu_guest_context_t;
> +DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
> +
> +struct xen_arch_domainconfig {
> +};

While these are okay to remain empty, ...

> +#endif
> +
> +struct arch_vcpu_info {
> +};
> +typedef struct arch_vcpu_info arch_vcpu_info_t;
> +
> +struct arch_shared_info {
> +};
> +typedef struct arch_shared_info arch_shared_info_t;

... these two need to gain a "todo" marker so that we won't forget
to at least add a placeholder entry if no real ones surface.

> +/* Maximum number of virtual CPUs in legacy multi-processor guests. */
> +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */

Nit: Style (missing full stop). And quite likely the two comments could
be joined to a single one.

> +#define XEN_LEGACY_MAX_VCPUS 1
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#ifndef __ASSEMBLY__

Why not continue the earlier !__ASSEMBLY__ section?

Jan
Oleksii Kurochko Aug. 18, 2023, 9:36 a.m. UTC | #2 
On Thu, 2023-08-17 at 17:00 +0200, Jan Beulich wrote:
> On 16.08.2023 12:19, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/include/public/arch-riscv.h
> > @@ -0,0 +1,90 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Guest OS interface to RISC-V Xen.
> > + * Initially based on the ARM implementation.
> > + */
> > +
> > +#ifndef __XEN_PUBLIC_ARCH_RISCV_H__
> > +#define __XEN_PUBLIC_ARCH_RISCV_H__
> > +
> > +#define  int64_aligned_t  int64_t __attribute__((__aligned__(8)))
> > +#define uint64_aligned_t uint64_t __attribute__((__aligned__(8)))
> > +
> > +#ifndef __ASSEMBLY__
> > +#define ___DEFINE_XEN_GUEST_HANDLE(name, type)                  \
> > +    typedef union { type *p; unsigned long q; }                 \
> > +        __guest_handle_ ## name;                                \
> > +    typedef union { type *p; uint64_aligned_t q; }              \
> > +        __guest_handle_64_ ## name
> > +
> > +/*
> > + * XEN_GUEST_HANDLE represents a guest pointer, when passed as a
> > field
> > + * in a struct in memory. On RISCV is always 8 bytes sizes and 8
> > bytes
> > + * aligned.
> > + * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed
> > as an
> > + * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on
> > aarch64.
> 
> Nit: aarch{32,64}?
Thanks. It should be updated to RISC-V.
> 
> > + */
> > +#define __DEFINE_XEN_GUEST_HANDLE(name, type) \
> > +    ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
> > +    ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
> > +#define DEFINE_XEN_GUEST_HANDLE(name)  
> > __DEFINE_XEN_GUEST_HANDLE(name, name)
> > +#define __XEN_GUEST_HANDLE(name)        __guest_handle_64_ ## name
> > +#define XEN_GUEST_HANDLE(name)          __XEN_GUEST_HANDLE(name)
> > +#define XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_ ## name
> > +#define set_xen_guest_handle_raw(hnd, val)                  \
> > +    do {                                                    \
> > +        typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
> > +        _sxghr_tmp->q = 0;                                  \
> > +        _sxghr_tmp->p = (val);                              \
> > +    } while ( 0 )
> 
> While I realize you simply took this from Arm, in new code I think it
> would be helpful to avoid name space violations from the beginning.
> Hence no leading underscore please for macro local variables.
> Trailing
> underscores is what we mean to use instead.
> 
> It's also not really valid to use a gcc extension here, but I guess
> that's hard to avoid.
Thank you. Understood. I'll make the update.
> 
> > +#define set_xen_guest_handle(hnd, val)
> > set_xen_guest_handle_raw(hnd, val)
> > +
> > +typedef uint64_t xen_pfn_t;
> > +#define PRI_xen_pfn PRIx64
> > +#define PRIu_xen_pfn PRIu64
> > +
> > +typedef uint64_t xen_ulong_t;
> > +#define PRI_xen_ulong PRIx64
> > +
> > +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> > +
> > +struct vcpu_guest_context {
> > +};
> > +typedef struct vcpu_guest_context vcpu_guest_context_t;
> > +DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
> > +
> > +struct xen_arch_domainconfig {
> > +};
> 
> While these are okay to remain empty, ...
> 
> > +#endif
> > +
> > +struct arch_vcpu_info {
> > +};
> > +typedef struct arch_vcpu_info arch_vcpu_info_t;
> > +
> > +struct arch_shared_info {
> > +};
> > +typedef struct arch_shared_info arch_shared_info_t;
> 
> ... these two need to gain a "todo" marker so that we won't forget
> to at least add a placeholder entry if no real ones surface.
I'll make the update. Thanks.

> 
> > +/* Maximum number of virtual CPUs in legacy multi-processor
> > guests. */
> > +/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
> 
> Nit: Style (missing full stop). And quite likely the two comments
> could
> be joined to a single one.
I'll take it into account.
> 
> > +#define XEN_LEGACY_MAX_VCPUS 1
> > +
> > +#endif /* __ASSEMBLY__ */
> > +
> > +#ifndef __ASSEMBLY__
> 
> Why not continue the earlier !__ASSEMBLY__ section?
Sure, we can continue the earlier !__ASSEBLY__ section. I'll update
this part.

~ Oleksii
diff mbox series

Patch

diff --git a/xen/include/public/arch-riscv.h b/xen/include/public/arch-riscv.h
new file mode 100644
index 0000000000..05f2c67f49
--- /dev/null
+++ b/xen/include/public/arch-riscv.h
@@ -0,0 +1,90 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Guest OS interface to RISC-V Xen.
+ * Initially based on the ARM implementation.
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_RISCV_H__
+#define __XEN_PUBLIC_ARCH_RISCV_H__
+
+#define  int64_aligned_t  int64_t __attribute__((__aligned__(8)))
+#define uint64_aligned_t uint64_t __attribute__((__aligned__(8)))
+
+#ifndef __ASSEMBLY__
+#define ___DEFINE_XEN_GUEST_HANDLE(name, type)                  \
+    typedef union { type *p; unsigned long q; }                 \
+        __guest_handle_ ## name;                                \
+    typedef union { type *p; uint64_aligned_t q; }              \
+        __guest_handle_64_ ## name
+
+/*
+ * XEN_GUEST_HANDLE represents a guest pointer, when passed as a field
+ * in a struct in memory. On RISCV is always 8 bytes sizes and 8 bytes
+ * aligned.
+ * XEN_GUEST_HANDLE_PARAM represents a guest pointer, when passed as an
+ * hypercall argument. It is 4 bytes on aarch32 and 8 bytes on aarch64.
+ */
+#define __DEFINE_XEN_GUEST_HANDLE(name, type) \
+    ___DEFINE_XEN_GUEST_HANDLE(name, type);   \
+    ___DEFINE_XEN_GUEST_HANDLE(const_##name, const type)
+#define DEFINE_XEN_GUEST_HANDLE(name)   __DEFINE_XEN_GUEST_HANDLE(name, name)
+#define __XEN_GUEST_HANDLE(name)        __guest_handle_64_ ## name
+#define XEN_GUEST_HANDLE(name)          __XEN_GUEST_HANDLE(name)
+#define XEN_GUEST_HANDLE_PARAM(name)    __guest_handle_ ## name
+#define set_xen_guest_handle_raw(hnd, val)                  \
+    do {                                                    \
+        typeof(&(hnd)) _sxghr_tmp = &(hnd);                 \
+        _sxghr_tmp->q = 0;                                  \
+        _sxghr_tmp->p = (val);                              \
+    } while ( 0 )
+#define set_xen_guest_handle(hnd, val) set_xen_guest_handle_raw(hnd, val)
+
+typedef uint64_t xen_pfn_t;
+#define PRI_xen_pfn PRIx64
+#define PRIu_xen_pfn PRIu64
+
+typedef uint64_t xen_ulong_t;
+#define PRI_xen_ulong PRIx64
+
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+
+struct vcpu_guest_context {
+};
+typedef struct vcpu_guest_context vcpu_guest_context_t;
+DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
+
+struct xen_arch_domainconfig {
+};
+
+#endif
+
+struct arch_vcpu_info {
+};
+typedef struct arch_vcpu_info arch_vcpu_info_t;
+
+struct arch_shared_info {
+};
+typedef struct arch_shared_info arch_shared_info_t;
+
+/* Maximum number of virtual CPUs in legacy multi-processor guests. */
+/* Only one. All other VCPUS must use VCPUOP_register_vcpu_info */
+#define XEN_LEGACY_MAX_VCPUS 1
+
+#endif /* __ASSEMBLY__ */
+
+#ifndef __ASSEMBLY__
+/* Stub definition of PMU structure */
+typedef struct xen_pmu_arch { uint8_t dummy; } xen_pmu_arch_t;
+#endif
+
+#endif /*  __XEN_PUBLIC_ARCH_RISCV_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 920567e006..8a07032796 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -16,6 +16,8 @@ 
 #include "arch-x86/xen.h"
 #elif defined(__arm__) || defined (__aarch64__)
 #include "arch-arm.h"
+#elif defined(__riscv)
+#include "arch-riscv.h"
 #else
 #error "Unsupported architecture"
 #endif