[PATCH v2 00/14] RISCV basic exception handling implementation

[PATCH v2 00/14] RISCV basic exception handling implementation

[Oleksii Kurochko]

The patch series is based on another one [Basic early_printk and smoke
test implementation] which hasn't been commited yet.

The patch series provides a basic implementation of exception handling.
It can do only basic things such as decode a cause of an exception,
save/restore registers and execute "wfi" instruction if an exception
can not be handled.

To verify that exception handling works well it was implemented macros
from <asm/bug.h> such as BUG/WARN/run_in_exception/assert_failed.
The implementation of macros is used "ebreak" instruction and set up bug
frame tables for each type of macros.
Also it was implemented register save/restore to return and continue work
after WARN/run_in_exception.
Not all functionality of the macros was implemented as some of them
require hard-panic the system which is not available now. Instead of
hard-panic 'wfi' instruction is used but it should be definitely changed
in the neareset future.
It wasn't implemented show_execution_state() and stack trace discovering
as it's not necessary now.

---
Changes in V2:
  - take the latest riscv_encoding.h from OpenSBI, update it with Xen
    related changes, and update the commit message with "Origin:"
    tag and the commit message itself.
  - add "Origin:" tag to the commit messag of the patch
    [xen/riscv: add <asm/csr.h> header].
  - Remove the patch [xen/riscv: add early_printk_hnum() function] as the
    functionality provided by the patch isn't used now.
  - Refactor prcoess.h: move structure offset defines to asm-offsets.c,
    change register_t to unsigned long.
  - Refactor entry.S to use offsets defined in asm-offsets.C
  - Rename {__,}handle_exception to handle_trap() and do_trap() to be more
    consistent with RISC-V spec.
  - Merge the pathc which introduces do_unexpected_trap() with the patch
    [xen/riscv: introduce exception handlers implementation].
  - Rename setup_trap_handler() to trap_init() and update correspondingly
    the patches in the patch series.
  - Refactor bug.h, remove bug_instr_t type from it.
  - Refactor decode_trap_cause() function to be more optimization-friendly.
  - Add two new empty headers: <cache.h> and <string.h> as they are needed to
    include <xen/lib.h> which provides ARRAY_SIZE and other macros.
  - Code style fixes.
---

Oleksii Kurochko (14):
  xen/riscv: add _zicsr to CFLAGS
  xen/riscv: add <asm/asm.h> header
  xen/riscv: add <asm/riscv_encoding.h header
  xen/riscv: add <asm/csr.h> header
  xen/riscv: introduce empty <asm/string.h>
  xen/riscv: introduce empty <asm/cache.h>
  xen/riscv: introduce exception context
  xen/riscv: introduce exception handlers implementation
  xen/riscv: introduce decode_cause() stuff
  xen/riscv: mask all interrupts
  xen/riscv: introduce trap_init()
  xen/riscv: introduce an implementation of macros from <asm/bug.h>
  xen/riscv: test basic handling stuff
  automation: add smoke test to verify macros from bug.h

 automation/scripts/qemu-smoke-riscv64.sh    |   2 +-
 xen/arch/riscv/Makefile                     |   2 +
 xen/arch/riscv/arch.mk                      |   2 +-
 xen/arch/riscv/entry.S                      |  94 ++
 xen/arch/riscv/include/asm/asm.h            |  54 ++
 xen/arch/riscv/include/asm/bug.h            | 118 +++
 xen/arch/riscv/include/asm/cache.h          |   6 +
 xen/arch/riscv/include/asm/csr.h            |  84 ++
 xen/arch/riscv/include/asm/processor.h      |  82 ++
 xen/arch/riscv/include/asm/riscv_encoding.h | 927 ++++++++++++++++++++
 xen/arch/riscv/include/asm/string.h         |   6 +
 xen/arch/riscv/include/asm/traps.h          |  14 +
 xen/arch/riscv/riscv64/asm-offsets.c        |  53 ++
 xen/arch/riscv/riscv64/head.S               |   5 +
 xen/arch/riscv/setup.c                      |  21 +
 xen/arch/riscv/traps.c                      | 231 +++++
 xen/arch/riscv/xen.lds.S                    |  10 +
 17 files changed, 1709 insertions(+), 2 deletions(-)
 create mode 100644 xen/arch/riscv/entry.S
 create mode 100644 xen/arch/riscv/include/asm/asm.h
 create mode 100644 xen/arch/riscv/include/asm/bug.h
 create mode 100644 xen/arch/riscv/include/asm/cache.h
 create mode 100644 xen/arch/riscv/include/asm/csr.h
 create mode 100644 xen/arch/riscv/include/asm/processor.h
 create mode 100644 xen/arch/riscv/include/asm/riscv_encoding.h
 create mode 100644 xen/arch/riscv/include/asm/string.h
 create mode 100644 xen/arch/riscv/include/asm/traps.h
 create mode 100644 xen/arch/riscv/traps.c

-- 
2.39.0

[PATCH v2 01/14] xen/riscv: add _zicsr to CFLAGS

[Oleksii Kurochko]

Work with some registers requires csr command which is part of
Zicsr.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Nothing changed
---
 xen/arch/riscv/arch.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 012dc677c3..95b41d9f3e 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -10,7 +10,7 @@ riscv-march-$(CONFIG_RISCV_ISA_C)       := $(riscv-march-y)c
 # into the upper half _or_ the lower half of the address space.
 # -mcmodel=medlow would force Xen into the lower half.
 
-CFLAGS += -march=$(riscv-march-y) -mstrict-align -mcmodel=medany
+CFLAGS += -march=$(riscv-march-y)_zicsr -mstrict-align -mcmodel=medany
 
 # TODO: Drop override when more of the build is working
 override ALL_OBJS-y = arch/$(TARGET_ARCH)/built_in.o
-- 
2.39.0

[PATCH v2 02/14] xen/riscv: add <asm/asm.h> header

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Nothing changed
---
 xen/arch/riscv/include/asm/asm.h | 54 ++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/asm.h

diff --git a/xen/arch/riscv/include/asm/asm.h b/xen/arch/riscv/include/asm/asm.h
new file mode 100644
index 0000000000..6d426ecea7
--- /dev/null
+++ b/xen/arch/riscv/include/asm/asm.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: (GPL-2.0-only) */
+/*
+ * Copyright (C) 2015 Regents of the University of California
+ */
+
+#ifndef _ASM_RISCV_ASM_H
+#define _ASM_RISCV_ASM_H
+
+#ifdef __ASSEMBLY__
+#define __ASM_STR(x)	x
+#else
+#define __ASM_STR(x)	#x
+#endif
+
+#if __riscv_xlen == 64
+#define __REG_SEL(a, b)	__ASM_STR(a)
+#elif __riscv_xlen == 32
+#define __REG_SEL(a, b)	__ASM_STR(b)
+#else
+#error "Unexpected __riscv_xlen"
+#endif
+
+#define REG_L		__REG_SEL(ld, lw)
+#define REG_S		__REG_SEL(sd, sw)
+
+#if __SIZEOF_POINTER__ == 8
+#ifdef __ASSEMBLY__
+#define RISCV_PTR		.dword
+#else
+#define RISCV_PTR		".dword"
+#endif
+#elif __SIZEOF_POINTER__ == 4
+#ifdef __ASSEMBLY__
+#define RISCV_PTR		.word
+#else
+#define RISCV_PTR		".word"
+#endif
+#else
+#error "Unexpected __SIZEOF_POINTER__"
+#endif
+
+#if (__SIZEOF_INT__ == 4)
+#define RISCV_INT		__ASM_STR(.word)
+#else
+#error "Unexpected __SIZEOF_INT__"
+#endif
+
+#if (__SIZEOF_SHORT__ == 2)
+#define RISCV_SHORT		__ASM_STR(.half)
+#else
+#error "Unexpected __SIZEOF_SHORT__"
+#endif
+
+#endif /* _ASM_RISCV_ASM_H */
-- 
2.39.0

[PATCH v2 03/14] xen/riscv: add <asm/riscv_encoding.h header

[Oleksii Kurochko]

The following changes were done in Xen code base in comparison with OpenSBI:
  * Remove "#include <sbi/sbi_const.h>" as most of the stuff inside
    it is present in Xen code base.
  * Add macros _UL and _ULL as they were in <sbi/sbi_const.h> before
  * Add SATP32_MODE_SHIFT/SATP64_MODE_SHIFT/SATP_MODE_SHIFT as they will
    be used in riscv/mm.c
  * Add CAUSE_IRQ_FLAG which is going to be used insised exception
    handler
  * Change ulong to unsigned long in macros REG_PTR(...)
  * Change s32 to int32_t

Originally authored by Anup Patel <anup.patel@wdc.com>

Origin: https://github.com/riscv-software-src/opensbi.git c45992cc2b12
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Take the latest version of riscv_encoding.h from OpenSBI.
  - Update riscv_encoding.h with Xen related changes mentioned in the
    commit message.
  - Update commit message and add "Origin:" tag
---
 xen/arch/riscv/include/asm/riscv_encoding.h | 927 ++++++++++++++++++++
 1 file changed, 927 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/riscv_encoding.h

diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h b/xen/arch/riscv/include/asm/riscv_encoding.h
new file mode 100644
index 0000000000..43dd4f6981
--- /dev/null
+++ b/xen/arch/riscv/include/asm/riscv_encoding.h
@@ -0,0 +1,927 @@
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ *
+ * Authors:
+ *   Anup Patel <anup.patel@wdc.com>
+ */
+
+#ifndef __RISCV_ENCODING_H__
+#define __RISCV_ENCODING_H__
+
+#define _UL(X) _AC(X, UL)
+#define _ULL(X) _AC(X, ULL)
+
+/* clang-format off */
+#define MSTATUS_SIE			_UL(0x00000002)
+#define MSTATUS_MIE			_UL(0x00000008)
+#define MSTATUS_SPIE_SHIFT		5
+#define MSTATUS_SPIE			(_UL(1) << MSTATUS_SPIE_SHIFT)
+#define MSTATUS_UBE			_UL(0x00000040)
+#define MSTATUS_MPIE			_UL(0x00000080)
+#define MSTATUS_SPP_SHIFT		8
+#define MSTATUS_SPP			(_UL(1) << MSTATUS_SPP_SHIFT)
+#define MSTATUS_MPP_SHIFT		11
+#define MSTATUS_MPP			(_UL(3) << MSTATUS_MPP_SHIFT)
+#define MSTATUS_FS			_UL(0x00006000)
+#define MSTATUS_XS			_UL(0x00018000)
+#define MSTATUS_VS			_UL(0x00000600)
+#define MSTATUS_MPRV			_UL(0x00020000)
+#define MSTATUS_SUM			_UL(0x00040000)
+#define MSTATUS_MXR			_UL(0x00080000)
+#define MSTATUS_TVM			_UL(0x00100000)
+#define MSTATUS_TW			_UL(0x00200000)
+#define MSTATUS_TSR			_UL(0x00400000)
+#define MSTATUS32_SD			_UL(0x80000000)
+#if __riscv_xlen == 64
+#define MSTATUS_UXL			_ULL(0x0000000300000000)
+#define MSTATUS_SXL			_ULL(0x0000000C00000000)
+#define MSTATUS_SBE			_ULL(0x0000001000000000)
+#define MSTATUS_MBE			_ULL(0x0000002000000000)
+#define MSTATUS_GVA			_ULL(0x0000004000000000)
+#define MSTATUS_GVA_SHIFT		38
+#define MSTATUS_MPV			_ULL(0x0000008000000000)
+#else
+#define MSTATUSH_SBE			_UL(0x00000010)
+#define MSTATUSH_MBE			_UL(0x00000020)
+#define MSTATUSH_GVA			_UL(0x00000040)
+#define MSTATUSH_GVA_SHIFT		6
+#define MSTATUSH_MPV			_UL(0x00000080)
+#endif
+#define MSTATUS32_SD			_UL(0x80000000)
+#define MSTATUS64_SD			_ULL(0x8000000000000000)
+
+#define SSTATUS_SIE			MSTATUS_SIE
+#define SSTATUS_SPIE_SHIFT		MSTATUS_SPIE_SHIFT
+#define SSTATUS_SPIE			MSTATUS_SPIE
+#define SSTATUS_SPP_SHIFT		MSTATUS_SPP_SHIFT
+#define SSTATUS_SPP			MSTATUS_SPP
+#define SSTATUS_FS			MSTATUS_FS
+#define SSTATUS_XS			MSTATUS_XS
+#define SSTATUS_VS			MSTATUS_VS
+#define SSTATUS_SUM			MSTATUS_SUM
+#define SSTATUS_MXR			MSTATUS_MXR
+#define SSTATUS32_SD			MSTATUS32_SD
+#define SSTATUS64_UXL			MSTATUS_UXL
+#define SSTATUS64_SD			MSTATUS64_SD
+
+#if __riscv_xlen == 64
+#define HSTATUS_VSXL			_UL(0x300000000)
+#define HSTATUS_VSXL_SHIFT		32
+#endif
+#define HSTATUS_VTSR			_UL(0x00400000)
+#define HSTATUS_VTW			_UL(0x00200000)
+#define HSTATUS_VTVM			_UL(0x00100000)
+#define HSTATUS_VGEIN			_UL(0x0003f000)
+#define HSTATUS_VGEIN_SHIFT		12
+#define HSTATUS_HU			_UL(0x00000200)
+#define HSTATUS_SPVP			_UL(0x00000100)
+#define HSTATUS_SPV			_UL(0x00000080)
+#define HSTATUS_GVA			_UL(0x00000040)
+#define HSTATUS_VSBE			_UL(0x00000020)
+
+#define IRQ_S_SOFT			1
+#define IRQ_VS_SOFT			2
+#define IRQ_M_SOFT			3
+#define IRQ_S_TIMER			5
+#define IRQ_VS_TIMER			6
+#define IRQ_M_TIMER			7
+#define IRQ_S_EXT			9
+#define IRQ_VS_EXT			10
+#define IRQ_M_EXT			11
+#define IRQ_S_GEXT			12
+#define IRQ_PMU_OVF			13
+
+#define MIP_SSIP			(_UL(1) << IRQ_S_SOFT)
+#define MIP_VSSIP			(_UL(1) << IRQ_VS_SOFT)
+#define MIP_MSIP			(_UL(1) << IRQ_M_SOFT)
+#define MIP_STIP			(_UL(1) << IRQ_S_TIMER)
+#define MIP_VSTIP			(_UL(1) << IRQ_VS_TIMER)
+#define MIP_MTIP			(_UL(1) << IRQ_M_TIMER)
+#define MIP_SEIP			(_UL(1) << IRQ_S_EXT)
+#define MIP_VSEIP			(_UL(1) << IRQ_VS_EXT)
+#define MIP_MEIP			(_UL(1) << IRQ_M_EXT)
+#define MIP_SGEIP			(_UL(1) << IRQ_S_GEXT)
+#define MIP_LCOFIP			(_UL(1) << IRQ_PMU_OVF)
+
+#define SIP_SSIP			MIP_SSIP
+#define SIP_STIP			MIP_STIP
+
+#define PRV_U				_UL(0)
+#define PRV_S				_UL(1)
+#define PRV_M				_UL(3)
+
+#define SATP32_MODE			_UL(0x80000000)
+#define SATP32_MODE_SHIFT		31
+#define SATP32_ASID			_UL(0x7FC00000)
+#define SATP32_PPN			_UL(0x003FFFFF)
+#define SATP64_MODE			_ULL(0xF000000000000000)
+#define SATP64_MODE_SHIFT		60
+#define SATP64_ASID			_ULL(0x0FFFF00000000000)
+#define SATP64_PPN			_ULL(0x00000FFFFFFFFFFF)
+
+#define SATP_MODE_OFF			_UL(0)
+#define SATP_MODE_SV32			_UL(1)
+#define SATP_MODE_SV39			_UL(8)
+#define SATP_MODE_SV48			_UL(9)
+#define SATP_MODE_SV57			_UL(10)
+#define SATP_MODE_SV64			_UL(11)
+
+#define HGATP_MODE_OFF			_UL(0)
+#define HGATP_MODE_SV32X4		_UL(1)
+#define HGATP_MODE_SV39X4		_UL(8)
+#define HGATP_MODE_SV48X4		_UL(9)
+
+#define HGATP32_MODE_SHIFT		31
+#define HGATP32_VMID_SHIFT		22
+#define HGATP32_VMID_MASK		_UL(0x1FC00000)
+#define HGATP32_PPN			_UL(0x003FFFFF)
+
+#define HGATP64_MODE_SHIFT		60
+#define HGATP64_VMID_SHIFT		44
+#define HGATP64_VMID_MASK		_ULL(0x03FFF00000000000)
+#define HGATP64_PPN			_ULL(0x00000FFFFFFFFFFF)
+
+#define PMP_R				_UL(0x01)
+#define PMP_W				_UL(0x02)
+#define PMP_X				_UL(0x04)
+#define PMP_A				_UL(0x18)
+#define PMP_A_TOR			_UL(0x08)
+#define PMP_A_NA4			_UL(0x10)
+#define PMP_A_NAPOT			_UL(0x18)
+#define PMP_L				_UL(0x80)
+
+#define PMP_SHIFT			2
+#define PMP_COUNT			64
+#if __riscv_xlen == 64
+#define PMP_ADDR_MASK			((_ULL(0x1) << 54) - 1)
+#else
+#define PMP_ADDR_MASK			_UL(0xFFFFFFFF)
+#endif
+
+#if __riscv_xlen == 64
+#define MSTATUS_SD			MSTATUS64_SD
+#define SSTATUS_SD			SSTATUS64_SD
+#define SATP_MODE			SATP64_MODE
+#define SATP_MODE_SHIFT			SATP64_MODE_SHIFT
+
+#define HGATP_PPN			HGATP64_PPN
+#define HGATP_VMID_SHIFT		HGATP64_VMID_SHIFT
+#define HGATP_VMID_MASK			HGATP64_VMID_MASK
+#define HGATP_MODE_SHIFT		HGATP64_MODE_SHIFT
+#else
+#define MSTATUS_SD			MSTATUS32_SD
+#define SSTATUS_SD			SSTATUS32_SD
+#define SATP_MODE			SATP32_MODE
+#define SATP_MODE_SHIFT			SATP32_MODE_SHIFT
+
+#define HGATP_PPN			HGATP32_PPN
+#define HGATP_VMID_SHIFT		HGATP32_VMID_SHIFT
+#define HGATP_VMID_MASK			HGATP32_VMID_MASK
+#define HGATP_MODE_SHIFT		HGATP32_MODE_SHIFT
+#endif
+
+#define TOPI_IID_SHIFT			16
+#define TOPI_IID_MASK			0xfff
+#define TOPI_IPRIO_MASK		0xff
+
+#if __riscv_xlen == 64
+#define MHPMEVENT_OF			(_UL(1) << 63)
+#define MHPMEVENT_MINH			(_UL(1) << 62)
+#define MHPMEVENT_SINH			(_UL(1) << 61)
+#define MHPMEVENT_UINH			(_UL(1) << 60)
+#define MHPMEVENT_VSINH			(_UL(1) << 59)
+#define MHPMEVENT_VUINH			(_UL(1) << 58)
+#else
+#define MHPMEVENTH_OF			(_ULL(1) << 31)
+#define MHPMEVENTH_MINH			(_ULL(1) << 30)
+#define MHPMEVENTH_SINH			(_ULL(1) << 29)
+#define MHPMEVENTH_UINH			(_ULL(1) << 28)
+#define MHPMEVENTH_VSINH		(_ULL(1) << 27)
+#define MHPMEVENTH_VUINH		(_ULL(1) << 26)
+
+#define MHPMEVENT_OF			(MHPMEVENTH_OF << 32)
+#define MHPMEVENT_MINH			(MHPMEVENTH_MINH << 32)
+#define MHPMEVENT_SINH			(MHPMEVENTH_SINH << 32)
+#define MHPMEVENT_UINH			(MHPMEVENTH_UINH << 32)
+#define MHPMEVENT_VSINH			(MHPMEVENTH_VSINH << 32)
+#define MHPMEVENT_VUINH			(MHPMEVENTH_VUINH << 32)
+
+#endif
+
+#define MHPMEVENT_SSCOF_MASK		_ULL(0xFFFF000000000000)
+
+#if __riscv_xlen > 32
+#define ENVCFG_STCE			(_ULL(1) << 63)
+#define ENVCFG_PBMTE			(_ULL(1) << 62)
+#else
+#define ENVCFGH_STCE			(_UL(1) << 31)
+#define ENVCFGH_PBMTE			(_UL(1) << 30)
+#endif
+#define ENVCFG_CBZE			(_UL(1) << 7)
+#define ENVCFG_CBCFE			(_UL(1) << 6)
+#define ENVCFG_CBIE_SHIFT		4
+#define ENVCFG_CBIE			(_UL(0x3) << ENVCFG_CBIE_SHIFT)
+#define ENVCFG_CBIE_ILL			_UL(0x0)
+#define ENVCFG_CBIE_FLUSH		_UL(0x1)
+#define ENVCFG_CBIE_INV			_UL(0x3)
+#define ENVCFG_FIOM			_UL(0x1)
+
+/* ===== User-level CSRs ===== */
+
+/* User Trap Setup (N-extension) */
+#define CSR_USTATUS			0x000
+#define CSR_UIE				0x004
+#define CSR_UTVEC			0x005
+
+/* User Trap Handling (N-extension) */
+#define CSR_USCRATCH			0x040
+#define CSR_UEPC			0x041
+#define CSR_UCAUSE			0x042
+#define CSR_UTVAL			0x043
+#define CSR_UIP				0x044
+
+/* User Floating-point CSRs */
+#define CSR_FFLAGS			0x001
+#define CSR_FRM				0x002
+#define CSR_FCSR			0x003
+
+/* User Counters/Timers */
+#define CSR_CYCLE			0xc00
+#define CSR_TIME			0xc01
+#define CSR_INSTRET			0xc02
+#define CSR_HPMCOUNTER3			0xc03
+#define CSR_HPMCOUNTER4			0xc04
+#define CSR_HPMCOUNTER5			0xc05
+#define CSR_HPMCOUNTER6			0xc06
+#define CSR_HPMCOUNTER7			0xc07
+#define CSR_HPMCOUNTER8			0xc08
+#define CSR_HPMCOUNTER9			0xc09
+#define CSR_HPMCOUNTER10		0xc0a
+#define CSR_HPMCOUNTER11		0xc0b
+#define CSR_HPMCOUNTER12		0xc0c
+#define CSR_HPMCOUNTER13		0xc0d
+#define CSR_HPMCOUNTER14		0xc0e
+#define CSR_HPMCOUNTER15		0xc0f
+#define CSR_HPMCOUNTER16		0xc10
+#define CSR_HPMCOUNTER17		0xc11
+#define CSR_HPMCOUNTER18		0xc12
+#define CSR_HPMCOUNTER19		0xc13
+#define CSR_HPMCOUNTER20		0xc14
+#define CSR_HPMCOUNTER21		0xc15
+#define CSR_HPMCOUNTER22		0xc16
+#define CSR_HPMCOUNTER23		0xc17
+#define CSR_HPMCOUNTER24		0xc18
+#define CSR_HPMCOUNTER25		0xc19
+#define CSR_HPMCOUNTER26		0xc1a
+#define CSR_HPMCOUNTER27		0xc1b
+#define CSR_HPMCOUNTER28		0xc1c
+#define CSR_HPMCOUNTER29		0xc1d
+#define CSR_HPMCOUNTER30		0xc1e
+#define CSR_HPMCOUNTER31		0xc1f
+#define CSR_CYCLEH			0xc80
+#define CSR_TIMEH			0xc81
+#define CSR_INSTRETH			0xc82
+#define CSR_HPMCOUNTER3H		0xc83
+#define CSR_HPMCOUNTER4H		0xc84
+#define CSR_HPMCOUNTER5H		0xc85
+#define CSR_HPMCOUNTER6H		0xc86
+#define CSR_HPMCOUNTER7H		0xc87
+#define CSR_HPMCOUNTER8H		0xc88
+#define CSR_HPMCOUNTER9H		0xc89
+#define CSR_HPMCOUNTER10H		0xc8a
+#define CSR_HPMCOUNTER11H		0xc8b
+#define CSR_HPMCOUNTER12H		0xc8c
+#define CSR_HPMCOUNTER13H		0xc8d
+#define CSR_HPMCOUNTER14H		0xc8e
+#define CSR_HPMCOUNTER15H		0xc8f
+#define CSR_HPMCOUNTER16H		0xc90
+#define CSR_HPMCOUNTER17H		0xc91
+#define CSR_HPMCOUNTER18H		0xc92
+#define CSR_HPMCOUNTER19H		0xc93
+#define CSR_HPMCOUNTER20H		0xc94
+#define CSR_HPMCOUNTER21H		0xc95
+#define CSR_HPMCOUNTER22H		0xc96
+#define CSR_HPMCOUNTER23H		0xc97
+#define CSR_HPMCOUNTER24H		0xc98
+#define CSR_HPMCOUNTER25H		0xc99
+#define CSR_HPMCOUNTER26H		0xc9a
+#define CSR_HPMCOUNTER27H		0xc9b
+#define CSR_HPMCOUNTER28H		0xc9c
+#define CSR_HPMCOUNTER29H		0xc9d
+#define CSR_HPMCOUNTER30H		0xc9e
+#define CSR_HPMCOUNTER31H		0xc9f
+
+/* ===== Supervisor-level CSRs ===== */
+
+/* Supervisor Trap Setup */
+#define CSR_SSTATUS			0x100
+#define CSR_SIE				0x104
+#define CSR_STVEC			0x105
+#define CSR_SCOUNTEREN			0x106
+
+/* Supervisor Configuration */
+#define CSR_SENVCFG			0x10a
+
+/* Supervisor Trap Handling */
+#define CSR_SSCRATCH			0x140
+#define CSR_SEPC			0x141
+#define CSR_SCAUSE			0x142
+#define CSR_STVAL			0x143
+#define CSR_SIP				0x144
+
+/* Sstc extension */
+#define CSR_STIMECMP			0x14D
+#define CSR_STIMECMPH			0x15D
+
+/* Supervisor Protection and Translation */
+#define CSR_SATP			0x180
+
+/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
+#define CSR_SISELECT			0x150
+#define CSR_SIREG			0x151
+
+/* Supervisor-Level Interrupts (AIA) */
+#define CSR_STOPEI			0x15c
+#define CSR_STOPI			0xdb0
+
+/* Supervisor-Level High-Half CSRs (AIA) */
+#define CSR_SIEH			0x114
+#define CSR_SIPH			0x154
+
+/* Supervisor stateen CSRs */
+#define CSR_SSTATEEN0			0x10C
+#define CSR_SSTATEEN1			0x10D
+#define CSR_SSTATEEN2			0x10E
+#define CSR_SSTATEEN3			0x10F
+
+/* ===== Hypervisor-level CSRs ===== */
+
+/* Hypervisor Trap Setup (H-extension) */
+#define CSR_HSTATUS			0x600
+#define CSR_HEDELEG			0x602
+#define CSR_HIDELEG			0x603
+#define CSR_HIE				0x604
+#define CSR_HCOUNTEREN			0x606
+#define CSR_HGEIE			0x607
+
+/* Hypervisor Configuration */
+#define CSR_HENVCFG			0x60a
+#define CSR_HENVCFGH			0x61a
+
+/* Hypervisor Trap Handling (H-extension) */
+#define CSR_HTVAL			0x643
+#define CSR_HIP				0x644
+#define CSR_HVIP			0x645
+#define CSR_HTINST			0x64a
+#define CSR_HGEIP			0xe12
+
+/* Hypervisor Protection and Translation (H-extension) */
+#define CSR_HGATP			0x680
+
+/* Hypervisor Counter/Timer Virtualization Registers (H-extension) */
+#define CSR_HTIMEDELTA			0x605
+#define CSR_HTIMEDELTAH			0x615
+
+/* Virtual Supervisor Registers (H-extension) */
+#define CSR_VSSTATUS			0x200
+#define CSR_VSIE			0x204
+#define CSR_VSTVEC			0x205
+#define CSR_VSSCRATCH			0x240
+#define CSR_VSEPC			0x241
+#define CSR_VSCAUSE			0x242
+#define CSR_VSTVAL			0x243
+#define CSR_VSIP			0x244
+#define CSR_VSATP			0x280
+
+/* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
+#define CSR_HVIEN			0x608
+#define CSR_HVICTL			0x609
+#define CSR_HVIPRIO1			0x646
+#define CSR_HVIPRIO2			0x647
+
+/* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA) */
+#define CSR_VSISELECT			0x250
+#define CSR_VSIREG			0x251
+
+/* VS-Level Interrupts (H-extension with AIA) */
+#define CSR_VSTOPEI			0x25c
+#define CSR_VSTOPI			0xeb0
+
+/* Hypervisor and VS-Level High-Half CSRs (H-extension with AIA) */
+#define CSR_HIDELEGH			0x613
+#define CSR_HVIENH			0x618
+#define CSR_HVIPH			0x655
+#define CSR_HVIPRIO1H			0x656
+#define CSR_HVIPRIO2H			0x657
+#define CSR_VSIEH			0x214
+#define CSR_VSIPH			0x254
+
+/* Hypervisor stateen CSRs */
+#define CSR_HSTATEEN0			0x60C
+#define CSR_HSTATEEN0H			0x61C
+#define CSR_HSTATEEN1			0x60D
+#define CSR_HSTATEEN1H			0x61D
+#define CSR_HSTATEEN2			0x60E
+#define CSR_HSTATEEN2H			0x61E
+#define CSR_HSTATEEN3			0x60F
+#define CSR_HSTATEEN3H			0x61F
+
+/* ===== Machine-level CSRs ===== */
+
+/* Machine Information Registers */
+#define CSR_MVENDORID			0xf11
+#define CSR_MARCHID			0xf12
+#define CSR_MIMPID			0xf13
+#define CSR_MHARTID			0xf14
+
+/* Machine Trap Setup */
+#define CSR_MSTATUS			0x300
+#define CSR_MISA			0x301
+#define CSR_MEDELEG			0x302
+#define CSR_MIDELEG			0x303
+#define CSR_MIE				0x304
+#define CSR_MTVEC			0x305
+#define CSR_MCOUNTEREN			0x306
+#define CSR_MSTATUSH			0x310
+
+/* Machine Configuration */
+#define CSR_MENVCFG			0x30a
+#define CSR_MENVCFGH			0x31a
+
+/* Machine Trap Handling */
+#define CSR_MSCRATCH			0x340
+#define CSR_MEPC			0x341
+#define CSR_MCAUSE			0x342
+#define CSR_MTVAL			0x343
+#define CSR_MIP				0x344
+#define CSR_MTINST			0x34a
+#define CSR_MTVAL2			0x34b
+
+/* Machine Memory Protection */
+#define CSR_PMPCFG0			0x3a0
+#define CSR_PMPCFG1			0x3a1
+#define CSR_PMPCFG2			0x3a2
+#define CSR_PMPCFG3			0x3a3
+#define CSR_PMPCFG4			0x3a4
+#define CSR_PMPCFG5			0x3a5
+#define CSR_PMPCFG6			0x3a6
+#define CSR_PMPCFG7			0x3a7
+#define CSR_PMPCFG8			0x3a8
+#define CSR_PMPCFG9			0x3a9
+#define CSR_PMPCFG10			0x3aa
+#define CSR_PMPCFG11			0x3ab
+#define CSR_PMPCFG12			0x3ac
+#define CSR_PMPCFG13			0x3ad
+#define CSR_PMPCFG14			0x3ae
+#define CSR_PMPCFG15			0x3af
+#define CSR_PMPADDR0			0x3b0
+#define CSR_PMPADDR1			0x3b1
+#define CSR_PMPADDR2			0x3b2
+#define CSR_PMPADDR3			0x3b3
+#define CSR_PMPADDR4			0x3b4
+#define CSR_PMPADDR5			0x3b5
+#define CSR_PMPADDR6			0x3b6
+#define CSR_PMPADDR7			0x3b7
+#define CSR_PMPADDR8			0x3b8
+#define CSR_PMPADDR9			0x3b9
+#define CSR_PMPADDR10			0x3ba
+#define CSR_PMPADDR11			0x3bb
+#define CSR_PMPADDR12			0x3bc
+#define CSR_PMPADDR13			0x3bd
+#define CSR_PMPADDR14			0x3be
+#define CSR_PMPADDR15			0x3bf
+#define CSR_PMPADDR16			0x3c0
+#define CSR_PMPADDR17			0x3c1
+#define CSR_PMPADDR18			0x3c2
+#define CSR_PMPADDR19			0x3c3
+#define CSR_PMPADDR20			0x3c4
+#define CSR_PMPADDR21			0x3c5
+#define CSR_PMPADDR22			0x3c6
+#define CSR_PMPADDR23			0x3c7
+#define CSR_PMPADDR24			0x3c8
+#define CSR_PMPADDR25			0x3c9
+#define CSR_PMPADDR26			0x3ca
+#define CSR_PMPADDR27			0x3cb
+#define CSR_PMPADDR28			0x3cc
+#define CSR_PMPADDR29			0x3cd
+#define CSR_PMPADDR30			0x3ce
+#define CSR_PMPADDR31			0x3cf
+#define CSR_PMPADDR32			0x3d0
+#define CSR_PMPADDR33			0x3d1
+#define CSR_PMPADDR34			0x3d2
+#define CSR_PMPADDR35			0x3d3
+#define CSR_PMPADDR36			0x3d4
+#define CSR_PMPADDR37			0x3d5
+#define CSR_PMPADDR38			0x3d6
+#define CSR_PMPADDR39			0x3d7
+#define CSR_PMPADDR40			0x3d8
+#define CSR_PMPADDR41			0x3d9
+#define CSR_PMPADDR42			0x3da
+#define CSR_PMPADDR43			0x3db
+#define CSR_PMPADDR44			0x3dc
+#define CSR_PMPADDR45			0x3dd
+#define CSR_PMPADDR46			0x3de
+#define CSR_PMPADDR47			0x3df
+#define CSR_PMPADDR48			0x3e0
+#define CSR_PMPADDR49			0x3e1
+#define CSR_PMPADDR50			0x3e2
+#define CSR_PMPADDR51			0x3e3
+#define CSR_PMPADDR52			0x3e4
+#define CSR_PMPADDR53			0x3e5
+#define CSR_PMPADDR54			0x3e6
+#define CSR_PMPADDR55			0x3e7
+#define CSR_PMPADDR56			0x3e8
+#define CSR_PMPADDR57			0x3e9
+#define CSR_PMPADDR58			0x3ea
+#define CSR_PMPADDR59			0x3eb
+#define CSR_PMPADDR60			0x3ec
+#define CSR_PMPADDR61			0x3ed
+#define CSR_PMPADDR62			0x3ee
+#define CSR_PMPADDR63			0x3ef
+
+/* Machine Counters/Timers */
+#define CSR_MCYCLE			0xb00
+#define CSR_MINSTRET			0xb02
+#define CSR_MHPMCOUNTER3		0xb03
+#define CSR_MHPMCOUNTER4		0xb04
+#define CSR_MHPMCOUNTER5		0xb05
+#define CSR_MHPMCOUNTER6		0xb06
+#define CSR_MHPMCOUNTER7		0xb07
+#define CSR_MHPMCOUNTER8		0xb08
+#define CSR_MHPMCOUNTER9		0xb09
+#define CSR_MHPMCOUNTER10		0xb0a
+#define CSR_MHPMCOUNTER11		0xb0b
+#define CSR_MHPMCOUNTER12		0xb0c
+#define CSR_MHPMCOUNTER13		0xb0d
+#define CSR_MHPMCOUNTER14		0xb0e
+#define CSR_MHPMCOUNTER15		0xb0f
+#define CSR_MHPMCOUNTER16		0xb10
+#define CSR_MHPMCOUNTER17		0xb11
+#define CSR_MHPMCOUNTER18		0xb12
+#define CSR_MHPMCOUNTER19		0xb13
+#define CSR_MHPMCOUNTER20		0xb14
+#define CSR_MHPMCOUNTER21		0xb15
+#define CSR_MHPMCOUNTER22		0xb16
+#define CSR_MHPMCOUNTER23		0xb17
+#define CSR_MHPMCOUNTER24		0xb18
+#define CSR_MHPMCOUNTER25		0xb19
+#define CSR_MHPMCOUNTER26		0xb1a
+#define CSR_MHPMCOUNTER27		0xb1b
+#define CSR_MHPMCOUNTER28		0xb1c
+#define CSR_MHPMCOUNTER29		0xb1d
+#define CSR_MHPMCOUNTER30		0xb1e
+#define CSR_MHPMCOUNTER31		0xb1f
+#define CSR_MCYCLEH			0xb80
+#define CSR_MINSTRETH			0xb82
+#define CSR_MHPMCOUNTER3H		0xb83
+#define CSR_MHPMCOUNTER4H		0xb84
+#define CSR_MHPMCOUNTER5H		0xb85
+#define CSR_MHPMCOUNTER6H		0xb86
+#define CSR_MHPMCOUNTER7H		0xb87
+#define CSR_MHPMCOUNTER8H		0xb88
+#define CSR_MHPMCOUNTER9H		0xb89
+#define CSR_MHPMCOUNTER10H		0xb8a
+#define CSR_MHPMCOUNTER11H		0xb8b
+#define CSR_MHPMCOUNTER12H		0xb8c
+#define CSR_MHPMCOUNTER13H		0xb8d
+#define CSR_MHPMCOUNTER14H		0xb8e
+#define CSR_MHPMCOUNTER15H		0xb8f
+#define CSR_MHPMCOUNTER16H		0xb90
+#define CSR_MHPMCOUNTER17H		0xb91
+#define CSR_MHPMCOUNTER18H		0xb92
+#define CSR_MHPMCOUNTER19H		0xb93
+#define CSR_MHPMCOUNTER20H		0xb94
+#define CSR_MHPMCOUNTER21H		0xb95
+#define CSR_MHPMCOUNTER22H		0xb96
+#define CSR_MHPMCOUNTER23H		0xb97
+#define CSR_MHPMCOUNTER24H		0xb98
+#define CSR_MHPMCOUNTER25H		0xb99
+#define CSR_MHPMCOUNTER26H		0xb9a
+#define CSR_MHPMCOUNTER27H		0xb9b
+#define CSR_MHPMCOUNTER28H		0xb9c
+#define CSR_MHPMCOUNTER29H		0xb9d
+#define CSR_MHPMCOUNTER30H		0xb9e
+#define CSR_MHPMCOUNTER31H		0xb9f
+
+/* Machine Counter Setup */
+#define CSR_MCOUNTINHIBIT		0x320
+#define CSR_MHPMEVENT3			0x323
+#define CSR_MHPMEVENT4			0x324
+#define CSR_MHPMEVENT5			0x325
+#define CSR_MHPMEVENT6			0x326
+#define CSR_MHPMEVENT7			0x327
+#define CSR_MHPMEVENT8			0x328
+#define CSR_MHPMEVENT9			0x329
+#define CSR_MHPMEVENT10			0x32a
+#define CSR_MHPMEVENT11			0x32b
+#define CSR_MHPMEVENT12			0x32c
+#define CSR_MHPMEVENT13			0x32d
+#define CSR_MHPMEVENT14			0x32e
+#define CSR_MHPMEVENT15			0x32f
+#define CSR_MHPMEVENT16			0x330
+#define CSR_MHPMEVENT17			0x331
+#define CSR_MHPMEVENT18			0x332
+#define CSR_MHPMEVENT19			0x333
+#define CSR_MHPMEVENT20			0x334
+#define CSR_MHPMEVENT21			0x335
+#define CSR_MHPMEVENT22			0x336
+#define CSR_MHPMEVENT23			0x337
+#define CSR_MHPMEVENT24			0x338
+#define CSR_MHPMEVENT25			0x339
+#define CSR_MHPMEVENT26			0x33a
+#define CSR_MHPMEVENT27			0x33b
+#define CSR_MHPMEVENT28			0x33c
+#define CSR_MHPMEVENT29			0x33d
+#define CSR_MHPMEVENT30			0x33e
+#define CSR_MHPMEVENT31			0x33f
+
+/* For RV32 */
+#define CSR_MHPMEVENT3H			0x723
+#define CSR_MHPMEVENT4H			0x724
+#define CSR_MHPMEVENT5H			0x725
+#define CSR_MHPMEVENT6H			0x726
+#define CSR_MHPMEVENT7H			0x727
+#define CSR_MHPMEVENT8H			0x728
+#define CSR_MHPMEVENT9H			0x729
+#define CSR_MHPMEVENT10H		0x72a
+#define CSR_MHPMEVENT11H		0x72b
+#define CSR_MHPMEVENT12H		0x72c
+#define CSR_MHPMEVENT13H		0x72d
+#define CSR_MHPMEVENT14H		0x72e
+#define CSR_MHPMEVENT15H		0x72f
+#define CSR_MHPMEVENT16H		0x730
+#define CSR_MHPMEVENT17H		0x731
+#define CSR_MHPMEVENT18H		0x732
+#define CSR_MHPMEVENT19H		0x733
+#define CSR_MHPMEVENT20H		0x734
+#define CSR_MHPMEVENT21H		0x735
+#define CSR_MHPMEVENT22H		0x736
+#define CSR_MHPMEVENT23H		0x737
+#define CSR_MHPMEVENT24H		0x738
+#define CSR_MHPMEVENT25H		0x739
+#define CSR_MHPMEVENT26H		0x73a
+#define CSR_MHPMEVENT27H		0x73b
+#define CSR_MHPMEVENT28H		0x73c
+#define CSR_MHPMEVENT29H		0x73d
+#define CSR_MHPMEVENT30H		0x73e
+#define CSR_MHPMEVENT31H		0x73f
+
+/* Counter Overflow CSR */
+#define CSR_SCOUNTOVF			0xda0
+
+/* Debug/Trace Registers */
+#define CSR_TSELECT			0x7a0
+#define CSR_TDATA1			0x7a1
+#define CSR_TDATA2			0x7a2
+#define CSR_TDATA3			0x7a3
+
+/* Debug Mode Registers */
+#define CSR_DCSR			0x7b0
+#define CSR_DPC				0x7b1
+#define CSR_DSCRATCH0			0x7b2
+#define CSR_DSCRATCH1			0x7b3
+
+/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
+#define CSR_MISELECT			0x350
+#define CSR_MIREG			0x351
+
+/* Machine-Level Interrupts (AIA) */
+#define CSR_MTOPEI			0x35c
+#define CSR_MTOPI			0xfb0
+
+/* Virtual Interrupts for Supervisor Level (AIA) */
+#define CSR_MVIEN			0x308
+#define CSR_MVIP			0x309
+
+/* Smstateen extension registers */
+/* Machine stateen CSRs */
+#define CSR_MSTATEEN0			0x30C
+#define CSR_MSTATEEN0H			0x31C
+#define CSR_MSTATEEN1			0x30D
+#define CSR_MSTATEEN1H			0x31D
+#define CSR_MSTATEEN2			0x30E
+#define CSR_MSTATEEN2H			0x31E
+#define CSR_MSTATEEN3			0x30F
+#define CSR_MSTATEEN3H			0x31F
+
+/* Machine-Level High-Half CSRs (AIA) */
+#define CSR_MIDELEGH			0x313
+#define CSR_MIEH			0x314
+#define CSR_MVIENH			0x318
+#define CSR_MVIPH			0x319
+#define CSR_MIPH			0x354
+
+/* ===== Trap/Exception Causes ===== */
+
+/* Exception cause high bit - is an interrupt if set */
+#define CAUSE_IRQ_FLAG			(_UL(1) << (__riscv_xlen - 1))
+
+#define CAUSE_MISALIGNED_FETCH		0x0
+#define CAUSE_FETCH_ACCESS		0x1
+#define CAUSE_ILLEGAL_INSTRUCTION	0x2
+#define CAUSE_BREAKPOINT		0x3
+#define CAUSE_MISALIGNED_LOAD		0x4
+#define CAUSE_LOAD_ACCESS		0x5
+#define CAUSE_MISALIGNED_STORE		0x6
+#define CAUSE_STORE_ACCESS		0x7
+#define CAUSE_USER_ECALL		0x8
+#define CAUSE_SUPERVISOR_ECALL		0x9
+#define CAUSE_VIRTUAL_SUPERVISOR_ECALL	0xa
+#define CAUSE_MACHINE_ECALL		0xb
+#define CAUSE_FETCH_PAGE_FAULT		0xc
+#define CAUSE_LOAD_PAGE_FAULT		0xd
+#define CAUSE_STORE_PAGE_FAULT		0xf
+#define CAUSE_FETCH_GUEST_PAGE_FAULT	0x14
+#define CAUSE_LOAD_GUEST_PAGE_FAULT	0x15
+#define CAUSE_VIRTUAL_INST_FAULT	0x16
+#define CAUSE_STORE_GUEST_PAGE_FAULT	0x17
+
+/* Common defines for all smstateen */
+#define SMSTATEEN_MAX_COUNT		4
+#define SMSTATEEN0_CS_SHIFT		0
+#define SMSTATEEN0_CS			(_ULL(1) << SMSTATEEN0_CS_SHIFT)
+#define SMSTATEEN0_FCSR_SHIFT		1
+#define SMSTATEEN0_FCSR			(_ULL(1) << SMSTATEEN0_FCSR_SHIFT)
+#define SMSTATEEN0_IMSIC_SHIFT		58
+#define SMSTATEEN0_IMSIC		(_ULL(1) << SMSTATEEN0_IMSIC_SHIFT)
+#define SMSTATEEN0_AIA_SHIFT		59
+#define SMSTATEEN0_AIA			(_ULL(1) << SMSTATEEN0_AIA_SHIFT)
+#define SMSTATEEN0_SVSLCT_SHIFT		60
+#define SMSTATEEN0_SVSLCT		(_ULL(1) << SMSTATEEN0_SVSLCT_SHIFT)
+#define SMSTATEEN0_HSENVCFG_SHIFT	62
+#define SMSTATEEN0_HSENVCFG		(_ULL(1) << SMSTATEEN0_HSENVCFG_SHIFT)
+#define SMSTATEEN_STATEN_SHIFT		63
+#define SMSTATEEN_STATEN		(_ULL(1) << SMSTATEEN_STATEN_SHIFT)
+
+/* ===== Instruction Encodings ===== */
+
+#define INSN_MATCH_LB			0x3
+#define INSN_MASK_LB			0x707f
+#define INSN_MATCH_LH			0x1003
+#define INSN_MASK_LH			0x707f
+#define INSN_MATCH_LW			0x2003
+#define INSN_MASK_LW			0x707f
+#define INSN_MATCH_LD			0x3003
+#define INSN_MASK_LD			0x707f
+#define INSN_MATCH_LBU			0x4003
+#define INSN_MASK_LBU			0x707f
+#define INSN_MATCH_LHU			0x5003
+#define INSN_MASK_LHU			0x707f
+#define INSN_MATCH_LWU			0x6003
+#define INSN_MASK_LWU			0x707f
+#define INSN_MATCH_SB			0x23
+#define INSN_MASK_SB			0x707f
+#define INSN_MATCH_SH			0x1023
+#define INSN_MASK_SH			0x707f
+#define INSN_MATCH_SW			0x2023
+#define INSN_MASK_SW			0x707f
+#define INSN_MATCH_SD			0x3023
+#define INSN_MASK_SD			0x707f
+
+#define INSN_MATCH_FLW			0x2007
+#define INSN_MASK_FLW			0x707f
+#define INSN_MATCH_FLD			0x3007
+#define INSN_MASK_FLD			0x707f
+#define INSN_MATCH_FLQ			0x4007
+#define INSN_MASK_FLQ			0x707f
+#define INSN_MATCH_FSW			0x2027
+#define INSN_MASK_FSW			0x707f
+#define INSN_MATCH_FSD			0x3027
+#define INSN_MASK_FSD			0x707f
+#define INSN_MATCH_FSQ			0x4027
+#define INSN_MASK_FSQ			0x707f
+
+#define INSN_MATCH_C_LD			0x6000
+#define INSN_MASK_C_LD			0xe003
+#define INSN_MATCH_C_SD			0xe000
+#define INSN_MASK_C_SD			0xe003
+#define INSN_MATCH_C_LW			0x4000
+#define INSN_MASK_C_LW			0xe003
+#define INSN_MATCH_C_SW			0xc000
+#define INSN_MASK_C_SW			0xe003
+#define INSN_MATCH_C_LDSP		0x6002
+#define INSN_MASK_C_LDSP		0xe003
+#define INSN_MATCH_C_SDSP		0xe002
+#define INSN_MASK_C_SDSP		0xe003
+#define INSN_MATCH_C_LWSP		0x4002
+#define INSN_MASK_C_LWSP		0xe003
+#define INSN_MATCH_C_SWSP		0xc002
+#define INSN_MASK_C_SWSP		0xe003
+
+#define INSN_MATCH_C_FLD		0x2000
+#define INSN_MASK_C_FLD			0xe003
+#define INSN_MATCH_C_FLW		0x6000
+#define INSN_MASK_C_FLW			0xe003
+#define INSN_MATCH_C_FSD		0xa000
+#define INSN_MASK_C_FSD			0xe003
+#define INSN_MATCH_C_FSW		0xe000
+#define INSN_MASK_C_FSW			0xe003
+#define INSN_MATCH_C_FLDSP		0x2002
+#define INSN_MASK_C_FLDSP		0xe003
+#define INSN_MATCH_C_FSDSP		0xa002
+#define INSN_MASK_C_FSDSP		0xe003
+#define INSN_MATCH_C_FLWSP		0x6002
+#define INSN_MASK_C_FLWSP		0xe003
+#define INSN_MATCH_C_FSWSP		0xe002
+#define INSN_MASK_C_FSWSP		0xe003
+
+#define INSN_MASK_WFI			0xffffff00
+#define INSN_MATCH_WFI			0x10500000
+
+#define INSN_MASK_FENCE_TSO		0xffffffff
+#define INSN_MATCH_FENCE_TSO		0x8330000f
+
+#if __riscv_xlen == 64
+
+/* 64-bit read for VS-stage address translation (RV64) */
+#define INSN_PSEUDO_VS_LOAD		0x00003000
+
+/* 64-bit write for VS-stage address translation (RV64) */
+#define INSN_PSEUDO_VS_STORE	0x00003020
+
+#elif __riscv_xlen == 32
+
+/* 32-bit read for VS-stage address translation (RV32) */
+#define INSN_PSEUDO_VS_LOAD		0x00002000
+
+/* 32-bit write for VS-stage address translation (RV32) */
+#define INSN_PSEUDO_VS_STORE	0x00002020
+
+#else
+#error "Unexpected __riscv_xlen"
+#endif
+
+#define INSN_16BIT_MASK			0x3
+#define INSN_32BIT_MASK			0x1c
+
+#define INSN_IS_16BIT(insn)		\
+	(((insn) & INSN_16BIT_MASK) != INSN_16BIT_MASK)
+#define INSN_IS_32BIT(insn)		\
+	(((insn) & INSN_16BIT_MASK) == INSN_16BIT_MASK && \
+	 ((insn) & INSN_32BIT_MASK) != INSN_32BIT_MASK)
+
+#define INSN_LEN(insn)			(INSN_IS_16BIT(insn) ? 2 : 4)
+
+#if __riscv_xlen == 64
+#define LOG_REGBYTES			3
+#else
+#define LOG_REGBYTES			2
+#endif
+#define REGBYTES			(1 << LOG_REGBYTES)
+
+#define SH_RD				7
+#define SH_RS1				15
+#define SH_RS2				20
+#define SH_RS2C				2
+
+#define RV_X(x, s, n)			(((x) >> (s)) & ((1 << (n)) - 1))
+#define RVC_LW_IMM(x)			((RV_X(x, 6, 1) << 2) | \
+					 (RV_X(x, 10, 3) << 3) | \
+					 (RV_X(x, 5, 1) << 6))
+#define RVC_LD_IMM(x)			((RV_X(x, 10, 3) << 3) | \
+					 (RV_X(x, 5, 2) << 6))
+#define RVC_LWSP_IMM(x)			((RV_X(x, 4, 3) << 2) | \
+					 (RV_X(x, 12, 1) << 5) | \
+					 (RV_X(x, 2, 2) << 6))
+#define RVC_LDSP_IMM(x)			((RV_X(x, 5, 2) << 3) | \
+					 (RV_X(x, 12, 1) << 5) | \
+					 (RV_X(x, 2, 3) << 6))
+#define RVC_SWSP_IMM(x)			((RV_X(x, 9, 4) << 2) | \
+					 (RV_X(x, 7, 2) << 6))
+#define RVC_SDSP_IMM(x)			((RV_X(x, 10, 3) << 3) | \
+					 (RV_X(x, 7, 3) << 6))
+#define RVC_RS1S(insn)			(8 + RV_X(insn, SH_RD, 3))
+#define RVC_RS2S(insn)			(8 + RV_X(insn, SH_RS2C, 3))
+#define RVC_RS2(insn)			RV_X(insn, SH_RS2C, 5)
+
+#define SHIFT_RIGHT(x, y)		\
+	((y) < 0 ? ((x) << -(y)) : ((x) >> (y)))
+
+#define REG_MASK			\
+	((1 << (5 + LOG_REGBYTES)) - (1 << LOG_REGBYTES))
+
+#define REG_OFFSET(insn, pos)		\
+	(SHIFT_RIGHT((insn), (pos) - LOG_REGBYTES) & REG_MASK)
+
+#define REG_PTR(insn, pos, regs)	\
+	(unsigned long *)((unsigned long)(regs) + REG_OFFSET(insn, pos))
+
+#define GET_RM(insn)			(((insn) >> 12) & 7)
+
+#define GET_RS1(insn, regs)		(*REG_PTR(insn, SH_RS1, regs))
+#define GET_RS2(insn, regs)		(*REG_PTR(insn, SH_RS2, regs))
+#define GET_RS1S(insn, regs)		(*REG_PTR(RVC_RS1S(insn), 0, regs))
+#define GET_RS2S(insn, regs)		(*REG_PTR(RVC_RS2S(insn), 0, regs))
+#define GET_RS2C(insn, regs)		(*REG_PTR(insn, SH_RS2C, regs))
+#define GET_SP(regs)			(*REG_PTR(2, 0, regs))
+#define SET_RD(insn, regs, val)		(*REG_PTR(insn, SH_RD, regs) = (val))
+#define IMM_I(insn)			((int32_t)(insn) >> 20)
+#define IMM_S(insn)			(((int32_t)(insn) >> 25 << 5) | \
+					 (int32_t)(((insn) >> 7) & 0x1f))
+#define MASK_FUNCT3			0x7000
+
+/* clang-format on */
+
+#endif
-- 
2.39.0

[PATCH v2 04/14] xen/riscv: add <asm/csr.h> header

[Oleksii Kurochko]

The following changes were made in comparison with <asm/csr.h> from
Linux:
  * remove all defines as they are defined in riscv_encoding.h
  * leave only csr_* macros

Origin: https://github.com/torvalds/linux.git 2475bf0250de
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Minor refactoring mentioned in the commit message, switch tabs to
    spaces and refactor things around __asm__ __volatile__.
  - Update the commit message and add "Origin:" tag.
---
 xen/arch/riscv/include/asm/csr.h | 84 ++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/csr.h

diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
new file mode 100644
index 0000000000..4275cf6515
--- /dev/null
+++ b/xen/arch/riscv/include/asm/csr.h
@@ -0,0 +1,84 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright (C) 2015 Regents of the University of California
+ */
+
+#ifndef _ASM_RISCV_CSR_H
+#define _ASM_RISCV_CSR_H
+
+#include <asm/asm.h>
+#include <xen/const.h>
+#include <asm/riscv_encoding.h>
+
+#ifndef __ASSEMBLY__
+
+#define csr_read(csr)                                           \
+({                                                              \
+    register unsigned long __v;                                 \
+    __asm__ __volatile__ (  "csrr %0, " __ASM_STR(csr)          \
+                            : "=r" (__v)                        \
+                            : : "memory" );                     \
+    __v;                                                        \
+})
+
+#define csr_write(csr, val)                                     \
+({                                                              \
+    unsigned long __v = (unsigned long)(val);                   \
+    __asm__ __volatile__ (  "csrw " __ASM_STR(csr) ", %0"       \
+                            : /* no outputs */                  \
+                            : "rK" (__v)                        \
+                            : "memory" );                       \
+})
+
+#define csr_swap(csr, val)                                      \
+({                                                              \
+    unsigned long __v = (unsigned long)(val);                   \
+    __asm__ __volatile__ (  "csrrw %0, " __ASM_STR(csr) ", %1"  \
+                            : "=r" (__v)                        \
+                            : "rK" (__v)                        \
+                            : "memory" );                       \
+    __v;                                                        \
+})
+
+#define csr_read_set(csr, val)                                  \
+({                                                              \
+    unsigned long __v = (unsigned long)(val);                   \
+    __asm__ __volatile__ (  "csrrs %0, " __ASM_STR(csr) ", %1"  \
+                            : "=r" (__v)                        \
+                            : "rK" (__v)                        \
+                            : "memory" );                       \
+    __v;                                                        \
+})
+
+#define csr_set(csr, val)                                       \
+({                                                              \
+    unsigned long __v = (unsigned long)(val);                   \
+    __asm__ __volatile__ (  "csrs " __ASM_STR(csr) ", %0"       \
+                            : /* no outputs */                  \
+                            : "rK" (__v)                        \
+                            : "memory" );                       \
+})
+
+#define csr_read_clear(csr, val)                                \
+({                                                              \
+    unsigned long __v = (unsigned long)(val);                   \
+    __asm__ __volatile__ (  "csrrc %0, " __ASM_STR(csr) ", %1"  \
+                            : "=r" (__v)                        \
+                            : "rK" (__v)                        \
+                            : "memory" );                       \
+    __v;                                                        \
+})
+
+#define csr_clear(csr, val)                                     \
+({                                                              \
+    unsigned long __v = (unsigned long)(val);                   \
+    __asm__ __volatile__ (  "csrc " __ASM_STR(csr) ", %0"       \
+                            : /*no outputs */                   \
+                            : "rK" (__v)                        \
+                            : "memory" );                       \
+})
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_RISCV_CSR_H */
-- 
2.39.0

[PATCH v2 05/14] xen/riscv: introduce empty <asm/string.h>

[Oleksii Kurochko]

To include <xen/lib.h> <asm/string.h> is required

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - <asm/string.h> is a new empty header which is required to include
    <xen/lib.h>
---
 xen/arch/riscv/include/asm/string.h | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/string.h

diff --git a/xen/arch/riscv/include/asm/string.h b/xen/arch/riscv/include/asm/string.h
new file mode 100644
index 0000000000..a26ba8f5c6
--- /dev/null
+++ b/xen/arch/riscv/include/asm/string.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_RISCV_STRING_H
+#define _ASM_RISCV_STRING_H
+
+#endif /* _ASM_RISCV_STRING_H */
-- 
2.39.0

[PATCH v2 06/14] xen/riscv: introduce empty <asm/cache.h>

[Oleksii Kurochko]

To include <xen/lib.h> <asm/cache.h> is required

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - <asm/cache.h> is a new empty header which is required to include
    <xen/lib.h>
---
 xen/arch/riscv/include/asm/cache.h | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/cache.h

diff --git a/xen/arch/riscv/include/asm/cache.h b/xen/arch/riscv/include/asm/cache.h
new file mode 100644
index 0000000000..69573eb051
--- /dev/null
+++ b/xen/arch/riscv/include/asm/cache.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_RISCV_CACHE_H
+#define _ASM_RISCV_CACHE_H
+
+#endif /* _ASM_RISCV_CACHE_H */
-- 
2.39.0

[PATCH v2 07/14] xen/riscv: introduce exception context

[Oleksii Kurochko]

The patch introduces a set of registers which should be saved to and
restored from a stack after an exception occurs and a set of defines
which will be used during exception context saving/restoring.

Originally <asm/processor.h> header was introduced in the patch series
from Bobby so partially it was
re-used and the following changes were done:
  - Move all RISCV_CPU_USER_REGS_* to asm/asm-offsets.c
  - Remove RISCV_CPU_USER_REGS_OFFSET & RISCV_CPU_USER_REGS_SIZE as
    there is no sense in them after RISCV_CPU_USER_REGS_* were moved to
    asm/asm-offsets.c
  - Remove RISCV_PCPUINFO_* as they aren't needed for current status of
    the RISC-V port
  - register_t renamed to unsigned long
  - rename wait_for_interrupt to wfi

Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - All the changes were added to the commit message.
  - temporarily was added function die() to stop exectution it will be
    removed after panic() will be available.
---
 xen/arch/riscv/include/asm/processor.h | 82 ++++++++++++++++++++++++++
 xen/arch/riscv/riscv64/asm-offsets.c   | 53 +++++++++++++++++
 2 files changed, 135 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/processor.h

diff --git a/xen/arch/riscv/include/asm/processor.h b/xen/arch/riscv/include/asm/processor.h
new file mode 100644
index 0000000000..4292de2efc
--- /dev/null
+++ b/xen/arch/riscv/include/asm/processor.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: MIT */
+/******************************************************************************
+ *
+ * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
+ * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
+ * Copyright 2023 (C) Vates
+ *
+ */
+
+#ifndef _ASM_RISCV_PROCESSOR_H
+#define _ASM_RISCV_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+/* On stack VCPU state */
+struct cpu_user_regs
+{
+    unsigned long zero;
+    unsigned long ra;
+    unsigned long sp;
+    unsigned long gp;
+    unsigned long tp;
+    unsigned long t0;
+    unsigned long t1;
+    unsigned long t2;
+    unsigned long s0;
+    unsigned long s1;
+    unsigned long a0;
+    unsigned long a1;
+    unsigned long a2;
+    unsigned long a3;
+    unsigned long a4;
+    unsigned long a5;
+    unsigned long a6;
+    unsigned long a7;
+    unsigned long s2;
+    unsigned long s3;
+    unsigned long s4;
+    unsigned long s5;
+    unsigned long s6;
+    unsigned long s7;
+    unsigned long s8;
+    unsigned long s9;
+    unsigned long s10;
+    unsigned long s11;
+    unsigned long t3;
+    unsigned long t4;
+    unsigned long t5;
+    unsigned long t6;
+    unsigned long sepc;
+    unsigned long sstatus;
+    /* pointer to previous stack_cpu_regs */
+    unsigned long pregs;
+};
+
+static inline void wfi(void)
+{
+    __asm__ __volatile__ ("wfi");
+}
+
+/* 
+ * panic() isn't available at the moment so an infinite loop will be
+ * used temporarily.
+ * TODO: change it to panic()
+ */
+static inline void die(void)
+{
+    for( ;; ) wfi();
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* _ASM_RISCV_PROCESSOR_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/riscv/riscv64/asm-offsets.c b/xen/arch/riscv/riscv64/asm-offsets.c
index e69de29bb2..d632b75c2a 100644
--- a/xen/arch/riscv/riscv64/asm-offsets.c
+++ b/xen/arch/riscv/riscv64/asm-offsets.c
@@ -0,0 +1,53 @@
+#define COMPILE_OFFSETS
+
+#include <asm/processor.h>
+#include <xen/types.h>
+
+#define DEFINE(_sym, _val)                                                 \
+    asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
+                  : : "i" (_val) )
+#define BLANK()                                                            \
+    asm volatile ( "\n.ascii\"==><==\"" : : )
+#define OFFSET(_sym, _str, _mem)                                           \
+    DEFINE(_sym, offsetof(_str, _mem));
+
+void asm_offsets(void)
+{
+    BLANK();
+    DEFINE(CPU_USER_REGS_SIZE, sizeof(struct cpu_user_regs));
+    OFFSET(CPU_USER_REGS_ZERO, struct cpu_user_regs, zero);
+    OFFSET(CPU_USER_REGS_RA, struct cpu_user_regs, ra);
+    OFFSET(CPU_USER_REGS_SP, struct cpu_user_regs, sp);
+    OFFSET(CPU_USER_REGS_GP, struct cpu_user_regs, gp);
+    OFFSET(CPU_USER_REGS_TP, struct cpu_user_regs, tp);
+    OFFSET(CPU_USER_REGS_T0, struct cpu_user_regs, t0);
+    OFFSET(CPU_USER_REGS_T1, struct cpu_user_regs, t1);
+    OFFSET(CPU_USER_REGS_T2, struct cpu_user_regs, t2);
+    OFFSET(CPU_USER_REGS_S0, struct cpu_user_regs, s0);
+    OFFSET(CPU_USER_REGS_S1, struct cpu_user_regs, s1);
+    OFFSET(CPU_USER_REGS_A0, struct cpu_user_regs, a0);
+    OFFSET(CPU_USER_REGS_A1, struct cpu_user_regs, a1);
+    OFFSET(CPU_USER_REGS_A2, struct cpu_user_regs, a2);
+    OFFSET(CPU_USER_REGS_A3, struct cpu_user_regs, a3);
+    OFFSET(CPU_USER_REGS_A4, struct cpu_user_regs, a4);
+    OFFSET(CPU_USER_REGS_A5, struct cpu_user_regs, a5);
+    OFFSET(CPU_USER_REGS_A6, struct cpu_user_regs, a6);
+    OFFSET(CPU_USER_REGS_A7, struct cpu_user_regs, a7);
+    OFFSET(CPU_USER_REGS_S2, struct cpu_user_regs, s2);
+    OFFSET(CPU_USER_REGS_S3, struct cpu_user_regs, s3);
+    OFFSET(CPU_USER_REGS_S4, struct cpu_user_regs, s4);
+    OFFSET(CPU_USER_REGS_S5, struct cpu_user_regs, s5);
+    OFFSET(CPU_USER_REGS_S6, struct cpu_user_regs, s6);
+    OFFSET(CPU_USER_REGS_S7, struct cpu_user_regs, s7);
+    OFFSET(CPU_USER_REGS_S8, struct cpu_user_regs, s8);
+    OFFSET(CPU_USER_REGS_S9, struct cpu_user_regs, s9);
+    OFFSET(CPU_USER_REGS_S10, struct cpu_user_regs, s10);
+    OFFSET(CPU_USER_REGS_S11, struct cpu_user_regs, s11);
+    OFFSET(CPU_USER_REGS_T3, struct cpu_user_regs, t3);
+    OFFSET(CPU_USER_REGS_T4, struct cpu_user_regs, t4);
+    OFFSET(CPU_USER_REGS_T5, struct cpu_user_regs, t5);
+    OFFSET(CPU_USER_REGS_T6, struct cpu_user_regs, t6);
+    OFFSET(CPU_USER_REGS_SEPC, struct cpu_user_regs, sepc);
+    OFFSET(CPU_USER_REGS_SSTATUS, struct cpu_user_regs, sstatus);
+    OFFSET(CPU_USER_REGS_PREGS, struct cpu_user_regs, pregs);
+}
-- 
2.39.0

[PATCH v2 08/14] xen/riscv: introduce exception handlers implementation

[Oleksii Kurochko]

The patch introduces an implementation of basic exception handlers:
- to save/restore context
- to handle an exception itself. The handler calls wait_for_interrupt
  now, nothing more.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Refactor entry.S to start using of defines introduced in asm_offsets.C
  - Rename {__,}handle_exception to handle_trap() and do_trap() to be more
    consistent with RISC-V spec.
  - Wrap handle_trap() to ENTRY().
---
 xen/arch/riscv/Makefile            |  2 +
 xen/arch/riscv/entry.S             | 94 ++++++++++++++++++++++++++++++
 xen/arch/riscv/include/asm/traps.h | 13 +++++
 xen/arch/riscv/traps.c             | 13 +++++
 4 files changed, 122 insertions(+)
 create mode 100644 xen/arch/riscv/entry.S
 create mode 100644 xen/arch/riscv/include/asm/traps.h
 create mode 100644 xen/arch/riscv/traps.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 1a4f1a6015..443f6bf15f 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,7 +1,9 @@
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
+obj-y += entry.o
 obj-$(CONFIG_RISCV_64) += riscv64/
 obj-y += sbi.o
 obj-y += setup.o
+obj-y += traps.o
 
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
diff --git a/xen/arch/riscv/entry.S b/xen/arch/riscv/entry.S
new file mode 100644
index 0000000000..0be543f8e0
--- /dev/null
+++ b/xen/arch/riscv/entry.S
@@ -0,0 +1,94 @@
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/processor.h>
+#include <asm/riscv_encoding.h>
+#include <asm/traps.h>
+
+/* WIP: only works while interrupting Xen context */
+ENTRY(handle_trap)
+
+    /* Exceptions from xen */
+save_to_stack:
+        /* Save context to stack */
+        REG_S   sp, (CPU_USER_REGS_SP - CPU_USER_REGS_SIZE) (sp)
+        addi    sp, sp, -CPU_USER_REGS_SIZE
+        REG_S   t0, CPU_USER_REGS_T0(sp)
+
+        /* Save registers */
+        REG_S   ra, CPU_USER_REGS_RA(sp)
+        REG_S   gp, CPU_USER_REGS_GP(sp)
+        REG_S   t1, CPU_USER_REGS_T1(sp)
+        REG_S   t2, CPU_USER_REGS_T2(sp)
+        REG_S   s0, CPU_USER_REGS_S0(sp)
+        REG_S   s1, CPU_USER_REGS_S1(sp)
+        REG_S   a0, CPU_USER_REGS_A0(sp)
+        REG_S   a1, CPU_USER_REGS_A1(sp)
+        REG_S   a2, CPU_USER_REGS_A2(sp)
+        REG_S   a3, CPU_USER_REGS_A3(sp)
+        REG_S   a4, CPU_USER_REGS_A4(sp)
+        REG_S   a5, CPU_USER_REGS_A5(sp)
+        REG_S   a6, CPU_USER_REGS_A6(sp)
+        REG_S   a7, CPU_USER_REGS_A7(sp)
+        REG_S   s2, CPU_USER_REGS_S2(sp)
+        REG_S   s3, CPU_USER_REGS_S3(sp)
+        REG_S   s4, CPU_USER_REGS_S4(sp)
+        REG_S   s5, CPU_USER_REGS_S5(sp)
+        REG_S   s6, CPU_USER_REGS_S6(sp)
+        REG_S   s7, CPU_USER_REGS_S7(sp)
+        REG_S   s8, CPU_USER_REGS_S8(sp)
+        REG_S   s9, CPU_USER_REGS_S9(sp)
+        REG_S   s10,CPU_USER_REGS_S10(sp)
+        REG_S   s11,CPU_USER_REGS_S11(sp)
+        REG_S   t3, CPU_USER_REGS_T3(sp)
+        REG_S   t4, CPU_USER_REGS_T4(sp)
+        REG_S   t5, CPU_USER_REGS_T5(sp)
+        REG_S   t6, CPU_USER_REGS_T6(sp)
+        csrr    t0, CSR_SEPC
+        REG_S   t0, CPU_USER_REGS_SEPC(sp)
+        csrr    t0, CSR_SSTATUS
+        REG_S   t0, CPU_USER_REGS_SSTATUS(sp)
+
+        mv      a0, sp
+        jal     do_trap
+
+restore_registers:
+        /* Restore stack_cpu_regs */
+        REG_L   t0, CPU_USER_REGS_SEPC(sp)
+        csrw    CSR_SEPC, t0
+        REG_L   t0, CPU_USER_REGS_SSTATUS(sp)
+        csrw    CSR_SSTATUS, t0
+
+        REG_L   ra, CPU_USER_REGS_RA(sp)
+        REG_L   gp, CPU_USER_REGS_GP(sp)
+        REG_L   t0, CPU_USER_REGS_T0(sp)
+        REG_L   t1, CPU_USER_REGS_T1(sp)
+        REG_L   t2, CPU_USER_REGS_T2(sp)
+        REG_L   s0, CPU_USER_REGS_S0(sp)
+        REG_L   s1, CPU_USER_REGS_S1(sp)
+        REG_L   a0, CPU_USER_REGS_A0(sp)
+        REG_L   a1, CPU_USER_REGS_A1(sp)
+        REG_L   a2, CPU_USER_REGS_A2(sp)
+        REG_L   a3, CPU_USER_REGS_A3(sp)
+        REG_L   a4, CPU_USER_REGS_A4(sp)
+        REG_L   a5, CPU_USER_REGS_A5(sp)
+        REG_L   a6, CPU_USER_REGS_A6(sp)
+        REG_L   a7, CPU_USER_REGS_A7(sp)
+        REG_L   s2, CPU_USER_REGS_S2(sp)
+        REG_L   s3, CPU_USER_REGS_S3(sp)
+        REG_L   s4, CPU_USER_REGS_S4(sp)
+        REG_L   s5, CPU_USER_REGS_S5(sp)
+        REG_L   s6, CPU_USER_REGS_S6(sp)
+        REG_L   s7, CPU_USER_REGS_S7(sp)
+        REG_L   s8, CPU_USER_REGS_S8(sp)
+        REG_L   s9, CPU_USER_REGS_S9(sp)
+        REG_L   s10, CPU_USER_REGS_S10(sp)
+        REG_L   s11, CPU_USER_REGS_S11(sp)
+        REG_L   t3, CPU_USER_REGS_T3(sp)
+        REG_L   t4, CPU_USER_REGS_T4(sp)
+        REG_L   t5, CPU_USER_REGS_T5(sp)
+        REG_L   t6, CPU_USER_REGS_T6(sp)
+
+        /* Restore sp */
+        REG_L   sp, CPU_USER_REGS_SP(sp)
+
+        sret
diff --git a/xen/arch/riscv/include/asm/traps.h b/xen/arch/riscv/include/asm/traps.h
new file mode 100644
index 0000000000..f3fb6b25d1
--- /dev/null
+++ b/xen/arch/riscv/include/asm/traps.h
@@ -0,0 +1,13 @@
+#ifndef __ASM_TRAPS_H__
+#define __ASM_TRAPS_H__
+
+#include <asm/processor.h>
+
+#ifndef __ASSEMBLY__
+
+void do_trap(struct cpu_user_regs *cpu_regs);
+void handle_trap(void);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_TRAPS_H__ */
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
new file mode 100644
index 0000000000..ccd3593f5a
--- /dev/null
+++ b/xen/arch/riscv/traps.c
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2023 Vates
+ *
+ * RISC-V Trap handlers
+ */
+#include <asm/processor.h>
+#include <asm/traps.h>
+
+void do_trap(struct cpu_user_regs *cpu_regs)
+{
+    die();
+}
-- 
2.39.0

[PATCH v2 09/14] xen/riscv: introduce decode_cause() stuff

[Oleksii Kurochko]

The patch introduces stuff needed to decode a reason of an
exception.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Make decode_trap_cause() more optimization friendly.
  - Merge the pathc which introduces do_unexpected_trap() to the current one.
---
 xen/arch/riscv/traps.c | 85 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index ccd3593f5a..f2a1e1ffcf 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -4,10 +4,93 @@
  *
  * RISC-V Trap handlers
  */
+#include <asm/csr.h>
+#include <asm/early_printk.h>
 #include <asm/processor.h>
 #include <asm/traps.h>
+#include <xen/errno.h>
+#include <xen/lib.h>
 
-void do_trap(struct cpu_user_regs *cpu_regs)
+static const char *decode_trap_cause(unsigned long cause)
+{
+    static const char *const trap_causes[] = {
+        [CAUSE_MISALIGNED_FETCH] = "Instruction Address Misaligned",
+        [CAUSE_FETCH_ACCESS] = "Instruction Access Fault",
+        [CAUSE_ILLEGAL_INSTRUCTION] = "Illegal Instruction",
+        [CAUSE_BREAKPOINT] = "Breakpoint",
+        [CAUSE_MISALIGNED_LOAD] = "Load Address Misaligned",
+        [CAUSE_LOAD_ACCESS] = "Load Access Fault",
+        [CAUSE_MISALIGNED_STORE] = "Store/AMO Address Misaligned",
+        [CAUSE_STORE_ACCESS] = "Store/AMO Access Fault",
+        [CAUSE_USER_ECALL] = "Environment Call from U-Mode",
+        [CAUSE_SUPERVISOR_ECALL] = "Environment Call from S-Mode",
+        [CAUSE_MACHINE_ECALL] = "Environment Call from M-Mode",
+        [CAUSE_FETCH_PAGE_FAULT] = "Instruction Page Fault",
+        [CAUSE_LOAD_PAGE_FAULT] = "Load Page Fault",
+        [CAUSE_STORE_PAGE_FAULT] = "Store/AMO Page Fault",
+        [CAUSE_FETCH_GUEST_PAGE_FAULT] = "Instruction Guest Page Fault",
+        [CAUSE_LOAD_GUEST_PAGE_FAULT] = "Load Guest Page Fault",
+        [CAUSE_VIRTUAL_INST_FAULT] = "Virtualized Instruction Fault",
+        [CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page Fault",
+    };
+
+    if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
+        return trap_causes[cause];
+    return "UNKNOWN";
+}
+
+const char *decode_reserved_interrupt_cause(unsigned long irq_cause)
+{
+    switch ( irq_cause )
+    {
+    case IRQ_M_SOFT:
+        return "M-mode Software Interrupt";
+    case IRQ_M_TIMER:
+        return "M-mode TIMER Interrupt";
+    case IRQ_M_EXT:
+        return "M-mode TIMER Interrupt";
+    default:
+        return "UNKNOWN IRQ type";
+    }
+}
+
+const char *decode_interrupt_cause(unsigned long cause)
+{
+    unsigned long irq_cause = cause & ~CAUSE_IRQ_FLAG;
+
+    switch ( irq_cause )
+    {
+    case IRQ_S_SOFT:
+        return "Supervisor Software Interrupt";
+    case IRQ_S_TIMER:
+        return "Supervisor Timer Interrupt";
+    case IRQ_S_EXT:
+        return "Supervisor External Interrupt";
+    default:
+        return decode_reserved_interrupt_cause(irq_cause);
+    }
+}
+
+const char *decode_cause(unsigned long cause)
+{
+    if ( cause & CAUSE_IRQ_FLAG )
+        return decode_interrupt_cause(cause);
+
+    return decode_trap_cause(cause);
+}
+
+static void do_unexpected_trap(const struct cpu_user_regs *regs)
 {
+    unsigned long cause = csr_read(CSR_SCAUSE);
+
+    early_printk("Unhandled exception: ");
+    early_printk(decode_cause(cause));
+    early_printk("\n");
+
     die();
 }
+
+void do_trap(struct cpu_user_regs *cpu_regs)
+{
+    do_unexpected_trap(cpu_regs);
+}
-- 
2.39.0

[PATCH v2 10/14] xen/riscv: mask all interrupts

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
Changes in V2:
 - Add Reviewed-by to the commit message
---
 xen/arch/riscv/riscv64/head.S | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index d444dd8aad..ffd95f9f89 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -1,6 +1,11 @@
+#include <asm/riscv_encoding.h>
+
         .section .text.header, "ax", %progbits
 
 ENTRY(start)
+        /* Mask all interrupts */
+        csrw    CSR_SIE, zero
+
         la      sp, cpu0_boot_stack
         li      t0, STACK_SIZE
         add     sp, sp, t0
-- 
2.39.0

[PATCH v2 11/14] xen/riscv: introduce trap_init()

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
Changes in V2:
  - Rename setup_trap_handler() to trap_init().
  - Add Reviewed-by to the commit message.
---
 xen/arch/riscv/include/asm/traps.h | 1 +
 xen/arch/riscv/setup.c             | 4 ++++
 xen/arch/riscv/traps.c             | 7 +++++++
 3 files changed, 12 insertions(+)

diff --git a/xen/arch/riscv/include/asm/traps.h b/xen/arch/riscv/include/asm/traps.h
index f3fb6b25d1..f1879294ef 100644
--- a/xen/arch/riscv/include/asm/traps.h
+++ b/xen/arch/riscv/include/asm/traps.h
@@ -7,6 +7,7 @@
 
 void do_trap(struct cpu_user_regs *cpu_regs);
 void handle_trap(void);
+void trap_init(void);
 
 #endif /* __ASSEMBLY__ */
 
diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index d09ffe1454..c8513ca4f8 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,7 +1,9 @@
 #include <xen/compile.h>
 #include <xen/init.h>
 
+#include <asm/csr.h>
 #include <asm/early_printk.h>
+#include <asm/traps.h>
 
 /* Xen stack for bringing up the first CPU. */
 unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
@@ -11,6 +13,8 @@ void __init noreturn start_xen(void)
 {
     early_printk("Hello from C env\n");
 
+    trap_init();
+
     for ( ;; )
         asm volatile ("wfi");
 
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index f2a1e1ffcf..31ed63e3c1 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -11,6 +11,13 @@
 #include <xen/errno.h>
 #include <xen/lib.h>
 
+void trap_init(void)
+{
+    unsigned long addr = (unsigned long)&handle_trap;
+
+    csr_write(CSR_STVEC, addr);
+}
+
 static const char *decode_trap_cause(unsigned long cause)
 {
     static const char *const trap_causes[] = {
-- 
2.39.0

[PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii Kurochko]

The patch introduces macros: BUG(), WARN(), run_in_exception(),
assert_failed.

The implementation uses "ebreak" instruction in combination with
diffrent bug frame tables (for each type) which contains useful
information.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes:
  - Remove __ in define namings
  - Update run_in_exception_handler() with
    register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
  - Remove bug_instr_t type and change it's usage to uint32_t
---
 xen/arch/riscv/include/asm/bug.h | 118 ++++++++++++++++++++++++++++
 xen/arch/riscv/traps.c           | 128 +++++++++++++++++++++++++++++++
 xen/arch/riscv/xen.lds.S         |  10 +++
 3 files changed, 256 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/bug.h

diff --git a/xen/arch/riscv/include/asm/bug.h b/xen/arch/riscv/include/asm/bug.h
new file mode 100644
index 0000000000..4b15d8eba6
--- /dev/null
+++ b/xen/arch/riscv/include/asm/bug.h
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2021-2023 Vates
+ *
+ */
+
+#ifndef _ASM_RISCV_BUG_H
+#define _ASM_RISCV_BUG_H
+
+#include <xen/stringify.h>
+#include <xen/types.h>
+
+#ifndef __ASSEMBLY__
+
+struct bug_frame {
+    signed int loc_disp;    /* Relative address to the bug address */
+    signed int file_disp;   /* Relative address to the filename */
+    signed int msg_disp;    /* Relative address to the predicate (for ASSERT) */
+    uint16_t line;          /* Line number */
+    uint32_t pad0:16;       /* Padding for 8-bytes align */
+};
+
+#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
+#define bug_file(b) ((const void *)(b) + (b)->file_disp);
+#define bug_line(b) ((b)->line)
+#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
+
+#define BUGFRAME_run_fn 0
+#define BUGFRAME_warn   1
+#define BUGFRAME_bug    2
+#define BUGFRAME_assert 3
+
+#define BUGFRAME_NR     4
+
+#define INSN_LENGTH_MASK        _UL(0x3)
+#define INSN_LENGTH_32          _UL(0x3)
+
+#define BUG_INSN_32             _UL(0x00100073) /* ebreak */
+#define BUG_INSN_16             _UL(0x9002) /* c.ebreak */
+#define COMPRESSED_INSN_MASK    _UL(0xffff)
+
+#define GET_INSN_LENGTH(insn)                               \
+({                                                          \
+    unsigned long len;                                      \
+    len = ((insn & INSN_LENGTH_MASK) == INSN_LENGTH_32) ?   \
+        4UL : 2UL;                                          \
+    len;                                                    \
+})
+
+/* These are defined by the architecture */
+int is_valid_bugaddr(uint32_t addr);
+
+#define BUG_FN_REG t0
+
+/* Many versions of GCC doesn't support the asm %c parameter which would
+ * be preferable to this unpleasantness. We use mergeable string
+ * sections to avoid multiple copies of the string appearing in the
+ * Xen image. BUGFRAME_run_fn needs to be handled separately.
+ */
+#define BUG_FRAME(type, line, file, has_msg, msg) do {                      \
+    asm ("1:ebreak\n"                                                       \
+         ".pushsection .rodata.str, \"aMS\", %progbits, 1\n"                \
+         "2:\t.asciz " __stringify(file) "\n"                               \
+         "3:\n"                                                             \
+         ".if " #has_msg "\n"                                               \
+         "\t.asciz " #msg "\n"                                              \
+         ".endif\n"                                                         \
+         ".popsection\n"                                                    \
+         ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
+         "4:\n"                                                             \
+         ".p2align 2\n"                                                     \
+         ".long (1b - 4b)\n"                                                \
+         ".long (2b - 4b)\n"                                                \
+         ".long (3b - 4b)\n"                                                \
+         ".hword " __stringify(line) ", 0\n"                                \
+         ".popsection");                                                    \
+} while (0)
+
+/*
+ * GCC will not allow to use "i"  when PIE is enabled (Xen doesn't set the
+ * flag but instead rely on the default value from the compiler). So the
+ * easiest way to implement run_in_exception_handler() is to pass the to
+ * be called function in a fixed register.
+ */
+#define  run_in_exception_handler(fn) do {                                  \
+    register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);                 \
+    asm ("1:ebreak\n"                                                       \
+         ".pushsection .bug_frames." __stringify(BUGFRAME_run_fn) ","       \
+         "             \"a\", %%progbits\n"                                 \
+         "2:\n"                                                             \
+         ".p2align 2\n"                                                     \
+         ".long (1b - 2b)\n"                                                \
+         ".long 0, 0, 0\n"                                                  \
+         ".popsection" :: "r" (fn_) );                                      \
+} while (0)
+
+#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
+
+#define BUG() do {                                              \
+    BUG_FRAME(BUGFRAME_bug,  __LINE__, __FILE__, 0, "");        \
+    unreachable();                                              \
+} while (0)
+
+#define assert_failed(msg) do {                                 \
+    BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg);     \
+    unreachable();                                              \
+} while (0)
+
+extern const struct bug_frame __start_bug_frames[],
+                              __stop_bug_frames_0[],
+                              __stop_bug_frames_1[],
+                              __stop_bug_frames_2[],
+                              __stop_bug_frames_3[];
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* _ASM_RISCV_BUG_H */
diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c
index 31ed63e3c1..0afb8e4e42 100644
--- a/xen/arch/riscv/traps.c
+++ b/xen/arch/riscv/traps.c
@@ -4,6 +4,7 @@
  *
  * RISC-V Trap handlers
  */
+#include <asm/bug.h>
 #include <asm/csr.h>
 #include <asm/early_printk.h>
 #include <asm/processor.h>
@@ -97,7 +98,134 @@ static void do_unexpected_trap(const struct cpu_user_regs *regs)
     die();
 }
 
+void show_execution_state(const struct cpu_user_regs *regs)
+{
+    early_printk("implement show_execution_state(regs)\n");
+}
+
+int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc)
+{
+    struct bug_frame *start, *end;
+    struct bug_frame *bug = NULL;
+    unsigned int id = 0;
+    const char *filename, *predicate;
+    int lineno;
+
+    unsigned long bug_frames[] = {
+        (unsigned long)&__start_bug_frames[0],
+        (unsigned long)&__stop_bug_frames_0[0],
+        (unsigned long)&__stop_bug_frames_1[0],
+        (unsigned long)&__stop_bug_frames_2[0],
+        (unsigned long)&__stop_bug_frames_3[0],
+    };
+
+    for ( id = 0; id < BUGFRAME_NR; id++ )
+    {
+        start = (struct  bug_frame *)bug_frames[id];
+        end = (struct  bug_frame *)bug_frames[id + 1];
+
+        while ( start != end )
+        {
+            if ( (vaddr_t)bug_loc(start) == pc )
+            {
+                bug = start;
+                goto found;
+            }
+
+            start++;
+        }
+    }
+
+ found:
+    if ( bug == NULL )
+        return -ENOENT;
+
+    if ( id == BUGFRAME_run_fn )
+    {
+        void (*fn)(const struct cpu_user_regs *) = (void *)regs->BUG_FN_REG;
+
+        fn(regs);
+
+        goto end;
+    }
+
+    /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
+    filename = bug_file(bug);
+    lineno = bug_line(bug);
+
+    switch ( id )
+    {
+    case BUGFRAME_warn:
+        /*
+         * TODO: change early_printk's function to early_printk with format
+         *       when s(n)printf() will be added.
+         */
+        early_printk("Xen WARN at ");
+        early_printk(filename);
+        early_printk(":");
+        // early_printk_hnum(lineno);
+
+        show_execution_state(regs);
+
+        goto end;
+
+    case BUGFRAME_bug:
+         /*
+          * TODO: change early_printk's function to early_printk with format
+          *       when s(n)printf() will be added.
+          */
+        early_printk("Xen BUG at ");
+        early_printk(filename);
+        early_printk(":");
+        // early_printk_hnum(lineno);
+
+        show_execution_state(regs);
+        early_printk("change wait_for_interrupt to panic() when common is available\n");
+        die();
+
+    case BUGFRAME_assert:
+        /* ASSERT: decode the predicate string pointer. */
+        predicate = bug_msg(bug);
+
+        /*
+         * TODO: change early_printk's function to early_printk with format
+         *       when s(n)printf() will be added.
+         */
+        early_printk("Assertion \'");
+        early_printk(predicate);
+        early_printk("\' failed at ");
+        early_printk(filename);
+        early_printk(":");
+        // early_printk_hnum(lineno);
+
+        show_execution_state(regs);
+        early_printk("change wait_for_interrupt to panic() when common is available\n");
+        die();
+    }
+
+    return -EINVAL;
+
+ end:
+    regs->sepc += GET_INSN_LENGTH(*(uint32_t *)pc);
+
+    return 0;
+}
+
+int is_valid_bugaddr(uint32_t insn)
+{
+    if ((insn & INSN_LENGTH_MASK) == INSN_LENGTH_32)
+        return (insn == BUG_INSN_32);
+    else
+        return ((insn & COMPRESSED_INSN_MASK) == BUG_INSN_16);
+}
+
 void do_trap(struct cpu_user_regs *cpu_regs)
 {
+    register_t pc = cpu_regs->sepc;
+    uint32_t instr = *(uint32_t *)pc;
+
+    if (is_valid_bugaddr(instr))
+        if (!do_bug_frame(cpu_regs, pc)) return;
+
     do_unexpected_trap(cpu_regs);
 }
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index ca57cce75c..139e2d04cb 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -39,6 +39,16 @@ SECTIONS
     . = ALIGN(PAGE_SIZE);
     .rodata : {
         _srodata = .;          /* Read-only data */
+        /* Bug frames table */
+       __start_bug_frames = .;
+       *(.bug_frames.0)
+       __stop_bug_frames_0 = .;
+       *(.bug_frames.1)
+       __stop_bug_frames_1 = .;
+       *(.bug_frames.2)
+       __stop_bug_frames_2 = .;
+       *(.bug_frames.3)
+       __stop_bug_frames_3 = .;
         *(.rodata)
         *(.rodata.*)
         *(.data.rel.ro)
-- 
2.39.0

[PATCH v2 13/14] xen/riscv: test basic handling stuff

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
  - Nothing changed
---
 xen/arch/riscv/setup.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c
index c8513ca4f8..bcff680fb5 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,6 +1,7 @@
 #include <xen/compile.h>
 #include <xen/init.h>
 
+#include <asm/bug.h>
 #include <asm/csr.h>
 #include <asm/early_printk.h>
 #include <asm/traps.h>
@@ -9,12 +10,28 @@
 unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
     __aligned(STACK_SIZE);
 
+static void test_run_in_exception(struct cpu_user_regs *regs)
+{
+    early_printk("If you see this message, ");
+    early_printk("run_in_exception_handler is most likely working\n");
+}
+
+static void test_macros_from_bug_h(void)
+{
+    run_in_exception_handler(test_run_in_exception);
+    WARN();
+    early_printk("If you see this message, ");
+    early_printk("WARN is most likely working\n");
+}
+
 void __init noreturn start_xen(void)
 {
     early_printk("Hello from C env\n");
 
     trap_init();
 
+    test_macros_from_bug_h();
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0

[PATCH v2 14/14] automation: add smoke test to verify macros from bug.h

[Oleksii Kurochko]

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
 - Leave only the latest "grep ..."
---
 automation/scripts/qemu-smoke-riscv64.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh
index e0f06360bc..02fc66be03 100755
--- a/automation/scripts/qemu-smoke-riscv64.sh
+++ b/automation/scripts/qemu-smoke-riscv64.sh
@@ -16,5 +16,5 @@ qemu-system-riscv64 \
     |& tee smoke.serial
 
 set -e
-(grep -q "Hello from C env" smoke.serial) || exit 1
+(grep -q "WARN is most likely working" smoke.serial) || exit 1
 exit 0
-- 
2.39.0

Re: [PATCH v2 04/14] xen/riscv: add <asm/csr.h> header

[Jan Beulich]

On 27.01.2023 14:59, Oleksii Kurochko wrote:
> The following changes were made in comparison with <asm/csr.h> from
> Linux:
>   * remove all defines as they are defined in riscv_encoding.h
>   * leave only csr_* macros
> 
> Origin: https://github.com/torvalds/linux.git 2475bf0250de

I'm sorry to be picky, but I think such references should be to the canonical
tree, which here aiui is the one at git.kernel.org.

Jan

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Jan Beulich]

On 27.01.2023 14:59, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/processor.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: MIT */
> +/******************************************************************************
> + *
> + * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
> + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
> + * Copyright 2023 (C) Vates
> + *
> + */
> +
> +#ifndef _ASM_RISCV_PROCESSOR_H
> +#define _ASM_RISCV_PROCESSOR_H
> +
> +#ifndef __ASSEMBLY__
> +
> +/* On stack VCPU state */
> +struct cpu_user_regs
> +{
> +    unsigned long zero;
> +    unsigned long ra;
> +    unsigned long sp;
> +    unsigned long gp;
> +    unsigned long tp;
> +    unsigned long t0;
> +    unsigned long t1;
> +    unsigned long t2;
> +    unsigned long s0;
> +    unsigned long s1;
> +    unsigned long a0;
> +    unsigned long a1;
> +    unsigned long a2;
> +    unsigned long a3;
> +    unsigned long a4;
> +    unsigned long a5;
> +    unsigned long a6;
> +    unsigned long a7;
> +    unsigned long s2;
> +    unsigned long s3;
> +    unsigned long s4;
> +    unsigned long s5;
> +    unsigned long s6;
> +    unsigned long s7;
> +    unsigned long s8;
> +    unsigned long s9;
> +    unsigned long s10;
> +    unsigned long s11;
> +    unsigned long t3;
> +    unsigned long t4;
> +    unsigned long t5;
> +    unsigned long t6;
> +    unsigned long sepc;
> +    unsigned long sstatus;
> +    /* pointer to previous stack_cpu_regs */
> +    unsigned long pregs;
> +};

Just to restate what I said on the earlier version: We have a struct of
this name in the public interface for x86. Besides to confusion about
re-using the name for something private, I'd still like to understand
what the public interface plans are. This is specifically because I
think it would be better to re-use suitable public interface structs
internally where possible. But that of course requires spelling out
such parts of the public interface first.

Jan

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Jan Beulich]

On 27.01.2023 14:59, Oleksii Kurochko wrote:
> The patch introduces macros: BUG(), WARN(), run_in_exception(),
> assert_failed.
> 
> The implementation uses "ebreak" instruction in combination with
> diffrent bug frame tables (for each type) which contains useful
> information.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes:
>   - Remove __ in define namings
>   - Update run_in_exception_handler() with
>     register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
>   - Remove bug_instr_t type and change it's usage to uint32_t

But that's not correct - as said before, you can't assume you can access
32 bits, there maybe only a 16-bit insn at the end of a page, with nothing
mapped to the VA of the subsequent page. Even more ...

> + end:
> +    regs->sepc += GET_INSN_LENGTH(*(uint32_t *)pc);

... you obtain insn length you don't even need to read 32 bits.

Jan

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Jan Beulich]

On 27.01.2023 14:59, Oleksii Kurochko wrote:
> +int is_valid_bugaddr(uint32_t insn)
> +{
> +    if ((insn & INSN_LENGTH_MASK) == INSN_LENGTH_32)
> +        return (insn == BUG_INSN_32);
> +    else
> +        return ((insn & COMPRESSED_INSN_MASK) == BUG_INSN_16);
> +}
> +
>  void do_trap(struct cpu_user_regs *cpu_regs)
>  {
> +    register_t pc = cpu_regs->sepc;
> +    uint32_t instr = *(uint32_t *)pc;
> +
> +    if (is_valid_bugaddr(instr))
> +        if (!do_bug_frame(cpu_regs, pc)) return;
> +
>      do_unexpected_trap(cpu_regs);
>  }

One more remark, style related: Even if some of the additions you're making
are temporary, it'll be better if you have everything in Xen style. That'll
reduce the risk of someone copying bad style from adjacent code, and it'll
also avoid people like me thinking whether to comment and request an
adjustment, or whether to assume that it's temporary code and will get
changed again anyway.

Jan

Re: [PATCH v2 14/14] automation: add smoke test to verify macros from bug.h

[Michal Orzel]

Hi Oleksii,

On 27/01/2023 14:59, Oleksii Kurochko wrote:
> 
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
>  - Leave only the latest "grep ..."
> ---
>  automation/scripts/qemu-smoke-riscv64.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh
> index e0f06360bc..02fc66be03 100755
> --- a/automation/scripts/qemu-smoke-riscv64.sh
> +++ b/automation/scripts/qemu-smoke-riscv64.sh
> @@ -16,5 +16,5 @@ qemu-system-riscv64 \
>      |& tee smoke.serial
> 
>  set -e
> -(grep -q "Hello from C env" smoke.serial) || exit 1
> +(grep -q "WARN is most likely working" smoke.serial) || exit 1
I think the commit msg is a bit misleading and should be changed.
FWICS, you are not *adding* any smoke test but instead modifying
the grep pattern to reflect the usage of WARN.

>  exit 0
> --
> 2.39.0
> 
> 

~Michal

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Julien Grall]

Hi Oleksii,

On 27/01/2023 13:59, Oleksii Kurochko wrote:
> +static inline void wfi(void)
> +{
> +    __asm__ __volatile__ ("wfi");

I have starred at this line for a while and I am not quite too sure to 
understand why we don't need to clobber the memory like we do on Arm.

FWIW, Linux is doing the same, so I guess this is correct. For Arm we 
also follow the Linux implementation.

But I am wondering whether we are just too strict on Arm, RISCv compiler 
offer a different guarantee, or you expect the user to be responsible to 
prevent the compiler to do harmful optimization.

> +/*
> + * panic() isn't available at the moment so an infinite loop will be
> + * used temporarily.
> + * TODO: change it to panic()
> + */
> +static inline void die(void)
> +{
> +    for( ;; ) wfi();

Please move wfi() to a newline.

> +}

-- 
Julien Grall

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Julien Grall]

Hi Oleksii,

On 27/01/2023 13:59, Oleksii Kurochko wrote:
> The patch introduces macros: BUG(), WARN(), run_in_exception(),
> assert_failed.
> 
> The implementation uses "ebreak" instruction in combination with
> diffrent bug frame tables (for each type) which contains useful
> information.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes:
>    - Remove __ in define namings
>    - Update run_in_exception_handler() with
>      register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
>    - Remove bug_instr_t type and change it's usage to uint32_t
> ---
>   xen/arch/riscv/include/asm/bug.h | 118 ++++++++++++++++++++++++++++
>   xen/arch/riscv/traps.c           | 128 +++++++++++++++++++++++++++++++
>   xen/arch/riscv/xen.lds.S         |  10 +++
>   3 files changed, 256 insertions(+)
>   create mode 100644 xen/arch/riscv/include/asm/bug.h
> 
> diff --git a/xen/arch/riscv/include/asm/bug.h b/xen/arch/riscv/include/asm/bug.h
> new file mode 100644
> index 0000000000..4b15d8eba6
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/bug.h
> @@ -0,0 +1,118 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2012 Regents of the University of California
> + * Copyright (C) 2021-2023 Vates

I have to question the two copyrights here given that the majority of 
the code seems to be taken from the arm implementation (see 
arch/arm/include/asm/bug.h).

With that said, we should consolidate the code rather than duplicating 
it on every architecture.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 14/14] automation: add smoke test to verify macros from bug.h

[Oleksii]

On Fri, 2023-01-27 at 15:43 +0100, Michal Orzel wrote:
> Hi Oleksii,
> 
> On 27/01/2023 14:59, Oleksii Kurochko wrote:
> > 
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V2:
> >  - Leave only the latest "grep ..."
> > ---
> >  automation/scripts/qemu-smoke-riscv64.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/automation/scripts/qemu-smoke-riscv64.sh
> > b/automation/scripts/qemu-smoke-riscv64.sh
> > index e0f06360bc..02fc66be03 100755
> > --- a/automation/scripts/qemu-smoke-riscv64.sh
> > +++ b/automation/scripts/qemu-smoke-riscv64.sh
> > @@ -16,5 +16,5 @@ qemu-system-riscv64 \
> >      |& tee smoke.serial
> > 
> >  set -e
> > -(grep -q "Hello from C env" smoke.serial) || exit 1
> > +(grep -q "WARN is most likely working" smoke.serial) || exit 1
> I think the commit msg is a bit misleading and should be changed.
> FWICS, you are not *adding* any smoke test but instead modifying
> the grep pattern to reflect the usage of WARN.
> 
It's incorrect so it will be changed in the new version of the patch
series.

Thanks.
> >  exit 0
> > --
> > 2.39.0
> > 
> > 
> 
> ~Michal

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii]

On Fri, 2023-01-27 at 15:34 +0100, Jan Beulich wrote:
> On 27.01.2023 14:59, Oleksii Kurochko wrote:
> > The patch introduces macros: BUG(), WARN(), run_in_exception(),
> > assert_failed.
> > 
> > The implementation uses "ebreak" instruction in combination with
> > diffrent bug frame tables (for each type) which contains useful
> > information.
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes:
> >   - Remove __ in define namings
> >   - Update run_in_exception_handler() with
> >     register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
> >   - Remove bug_instr_t type and change it's usage to uint32_t
> 
> But that's not correct - as said before, you can't assume you can
> access
> 32 bits, there maybe only a 16-bit insn at the end of a page, with
> nothing
> mapped to the VA of the subsequent page. Even more ...
> 
Agree that it will be an issue if 16-bit insn will be at the end of a
page.
The code is based on Linux
(https://elixir.bootlin.com/linux/latest/source/arch/riscv/kernel/traps.c#L152)
and it seems they might have the same issue.
> > + end:
> > +    regs->sepc += GET_INSN_LENGTH(*(uint32_t *)pc);
> 
> ... you obtain insn length you don't even need to read 32 bits.
> 
It looks you are right so I'll change that in the new version of the
patch series.

> Jan
Oleksii

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii]

Hi Julien,
On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > The patch introduces macros: BUG(), WARN(), run_in_exception(),
> > assert_failed.
> > 
> > The implementation uses "ebreak" instruction in combination with
> > diffrent bug frame tables (for each type) which contains useful
> > information.
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes:
> >    - Remove __ in define namings
> >    - Update run_in_exception_handler() with
> >      register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
> >    - Remove bug_instr_t type and change it's usage to uint32_t
> > ---
> >   xen/arch/riscv/include/asm/bug.h | 118
> > ++++++++++++++++++++++++++++
> >   xen/arch/riscv/traps.c           | 128
> > +++++++++++++++++++++++++++++++
> >   xen/arch/riscv/xen.lds.S         |  10 +++
> >   3 files changed, 256 insertions(+)
> >   create mode 100644 xen/arch/riscv/include/asm/bug.h
> > 
> > diff --git a/xen/arch/riscv/include/asm/bug.h
> > b/xen/arch/riscv/include/asm/bug.h
> > new file mode 100644
> > index 0000000000..4b15d8eba6
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/bug.h
> > @@ -0,0 +1,118 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2012 Regents of the University of California
> > + * Copyright (C) 2021-2023 Vates
> 
> I have to question the two copyrights here given that the majority of
> the code seems to be taken from the arm implementation (see 
> arch/arm/include/asm/bug.h).
> 
> With that said, we should consolidate the code rather than
> duplicating 
> it on every architecture.
> 
Copyrights should be removed. They were taken from the previous
implementation of bug.h for RISC-V so I just forgot to remove them.

It looks like we should have common bug.h for ARM and RISCV but I am
not sure that I know how to do that better.
Probably the code wants to be moved to xen/include/bug.h and using
ifdef ARM && RISCV ...
But still I am not sure that this is the best one option as at least we
have different implementation for x86_64.
> Cheers,
> 
~ Oleksii

Re: [PATCH v2 04/14] xen/riscv: add <asm/csr.h> header

[Oleksii]

On Fri, 2023-01-27 at 15:10 +0100, Jan Beulich wrote:
> On 27.01.2023 14:59, Oleksii Kurochko wrote:
> > The following changes were made in comparison with <asm/csr.h> from
> > Linux:
> >   * remove all defines as they are defined in riscv_encoding.h
> >   * leave only csr_* macros
> > 
> > Origin: https://github.com/torvalds/linux.git 2475bf0250de
> 
> I'm sorry to be picky, but I think such references should be to the
> canonical
> tree, which here aiui is the one at git.kernel.org.
> 
Thanks for clarification.
I will change it then.

> Jan
~ Oleksii

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Oleksii]

Hi Julien,

On Fri, 2023-01-27 at 14:54 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > +static inline void wfi(void)
> > +{
> > +    __asm__ __volatile__ ("wfi");
> 
> I have starred at this line for a while and I am not quite too sure
> to 
> understand why we don't need to clobber the memory like we do on Arm.
> 
I don't have an answer. The code was based on Linux so...
> FWIW, Linux is doing the same, so I guess this is correct. For Arm we
> also follow the Linux implementation.
> 
> But I am wondering whether we are just too strict on Arm, RISCv
> compiler 
> offer a different guarantee, or you expect the user to be responsible
> to 
> prevent the compiler to do harmful optimization.
> 
> > +/*
> > + * panic() isn't available at the moment so an infinite loop will
> > be
> > + * used temporarily.
> > + * TODO: change it to panic()
> > + */
> > +static inline void die(void)
> > +{
> > +    for( ;; ) wfi();
> 
> Please move wfi() to a newline.
Thanks.

I thought that it is fine to put into one line in this case but I'll
move it to a newline. It's fine.
> 
> > +}
> 
~Oleksii

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 2707 bytes --]

On 30.01.23 12:35, Oleksii wrote:
> Hi Julien,
> On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
>> Hi Oleksii,
>>
>> On 27/01/2023 13:59, Oleksii Kurochko wrote:
>>> The patch introduces macros: BUG(), WARN(), run_in_exception(),
>>> assert_failed.
>>>
>>> The implementation uses "ebreak" instruction in combination with
>>> diffrent bug frame tables (for each type) which contains useful
>>> information.
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> ---
>>> Changes:
>>>     - Remove __ in define namings
>>>     - Update run_in_exception_handler() with
>>>       register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
>>>     - Remove bug_instr_t type and change it's usage to uint32_t
>>> ---
>>>    xen/arch/riscv/include/asm/bug.h | 118
>>> ++++++++++++++++++++++++++++
>>>    xen/arch/riscv/traps.c           | 128
>>> +++++++++++++++++++++++++++++++
>>>    xen/arch/riscv/xen.lds.S         |  10 +++
>>>    3 files changed, 256 insertions(+)
>>>    create mode 100644 xen/arch/riscv/include/asm/bug.h
>>>
>>> diff --git a/xen/arch/riscv/include/asm/bug.h
>>> b/xen/arch/riscv/include/asm/bug.h
>>> new file mode 100644
>>> index 0000000000..4b15d8eba6
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/bug.h
>>> @@ -0,0 +1,118 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (C) 2012 Regents of the University of California
>>> + * Copyright (C) 2021-2023 Vates
>>
>> I have to question the two copyrights here given that the majority of
>> the code seems to be taken from the arm implementation (see
>> arch/arm/include/asm/bug.h).
>>
>> With that said, we should consolidate the code rather than
>> duplicating
>> it on every architecture.
>>
> Copyrights should be removed. They were taken from the previous
> implementation of bug.h for RISC-V so I just forgot to remove them.
> 
> It looks like we should have common bug.h for ARM and RISCV but I am
> not sure that I know how to do that better.
> Probably the code wants to be moved to xen/include/bug.h and using
> ifdef ARM && RISCV ...
> But still I am not sure that this is the best one option as at least we
> have different implementation for x86_64.

There are already a lot of duplicated #defines in the Arm and x86 asm/bug.h
files.

I'd create xen/include/xen/bug.h including asm/bug.h first and then adding
all the common stuff.

In case 2 archs are sharing some #define FOO you could #define FOO in the
asm/bug.h for the arch not using the common definition and do #ifndef FOO
in xen/include/xen/bug.h around the variant shared by the other archs.


Juergen


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Oleksii]

Hi Jan,

On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
> On 27.01.2023 14:59, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/processor.h
> > @@ -0,0 +1,82 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*****************************************************************
> > *************
> > + *
> > + * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
> > + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
> > + * Copyright 2023 (C) Vates
> > + *
> > + */
> > +
> > +#ifndef _ASM_RISCV_PROCESSOR_H
> > +#define _ASM_RISCV_PROCESSOR_H
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +/* On stack VCPU state */
> > +struct cpu_user_regs
> > +{
> > +    unsigned long zero;
> > +    unsigned long ra;
> > +    unsigned long sp;
> > +    unsigned long gp;
> > +    unsigned long tp;
> > +    unsigned long t0;
> > +    unsigned long t1;
> > +    unsigned long t2;
> > +    unsigned long s0;
> > +    unsigned long s1;
> > +    unsigned long a0;
> > +    unsigned long a1;
> > +    unsigned long a2;
> > +    unsigned long a3;
> > +    unsigned long a4;
> > +    unsigned long a5;
> > +    unsigned long a6;
> > +    unsigned long a7;
> > +    unsigned long s2;
> > +    unsigned long s3;
> > +    unsigned long s4;
> > +    unsigned long s5;
> > +    unsigned long s6;
> > +    unsigned long s7;
> > +    unsigned long s8;
> > +    unsigned long s9;
> > +    unsigned long s10;
> > +    unsigned long s11;
> > +    unsigned long t3;
> > +    unsigned long t4;
> > +    unsigned long t5;
> > +    unsigned long t6;
> > +    unsigned long sepc;
> > +    unsigned long sstatus;
> > +    /* pointer to previous stack_cpu_regs */
> > +    unsigned long pregs;
> > +};
> 
> Just to restate what I said on the earlier version: We have a struct
> of
> this name in the public interface for x86. Besides to confusion about
> re-using the name for something private, I'd still like to understand
> what the public interface plans are. This is specifically because I
> think it would be better to re-use suitable public interface structs
> internally where possible. But that of course requires spelling out
> such parts of the public interface first.
> 
I am not sure that I get you here.
I greped a little bit and found out that each architecture declares
this structure inside arch-specific folder.

Mostly the usage of the cpu_user_regs is to save/restore current state
of CPU during traps ( exceptions/interrupts ) and context_switch().
Also some registers are modified during construction of a domain.
Thereby I prefer here to see the arch-specific register names instead
of common.

> Jan
~ Oleksii

Re: [PATCH v2 04/14] xen/riscv: add <asm/csr.h> header

[Alistair Francis]

On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> The following changes were made in comparison with <asm/csr.h> from
> Linux:
>   * remove all defines as they are defined in riscv_encoding.h
>   * leave only csr_* macros
>
> Origin: https://github.com/torvalds/linux.git 2475bf0250de
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V2:
>   - Minor refactoring mentioned in the commit message, switch tabs to
>     spaces and refactor things around __asm__ __volatile__.
>   - Update the commit message and add "Origin:" tag.
> ---
>  xen/arch/riscv/include/asm/csr.h | 84 ++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/csr.h
>
> diff --git a/xen/arch/riscv/include/asm/csr.h b/xen/arch/riscv/include/asm/csr.h
> new file mode 100644
> index 0000000000..4275cf6515
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/csr.h
> @@ -0,0 +1,84 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-only
> + *
> + * Copyright (C) 2015 Regents of the University of California
> + */
> +
> +#ifndef _ASM_RISCV_CSR_H
> +#define _ASM_RISCV_CSR_H
> +
> +#include <asm/asm.h>
> +#include <xen/const.h>
> +#include <asm/riscv_encoding.h>
> +
> +#ifndef __ASSEMBLY__
> +
> +#define csr_read(csr)                                           \
> +({                                                              \
> +    register unsigned long __v;                                 \
> +    __asm__ __volatile__ (  "csrr %0, " __ASM_STR(csr)          \
> +                            : "=r" (__v)                        \
> +                            : : "memory" );                     \
> +    __v;                                                        \
> +})
> +
> +#define csr_write(csr, val)                                     \
> +({                                                              \
> +    unsigned long __v = (unsigned long)(val);                   \
> +    __asm__ __volatile__ (  "csrw " __ASM_STR(csr) ", %0"       \
> +                            : /* no outputs */                  \
> +                            : "rK" (__v)                        \
> +                            : "memory" );                       \
> +})
> +
> +#define csr_swap(csr, val)                                      \
> +({                                                              \
> +    unsigned long __v = (unsigned long)(val);                   \
> +    __asm__ __volatile__ (  "csrrw %0, " __ASM_STR(csr) ", %1"  \
> +                            : "=r" (__v)                        \
> +                            : "rK" (__v)                        \
> +                            : "memory" );                       \
> +    __v;                                                        \
> +})
> +
> +#define csr_read_set(csr, val)                                  \
> +({                                                              \
> +    unsigned long __v = (unsigned long)(val);                   \
> +    __asm__ __volatile__ (  "csrrs %0, " __ASM_STR(csr) ", %1"  \
> +                            : "=r" (__v)                        \
> +                            : "rK" (__v)                        \
> +                            : "memory" );                       \
> +    __v;                                                        \
> +})
> +
> +#define csr_set(csr, val)                                       \
> +({                                                              \
> +    unsigned long __v = (unsigned long)(val);                   \
> +    __asm__ __volatile__ (  "csrs " __ASM_STR(csr) ", %0"       \
> +                            : /* no outputs */                  \
> +                            : "rK" (__v)                        \
> +                            : "memory" );                       \
> +})
> +
> +#define csr_read_clear(csr, val)                                \
> +({                                                              \
> +    unsigned long __v = (unsigned long)(val);                   \
> +    __asm__ __volatile__ (  "csrrc %0, " __ASM_STR(csr) ", %1"  \
> +                            : "=r" (__v)                        \
> +                            : "rK" (__v)                        \
> +                            : "memory" );                       \
> +    __v;                                                        \
> +})
> +
> +#define csr_clear(csr, val)                                     \
> +({                                                              \
> +    unsigned long __v = (unsigned long)(val);                   \
> +    __asm__ __volatile__ (  "csrc " __ASM_STR(csr) ", %0"       \
> +                            : /*no outputs */                   \
> +                            : "rK" (__v)                        \
> +                            : "memory" );                       \
> +})
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* _ASM_RISCV_CSR_H */
> --
> 2.39.0
>
>

Re: [PATCH v2 03/14] xen/riscv: add <asm/riscv_encoding.h header

[Alistair Francis]

On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> The following changes were done in Xen code base in comparison with OpenSBI:
>   * Remove "#include <sbi/sbi_const.h>" as most of the stuff inside
>     it is present in Xen code base.
>   * Add macros _UL and _ULL as they were in <sbi/sbi_const.h> before
>   * Add SATP32_MODE_SHIFT/SATP64_MODE_SHIFT/SATP_MODE_SHIFT as they will
>     be used in riscv/mm.c
>   * Add CAUSE_IRQ_FLAG which is going to be used insised exception
>     handler
>   * Change ulong to unsigned long in macros REG_PTR(...)
>   * Change s32 to int32_t
>
> Originally authored by Anup Patel <anup.patel@wdc.com>
>
> Origin: https://github.com/riscv-software-src/opensbi.git c45992cc2b12
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V2:
>   - Take the latest version of riscv_encoding.h from OpenSBI.
>   - Update riscv_encoding.h with Xen related changes mentioned in the
>     commit message.
>   - Update commit message and add "Origin:" tag
> ---
>  xen/arch/riscv/include/asm/riscv_encoding.h | 927 ++++++++++++++++++++
>  1 file changed, 927 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/riscv_encoding.h
>
> diff --git a/xen/arch/riscv/include/asm/riscv_encoding.h b/xen/arch/riscv/include/asm/riscv_encoding.h
> new file mode 100644
> index 0000000000..43dd4f6981
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/riscv_encoding.h
> @@ -0,0 +1,927 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + *
> + * Authors:
> + *   Anup Patel <anup.patel@wdc.com>
> + */
> +
> +#ifndef __RISCV_ENCODING_H__
> +#define __RISCV_ENCODING_H__
> +
> +#define _UL(X) _AC(X, UL)
> +#define _ULL(X) _AC(X, ULL)
> +
> +/* clang-format off */
> +#define MSTATUS_SIE                    _UL(0x00000002)
> +#define MSTATUS_MIE                    _UL(0x00000008)
> +#define MSTATUS_SPIE_SHIFT             5
> +#define MSTATUS_SPIE                   (_UL(1) << MSTATUS_SPIE_SHIFT)
> +#define MSTATUS_UBE                    _UL(0x00000040)
> +#define MSTATUS_MPIE                   _UL(0x00000080)
> +#define MSTATUS_SPP_SHIFT              8
> +#define MSTATUS_SPP                    (_UL(1) << MSTATUS_SPP_SHIFT)
> +#define MSTATUS_MPP_SHIFT              11
> +#define MSTATUS_MPP                    (_UL(3) << MSTATUS_MPP_SHIFT)
> +#define MSTATUS_FS                     _UL(0x00006000)
> +#define MSTATUS_XS                     _UL(0x00018000)
> +#define MSTATUS_VS                     _UL(0x00000600)
> +#define MSTATUS_MPRV                   _UL(0x00020000)
> +#define MSTATUS_SUM                    _UL(0x00040000)
> +#define MSTATUS_MXR                    _UL(0x00080000)
> +#define MSTATUS_TVM                    _UL(0x00100000)
> +#define MSTATUS_TW                     _UL(0x00200000)
> +#define MSTATUS_TSR                    _UL(0x00400000)
> +#define MSTATUS32_SD                   _UL(0x80000000)
> +#if __riscv_xlen == 64
> +#define MSTATUS_UXL                    _ULL(0x0000000300000000)
> +#define MSTATUS_SXL                    _ULL(0x0000000C00000000)
> +#define MSTATUS_SBE                    _ULL(0x0000001000000000)
> +#define MSTATUS_MBE                    _ULL(0x0000002000000000)
> +#define MSTATUS_GVA                    _ULL(0x0000004000000000)
> +#define MSTATUS_GVA_SHIFT              38
> +#define MSTATUS_MPV                    _ULL(0x0000008000000000)
> +#else
> +#define MSTATUSH_SBE                   _UL(0x00000010)
> +#define MSTATUSH_MBE                   _UL(0x00000020)
> +#define MSTATUSH_GVA                   _UL(0x00000040)
> +#define MSTATUSH_GVA_SHIFT             6
> +#define MSTATUSH_MPV                   _UL(0x00000080)
> +#endif
> +#define MSTATUS32_SD                   _UL(0x80000000)
> +#define MSTATUS64_SD                   _ULL(0x8000000000000000)
> +
> +#define SSTATUS_SIE                    MSTATUS_SIE
> +#define SSTATUS_SPIE_SHIFT             MSTATUS_SPIE_SHIFT
> +#define SSTATUS_SPIE                   MSTATUS_SPIE
> +#define SSTATUS_SPP_SHIFT              MSTATUS_SPP_SHIFT
> +#define SSTATUS_SPP                    MSTATUS_SPP
> +#define SSTATUS_FS                     MSTATUS_FS
> +#define SSTATUS_XS                     MSTATUS_XS
> +#define SSTATUS_VS                     MSTATUS_VS
> +#define SSTATUS_SUM                    MSTATUS_SUM
> +#define SSTATUS_MXR                    MSTATUS_MXR
> +#define SSTATUS32_SD                   MSTATUS32_SD
> +#define SSTATUS64_UXL                  MSTATUS_UXL
> +#define SSTATUS64_SD                   MSTATUS64_SD
> +
> +#if __riscv_xlen == 64
> +#define HSTATUS_VSXL                   _UL(0x300000000)
> +#define HSTATUS_VSXL_SHIFT             32
> +#endif
> +#define HSTATUS_VTSR                   _UL(0x00400000)
> +#define HSTATUS_VTW                    _UL(0x00200000)
> +#define HSTATUS_VTVM                   _UL(0x00100000)
> +#define HSTATUS_VGEIN                  _UL(0x0003f000)
> +#define HSTATUS_VGEIN_SHIFT            12
> +#define HSTATUS_HU                     _UL(0x00000200)
> +#define HSTATUS_SPVP                   _UL(0x00000100)
> +#define HSTATUS_SPV                    _UL(0x00000080)
> +#define HSTATUS_GVA                    _UL(0x00000040)
> +#define HSTATUS_VSBE                   _UL(0x00000020)
> +
> +#define IRQ_S_SOFT                     1
> +#define IRQ_VS_SOFT                    2
> +#define IRQ_M_SOFT                     3
> +#define IRQ_S_TIMER                    5
> +#define IRQ_VS_TIMER                   6
> +#define IRQ_M_TIMER                    7
> +#define IRQ_S_EXT                      9
> +#define IRQ_VS_EXT                     10
> +#define IRQ_M_EXT                      11
> +#define IRQ_S_GEXT                     12
> +#define IRQ_PMU_OVF                    13
> +
> +#define MIP_SSIP                       (_UL(1) << IRQ_S_SOFT)
> +#define MIP_VSSIP                      (_UL(1) << IRQ_VS_SOFT)
> +#define MIP_MSIP                       (_UL(1) << IRQ_M_SOFT)
> +#define MIP_STIP                       (_UL(1) << IRQ_S_TIMER)
> +#define MIP_VSTIP                      (_UL(1) << IRQ_VS_TIMER)
> +#define MIP_MTIP                       (_UL(1) << IRQ_M_TIMER)
> +#define MIP_SEIP                       (_UL(1) << IRQ_S_EXT)
> +#define MIP_VSEIP                      (_UL(1) << IRQ_VS_EXT)
> +#define MIP_MEIP                       (_UL(1) << IRQ_M_EXT)
> +#define MIP_SGEIP                      (_UL(1) << IRQ_S_GEXT)
> +#define MIP_LCOFIP                     (_UL(1) << IRQ_PMU_OVF)
> +
> +#define SIP_SSIP                       MIP_SSIP
> +#define SIP_STIP                       MIP_STIP
> +
> +#define PRV_U                          _UL(0)
> +#define PRV_S                          _UL(1)
> +#define PRV_M                          _UL(3)
> +
> +#define SATP32_MODE                    _UL(0x80000000)
> +#define SATP32_MODE_SHIFT              31
> +#define SATP32_ASID                    _UL(0x7FC00000)
> +#define SATP32_PPN                     _UL(0x003FFFFF)
> +#define SATP64_MODE                    _ULL(0xF000000000000000)
> +#define SATP64_MODE_SHIFT              60
> +#define SATP64_ASID                    _ULL(0x0FFFF00000000000)
> +#define SATP64_PPN                     _ULL(0x00000FFFFFFFFFFF)
> +
> +#define SATP_MODE_OFF                  _UL(0)
> +#define SATP_MODE_SV32                 _UL(1)
> +#define SATP_MODE_SV39                 _UL(8)
> +#define SATP_MODE_SV48                 _UL(9)
> +#define SATP_MODE_SV57                 _UL(10)
> +#define SATP_MODE_SV64                 _UL(11)
> +
> +#define HGATP_MODE_OFF                 _UL(0)
> +#define HGATP_MODE_SV32X4              _UL(1)
> +#define HGATP_MODE_SV39X4              _UL(8)
> +#define HGATP_MODE_SV48X4              _UL(9)
> +
> +#define HGATP32_MODE_SHIFT             31
> +#define HGATP32_VMID_SHIFT             22
> +#define HGATP32_VMID_MASK              _UL(0x1FC00000)
> +#define HGATP32_PPN                    _UL(0x003FFFFF)
> +
> +#define HGATP64_MODE_SHIFT             60
> +#define HGATP64_VMID_SHIFT             44
> +#define HGATP64_VMID_MASK              _ULL(0x03FFF00000000000)
> +#define HGATP64_PPN                    _ULL(0x00000FFFFFFFFFFF)
> +
> +#define PMP_R                          _UL(0x01)
> +#define PMP_W                          _UL(0x02)
> +#define PMP_X                          _UL(0x04)
> +#define PMP_A                          _UL(0x18)
> +#define PMP_A_TOR                      _UL(0x08)
> +#define PMP_A_NA4                      _UL(0x10)
> +#define PMP_A_NAPOT                    _UL(0x18)
> +#define PMP_L                          _UL(0x80)
> +
> +#define PMP_SHIFT                      2
> +#define PMP_COUNT                      64
> +#if __riscv_xlen == 64
> +#define PMP_ADDR_MASK                  ((_ULL(0x1) << 54) - 1)
> +#else
> +#define PMP_ADDR_MASK                  _UL(0xFFFFFFFF)
> +#endif
> +
> +#if __riscv_xlen == 64
> +#define MSTATUS_SD                     MSTATUS64_SD
> +#define SSTATUS_SD                     SSTATUS64_SD
> +#define SATP_MODE                      SATP64_MODE
> +#define SATP_MODE_SHIFT                        SATP64_MODE_SHIFT
> +
> +#define HGATP_PPN                      HGATP64_PPN
> +#define HGATP_VMID_SHIFT               HGATP64_VMID_SHIFT
> +#define HGATP_VMID_MASK                        HGATP64_VMID_MASK
> +#define HGATP_MODE_SHIFT               HGATP64_MODE_SHIFT
> +#else
> +#define MSTATUS_SD                     MSTATUS32_SD
> +#define SSTATUS_SD                     SSTATUS32_SD
> +#define SATP_MODE                      SATP32_MODE
> +#define SATP_MODE_SHIFT                        SATP32_MODE_SHIFT
> +
> +#define HGATP_PPN                      HGATP32_PPN
> +#define HGATP_VMID_SHIFT               HGATP32_VMID_SHIFT
> +#define HGATP_VMID_MASK                        HGATP32_VMID_MASK
> +#define HGATP_MODE_SHIFT               HGATP32_MODE_SHIFT
> +#endif
> +
> +#define TOPI_IID_SHIFT                 16
> +#define TOPI_IID_MASK                  0xfff
> +#define TOPI_IPRIO_MASK                0xff
> +
> +#if __riscv_xlen == 64
> +#define MHPMEVENT_OF                   (_UL(1) << 63)
> +#define MHPMEVENT_MINH                 (_UL(1) << 62)
> +#define MHPMEVENT_SINH                 (_UL(1) << 61)
> +#define MHPMEVENT_UINH                 (_UL(1) << 60)
> +#define MHPMEVENT_VSINH                        (_UL(1) << 59)
> +#define MHPMEVENT_VUINH                        (_UL(1) << 58)
> +#else
> +#define MHPMEVENTH_OF                  (_ULL(1) << 31)
> +#define MHPMEVENTH_MINH                        (_ULL(1) << 30)
> +#define MHPMEVENTH_SINH                        (_ULL(1) << 29)
> +#define MHPMEVENTH_UINH                        (_ULL(1) << 28)
> +#define MHPMEVENTH_VSINH               (_ULL(1) << 27)
> +#define MHPMEVENTH_VUINH               (_ULL(1) << 26)
> +
> +#define MHPMEVENT_OF                   (MHPMEVENTH_OF << 32)
> +#define MHPMEVENT_MINH                 (MHPMEVENTH_MINH << 32)
> +#define MHPMEVENT_SINH                 (MHPMEVENTH_SINH << 32)
> +#define MHPMEVENT_UINH                 (MHPMEVENTH_UINH << 32)
> +#define MHPMEVENT_VSINH                        (MHPMEVENTH_VSINH << 32)
> +#define MHPMEVENT_VUINH                        (MHPMEVENTH_VUINH << 32)
> +
> +#endif
> +
> +#define MHPMEVENT_SSCOF_MASK           _ULL(0xFFFF000000000000)
> +
> +#if __riscv_xlen > 32
> +#define ENVCFG_STCE                    (_ULL(1) << 63)
> +#define ENVCFG_PBMTE                   (_ULL(1) << 62)
> +#else
> +#define ENVCFGH_STCE                   (_UL(1) << 31)
> +#define ENVCFGH_PBMTE                  (_UL(1) << 30)
> +#endif
> +#define ENVCFG_CBZE                    (_UL(1) << 7)
> +#define ENVCFG_CBCFE                   (_UL(1) << 6)
> +#define ENVCFG_CBIE_SHIFT              4
> +#define ENVCFG_CBIE                    (_UL(0x3) << ENVCFG_CBIE_SHIFT)
> +#define ENVCFG_CBIE_ILL                        _UL(0x0)
> +#define ENVCFG_CBIE_FLUSH              _UL(0x1)
> +#define ENVCFG_CBIE_INV                        _UL(0x3)
> +#define ENVCFG_FIOM                    _UL(0x1)
> +
> +/* ===== User-level CSRs ===== */
> +
> +/* User Trap Setup (N-extension) */
> +#define CSR_USTATUS                    0x000
> +#define CSR_UIE                                0x004
> +#define CSR_UTVEC                      0x005
> +
> +/* User Trap Handling (N-extension) */
> +#define CSR_USCRATCH                   0x040
> +#define CSR_UEPC                       0x041
> +#define CSR_UCAUSE                     0x042
> +#define CSR_UTVAL                      0x043
> +#define CSR_UIP                                0x044
> +
> +/* User Floating-point CSRs */
> +#define CSR_FFLAGS                     0x001
> +#define CSR_FRM                                0x002
> +#define CSR_FCSR                       0x003
> +
> +/* User Counters/Timers */
> +#define CSR_CYCLE                      0xc00
> +#define CSR_TIME                       0xc01
> +#define CSR_INSTRET                    0xc02
> +#define CSR_HPMCOUNTER3                        0xc03
> +#define CSR_HPMCOUNTER4                        0xc04
> +#define CSR_HPMCOUNTER5                        0xc05
> +#define CSR_HPMCOUNTER6                        0xc06
> +#define CSR_HPMCOUNTER7                        0xc07
> +#define CSR_HPMCOUNTER8                        0xc08
> +#define CSR_HPMCOUNTER9                        0xc09
> +#define CSR_HPMCOUNTER10               0xc0a
> +#define CSR_HPMCOUNTER11               0xc0b
> +#define CSR_HPMCOUNTER12               0xc0c
> +#define CSR_HPMCOUNTER13               0xc0d
> +#define CSR_HPMCOUNTER14               0xc0e
> +#define CSR_HPMCOUNTER15               0xc0f
> +#define CSR_HPMCOUNTER16               0xc10
> +#define CSR_HPMCOUNTER17               0xc11
> +#define CSR_HPMCOUNTER18               0xc12
> +#define CSR_HPMCOUNTER19               0xc13
> +#define CSR_HPMCOUNTER20               0xc14
> +#define CSR_HPMCOUNTER21               0xc15
> +#define CSR_HPMCOUNTER22               0xc16
> +#define CSR_HPMCOUNTER23               0xc17
> +#define CSR_HPMCOUNTER24               0xc18
> +#define CSR_HPMCOUNTER25               0xc19
> +#define CSR_HPMCOUNTER26               0xc1a
> +#define CSR_HPMCOUNTER27               0xc1b
> +#define CSR_HPMCOUNTER28               0xc1c
> +#define CSR_HPMCOUNTER29               0xc1d
> +#define CSR_HPMCOUNTER30               0xc1e
> +#define CSR_HPMCOUNTER31               0xc1f
> +#define CSR_CYCLEH                     0xc80
> +#define CSR_TIMEH                      0xc81
> +#define CSR_INSTRETH                   0xc82
> +#define CSR_HPMCOUNTER3H               0xc83
> +#define CSR_HPMCOUNTER4H               0xc84
> +#define CSR_HPMCOUNTER5H               0xc85
> +#define CSR_HPMCOUNTER6H               0xc86
> +#define CSR_HPMCOUNTER7H               0xc87
> +#define CSR_HPMCOUNTER8H               0xc88
> +#define CSR_HPMCOUNTER9H               0xc89
> +#define CSR_HPMCOUNTER10H              0xc8a
> +#define CSR_HPMCOUNTER11H              0xc8b
> +#define CSR_HPMCOUNTER12H              0xc8c
> +#define CSR_HPMCOUNTER13H              0xc8d
> +#define CSR_HPMCOUNTER14H              0xc8e
> +#define CSR_HPMCOUNTER15H              0xc8f
> +#define CSR_HPMCOUNTER16H              0xc90
> +#define CSR_HPMCOUNTER17H              0xc91
> +#define CSR_HPMCOUNTER18H              0xc92
> +#define CSR_HPMCOUNTER19H              0xc93
> +#define CSR_HPMCOUNTER20H              0xc94
> +#define CSR_HPMCOUNTER21H              0xc95
> +#define CSR_HPMCOUNTER22H              0xc96
> +#define CSR_HPMCOUNTER23H              0xc97
> +#define CSR_HPMCOUNTER24H              0xc98
> +#define CSR_HPMCOUNTER25H              0xc99
> +#define CSR_HPMCOUNTER26H              0xc9a
> +#define CSR_HPMCOUNTER27H              0xc9b
> +#define CSR_HPMCOUNTER28H              0xc9c
> +#define CSR_HPMCOUNTER29H              0xc9d
> +#define CSR_HPMCOUNTER30H              0xc9e
> +#define CSR_HPMCOUNTER31H              0xc9f
> +
> +/* ===== Supervisor-level CSRs ===== */
> +
> +/* Supervisor Trap Setup */
> +#define CSR_SSTATUS                    0x100
> +#define CSR_SIE                                0x104
> +#define CSR_STVEC                      0x105
> +#define CSR_SCOUNTEREN                 0x106
> +
> +/* Supervisor Configuration */
> +#define CSR_SENVCFG                    0x10a
> +
> +/* Supervisor Trap Handling */
> +#define CSR_SSCRATCH                   0x140
> +#define CSR_SEPC                       0x141
> +#define CSR_SCAUSE                     0x142
> +#define CSR_STVAL                      0x143
> +#define CSR_SIP                                0x144
> +
> +/* Sstc extension */
> +#define CSR_STIMECMP                   0x14D
> +#define CSR_STIMECMPH                  0x15D
> +
> +/* Supervisor Protection and Translation */
> +#define CSR_SATP                       0x180
> +
> +/* Supervisor-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_SISELECT                   0x150
> +#define CSR_SIREG                      0x151
> +
> +/* Supervisor-Level Interrupts (AIA) */
> +#define CSR_STOPEI                     0x15c
> +#define CSR_STOPI                      0xdb0
> +
> +/* Supervisor-Level High-Half CSRs (AIA) */
> +#define CSR_SIEH                       0x114
> +#define CSR_SIPH                       0x154
> +
> +/* Supervisor stateen CSRs */
> +#define CSR_SSTATEEN0                  0x10C
> +#define CSR_SSTATEEN1                  0x10D
> +#define CSR_SSTATEEN2                  0x10E
> +#define CSR_SSTATEEN3                  0x10F
> +
> +/* ===== Hypervisor-level CSRs ===== */
> +
> +/* Hypervisor Trap Setup (H-extension) */
> +#define CSR_HSTATUS                    0x600
> +#define CSR_HEDELEG                    0x602
> +#define CSR_HIDELEG                    0x603
> +#define CSR_HIE                                0x604
> +#define CSR_HCOUNTEREN                 0x606
> +#define CSR_HGEIE                      0x607
> +
> +/* Hypervisor Configuration */
> +#define CSR_HENVCFG                    0x60a
> +#define CSR_HENVCFGH                   0x61a
> +
> +/* Hypervisor Trap Handling (H-extension) */
> +#define CSR_HTVAL                      0x643
> +#define CSR_HIP                                0x644
> +#define CSR_HVIP                       0x645
> +#define CSR_HTINST                     0x64a
> +#define CSR_HGEIP                      0xe12
> +
> +/* Hypervisor Protection and Translation (H-extension) */
> +#define CSR_HGATP                      0x680
> +
> +/* Hypervisor Counter/Timer Virtualization Registers (H-extension) */
> +#define CSR_HTIMEDELTA                 0x605
> +#define CSR_HTIMEDELTAH                        0x615
> +
> +/* Virtual Supervisor Registers (H-extension) */
> +#define CSR_VSSTATUS                   0x200
> +#define CSR_VSIE                       0x204
> +#define CSR_VSTVEC                     0x205
> +#define CSR_VSSCRATCH                  0x240
> +#define CSR_VSEPC                      0x241
> +#define CSR_VSCAUSE                    0x242
> +#define CSR_VSTVAL                     0x243
> +#define CSR_VSIP                       0x244
> +#define CSR_VSATP                      0x280
> +
> +/* Virtual Interrupts and Interrupt Priorities (H-extension with AIA) */
> +#define CSR_HVIEN                      0x608
> +#define CSR_HVICTL                     0x609
> +#define CSR_HVIPRIO1                   0x646
> +#define CSR_HVIPRIO2                   0x647
> +
> +/* VS-Level Window to Indirectly Accessed Registers (H-extension with AIA) */
> +#define CSR_VSISELECT                  0x250
> +#define CSR_VSIREG                     0x251
> +
> +/* VS-Level Interrupts (H-extension with AIA) */
> +#define CSR_VSTOPEI                    0x25c
> +#define CSR_VSTOPI                     0xeb0
> +
> +/* Hypervisor and VS-Level High-Half CSRs (H-extension with AIA) */
> +#define CSR_HIDELEGH                   0x613
> +#define CSR_HVIENH                     0x618
> +#define CSR_HVIPH                      0x655
> +#define CSR_HVIPRIO1H                  0x656
> +#define CSR_HVIPRIO2H                  0x657
> +#define CSR_VSIEH                      0x214
> +#define CSR_VSIPH                      0x254
> +
> +/* Hypervisor stateen CSRs */
> +#define CSR_HSTATEEN0                  0x60C
> +#define CSR_HSTATEEN0H                 0x61C
> +#define CSR_HSTATEEN1                  0x60D
> +#define CSR_HSTATEEN1H                 0x61D
> +#define CSR_HSTATEEN2                  0x60E
> +#define CSR_HSTATEEN2H                 0x61E
> +#define CSR_HSTATEEN3                  0x60F
> +#define CSR_HSTATEEN3H                 0x61F
> +
> +/* ===== Machine-level CSRs ===== */
> +
> +/* Machine Information Registers */
> +#define CSR_MVENDORID                  0xf11
> +#define CSR_MARCHID                    0xf12
> +#define CSR_MIMPID                     0xf13
> +#define CSR_MHARTID                    0xf14
> +
> +/* Machine Trap Setup */
> +#define CSR_MSTATUS                    0x300
> +#define CSR_MISA                       0x301
> +#define CSR_MEDELEG                    0x302
> +#define CSR_MIDELEG                    0x303
> +#define CSR_MIE                                0x304
> +#define CSR_MTVEC                      0x305
> +#define CSR_MCOUNTEREN                 0x306
> +#define CSR_MSTATUSH                   0x310
> +
> +/* Machine Configuration */
> +#define CSR_MENVCFG                    0x30a
> +#define CSR_MENVCFGH                   0x31a
> +
> +/* Machine Trap Handling */
> +#define CSR_MSCRATCH                   0x340
> +#define CSR_MEPC                       0x341
> +#define CSR_MCAUSE                     0x342
> +#define CSR_MTVAL                      0x343
> +#define CSR_MIP                                0x344
> +#define CSR_MTINST                     0x34a
> +#define CSR_MTVAL2                     0x34b
> +
> +/* Machine Memory Protection */
> +#define CSR_PMPCFG0                    0x3a0
> +#define CSR_PMPCFG1                    0x3a1
> +#define CSR_PMPCFG2                    0x3a2
> +#define CSR_PMPCFG3                    0x3a3
> +#define CSR_PMPCFG4                    0x3a4
> +#define CSR_PMPCFG5                    0x3a5
> +#define CSR_PMPCFG6                    0x3a6
> +#define CSR_PMPCFG7                    0x3a7
> +#define CSR_PMPCFG8                    0x3a8
> +#define CSR_PMPCFG9                    0x3a9
> +#define CSR_PMPCFG10                   0x3aa
> +#define CSR_PMPCFG11                   0x3ab
> +#define CSR_PMPCFG12                   0x3ac
> +#define CSR_PMPCFG13                   0x3ad
> +#define CSR_PMPCFG14                   0x3ae
> +#define CSR_PMPCFG15                   0x3af
> +#define CSR_PMPADDR0                   0x3b0
> +#define CSR_PMPADDR1                   0x3b1
> +#define CSR_PMPADDR2                   0x3b2
> +#define CSR_PMPADDR3                   0x3b3
> +#define CSR_PMPADDR4                   0x3b4
> +#define CSR_PMPADDR5                   0x3b5
> +#define CSR_PMPADDR6                   0x3b6
> +#define CSR_PMPADDR7                   0x3b7
> +#define CSR_PMPADDR8                   0x3b8
> +#define CSR_PMPADDR9                   0x3b9
> +#define CSR_PMPADDR10                  0x3ba
> +#define CSR_PMPADDR11                  0x3bb
> +#define CSR_PMPADDR12                  0x3bc
> +#define CSR_PMPADDR13                  0x3bd
> +#define CSR_PMPADDR14                  0x3be
> +#define CSR_PMPADDR15                  0x3bf
> +#define CSR_PMPADDR16                  0x3c0
> +#define CSR_PMPADDR17                  0x3c1
> +#define CSR_PMPADDR18                  0x3c2
> +#define CSR_PMPADDR19                  0x3c3
> +#define CSR_PMPADDR20                  0x3c4
> +#define CSR_PMPADDR21                  0x3c5
> +#define CSR_PMPADDR22                  0x3c6
> +#define CSR_PMPADDR23                  0x3c7
> +#define CSR_PMPADDR24                  0x3c8
> +#define CSR_PMPADDR25                  0x3c9
> +#define CSR_PMPADDR26                  0x3ca
> +#define CSR_PMPADDR27                  0x3cb
> +#define CSR_PMPADDR28                  0x3cc
> +#define CSR_PMPADDR29                  0x3cd
> +#define CSR_PMPADDR30                  0x3ce
> +#define CSR_PMPADDR31                  0x3cf
> +#define CSR_PMPADDR32                  0x3d0
> +#define CSR_PMPADDR33                  0x3d1
> +#define CSR_PMPADDR34                  0x3d2
> +#define CSR_PMPADDR35                  0x3d3
> +#define CSR_PMPADDR36                  0x3d4
> +#define CSR_PMPADDR37                  0x3d5
> +#define CSR_PMPADDR38                  0x3d6
> +#define CSR_PMPADDR39                  0x3d7
> +#define CSR_PMPADDR40                  0x3d8
> +#define CSR_PMPADDR41                  0x3d9
> +#define CSR_PMPADDR42                  0x3da
> +#define CSR_PMPADDR43                  0x3db
> +#define CSR_PMPADDR44                  0x3dc
> +#define CSR_PMPADDR45                  0x3dd
> +#define CSR_PMPADDR46                  0x3de
> +#define CSR_PMPADDR47                  0x3df
> +#define CSR_PMPADDR48                  0x3e0
> +#define CSR_PMPADDR49                  0x3e1
> +#define CSR_PMPADDR50                  0x3e2
> +#define CSR_PMPADDR51                  0x3e3
> +#define CSR_PMPADDR52                  0x3e4
> +#define CSR_PMPADDR53                  0x3e5
> +#define CSR_PMPADDR54                  0x3e6
> +#define CSR_PMPADDR55                  0x3e7
> +#define CSR_PMPADDR56                  0x3e8
> +#define CSR_PMPADDR57                  0x3e9
> +#define CSR_PMPADDR58                  0x3ea
> +#define CSR_PMPADDR59                  0x3eb
> +#define CSR_PMPADDR60                  0x3ec
> +#define CSR_PMPADDR61                  0x3ed
> +#define CSR_PMPADDR62                  0x3ee
> +#define CSR_PMPADDR63                  0x3ef
> +
> +/* Machine Counters/Timers */
> +#define CSR_MCYCLE                     0xb00
> +#define CSR_MINSTRET                   0xb02
> +#define CSR_MHPMCOUNTER3               0xb03
> +#define CSR_MHPMCOUNTER4               0xb04
> +#define CSR_MHPMCOUNTER5               0xb05
> +#define CSR_MHPMCOUNTER6               0xb06
> +#define CSR_MHPMCOUNTER7               0xb07
> +#define CSR_MHPMCOUNTER8               0xb08
> +#define CSR_MHPMCOUNTER9               0xb09
> +#define CSR_MHPMCOUNTER10              0xb0a
> +#define CSR_MHPMCOUNTER11              0xb0b
> +#define CSR_MHPMCOUNTER12              0xb0c
> +#define CSR_MHPMCOUNTER13              0xb0d
> +#define CSR_MHPMCOUNTER14              0xb0e
> +#define CSR_MHPMCOUNTER15              0xb0f
> +#define CSR_MHPMCOUNTER16              0xb10
> +#define CSR_MHPMCOUNTER17              0xb11
> +#define CSR_MHPMCOUNTER18              0xb12
> +#define CSR_MHPMCOUNTER19              0xb13
> +#define CSR_MHPMCOUNTER20              0xb14
> +#define CSR_MHPMCOUNTER21              0xb15
> +#define CSR_MHPMCOUNTER22              0xb16
> +#define CSR_MHPMCOUNTER23              0xb17
> +#define CSR_MHPMCOUNTER24              0xb18
> +#define CSR_MHPMCOUNTER25              0xb19
> +#define CSR_MHPMCOUNTER26              0xb1a
> +#define CSR_MHPMCOUNTER27              0xb1b
> +#define CSR_MHPMCOUNTER28              0xb1c
> +#define CSR_MHPMCOUNTER29              0xb1d
> +#define CSR_MHPMCOUNTER30              0xb1e
> +#define CSR_MHPMCOUNTER31              0xb1f
> +#define CSR_MCYCLEH                    0xb80
> +#define CSR_MINSTRETH                  0xb82
> +#define CSR_MHPMCOUNTER3H              0xb83
> +#define CSR_MHPMCOUNTER4H              0xb84
> +#define CSR_MHPMCOUNTER5H              0xb85
> +#define CSR_MHPMCOUNTER6H              0xb86
> +#define CSR_MHPMCOUNTER7H              0xb87
> +#define CSR_MHPMCOUNTER8H              0xb88
> +#define CSR_MHPMCOUNTER9H              0xb89
> +#define CSR_MHPMCOUNTER10H             0xb8a
> +#define CSR_MHPMCOUNTER11H             0xb8b
> +#define CSR_MHPMCOUNTER12H             0xb8c
> +#define CSR_MHPMCOUNTER13H             0xb8d
> +#define CSR_MHPMCOUNTER14H             0xb8e
> +#define CSR_MHPMCOUNTER15H             0xb8f
> +#define CSR_MHPMCOUNTER16H             0xb90
> +#define CSR_MHPMCOUNTER17H             0xb91
> +#define CSR_MHPMCOUNTER18H             0xb92
> +#define CSR_MHPMCOUNTER19H             0xb93
> +#define CSR_MHPMCOUNTER20H             0xb94
> +#define CSR_MHPMCOUNTER21H             0xb95
> +#define CSR_MHPMCOUNTER22H             0xb96
> +#define CSR_MHPMCOUNTER23H             0xb97
> +#define CSR_MHPMCOUNTER24H             0xb98
> +#define CSR_MHPMCOUNTER25H             0xb99
> +#define CSR_MHPMCOUNTER26H             0xb9a
> +#define CSR_MHPMCOUNTER27H             0xb9b
> +#define CSR_MHPMCOUNTER28H             0xb9c
> +#define CSR_MHPMCOUNTER29H             0xb9d
> +#define CSR_MHPMCOUNTER30H             0xb9e
> +#define CSR_MHPMCOUNTER31H             0xb9f
> +
> +/* Machine Counter Setup */
> +#define CSR_MCOUNTINHIBIT              0x320
> +#define CSR_MHPMEVENT3                 0x323
> +#define CSR_MHPMEVENT4                 0x324
> +#define CSR_MHPMEVENT5                 0x325
> +#define CSR_MHPMEVENT6                 0x326
> +#define CSR_MHPMEVENT7                 0x327
> +#define CSR_MHPMEVENT8                 0x328
> +#define CSR_MHPMEVENT9                 0x329
> +#define CSR_MHPMEVENT10                        0x32a
> +#define CSR_MHPMEVENT11                        0x32b
> +#define CSR_MHPMEVENT12                        0x32c
> +#define CSR_MHPMEVENT13                        0x32d
> +#define CSR_MHPMEVENT14                        0x32e
> +#define CSR_MHPMEVENT15                        0x32f
> +#define CSR_MHPMEVENT16                        0x330
> +#define CSR_MHPMEVENT17                        0x331
> +#define CSR_MHPMEVENT18                        0x332
> +#define CSR_MHPMEVENT19                        0x333
> +#define CSR_MHPMEVENT20                        0x334
> +#define CSR_MHPMEVENT21                        0x335
> +#define CSR_MHPMEVENT22                        0x336
> +#define CSR_MHPMEVENT23                        0x337
> +#define CSR_MHPMEVENT24                        0x338
> +#define CSR_MHPMEVENT25                        0x339
> +#define CSR_MHPMEVENT26                        0x33a
> +#define CSR_MHPMEVENT27                        0x33b
> +#define CSR_MHPMEVENT28                        0x33c
> +#define CSR_MHPMEVENT29                        0x33d
> +#define CSR_MHPMEVENT30                        0x33e
> +#define CSR_MHPMEVENT31                        0x33f
> +
> +/* For RV32 */
> +#define CSR_MHPMEVENT3H                        0x723
> +#define CSR_MHPMEVENT4H                        0x724
> +#define CSR_MHPMEVENT5H                        0x725
> +#define CSR_MHPMEVENT6H                        0x726
> +#define CSR_MHPMEVENT7H                        0x727
> +#define CSR_MHPMEVENT8H                        0x728
> +#define CSR_MHPMEVENT9H                        0x729
> +#define CSR_MHPMEVENT10H               0x72a
> +#define CSR_MHPMEVENT11H               0x72b
> +#define CSR_MHPMEVENT12H               0x72c
> +#define CSR_MHPMEVENT13H               0x72d
> +#define CSR_MHPMEVENT14H               0x72e
> +#define CSR_MHPMEVENT15H               0x72f
> +#define CSR_MHPMEVENT16H               0x730
> +#define CSR_MHPMEVENT17H               0x731
> +#define CSR_MHPMEVENT18H               0x732
> +#define CSR_MHPMEVENT19H               0x733
> +#define CSR_MHPMEVENT20H               0x734
> +#define CSR_MHPMEVENT21H               0x735
> +#define CSR_MHPMEVENT22H               0x736
> +#define CSR_MHPMEVENT23H               0x737
> +#define CSR_MHPMEVENT24H               0x738
> +#define CSR_MHPMEVENT25H               0x739
> +#define CSR_MHPMEVENT26H               0x73a
> +#define CSR_MHPMEVENT27H               0x73b
> +#define CSR_MHPMEVENT28H               0x73c
> +#define CSR_MHPMEVENT29H               0x73d
> +#define CSR_MHPMEVENT30H               0x73e
> +#define CSR_MHPMEVENT31H               0x73f
> +
> +/* Counter Overflow CSR */
> +#define CSR_SCOUNTOVF                  0xda0
> +
> +/* Debug/Trace Registers */
> +#define CSR_TSELECT                    0x7a0
> +#define CSR_TDATA1                     0x7a1
> +#define CSR_TDATA2                     0x7a2
> +#define CSR_TDATA3                     0x7a3
> +
> +/* Debug Mode Registers */
> +#define CSR_DCSR                       0x7b0
> +#define CSR_DPC                                0x7b1
> +#define CSR_DSCRATCH0                  0x7b2
> +#define CSR_DSCRATCH1                  0x7b3
> +
> +/* Machine-Level Window to Indirectly Accessed Registers (AIA) */
> +#define CSR_MISELECT                   0x350
> +#define CSR_MIREG                      0x351
> +
> +/* Machine-Level Interrupts (AIA) */
> +#define CSR_MTOPEI                     0x35c
> +#define CSR_MTOPI                      0xfb0
> +
> +/* Virtual Interrupts for Supervisor Level (AIA) */
> +#define CSR_MVIEN                      0x308
> +#define CSR_MVIP                       0x309
> +
> +/* Smstateen extension registers */
> +/* Machine stateen CSRs */
> +#define CSR_MSTATEEN0                  0x30C
> +#define CSR_MSTATEEN0H                 0x31C
> +#define CSR_MSTATEEN1                  0x30D
> +#define CSR_MSTATEEN1H                 0x31D
> +#define CSR_MSTATEEN2                  0x30E
> +#define CSR_MSTATEEN2H                 0x31E
> +#define CSR_MSTATEEN3                  0x30F
> +#define CSR_MSTATEEN3H                 0x31F
> +
> +/* Machine-Level High-Half CSRs (AIA) */
> +#define CSR_MIDELEGH                   0x313
> +#define CSR_MIEH                       0x314
> +#define CSR_MVIENH                     0x318
> +#define CSR_MVIPH                      0x319
> +#define CSR_MIPH                       0x354
> +
> +/* ===== Trap/Exception Causes ===== */
> +
> +/* Exception cause high bit - is an interrupt if set */
> +#define CAUSE_IRQ_FLAG                 (_UL(1) << (__riscv_xlen - 1))
> +
> +#define CAUSE_MISALIGNED_FETCH         0x0
> +#define CAUSE_FETCH_ACCESS             0x1
> +#define CAUSE_ILLEGAL_INSTRUCTION      0x2
> +#define CAUSE_BREAKPOINT               0x3
> +#define CAUSE_MISALIGNED_LOAD          0x4
> +#define CAUSE_LOAD_ACCESS              0x5
> +#define CAUSE_MISALIGNED_STORE         0x6
> +#define CAUSE_STORE_ACCESS             0x7
> +#define CAUSE_USER_ECALL               0x8
> +#define CAUSE_SUPERVISOR_ECALL         0x9
> +#define CAUSE_VIRTUAL_SUPERVISOR_ECALL 0xa
> +#define CAUSE_MACHINE_ECALL            0xb
> +#define CAUSE_FETCH_PAGE_FAULT         0xc
> +#define CAUSE_LOAD_PAGE_FAULT          0xd
> +#define CAUSE_STORE_PAGE_FAULT         0xf
> +#define CAUSE_FETCH_GUEST_PAGE_FAULT   0x14
> +#define CAUSE_LOAD_GUEST_PAGE_FAULT    0x15
> +#define CAUSE_VIRTUAL_INST_FAULT       0x16
> +#define CAUSE_STORE_GUEST_PAGE_FAULT   0x17
> +
> +/* Common defines for all smstateen */
> +#define SMSTATEEN_MAX_COUNT            4
> +#define SMSTATEEN0_CS_SHIFT            0
> +#define SMSTATEEN0_CS                  (_ULL(1) << SMSTATEEN0_CS_SHIFT)
> +#define SMSTATEEN0_FCSR_SHIFT          1
> +#define SMSTATEEN0_FCSR                        (_ULL(1) << SMSTATEEN0_FCSR_SHIFT)
> +#define SMSTATEEN0_IMSIC_SHIFT         58
> +#define SMSTATEEN0_IMSIC               (_ULL(1) << SMSTATEEN0_IMSIC_SHIFT)
> +#define SMSTATEEN0_AIA_SHIFT           59
> +#define SMSTATEEN0_AIA                 (_ULL(1) << SMSTATEEN0_AIA_SHIFT)
> +#define SMSTATEEN0_SVSLCT_SHIFT                60
> +#define SMSTATEEN0_SVSLCT              (_ULL(1) << SMSTATEEN0_SVSLCT_SHIFT)
> +#define SMSTATEEN0_HSENVCFG_SHIFT      62
> +#define SMSTATEEN0_HSENVCFG            (_ULL(1) << SMSTATEEN0_HSENVCFG_SHIFT)
> +#define SMSTATEEN_STATEN_SHIFT         63
> +#define SMSTATEEN_STATEN               (_ULL(1) << SMSTATEEN_STATEN_SHIFT)
> +
> +/* ===== Instruction Encodings ===== */
> +
> +#define INSN_MATCH_LB                  0x3
> +#define INSN_MASK_LB                   0x707f
> +#define INSN_MATCH_LH                  0x1003
> +#define INSN_MASK_LH                   0x707f
> +#define INSN_MATCH_LW                  0x2003
> +#define INSN_MASK_LW                   0x707f
> +#define INSN_MATCH_LD                  0x3003
> +#define INSN_MASK_LD                   0x707f
> +#define INSN_MATCH_LBU                 0x4003
> +#define INSN_MASK_LBU                  0x707f
> +#define INSN_MATCH_LHU                 0x5003
> +#define INSN_MASK_LHU                  0x707f
> +#define INSN_MATCH_LWU                 0x6003
> +#define INSN_MASK_LWU                  0x707f
> +#define INSN_MATCH_SB                  0x23
> +#define INSN_MASK_SB                   0x707f
> +#define INSN_MATCH_SH                  0x1023
> +#define INSN_MASK_SH                   0x707f
> +#define INSN_MATCH_SW                  0x2023
> +#define INSN_MASK_SW                   0x707f
> +#define INSN_MATCH_SD                  0x3023
> +#define INSN_MASK_SD                   0x707f
> +
> +#define INSN_MATCH_FLW                 0x2007
> +#define INSN_MASK_FLW                  0x707f
> +#define INSN_MATCH_FLD                 0x3007
> +#define INSN_MASK_FLD                  0x707f
> +#define INSN_MATCH_FLQ                 0x4007
> +#define INSN_MASK_FLQ                  0x707f
> +#define INSN_MATCH_FSW                 0x2027
> +#define INSN_MASK_FSW                  0x707f
> +#define INSN_MATCH_FSD                 0x3027
> +#define INSN_MASK_FSD                  0x707f
> +#define INSN_MATCH_FSQ                 0x4027
> +#define INSN_MASK_FSQ                  0x707f
> +
> +#define INSN_MATCH_C_LD                        0x6000
> +#define INSN_MASK_C_LD                 0xe003
> +#define INSN_MATCH_C_SD                        0xe000
> +#define INSN_MASK_C_SD                 0xe003
> +#define INSN_MATCH_C_LW                        0x4000
> +#define INSN_MASK_C_LW                 0xe003
> +#define INSN_MATCH_C_SW                        0xc000
> +#define INSN_MASK_C_SW                 0xe003
> +#define INSN_MATCH_C_LDSP              0x6002
> +#define INSN_MASK_C_LDSP               0xe003
> +#define INSN_MATCH_C_SDSP              0xe002
> +#define INSN_MASK_C_SDSP               0xe003
> +#define INSN_MATCH_C_LWSP              0x4002
> +#define INSN_MASK_C_LWSP               0xe003
> +#define INSN_MATCH_C_SWSP              0xc002
> +#define INSN_MASK_C_SWSP               0xe003
> +
> +#define INSN_MATCH_C_FLD               0x2000
> +#define INSN_MASK_C_FLD                        0xe003
> +#define INSN_MATCH_C_FLW               0x6000
> +#define INSN_MASK_C_FLW                        0xe003
> +#define INSN_MATCH_C_FSD               0xa000
> +#define INSN_MASK_C_FSD                        0xe003
> +#define INSN_MATCH_C_FSW               0xe000
> +#define INSN_MASK_C_FSW                        0xe003
> +#define INSN_MATCH_C_FLDSP             0x2002
> +#define INSN_MASK_C_FLDSP              0xe003
> +#define INSN_MATCH_C_FSDSP             0xa002
> +#define INSN_MASK_C_FSDSP              0xe003
> +#define INSN_MATCH_C_FLWSP             0x6002
> +#define INSN_MASK_C_FLWSP              0xe003
> +#define INSN_MATCH_C_FSWSP             0xe002
> +#define INSN_MASK_C_FSWSP              0xe003
> +
> +#define INSN_MASK_WFI                  0xffffff00
> +#define INSN_MATCH_WFI                 0x10500000
> +
> +#define INSN_MASK_FENCE_TSO            0xffffffff
> +#define INSN_MATCH_FENCE_TSO           0x8330000f
> +
> +#if __riscv_xlen == 64
> +
> +/* 64-bit read for VS-stage address translation (RV64) */
> +#define INSN_PSEUDO_VS_LOAD            0x00003000
> +
> +/* 64-bit write for VS-stage address translation (RV64) */
> +#define INSN_PSEUDO_VS_STORE   0x00003020
> +
> +#elif __riscv_xlen == 32
> +
> +/* 32-bit read for VS-stage address translation (RV32) */
> +#define INSN_PSEUDO_VS_LOAD            0x00002000
> +
> +/* 32-bit write for VS-stage address translation (RV32) */
> +#define INSN_PSEUDO_VS_STORE   0x00002020
> +
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif
> +
> +#define INSN_16BIT_MASK                        0x3
> +#define INSN_32BIT_MASK                        0x1c
> +
> +#define INSN_IS_16BIT(insn)            \
> +       (((insn) & INSN_16BIT_MASK) != INSN_16BIT_MASK)
> +#define INSN_IS_32BIT(insn)            \
> +       (((insn) & INSN_16BIT_MASK) == INSN_16BIT_MASK && \
> +        ((insn) & INSN_32BIT_MASK) != INSN_32BIT_MASK)
> +
> +#define INSN_LEN(insn)                 (INSN_IS_16BIT(insn) ? 2 : 4)
> +
> +#if __riscv_xlen == 64
> +#define LOG_REGBYTES                   3
> +#else
> +#define LOG_REGBYTES                   2
> +#endif
> +#define REGBYTES                       (1 << LOG_REGBYTES)
> +
> +#define SH_RD                          7
> +#define SH_RS1                         15
> +#define SH_RS2                         20
> +#define SH_RS2C                                2
> +
> +#define RV_X(x, s, n)                  (((x) >> (s)) & ((1 << (n)) - 1))
> +#define RVC_LW_IMM(x)                  ((RV_X(x, 6, 1) << 2) | \
> +                                        (RV_X(x, 10, 3) << 3) | \
> +                                        (RV_X(x, 5, 1) << 6))
> +#define RVC_LD_IMM(x)                  ((RV_X(x, 10, 3) << 3) | \
> +                                        (RV_X(x, 5, 2) << 6))
> +#define RVC_LWSP_IMM(x)                        ((RV_X(x, 4, 3) << 2) | \
> +                                        (RV_X(x, 12, 1) << 5) | \
> +                                        (RV_X(x, 2, 2) << 6))
> +#define RVC_LDSP_IMM(x)                        ((RV_X(x, 5, 2) << 3) | \
> +                                        (RV_X(x, 12, 1) << 5) | \
> +                                        (RV_X(x, 2, 3) << 6))
> +#define RVC_SWSP_IMM(x)                        ((RV_X(x, 9, 4) << 2) | \
> +                                        (RV_X(x, 7, 2) << 6))
> +#define RVC_SDSP_IMM(x)                        ((RV_X(x, 10, 3) << 3) | \
> +                                        (RV_X(x, 7, 3) << 6))
> +#define RVC_RS1S(insn)                 (8 + RV_X(insn, SH_RD, 3))
> +#define RVC_RS2S(insn)                 (8 + RV_X(insn, SH_RS2C, 3))
> +#define RVC_RS2(insn)                  RV_X(insn, SH_RS2C, 5)
> +
> +#define SHIFT_RIGHT(x, y)              \
> +       ((y) < 0 ? ((x) << -(y)) : ((x) >> (y)))
> +
> +#define REG_MASK                       \
> +       ((1 << (5 + LOG_REGBYTES)) - (1 << LOG_REGBYTES))
> +
> +#define REG_OFFSET(insn, pos)          \
> +       (SHIFT_RIGHT((insn), (pos) - LOG_REGBYTES) & REG_MASK)
> +
> +#define REG_PTR(insn, pos, regs)       \
> +       (unsigned long *)((unsigned long)(regs) + REG_OFFSET(insn, pos))
> +
> +#define GET_RM(insn)                   (((insn) >> 12) & 7)
> +
> +#define GET_RS1(insn, regs)            (*REG_PTR(insn, SH_RS1, regs))
> +#define GET_RS2(insn, regs)            (*REG_PTR(insn, SH_RS2, regs))
> +#define GET_RS1S(insn, regs)           (*REG_PTR(RVC_RS1S(insn), 0, regs))
> +#define GET_RS2S(insn, regs)           (*REG_PTR(RVC_RS2S(insn), 0, regs))
> +#define GET_RS2C(insn, regs)           (*REG_PTR(insn, SH_RS2C, regs))
> +#define GET_SP(regs)                   (*REG_PTR(2, 0, regs))
> +#define SET_RD(insn, regs, val)                (*REG_PTR(insn, SH_RD, regs) = (val))
> +#define IMM_I(insn)                    ((int32_t)(insn) >> 20)
> +#define IMM_S(insn)                    (((int32_t)(insn) >> 25 << 5) | \
> +                                        (int32_t)(((insn) >> 7) & 0x1f))
> +#define MASK_FUNCT3                    0x7000
> +
> +/* clang-format on */
> +
> +#endif
> --
> 2.39.0
>
>

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Jan Beulich]

On 30.01.2023 12:54, Oleksii wrote:
> Hi Jan,
> 
> On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
>> On 27.01.2023 14:59, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/processor.h
>>> @@ -0,0 +1,82 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*****************************************************************
>>> *************
>>> + *
>>> + * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
>>> + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
>>> + * Copyright 2023 (C) Vates
>>> + *
>>> + */
>>> +
>>> +#ifndef _ASM_RISCV_PROCESSOR_H
>>> +#define _ASM_RISCV_PROCESSOR_H
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +
>>> +/* On stack VCPU state */
>>> +struct cpu_user_regs
>>> +{
>>> +    unsigned long zero;
>>> +    unsigned long ra;
>>> +    unsigned long sp;
>>> +    unsigned long gp;
>>> +    unsigned long tp;
>>> +    unsigned long t0;
>>> +    unsigned long t1;
>>> +    unsigned long t2;
>>> +    unsigned long s0;
>>> +    unsigned long s1;
>>> +    unsigned long a0;
>>> +    unsigned long a1;
>>> +    unsigned long a2;
>>> +    unsigned long a3;
>>> +    unsigned long a4;
>>> +    unsigned long a5;
>>> +    unsigned long a6;
>>> +    unsigned long a7;
>>> +    unsigned long s2;
>>> +    unsigned long s3;
>>> +    unsigned long s4;
>>> +    unsigned long s5;
>>> +    unsigned long s6;
>>> +    unsigned long s7;
>>> +    unsigned long s8;
>>> +    unsigned long s9;
>>> +    unsigned long s10;
>>> +    unsigned long s11;
>>> +    unsigned long t3;
>>> +    unsigned long t4;
>>> +    unsigned long t5;
>>> +    unsigned long t6;
>>> +    unsigned long sepc;
>>> +    unsigned long sstatus;
>>> +    /* pointer to previous stack_cpu_regs */
>>> +    unsigned long pregs;
>>> +};
>>
>> Just to restate what I said on the earlier version: We have a struct
>> of
>> this name in the public interface for x86. Besides to confusion about
>> re-using the name for something private, I'd still like to understand
>> what the public interface plans are. This is specifically because I
>> think it would be better to re-use suitable public interface structs
>> internally where possible. But that of course requires spelling out
>> such parts of the public interface first.
>>
> I am not sure that I get you here.
> I greped a little bit and found out that each architecture declares
> this structure inside arch-specific folder.
> 
> Mostly the usage of the cpu_user_regs is to save/restore current state
> of CPU during traps ( exceptions/interrupts ) and context_switch().

Arm effectively duplicates the public interface struct vcpu_guest_core_regs
and the internal struct cpu_user_regs (and this goes as far as also
duplicating the __DECL_REG() helper). Personally I find such duplication
odd at the first glance at least; maybe there is a specific reason for this
in Arm. But whether the public interface struct can be re-used can likely
only be known once it was spelled out.

> Also some registers are modified during construction of a domain.
> Thereby I prefer here to see the arch-specific register names instead
> of common.

Not sure what meaning of "common" you imply here. Surely register names
want to be arch-specific, and hence can't be "common" with other arch-es.

Jan

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Julien Grall]

Hi,

On 30/01/2023 11:40, Oleksii wrote:
> On Fri, 2023-01-27 at 14:54 +0000, Julien Grall wrote:
>> Hi Oleksii,
>>
>> On 27/01/2023 13:59, Oleksii Kurochko wrote:
>>> +static inline void wfi(void)
>>> +{
>>> +    __asm__ __volatile__ ("wfi");
>>
>> I have starred at this line for a while and I am not quite too sure
>> to
>> understand why we don't need to clobber the memory like we do on Arm.
>>
> I don't have an answer. The code was based on Linux so...

Hmmm ok. It would probably wise to understand how code imported from 
Linux work so we don't end up introducing bug when calling such function.

 From your current use in this patch, I don't expect any issue. That may 
chance for the others use.

>> FWIW, Linux is doing the same, so I guess this is correct. For Arm we
>> also follow the Linux implementation.
>>
>> But I am wondering whether we are just too strict on Arm, RISCv
>> compiler
>> offer a different guarantee, or you expect the user to be responsible
>> to
>> prevent the compiler to do harmful optimization.
>>
>>> +/*
>>> + * panic() isn't available at the moment so an infinite loop will
>>> be
>>> + * used temporarily.
>>> + * TODO: change it to panic()
>>> + */
>>> +static inline void die(void)
>>> +{
>>> +    for( ;; ) wfi();
>>
>> Please move wfi() to a newline.
> Thanks.
> 
> I thought that it is fine to put into one line in this case but I'll
> move it to a newline. It's fine.

I am not aware of any place in Xen where we would combine the lines.
Also, you want a space after 'for'.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Julien Grall]

Hi Oleksii,

On 30/01/2023 11:35, Oleksii wrote:
> Hi Julien,
> On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
>> Hi Oleksii,
>>
>> On 27/01/2023 13:59, Oleksii Kurochko wrote:
>>> The patch introduces macros: BUG(), WARN(), run_in_exception(),
>>> assert_failed.
>>>
>>> The implementation uses "ebreak" instruction in combination with
>>> diffrent bug frame tables (for each type) which contains useful
>>> information.
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> ---
>>> Changes:
>>>     - Remove __ in define namings
>>>     - Update run_in_exception_handler() with
>>>       register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
>>>     - Remove bug_instr_t type and change it's usage to uint32_t
>>> ---
>>>    xen/arch/riscv/include/asm/bug.h | 118
>>> ++++++++++++++++++++++++++++
>>>    xen/arch/riscv/traps.c           | 128
>>> +++++++++++++++++++++++++++++++
>>>    xen/arch/riscv/xen.lds.S         |  10 +++
>>>    3 files changed, 256 insertions(+)
>>>    create mode 100644 xen/arch/riscv/include/asm/bug.h
>>>
>>> diff --git a/xen/arch/riscv/include/asm/bug.h
>>> b/xen/arch/riscv/include/asm/bug.h
>>> new file mode 100644
>>> index 0000000000..4b15d8eba6
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/bug.h
>>> @@ -0,0 +1,118 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (C) 2012 Regents of the University of California
>>> + * Copyright (C) 2021-2023 Vates
>>
>> I have to question the two copyrights here given that the majority of
>> the code seems to be taken from the arm implementation (see
>> arch/arm/include/asm/bug.h).
>>
>> With that said, we should consolidate the code rather than
>> duplicating
>> it on every architecture.
>>
> Copyrights should be removed. They were taken from the previous
> implementation of bug.h for RISC-V so I just forgot to remove them.
> 
> It looks like we should have common bug.h for ARM and RISCV but I am
> not sure that I know how to do that better.
> Probably the code wants to be moved to xen/include/bug.h and using
> ifdef ARM && RISCV ...

Or you could introduce CONFIG_BUG_GENERIC or else, so it is easily 
selectable by other architecture.

> But still I am not sure that this is the best one option as at least we
> have different implementation for x86_64.

My main concern is the maintainance effort. For every bug, we would need 
to fix it in two places. The risk is we may forget to fix one architecture.
This is not a very ideal situation.

So I think sharing the header between RISC-V and Arm (or x86, see below) 
is at least a must. We can do the 3rd architecture at a leisure pace.

One option would be to introduce asm-generic like Linux (IIRC this was a 
suggestion from Andrew). This would also to share code between two of 
the archs.

Also, from a brief look, the difference in implementation is mainly 
because on Arm we can't use %c (some version of GCC didn't support it). 
Is this also the case on RISC-V? If not, you may want to consider to use 
the x86 version.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Julien Grall]

Hi Jan,

On 30/01/2023 13:50, Jan Beulich wrote:
> On 30.01.2023 12:54, Oleksii wrote:
>> Hi Jan,
>>
>> On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
>>> On 27.01.2023 14:59, Oleksii Kurochko wrote:
>>>> --- /dev/null
>>>> +++ b/xen/arch/riscv/include/asm/processor.h
>>>> @@ -0,0 +1,82 @@
>>>> +/* SPDX-License-Identifier: MIT */
>>>> +/*****************************************************************
>>>> *************
>>>> + *
>>>> + * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
>>>> + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
>>>> + * Copyright 2023 (C) Vates
>>>> + *
>>>> + */
>>>> +
>>>> +#ifndef _ASM_RISCV_PROCESSOR_H
>>>> +#define _ASM_RISCV_PROCESSOR_H
>>>> +
>>>> +#ifndef __ASSEMBLY__
>>>> +
>>>> +/* On stack VCPU state */
>>>> +struct cpu_user_regs
>>>> +{
>>>> +    unsigned long zero;
>>>> +    unsigned long ra;
>>>> +    unsigned long sp;
>>>> +    unsigned long gp;
>>>> +    unsigned long tp;
>>>> +    unsigned long t0;
>>>> +    unsigned long t1;
>>>> +    unsigned long t2;
>>>> +    unsigned long s0;
>>>> +    unsigned long s1;
>>>> +    unsigned long a0;
>>>> +    unsigned long a1;
>>>> +    unsigned long a2;
>>>> +    unsigned long a3;
>>>> +    unsigned long a4;
>>>> +    unsigned long a5;
>>>> +    unsigned long a6;
>>>> +    unsigned long a7;
>>>> +    unsigned long s2;
>>>> +    unsigned long s3;
>>>> +    unsigned long s4;
>>>> +    unsigned long s5;
>>>> +    unsigned long s6;
>>>> +    unsigned long s7;
>>>> +    unsigned long s8;
>>>> +    unsigned long s9;
>>>> +    unsigned long s10;
>>>> +    unsigned long s11;
>>>> +    unsigned long t3;
>>>> +    unsigned long t4;
>>>> +    unsigned long t5;
>>>> +    unsigned long t6;
>>>> +    unsigned long sepc;
>>>> +    unsigned long sstatus;
>>>> +    /* pointer to previous stack_cpu_regs */
>>>> +    unsigned long pregs;
>>>> +};
>>>
>>> Just to restate what I said on the earlier version: We have a struct
>>> of
>>> this name in the public interface for x86. Besides to confusion about
>>> re-using the name for something private, I'd still like to understand
>>> what the public interface plans are. This is specifically because I
>>> think it would be better to re-use suitable public interface structs
>>> internally where possible. But that of course requires spelling out
>>> such parts of the public interface first.
>>>
>> I am not sure that I get you here.
>> I greped a little bit and found out that each architecture declares
>> this structure inside arch-specific folder.
>>
>> Mostly the usage of the cpu_user_regs is to save/restore current state
>> of CPU during traps ( exceptions/interrupts ) and context_switch().
> 
> Arm effectively duplicates the public interface struct vcpu_guest_core_regs
> and the internal struct cpu_user_regs (and this goes as far as also
> duplicating the __DECL_REG() helper). Personally I find such duplication
> odd at the first glance at least; maybe there is a specific reason for this
> in Arm. But whether the public interface struct can be re-used can likely
> only be known once it was spelled out.

There are some force padding, a different ordering and some extra fields 
in the internal version to simplify the assembly code.

The rationale is explained in 1c38a1e937d3 ("xen: arm: separate guest 
user regs from internal guest state").

We also have a split between 32-bit and 64-bit to avoid doubling up the 
size in the former case.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 01/14] xen/riscv: add _zicsr to CFLAGS

[Alistair Francis]

On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> Work with some registers requires csr command which is part of
> Zicsr.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V2:
>   - Nothing changed
> ---
>  xen/arch/riscv/arch.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
> index 012dc677c3..95b41d9f3e 100644
> --- a/xen/arch/riscv/arch.mk
> +++ b/xen/arch/riscv/arch.mk
> @@ -10,7 +10,7 @@ riscv-march-$(CONFIG_RISCV_ISA_C)       := $(riscv-march-y)c
>  # into the upper half _or_ the lower half of the address space.
>  # -mcmodel=medlow would force Xen into the lower half.
>
> -CFLAGS += -march=$(riscv-march-y) -mstrict-align -mcmodel=medany
> +CFLAGS += -march=$(riscv-march-y)_zicsr -mstrict-align -mcmodel=medany
>
>  # TODO: Drop override when more of the build is working
>  override ALL_OBJS-y = arch/$(TARGET_ARCH)/built_in.o
> --
> 2.39.0
>
>

Re: [PATCH v2 02/14] xen/riscv: add <asm/asm.h> header

[Alistair Francis]

On Fri, Jan 27, 2023 at 11:59 PM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V2:
>   - Nothing changed
> ---
>  xen/arch/riscv/include/asm/asm.h | 54 ++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/asm.h
>
> diff --git a/xen/arch/riscv/include/asm/asm.h b/xen/arch/riscv/include/asm/asm.h
> new file mode 100644
> index 0000000000..6d426ecea7
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/asm.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only) */
> +/*
> + * Copyright (C) 2015 Regents of the University of California
> + */
> +
> +#ifndef _ASM_RISCV_ASM_H
> +#define _ASM_RISCV_ASM_H
> +
> +#ifdef __ASSEMBLY__
> +#define __ASM_STR(x)   x
> +#else
> +#define __ASM_STR(x)   #x
> +#endif
> +
> +#if __riscv_xlen == 64
> +#define __REG_SEL(a, b)        __ASM_STR(a)
> +#elif __riscv_xlen == 32
> +#define __REG_SEL(a, b)        __ASM_STR(b)
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif
> +
> +#define REG_L          __REG_SEL(ld, lw)
> +#define REG_S          __REG_SEL(sd, sw)
> +
> +#if __SIZEOF_POINTER__ == 8
> +#ifdef __ASSEMBLY__
> +#define RISCV_PTR              .dword
> +#else
> +#define RISCV_PTR              ".dword"
> +#endif
> +#elif __SIZEOF_POINTER__ == 4
> +#ifdef __ASSEMBLY__
> +#define RISCV_PTR              .word
> +#else
> +#define RISCV_PTR              ".word"
> +#endif
> +#else
> +#error "Unexpected __SIZEOF_POINTER__"
> +#endif
> +
> +#if (__SIZEOF_INT__ == 4)
> +#define RISCV_INT              __ASM_STR(.word)
> +#else
> +#error "Unexpected __SIZEOF_INT__"
> +#endif
> +
> +#if (__SIZEOF_SHORT__ == 2)
> +#define RISCV_SHORT            __ASM_STR(.half)
> +#else
> +#error "Unexpected __SIZEOF_SHORT__"
> +#endif
> +
> +#endif /* _ASM_RISCV_ASM_H */
> --
> 2.39.0
>
>

Re: [PATCH v2 05/14] xen/riscv: introduce empty <asm/string.h>

[Alistair Francis]

On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> To include <xen/lib.h> <asm/string.h> is required
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V2:
>   - <asm/string.h> is a new empty header which is required to include
>     <xen/lib.h>
> ---
>  xen/arch/riscv/include/asm/string.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/string.h
>
> diff --git a/xen/arch/riscv/include/asm/string.h b/xen/arch/riscv/include/asm/string.h
> new file mode 100644
> index 0000000000..a26ba8f5c6
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/string.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_RISCV_STRING_H
> +#define _ASM_RISCV_STRING_H
> +
> +#endif /* _ASM_RISCV_STRING_H */
> --
> 2.39.0
>
>

Re: [PATCH v2 06/14] xen/riscv: introduce empty <asm/cache.h>

[Alistair Francis]

On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> To include <xen/lib.h> <asm/cache.h> is required
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V2:
>   - <asm/cache.h> is a new empty header which is required to include
>     <xen/lib.h>
> ---
>  xen/arch/riscv/include/asm/cache.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/cache.h
>
> diff --git a/xen/arch/riscv/include/asm/cache.h b/xen/arch/riscv/include/asm/cache.h
> new file mode 100644
> index 0000000000..69573eb051
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/cache.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_RISCV_CACHE_H
> +#define _ASM_RISCV_CACHE_H
> +
> +#endif /* _ASM_RISCV_CACHE_H */
> --
> 2.39.0
>
>

Re: [PATCH v2 01/14] xen/riscv: add _zicsr to CFLAGS

[Jan Beulich]

On 31.01.2023 01:21, Alistair Francis wrote:
> On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
> <oleksii.kurochko@gmail.com> wrote:
>>
>> Work with some registers requires csr command which is part of
>> Zicsr.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

There is an open aspect Andrew has pointed out on an earlier version.
I think it would be quite helpful if that could be settled one way or
another before this patch gets committed (which formally may now be
possible, depending on whether that open aspect is considered an
"open" objection, as per ./MAINTAINERS).

Jan

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Oleksii]

Hi Julien,

On Mon, 2023-01-30 at 22:11 +0000, Julien Grall wrote:
> Hi,
> 
> On 30/01/2023 11:40, Oleksii wrote:
> > On Fri, 2023-01-27 at 14:54 +0000, Julien Grall wrote:
> > > Hi Oleksii,
> > > 
> > > On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > > > +static inline void wfi(void)
> > > > +{
> > > > +    __asm__ __volatile__ ("wfi");
> > > 
> > > I have starred at this line for a while and I am not quite too
> > > sure
> > > to
> > > understand why we don't need to clobber the memory like we do on
> > > Arm.
> > > 
> > I don't have an answer. The code was based on Linux so...
> 
> Hmmm ok. It would probably wise to understand how code imported from 
> Linux work so we don't end up introducing bug when calling such
> function.
> 
>  From your current use in this patch, I don't expect any issue. That
> may 
> chance for the others use.
> 
Could you please share with me a link or explain what kind of problems
may occur in case when we don't clobber the memory in the others use
case during usage of "wfi" ?

As I understand the reason for clobber the memory is to cause GCC to
not keep memory values cached in registers across the
assembler	instruction and not optimize stores/load to the memory.
But current one instruction isn't expected to work with the memory so
it should be safe enough to stall current hart ( CPU ) until an
interrupt might need servicing.

Anyway we can change the code to:
    __asm__ __volatile__ ("wfi" ::: "memory")
In order to be sure that no problems will arise in the future.

> > > FWIW, Linux is doing the same, so I guess this is correct. For
> > > Arm we
> > > also follow the Linux implementation.
> > > 
> > > But I am wondering whether we are just too strict on Arm, RISCv
> > > compiler
> > > offer a different guarantee, or you expect the user to be
> > > responsible
> > > to
> > > prevent the compiler to do harmful optimization.
> > > 
> > > > +/*
> > > > + * panic() isn't available at the moment so an infinite loop
> > > > will
> > > > be
> > > > + * used temporarily.
> > > > + * TODO: change it to panic()
> > > > + */
> > > > +static inline void die(void)
> > > > +{
> > > > +    for( ;; ) wfi();
> > > 
> > > Please move wfi() to a newline.
> > Thanks.
> > 
> > I thought that it is fine to put into one line in this case but
> > I'll
> > move it to a newline. It's fine.
> 
> I am not aware of any place in Xen where we would combine the lines.
> Also, you want a space after 'for'.
> 
> Cheers,
> 

~ Oleksii

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii]

On Mon, 2023-01-30 at 22:28 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 30/01/2023 11:35, Oleksii wrote:
> > Hi Julien,
> > On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
> > > Hi Oleksii,
> > > 
> > > On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > > > The patch introduces macros: BUG(), WARN(), run_in_exception(),
> > > > assert_failed.
> > > > 
> > > > The implementation uses "ebreak" instruction in combination
> > > > with
> > > > diffrent bug frame tables (for each type) which contains useful
> > > > information.
> > > > 
> > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > > > ---
> > > > Changes:
> > > >     - Remove __ in define namings
> > > >     - Update run_in_exception_handler() with
> > > >       register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
> > > >     - Remove bug_instr_t type and change it's usage to uint32_t
> > > > ---
> > > >    xen/arch/riscv/include/asm/bug.h | 118
> > > > ++++++++++++++++++++++++++++
> > > >    xen/arch/riscv/traps.c           | 128
> > > > +++++++++++++++++++++++++++++++
> > > >    xen/arch/riscv/xen.lds.S         |  10 +++
> > > >    3 files changed, 256 insertions(+)
> > > >    create mode 100644 xen/arch/riscv/include/asm/bug.h
> > > > 
> > > > diff --git a/xen/arch/riscv/include/asm/bug.h
> > > > b/xen/arch/riscv/include/asm/bug.h
> > > > new file mode 100644
> > > > index 0000000000..4b15d8eba6
> > > > --- /dev/null
> > > > +++ b/xen/arch/riscv/include/asm/bug.h
> > > > @@ -0,0 +1,118 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Copyright (C) 2012 Regents of the University of California
> > > > + * Copyright (C) 2021-2023 Vates
> > > 
> > > I have to question the two copyrights here given that the
> > > majority of
> > > the code seems to be taken from the arm implementation (see
> > > arch/arm/include/asm/bug.h).
> > > 
> > > With that said, we should consolidate the code rather than
> > > duplicating
> > > it on every architecture.
> > > 
> > Copyrights should be removed. They were taken from the previous
> > implementation of bug.h for RISC-V so I just forgot to remove them.
> > 
> > It looks like we should have common bug.h for ARM and RISCV but I
> > am
> > not sure that I know how to do that better.
> > Probably the code wants to be moved to xen/include/bug.h and using
> > ifdef ARM && RISCV ...
> 
> Or you could introduce CONFIG_BUG_GENERIC or else, so it is easily 
> selectable by other architecture.
> 
> > But still I am not sure that this is the best one option as at
> > least we
> > have different implementation for x86_64.
> 
> My main concern is the maintainance effort. For every bug, we would
> need 
> to fix it in two places. The risk is we may forget to fix one
> architecture.
> This is not a very ideal situation.
> 
> So I think sharing the header between RISC-V and Arm (or x86, see
> below) 
> is at least a must. We can do the 3rd architecture at a leisure pace.
> 
> One option would be to introduce asm-generic like Linux (IIRC this
> was a 
> suggestion from Andrew). This would also to share code between two of
> the archs.
> 
> Also, from a brief look, the difference in implementation is mainly 
> because on Arm we can't use %c (some version of GCC didn't support
> it). 
> Is this also the case on RISC-V? If not, you may want to consider to
> use 
> the x86 version.
> 
No, it shouldn't be an issue for RISC-V. I'll double check.
Any way it should bug.h should be shared between archs so I am going to
rework that in this patch series and sent the changes in the next
version of the patch series.

Thanks.

~Oleksii
> Cheers,
> 

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Julien Grall]

On 31/01/2023 12:24, Oleksii wrote:
> Hi Julien,

Hi Oleksii,

> On Mon, 2023-01-30 at 22:11 +0000, Julien Grall wrote:
>> Hi,
>>
>> On 30/01/2023 11:40, Oleksii wrote:
>>> On Fri, 2023-01-27 at 14:54 +0000, Julien Grall wrote:
>>>> Hi Oleksii,
>>>>
>>>> On 27/01/2023 13:59, Oleksii Kurochko wrote:
>>>>> +static inline void wfi(void)
>>>>> +{
>>>>> +    __asm__ __volatile__ ("wfi");
>>>>
>>>> I have starred at this line for a while and I am not quite too
>>>> sure
>>>> to
>>>> understand why we don't need to clobber the memory like we do on
>>>> Arm.
>>>>
>>> I don't have an answer. The code was based on Linux so...
>>
>> Hmmm ok. It would probably wise to understand how code imported from
>> Linux work so we don't end up introducing bug when calling such
>> function.
>>
>>   From your current use in this patch, I don't expect any issue. That
>> may
>> chance for the others use.
>>
> Could you please share with me a link or explain what kind of problems
> may occur in case when we don't clobber the memory in the others use
> case during usage of "wfi" ?

I don't have a link and that's why I was asking the question here.

The concern I have is the following:

1)
    wfi();
    val = *addr;

2)
    *addr = val;
    wfi();


Is the compiler allowed to re-order the sequence so '*addr' will be read 
before 'wfi' or (for the second case) write after 'wfi'?

At the moment, I believe this is why we have the 'memory' clobber on 
Arm. But then I couldn't find any documentation implying that the 
compiler cannot do the re-ordering.

> 
> As I understand the reason for clobber the memory is to cause GCC to
> not keep memory values cached in registers across the
> assembler	instruction and not optimize stores/load to the memory.
> But current one instruction isn't expected to work with the memory so
> it should be safe enough to stall current hart ( CPU ) until an
> interrupt might need servicing.
> 
> Anyway we can change the code to:
>      __asm__ __volatile__ ("wfi" ::: "memory")
> In order to be sure that no problems will arise in the future.

As I wrote earlier, so far, I didn't suggest to change any code. I am 
simply trying to understand how this is meant to work.

One action may be that we can remove the memory clobber on Arm.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 4253 bytes --]

On Mon, 30 Jan 2023, Jan Beulich wrote:
> On 30.01.2023 12:54, Oleksii wrote:
> > Hi Jan,
> > 
> > On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
> >> On 27.01.2023 14:59, Oleksii Kurochko wrote:
> >>> --- /dev/null
> >>> +++ b/xen/arch/riscv/include/asm/processor.h
> >>> @@ -0,0 +1,82 @@
> >>> +/* SPDX-License-Identifier: MIT */
> >>> +/*****************************************************************
> >>> *************
> >>> + *
> >>> + * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
> >>> + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
> >>> + * Copyright 2023 (C) Vates
> >>> + *
> >>> + */
> >>> +
> >>> +#ifndef _ASM_RISCV_PROCESSOR_H
> >>> +#define _ASM_RISCV_PROCESSOR_H
> >>> +
> >>> +#ifndef __ASSEMBLY__
> >>> +
> >>> +/* On stack VCPU state */
> >>> +struct cpu_user_regs
> >>> +{
> >>> +    unsigned long zero;
> >>> +    unsigned long ra;
> >>> +    unsigned long sp;
> >>> +    unsigned long gp;
> >>> +    unsigned long tp;
> >>> +    unsigned long t0;
> >>> +    unsigned long t1;
> >>> +    unsigned long t2;
> >>> +    unsigned long s0;
> >>> +    unsigned long s1;
> >>> +    unsigned long a0;
> >>> +    unsigned long a1;
> >>> +    unsigned long a2;
> >>> +    unsigned long a3;
> >>> +    unsigned long a4;
> >>> +    unsigned long a5;
> >>> +    unsigned long a6;
> >>> +    unsigned long a7;
> >>> +    unsigned long s2;
> >>> +    unsigned long s3;
> >>> +    unsigned long s4;
> >>> +    unsigned long s5;
> >>> +    unsigned long s6;
> >>> +    unsigned long s7;
> >>> +    unsigned long s8;
> >>> +    unsigned long s9;
> >>> +    unsigned long s10;
> >>> +    unsigned long s11;
> >>> +    unsigned long t3;
> >>> +    unsigned long t4;
> >>> +    unsigned long t5;
> >>> +    unsigned long t6;
> >>> +    unsigned long sepc;
> >>> +    unsigned long sstatus;
> >>> +    /* pointer to previous stack_cpu_regs */
> >>> +    unsigned long pregs;
> >>> +};
> >>
> >> Just to restate what I said on the earlier version: We have a struct
> >> of
> >> this name in the public interface for x86. Besides to confusion about
> >> re-using the name for something private, I'd still like to understand
> >> what the public interface plans are. This is specifically because I
> >> think it would be better to re-use suitable public interface structs
> >> internally where possible. But that of course requires spelling out
> >> such parts of the public interface first.
> >>
> > I am not sure that I get you here.
> > I greped a little bit and found out that each architecture declares
> > this structure inside arch-specific folder.
> > 
> > Mostly the usage of the cpu_user_regs is to save/restore current state
> > of CPU during traps ( exceptions/interrupts ) and context_switch().
> 
> Arm effectively duplicates the public interface struct vcpu_guest_core_regs
> and the internal struct cpu_user_regs (and this goes as far as also
> duplicating the __DECL_REG() helper). Personally I find such duplication
> odd at the first glance at least; maybe there is a specific reason for this
> in Arm. But whether the public interface struct can be re-used can likely
> only be known once it was spelled out.

struct vcpu_guest_core_regs is used as part of struct
vcpu_guest_context, which is used for VCPUOP_initialise, which is not
used on ARM and RISC-V (we use a standard firmware interface instead),
and for save/restore, which also is not going to work any time soon on
ARM and RISC-V.

This is to say that we are probably not going to need the public
interface for the next year or two, so it is difficult to tell at this
stage if it aligns well with struct cpu_user_regs. I think we'll have to
cross that bridge when we come to it.


> > Also some registers are modified during construction of a domain.
> > Thereby I prefer here to see the arch-specific register names instead
> > of common.
> 
> Not sure what meaning of "common" you imply here. Surely register names
> want to be arch-specific, and hence can't be "common" with other arch-es.

I think Oleksii misunderstood your request and believed you were asking
him to make struct cpu_user_regs common across arches.

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Andrew Cooper]

On 30/01/2023 10:44 pm, Julien Grall wrote:
> Hi Jan,
>
> On 30/01/2023 13:50, Jan Beulich wrote:
>> On 30.01.2023 12:54, Oleksii wrote:
>>> Hi Jan,
>>>
>>> On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
>>>> On 27.01.2023 14:59, Oleksii Kurochko wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/riscv/include/asm/processor.h
>>>>> @@ -0,0 +1,82 @@
>>>>> +/* SPDX-License-Identifier: MIT */
>>>>> +/*****************************************************************
>>>>> *************
>>>>> + *
>>>>> + * Copyright 2019 (C) Alistair Francis <alistair.francis@wdc.com>
>>>>> + * Copyright 2021 (C) Bobby Eshleman <bobby.eshleman@gmail.com>
>>>>> + * Copyright 2023 (C) Vates
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#ifndef _ASM_RISCV_PROCESSOR_H
>>>>> +#define _ASM_RISCV_PROCESSOR_H
>>>>> +
>>>>> +#ifndef __ASSEMBLY__
>>>>> +
>>>>> +/* On stack VCPU state */
>>>>> +struct cpu_user_regs
>>>>> +{
>>>>> +    unsigned long zero;
>>>>> +    unsigned long ra;
>>>>> +    unsigned long sp;
>>>>> +    unsigned long gp;
>>>>> +    unsigned long tp;
>>>>> +    unsigned long t0;
>>>>> +    unsigned long t1;
>>>>> +    unsigned long t2;
>>>>> +    unsigned long s0;
>>>>> +    unsigned long s1;
>>>>> +    unsigned long a0;
>>>>> +    unsigned long a1;
>>>>> +    unsigned long a2;
>>>>> +    unsigned long a3;
>>>>> +    unsigned long a4;
>>>>> +    unsigned long a5;
>>>>> +    unsigned long a6;
>>>>> +    unsigned long a7;
>>>>> +    unsigned long s2;
>>>>> +    unsigned long s3;
>>>>> +    unsigned long s4;
>>>>> +    unsigned long s5;
>>>>> +    unsigned long s6;
>>>>> +    unsigned long s7;
>>>>> +    unsigned long s8;
>>>>> +    unsigned long s9;
>>>>> +    unsigned long s10;
>>>>> +    unsigned long s11;
>>>>> +    unsigned long t3;
>>>>> +    unsigned long t4;
>>>>> +    unsigned long t5;
>>>>> +    unsigned long t6;
>>>>> +    unsigned long sepc;
>>>>> +    unsigned long sstatus;
>>>>> +    /* pointer to previous stack_cpu_regs */
>>>>> +    unsigned long pregs;
>>>>> +};
>>>>
>>>> Just to restate what I said on the earlier version: We have a struct
>>>> of
>>>> this name in the public interface for x86. Besides to confusion about
>>>> re-using the name for something private, I'd still like to understand
>>>> what the public interface plans are. This is specifically because I
>>>> think it would be better to re-use suitable public interface structs
>>>> internally where possible. But that of course requires spelling out
>>>> such parts of the public interface first.
>>>>
>>> I am not sure that I get you here.
>>> I greped a little bit and found out that each architecture declares
>>> this structure inside arch-specific folder.
>>>
>>> Mostly the usage of the cpu_user_regs is to save/restore current state
>>> of CPU during traps ( exceptions/interrupts ) and context_switch().
>>
>> Arm effectively duplicates the public interface struct
>> vcpu_guest_core_regs
>> and the internal struct cpu_user_regs (and this goes as far as also
>> duplicating the __DECL_REG() helper). Personally I find such duplication
>> odd at the first glance at least; maybe there is a specific reason
>> for this
>> in Arm. But whether the public interface struct can be re-used can
>> likely
>> only be known once it was spelled out.
>
> There are some force padding, a different ordering and some extra
> fields in the internal version to simplify the assembly code.
>
> The rationale is explained in 1c38a1e937d3 ("xen: arm: separate guest
> user regs from internal guest state").
>
> We also have a split between 32-bit and 64-bit to avoid doubling up
> the size in the former case.

And on top of these reasons, I feel I need to remind you that we still
need to break these apart on x86 to fix a stack OoB access in the #DF
handler, and to fix stack alignment for UEFI, and to remove the relics
of vm86 mode, and not to mention adding support for FRED.

It was a fundamental design error that Xen's internal representation
ever made it into the public API, and it absolutely does not want
repeating again.

~Andrew

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii]

Hi Julien,

On Mon, 2023-01-30 at 22:28 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 30/01/2023 11:35, Oleksii wrote:
> > Hi Julien,
> > On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
> > > Hi Oleksii,
> > > 
> > > On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > > > The patch introduces macros: BUG(), WARN(), run_in_exception(),
> > > > assert_failed.
> > > > 
> > > > The implementation uses "ebreak" instruction in combination
> > > > with
> > > > diffrent bug frame tables (for each type) which contains useful
> > > > information.
> > > > 
> > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > > > ---
> > > > Changes:
> > > >     - Remove __ in define namings
> > > >     - Update run_in_exception_handler() with
> > > >       register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
> > > >     - Remove bug_instr_t type and change it's usage to uint32_t
> > > > ---
> > > >    xen/arch/riscv/include/asm/bug.h | 118
> > > > ++++++++++++++++++++++++++++
> > > >    xen/arch/riscv/traps.c           | 128
> > > > +++++++++++++++++++++++++++++++
> > > >    xen/arch/riscv/xen.lds.S         |  10 +++
> > > >    3 files changed, 256 insertions(+)
> > > >    create mode 100644 xen/arch/riscv/include/asm/bug.h
> > > > 
> > > > diff --git a/xen/arch/riscv/include/asm/bug.h
> > > > b/xen/arch/riscv/include/asm/bug.h
> > > > new file mode 100644
> > > > index 0000000000..4b15d8eba6
> > > > --- /dev/null
> > > > +++ b/xen/arch/riscv/include/asm/bug.h
> > > > @@ -0,0 +1,118 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Copyright (C) 2012 Regents of the University of California
> > > > + * Copyright (C) 2021-2023 Vates
> > > 
> > > I have to question the two copyrights here given that the
> > > majority of
> > > the code seems to be taken from the arm implementation (see
> > > arch/arm/include/asm/bug.h).
> > > 
> > > With that said, we should consolidate the code rather than
> > > duplicating
> > > it on every architecture.
> > > 
> > Copyrights should be removed. They were taken from the previous
> > implementation of bug.h for RISC-V so I just forgot to remove them.
> > 
> > It looks like we should have common bug.h for ARM and RISCV but I
> > am
> > not sure that I know how to do that better.
> > Probably the code wants to be moved to xen/include/bug.h and using
> > ifdef ARM && RISCV ...
> 
> Or you could introduce CONFIG_BUG_GENERIC or else, so it is easily 
> selectable by other architecture.
> 
> > But still I am not sure that this is the best one option as at
> > least we
> > have different implementation for x86_64.
> 
> My main concern is the maintainance effort. For every bug, we would
> need 
> to fix it in two places. The risk is we may forget to fix one
> architecture.
> This is not a very ideal situation.
> 
> So I think sharing the header between RISC-V and Arm (or x86, see
> below) 
> is at least a must. We can do the 3rd architecture at a leisure pace.
> 
> One option would be to introduce asm-generic like Linux (IIRC this
> was a 
> suggestion from Andrew). This would also to share code between two of
> the archs.
> 
> Also, from a brief look, the difference in implementation is mainly 
> because on Arm we can't use %c (some version of GCC didn't support
> it).
> Is this also the case on RISC-V? If not, you may want to consider to
> use 
> the x86 version.
> 
I did several experiments related to '%c' in inline assembly for RISC-V
and it seems that '%c' doesn't support all forms of the use of '%c'.
I wrote the following macros and they have been compiled without any
errors:
                        .....
#define _ASM_BUGFRAME_TEXT(second_frame)                       \
    ".Lbug%=: ebreak\n"                                        \
    ".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
    ".p2align 2\n"                                             \
    ".Lfrm%=:\n"                                               \
    ".long (.Lfrm%=)\n"                                        \
    ".long (0x55555555)\n"                                     \
    ".long (.Lbug%=)\n"                                        \
    ".long (0x55555555)\n"                                     \
    ".long %c[bf_line_hi]\n"                                   \
    ".long (0x55555555)\n"                                     \
    ".long %[bf_line_hi]\n"                                    \
    ".long (0x55555555)\n"                                     \
    ".long %[bf_line_lo]\n"                                    \
    ".long (0x55555555)\n"                                     \
    ".long %[bf_ptr]\n"                                        \
    ".long (0x55555555)\n"                                     \
    ".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n"             \
    ".popsection\n"                                            \

#define _ASM_BUGFRAME_INFO(type, line, ptr, msg)               \
    [bf_type]    "i" (type),                                   \
    [bf_ptr]     "i" (ptr),                                    \
    [bf_msg]     "i" (msg),                                    \
    [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1))  \
                      << BUG_DISP_WIDTH),                      \
    [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)

#define BUG_FRAME(type, line, ptr, second_frame, msg) do {     \
    __asm__ __volatile__ ( _ASM_BUGFRAME_TEXT(second_frame)    \
                   :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
} while (0)
                          ....


But if add ".long %c[bf_ptr]\n" then the following compilation error
will occur: [ error: invalid 'asm': invalid use of '%c'. ]

If you look at the dissembler of _bug_frames_...
                               ......
    80201010:   1010                    addi    a2,sp,32   # .Lfrm%=
    80201012:   8020                    .2byte  0x8020
    80201014:   5555                    li      a0,-11
    80201016:   5555                    li      a0,-11
    80201018:   3022                    .2byte  0x3022  # .Lbug%=
    8020101a:   8020                    .2byte  0x8020
    8020101c:   5555                    li      a0,-11
    8020101e:   5555                    li      a0,-11
    80201020:   0000                    unimp          # %c[bf_line_hi]
    80201022:   0000                    unimp
    80201024:   5555                    li      a0,-11
    80201026:   5555                    li      a0,-11
    80201028:   0000                    unimp           # %[bf_line_hi]
    8020102a:   0000                    unimp
    8020102c:   5555                    li      a0,-11
    8020102e:   5555                    li      a0,-11
    80201030:   0000                    unimp           # %[bf_line_lo]
    80201032:   1600                    addi    s0,sp,800
    80201034:   5555                    li      a0,-11
    80201036:   5555                    li      a0,-11
    80201038:   10b8                    addi    a4,sp,104   # %[bf_ptr]
    8020103a:   8020                    .2byte  0x8020
    8020103c:   5555                    li      a0,-11
    8020103e:   5555                    li      a0,-11
    80201040:   2012                    .2byte  0x2012   # (.Lbug%= -
.Lfrm%=) + %c[bf_line_hi]
                               .....
... it looks like the error will be generated if the name in %c[name]
isn't equal to 0.

Another thing I noticed is that %[name] can be used instead of %c[name]
for RISC-V ( i did a quick check and it works) so it is still possible
to follow Intel implementation but required a redefinition of 
_ASM_BUGFRAME_TEXT where %c[...] won't be used. All the rest will be
the same as in x86 implementation:
                                .....
#define _ASM_BUGFRAME_TEXT(second_frame)                      \
    ".Lbug%=: ebreak\n"                                       \
    ".pushsection .bug_frames.%[bf_type], \"a\", @progbits\n" \
    ".p2align 2\n"                                            \
    ".Lfrm%=:\n"                                              \
    ".long (.Lbug%= - .Lfrm%=) + %[bf_line_hi]\n"             \
    ".long (%[bf_ptr] - .Lfrm%=) + %[bf_line_lo]\n"           \
    ".if " #second_frame "\n"                                 \
    ".long 0, %[bf_msg] - .Lfrm%=\n"                          \
    ".endif\n"                                                \
    ".popsection\n"                                           \
                                 .....

One thing I would like to ask you is why it's better to follow/use x86
implementation instead of ARM?
It seems that "%c[...]" has the best support only for Intel GCC and
thereby ARM implementation looks more universal, doesn't it?

> Cheers,
> 

~ Oleksii

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Julien Grall]

On 01/02/2023 17:40, Oleksii wrote:
> Hi Julien,

Hi Oleksii,

> On Mon, 2023-01-30 at 22:28 +0000, Julien Grall wrote:
>> Hi Oleksii,
>>
>> On 30/01/2023 11:35, Oleksii wrote:
>>> Hi Julien,
>>> On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
>>>> Hi Oleksii,
>>>>
>>>> On 27/01/2023 13:59, Oleksii Kurochko wrote:
>>>>> The patch introduces macros: BUG(), WARN(), run_in_exception(),
>>>>> assert_failed.
>>>>>
>>>>> The implementation uses "ebreak" instruction in combination
>>>>> with
>>>>> diffrent bug frame tables (for each type) which contains useful
>>>>> information.
>>>>>
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> ---
>>>>> Changes:
>>>>>      - Remove __ in define namings
>>>>>      - Update run_in_exception_handler() with
>>>>>        register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
>>>>>      - Remove bug_instr_t type and change it's usage to uint32_t
>>>>> ---
>>>>>     xen/arch/riscv/include/asm/bug.h | 118
>>>>> ++++++++++++++++++++++++++++
>>>>>     xen/arch/riscv/traps.c           | 128
>>>>> +++++++++++++++++++++++++++++++
>>>>>     xen/arch/riscv/xen.lds.S         |  10 +++
>>>>>     3 files changed, 256 insertions(+)
>>>>>     create mode 100644 xen/arch/riscv/include/asm/bug.h
>>>>>
>>>>> diff --git a/xen/arch/riscv/include/asm/bug.h
>>>>> b/xen/arch/riscv/include/asm/bug.h
>>>>> new file mode 100644
>>>>> index 0000000000..4b15d8eba6
>>>>> --- /dev/null
>>>>> +++ b/xen/arch/riscv/include/asm/bug.h
>>>>> @@ -0,0 +1,118 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>> +/*
>>>>> + * Copyright (C) 2012 Regents of the University of California
>>>>> + * Copyright (C) 2021-2023 Vates
>>>>
>>>> I have to question the two copyrights here given that the
>>>> majority of
>>>> the code seems to be taken from the arm implementation (see
>>>> arch/arm/include/asm/bug.h).
>>>>
>>>> With that said, we should consolidate the code rather than
>>>> duplicating
>>>> it on every architecture.
>>>>
>>> Copyrights should be removed. They were taken from the previous
>>> implementation of bug.h for RISC-V so I just forgot to remove them.
>>>
>>> It looks like we should have common bug.h for ARM and RISCV but I
>>> am
>>> not sure that I know how to do that better.
>>> Probably the code wants to be moved to xen/include/bug.h and using
>>> ifdef ARM && RISCV ...
>>
>> Or you could introduce CONFIG_BUG_GENERIC or else, so it is easily
>> selectable by other architecture.
>>
>>> But still I am not sure that this is the best one option as at
>>> least we
>>> have different implementation for x86_64.
>>
>> My main concern is the maintainance effort. For every bug, we would
>> need
>> to fix it in two places. The risk is we may forget to fix one
>> architecture.
>> This is not a very ideal situation.
>>
>> So I think sharing the header between RISC-V and Arm (or x86, see
>> below)
>> is at least a must. We can do the 3rd architecture at a leisure pace.
>>
>> One option would be to introduce asm-generic like Linux (IIRC this
>> was a
>> suggestion from Andrew). This would also to share code between two of
>> the archs.
>>
>> Also, from a brief look, the difference in implementation is mainly
>> because on Arm we can't use %c (some version of GCC didn't support
>> it).
>> Is this also the case on RISC-V? If not, you may want to consider to
>> use
>> the x86 version.
>>
> I did several experiments related to '%c' in inline assembly for RISC-V
> and it seems that '%c' doesn't support all forms of the use of '%c'.

Thanks for checking!

> I wrote the following macros and they have been compiled without any
> errors:
>                          .....
> #define _ASM_BUGFRAME_TEXT(second_frame)                       \
>      ".Lbug%=: ebreak\n"                                        \
>      ".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
>      ".p2align 2\n"                                             \
>      ".Lfrm%=:\n"                                               \
>      ".long (.Lfrm%=)\n"                                        \
>      ".long (0x55555555)\n"                                     \
>      ".long (.Lbug%=)\n"                                        \
>      ".long (0x55555555)\n"                                     \
>      ".long %c[bf_line_hi]\n"                                   \
>      ".long (0x55555555)\n"                                     \
>      ".long %[bf_line_hi]\n"                                    \
>      ".long (0x55555555)\n"                                     \
>      ".long %[bf_line_lo]\n"                                    \
>      ".long (0x55555555)\n"                                     \
>      ".long %[bf_ptr]\n"                                        \
>      ".long (0x55555555)\n"                                     \
>      ".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n"             \
>      ".popsection\n"                                            \
> 
> #define _ASM_BUGFRAME_INFO(type, line, ptr, msg)               \
>      [bf_type]    "i" (type),                                   \
>      [bf_ptr]     "i" (ptr),                                    \
>      [bf_msg]     "i" (msg),                                    \
>      [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1))  \
>                        << BUG_DISP_WIDTH),                      \
>      [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
> 
> #define BUG_FRAME(type, line, ptr, second_frame, msg) do {     \
>      __asm__ __volatile__ ( _ASM_BUGFRAME_TEXT(second_frame)    \
>                     :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
> } while (0)
>                            ....
> 
> 
> But if add ".long %c[bf_ptr]\n" then the following compilation error
> will occur: [ error: invalid 'asm': invalid use of '%c'. ]
> 
> If you look at the dissembler of _bug_frames_...
>                                 ......
>      80201010:   1010                    addi    a2,sp,32   # .Lfrm%=
>      80201012:   8020                    .2byte  0x8020
>      80201014:   5555                    li      a0,-11
>      80201016:   5555                    li      a0,-11
>      80201018:   3022                    .2byte  0x3022  # .Lbug%=
>      8020101a:   8020                    .2byte  0x8020
>      8020101c:   5555                    li      a0,-11
>      8020101e:   5555                    li      a0,-11
>      80201020:   0000                    unimp          # %c[bf_line_hi]
>      80201022:   0000                    unimp
>      80201024:   5555                    li      a0,-11
>      80201026:   5555                    li      a0,-11
>      80201028:   0000                    unimp           # %[bf_line_hi]
>      8020102a:   0000                    unimp
>      8020102c:   5555                    li      a0,-11
>      8020102e:   5555                    li      a0,-11
>      80201030:   0000                    unimp           # %[bf_line_lo]
>      80201032:   1600                    addi    s0,sp,800
>      80201034:   5555                    li      a0,-11
>      80201036:   5555                    li      a0,-11
>      80201038:   10b8                    addi    a4,sp,104   # %[bf_ptr]
>      8020103a:   8020                    .2byte  0x8020
>      8020103c:   5555                    li      a0,-11
>      8020103e:   5555                    li      a0,-11
>      80201040:   2012                    .2byte  0x2012   # (.Lbug%= -
> .Lfrm%=) + %c[bf_line_hi]
>                                 .....
> ... it looks like the error will be generated if the name in %c[name]
> isn't equal to 0.
> 
> Another thing I noticed is that %[name] can be used instead of %c[name]
> for RISC-V ( i did a quick check and it works) so it is still possible
> to follow Intel implementation but required a redefinition of
> _ASM_BUGFRAME_TEXT where %c[...] won't be used. All the rest will be
> the same as in x86 implementation:
>                                  .....
> #define _ASM_BUGFRAME_TEXT(second_frame)                      \
>      ".Lbug%=: ebreak\n"                                       \
>      ".pushsection .bug_frames.%[bf_type], \"a\", @progbits\n" \
>      ".p2align 2\n"                                            \
>      ".Lfrm%=:\n"                                              \
>      ".long (.Lbug%= - .Lfrm%=) + %[bf_line_hi]\n"             \
>      ".long (%[bf_ptr] - .Lfrm%=) + %[bf_line_lo]\n"           \
>      ".if " #second_frame "\n"                                 \
>      ".long 0, %[bf_msg] - .Lfrm%=\n"                          \
>      ".endif\n"                                                \
>      ".popsection\n"                                           \
>                                   .....
> 
> One thing I would like to ask you is why it's better to follow/use x86
> implementation instead of ARM?

IMHO, the x86 version is cleaner. But...

> It seems that "%c[...]" has the best support only for Intel GCC and
> thereby ARM implementation looks more universal, doesn't it?

... you are right, the Arm version is more portable. Although, I do 
wonder how GCC managed to have a different behavior between architecture 
as I would have expected %c to be a generic implementation.

Anyway, if you are basing on the Arm one, then you should be able to
  1) move arch/arm/include/asm/bug.h in asm-generic/bug.h (or similar)
  2) Rename the guard and remove arm specific code.(I am not sure from 
where to include arm{32, 64}/bug.h)
  3) Define BUG_INSTR to ebreak on RISC-V.
  4) Find a place for all the RISC-V specific header
  5) Move do_bug_frame() in common/bug.c

I am happy to help testing the Arm version and/or help moving the code 
to common.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Jan Beulich]

On 01.02.2023 23:11, Julien Grall wrote:
> On 01/02/2023 17:40, Oleksii wrote:
>> I wrote the following macros and they have been compiled without any
>> errors:
>>                          .....
>> #define _ASM_BUGFRAME_TEXT(second_frame)                       \
>>      ".Lbug%=: ebreak\n"                                        \
>>      ".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
>>      ".p2align 2\n"                                             \
>>      ".Lfrm%=:\n"                                               \
>>      ".long (.Lfrm%=)\n"                                        \
>>      ".long (0x55555555)\n"                                     \
>>      ".long (.Lbug%=)\n"                                        \
>>      ".long (0x55555555)\n"                                     \
>>      ".long %c[bf_line_hi]\n"                                   \
>>      ".long (0x55555555)\n"                                     \
>>      ".long %[bf_line_hi]\n"                                    \
>>      ".long (0x55555555)\n"                                     \
>>      ".long %[bf_line_lo]\n"                                    \
>>      ".long (0x55555555)\n"                                     \
>>      ".long %[bf_ptr]\n"                                        \
>>      ".long (0x55555555)\n"                                     \
>>      ".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n"             \
>>      ".popsection\n"                                            \
>>
>> #define _ASM_BUGFRAME_INFO(type, line, ptr, msg)               \
>>      [bf_type]    "i" (type),                                   \
>>      [bf_ptr]     "i" (ptr),                                    \
>>      [bf_msg]     "i" (msg),                                    \
>>      [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1))  \
>>                        << BUG_DISP_WIDTH),                      \
>>      [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
>>
>> #define BUG_FRAME(type, line, ptr, second_frame, msg) do {     \
>>      __asm__ __volatile__ ( _ASM_BUGFRAME_TEXT(second_frame)    \
>>                     :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
>> } while (0)
>>                            ....
>>
>>
>> But if add ".long %c[bf_ptr]\n" then the following compilation error
>> will occur: [ error: invalid 'asm': invalid use of '%c'. ]
>>
>> If you look at the dissembler of _bug_frames_...
>>                                 ......
>>      80201010:   1010                    addi    a2,sp,32   # .Lfrm%=
>>      80201012:   8020                    .2byte  0x8020
>>      80201014:   5555                    li      a0,-11
>>      80201016:   5555                    li      a0,-11
>>      80201018:   3022                    .2byte  0x3022  # .Lbug%=
>>      8020101a:   8020                    .2byte  0x8020
>>      8020101c:   5555                    li      a0,-11
>>      8020101e:   5555                    li      a0,-11
>>      80201020:   0000                    unimp          # %c[bf_line_hi]
>>      80201022:   0000                    unimp
>>      80201024:   5555                    li      a0,-11
>>      80201026:   5555                    li      a0,-11
>>      80201028:   0000                    unimp           # %[bf_line_hi]
>>      8020102a:   0000                    unimp
>>      8020102c:   5555                    li      a0,-11
>>      8020102e:   5555                    li      a0,-11
>>      80201030:   0000                    unimp           # %[bf_line_lo]
>>      80201032:   1600                    addi    s0,sp,800
>>      80201034:   5555                    li      a0,-11
>>      80201036:   5555                    li      a0,-11
>>      80201038:   10b8                    addi    a4,sp,104   # %[bf_ptr]
>>      8020103a:   8020                    .2byte  0x8020
>>      8020103c:   5555                    li      a0,-11
>>      8020103e:   5555                    li      a0,-11
>>      80201040:   2012                    .2byte  0x2012   # (.Lbug%= -
>> .Lfrm%=) + %c[bf_line_hi]
>>                                 .....
>> ... it looks like the error will be generated if the name in %c[name]
>> isn't equal to 0.
>>
>> Another thing I noticed is that %[name] can be used instead of %c[name]
>> for RISC-V ( i did a quick check and it works) so it is still possible
>> to follow Intel implementation but required a redefinition of
>> _ASM_BUGFRAME_TEXT where %c[...] won't be used. All the rest will be
>> the same as in x86 implementation:
>>                                  .....
>> #define _ASM_BUGFRAME_TEXT(second_frame)                      \
>>      ".Lbug%=: ebreak\n"                                       \
>>      ".pushsection .bug_frames.%[bf_type], \"a\", @progbits\n" \
>>      ".p2align 2\n"                                            \
>>      ".Lfrm%=:\n"                                              \
>>      ".long (.Lbug%= - .Lfrm%=) + %[bf_line_hi]\n"             \
>>      ".long (%[bf_ptr] - .Lfrm%=) + %[bf_line_lo]\n"           \
>>      ".if " #second_frame "\n"                                 \
>>      ".long 0, %[bf_msg] - .Lfrm%=\n"                          \
>>      ".endif\n"                                                \
>>      ".popsection\n"                                           \
>>                                   .....
>>
>> One thing I would like to ask you is why it's better to follow/use x86
>> implementation instead of ARM?
> 
> IMHO, the x86 version is cleaner. But...
> 
>> It seems that "%c[...]" has the best support only for Intel GCC and
>> thereby ARM implementation looks more universal, doesn't it?
> 
> ... you are right, the Arm version is more portable. Although, I do 
> wonder how GCC managed to have a different behavior between architecture 
> as I would have expected %c to be a generic implementation.

While the implementation is common, the condition when 'c' is legitimate
can be customized by arch-specific code. While all code for all of the
four architectures does so, perhaps x86'es goes farther with what it
permits.

Jan

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii]

Hi Julien,

On Wed, 2023-02-01 at 22:11 +0000, Julien Grall wrote:
> 
> 
> On 01/02/2023 17:40, Oleksii wrote:
> > Hi Julien,
> 
> Hi Oleksii,
> 
> > On Mon, 2023-01-30 at 22:28 +0000, Julien Grall wrote:
> > > Hi Oleksii,
> > > 
> > > On 30/01/2023 11:35, Oleksii wrote:
> > > > Hi Julien,
> > > > On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
> > > > > Hi Oleksii,
> > > > > 
> > > > > On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > > > > > The patch introduces macros: BUG(), WARN(),
> > > > > > run_in_exception(),
> > > > > > assert_failed.
> > > > > > 
> > > > > > The implementation uses "ebreak" instruction in combination
> > > > > > with
> > > > > > diffrent bug frame tables (for each type) which contains
> > > > > > useful
> > > > > > information.
> > > > > > 
> > > > > > Signed-off-by: Oleksii Kurochko
> > > > > > <oleksii.kurochko@gmail.com>
> > > > > > ---
> > > > > > Changes:
> > > > > >      - Remove __ in define namings
> > > > > >      - Update run_in_exception_handler() with
> > > > > >        register void *fn_ asm(__stringify(BUG_FN_REG)) =
> > > > > > (fn);
> > > > > >      - Remove bug_instr_t type and change it's usage to
> > > > > > uint32_t
> > > > > > ---
> > > > > >     xen/arch/riscv/include/asm/bug.h | 118
> > > > > > ++++++++++++++++++++++++++++
> > > > > >     xen/arch/riscv/traps.c           | 128
> > > > > > +++++++++++++++++++++++++++++++
> > > > > >     xen/arch/riscv/xen.lds.S         |  10 +++
> > > > > >     3 files changed, 256 insertions(+)
> > > > > >     create mode 100644 xen/arch/riscv/include/asm/bug.h
> > > > > > 
> > > > > > diff --git a/xen/arch/riscv/include/asm/bug.h
> > > > > > b/xen/arch/riscv/include/asm/bug.h
> > > > > > new file mode 100644
> > > > > > index 0000000000..4b15d8eba6
> > > > > > --- /dev/null
> > > > > > +++ b/xen/arch/riscv/include/asm/bug.h
> > > > > > @@ -0,0 +1,118 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > > +/*
> > > > > > + * Copyright (C) 2012 Regents of the University of
> > > > > > California
> > > > > > + * Copyright (C) 2021-2023 Vates
> > > > > 
> > > > > I have to question the two copyrights here given that the
> > > > > majority of
> > > > > the code seems to be taken from the arm implementation (see
> > > > > arch/arm/include/asm/bug.h).
> > > > > 
> > > > > With that said, we should consolidate the code rather than
> > > > > duplicating
> > > > > it on every architecture.
> > > > > 
> > > > Copyrights should be removed. They were taken from the previous
> > > > implementation of bug.h for RISC-V so I just forgot to remove
> > > > them.
> > > > 
> > > > It looks like we should have common bug.h for ARM and RISCV but
> > > > I
> > > > am
> > > > not sure that I know how to do that better.
> > > > Probably the code wants to be moved to xen/include/bug.h and
> > > > using
> > > > ifdef ARM && RISCV ...
> > > 
> > > Or you could introduce CONFIG_BUG_GENERIC or else, so it is
> > > easily
> > > selectable by other architecture.
> > > 
> > > > But still I am not sure that this is the best one option as at
> > > > least we
> > > > have different implementation for x86_64.
> > > 
> > > My main concern is the maintainance effort. For every bug, we
> > > would
> > > need
> > > to fix it in two places. The risk is we may forget to fix one
> > > architecture.
> > > This is not a very ideal situation.
> > > 
> > > So I think sharing the header between RISC-V and Arm (or x86, see
> > > below)
> > > is at least a must. We can do the 3rd architecture at a leisure
> > > pace.
> > > 
> > > One option would be to introduce asm-generic like Linux (IIRC
> > > this
> > > was a
> > > suggestion from Andrew). This would also to share code between
> > > two of
> > > the archs.
> > > 
> > > Also, from a brief look, the difference in implementation is
> > > mainly
> > > because on Arm we can't use %c (some version of GCC didn't
> > > support
> > > it).
> > > Is this also the case on RISC-V? If not, you may want to consider
> > > to
> > > use
> > > the x86 version.
> > > 
> > I did several experiments related to '%c' in inline assembly for
> > RISC-V
> > and it seems that '%c' doesn't support all forms of the use of
> > '%c'.
> 
> Thanks for checking!
> 
> > I wrote the following macros and they have been compiled without
> > any
> > errors:
> >                          .....
> > #define _ASM_BUGFRAME_TEXT(second_frame)                       \
> >      ".Lbug%=: ebreak\n"                                        \
> >      ".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
> >      ".p2align 2\n"                                             \
> >      ".Lfrm%=:\n"                                               \
> >      ".long (.Lfrm%=)\n"                                        \
> >      ".long (0x55555555)\n"                                     \
> >      ".long (.Lbug%=)\n"                                        \
> >      ".long (0x55555555)\n"                                     \
> >      ".long %c[bf_line_hi]\n"                                   \
> >      ".long (0x55555555)\n"                                     \
> >      ".long %[bf_line_hi]\n"                                    \
> >      ".long (0x55555555)\n"                                     \
> >      ".long %[bf_line_lo]\n"                                    \
> >      ".long (0x55555555)\n"                                     \
> >      ".long %[bf_ptr]\n"                                        \
> >      ".long (0x55555555)\n"                                     \
> >      ".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n"             \
> >      ".popsection\n"                                            \
> > 
> > #define _ASM_BUGFRAME_INFO(type, line, ptr, msg)               \
> >      [bf_type]    "i" (type),                                   \
> >      [bf_ptr]     "i" (ptr),                                    \
> >      [bf_msg]     "i" (msg),                                    \
> >      [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1))  \
> >                        << BUG_DISP_WIDTH),                      \
> >      [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) <<
> > BUG_DISP_WIDTH)
> > 
> > #define BUG_FRAME(type, line, ptr, second_frame, msg) do {     \
> >      __asm__ __volatile__ ( _ASM_BUGFRAME_TEXT(second_frame)    \
> >                     :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) );
> > \
> > } while (0)
> >                            ....
> > 
> > 
> > But if add ".long %c[bf_ptr]\n" then the following compilation
> > error
> > will occur: [ error: invalid 'asm': invalid use of '%c'. ]
> > 
> > If you look at the dissembler of _bug_frames_...
> >                                 ......
> >      80201010:   1010                    addi    a2,sp,32   #
> > .Lfrm%=
> >      80201012:   8020                    .2byte  0x8020
> >      80201014:   5555                    li      a0,-11
> >      80201016:   5555                    li      a0,-11
> >      80201018:   3022                    .2byte  0x3022  # .Lbug%=
> >      8020101a:   8020                    .2byte  0x8020
> >      8020101c:   5555                    li      a0,-11
> >      8020101e:   5555                    li      a0,-11
> >      80201020:   0000                    unimp          #
> > %c[bf_line_hi]
> >      80201022:   0000                    unimp
> >      80201024:   5555                    li      a0,-11
> >      80201026:   5555                    li      a0,-11
> >      80201028:   0000                    unimp           #
> > %[bf_line_hi]
> >      8020102a:   0000                    unimp
> >      8020102c:   5555                    li      a0,-11
> >      8020102e:   5555                    li      a0,-11
> >      80201030:   0000                    unimp           #
> > %[bf_line_lo]
> >      80201032:   1600                    addi    s0,sp,800
> >      80201034:   5555                    li      a0,-11
> >      80201036:   5555                    li      a0,-11
> >      80201038:   10b8                    addi    a4,sp,104   #
> > %[bf_ptr]
> >      8020103a:   8020                    .2byte  0x8020
> >      8020103c:   5555                    li      a0,-11
> >      8020103e:   5555                    li      a0,-11
> >      80201040:   2012                    .2byte  0x2012   #
> > (.Lbug%= -
> > .Lfrm%=) + %c[bf_line_hi]
> >                                 .....
> > ... it looks like the error will be generated if the name in
> > %c[name]
> > isn't equal to 0.
> > 
> > Another thing I noticed is that %[name] can be used instead of
> > %c[name]
> > for RISC-V ( i did a quick check and it works) so it is still
> > possible
> > to follow Intel implementation but required a redefinition of
> > _ASM_BUGFRAME_TEXT where %c[...] won't be used. All the rest will
> > be
> > the same as in x86 implementation:
> >                                  .....
> > #define _ASM_BUGFRAME_TEXT(second_frame)                      \
> >      ".Lbug%=: ebreak\n"                                       \
> >      ".pushsection .bug_frames.%[bf_type], \"a\", @progbits\n" \
> >      ".p2align 2\n"                                            \
> >      ".Lfrm%=:\n"                                              \
> >      ".long (.Lbug%= - .Lfrm%=) + %[bf_line_hi]\n"             \
> >      ".long (%[bf_ptr] - .Lfrm%=) + %[bf_line_lo]\n"           \
> >      ".if " #second_frame "\n"                                 \
> >      ".long 0, %[bf_msg] - .Lfrm%=\n"                          \
> >      ".endif\n"                                                \
> >      ".popsection\n"                                           \
> >                                   .....
> > 
> > One thing I would like to ask you is why it's better to follow/use
> > x86
> > implementation instead of ARM?
> 
> IMHO, the x86 version is cleaner. But...
> 
> > It seems that "%c[...]" has the best support only for Intel GCC and
> > thereby ARM implementation looks more universal, doesn't it?
> 
> ... you are right, the Arm version is more portable. Although, I do 
> wonder how GCC managed to have a different behavior between
> architecture 
> as I would have expected %c to be a generic implementation.
> 
> Anyway, if you are basing on the Arm one, then you should be able to
>   1) move arch/arm/include/asm/bug.h in asm-generic/bug.h (or
> similar)
>   2) Rename the guard and remove arm specific code.(I am not sure
> from 
> where to include arm{32, 64}/bug.h)
>   3) Define BUG_INSTR to ebreak on RISC-V.
>   4) Find a place for all the RISC-V specific header
>   5) Move do_bug_frame() in common/bug.c
> 
> I am happy to help testing the Arm version and/or help moving the
> code 
> to common.
> 
Thanks a lot for the help offered.

I've started to rework bug.h stuff but faced an issue.

I am trying to introduce GENERIC_BUG_FRAME config ( only for ARM now as
some stuff isn't available now for RISC-V such as find_text_region(),
printk() and so on... Thereby I will switch to do_bug_frame() to
generic one a little bit later for RISCV ) so I added the following to
Kconfig:

    config GENERIC_DO_BUG_FRAME
    	bool "Generic implementation of do_bug_frame()"
	default y if ARM
	default n
	help
	  ...

But when I pushed the commit to GitLab all ARM randconfig jobs failed
because they decided not to set GENERIC_BUG_FRAME=y.
Could you please give me a suggestion how I can work around this
problem? ( I thought that it would be enough to use default y but
randconfig can override it ).
Or is it needed to provide an empty implementation for do_bug_frame()
GENERIC_BUG_FRAME=n?
Also I thought about weak function as an option...

Here is pipeline for generic bug frame feature and the patch ( of
course not ready for upstream but at least it shows an idea ):
 *
https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/766581174
 *
https://gitlab.com/xen-project/people/olkur/xen/-/commit/6fc6481202852e0aa2c2cb3877f2d71ac0213511
 
P.S.: Probably you have some comments ( something that is unacceptable
even now ) about the patch. I will happy to hear them too.

Thanks in advance.

> Cheers,
> 
~ Oleksii

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Julien Grall]

On 03/02/2023 13:15, Oleksii wrote:
> Hi Julien,

Hi Oleksii,

> On Wed, 2023-02-01 at 22:11 +0000, Julien Grall wrote:
> I am trying to introduce GENERIC_BUG_FRAME config ( only for ARM now as
> some stuff isn't available now for RISC-V such as find_text_region(),
> printk() and so on... Thereby I will switch to do_bug_frame() to
> generic one a little bit later for RISCV ) so I added the following to
> Kconfig:
> 
>      config GENERIC_DO_BUG_FRAME
>      	bool "Generic implementation of do_bug_frame()"
> 	default y if ARM
> 	default n
> 	help
> 	  ...
> 
> But when I pushed the commit to GitLab all ARM randconfig jobs failed
> because they decided not to set GENERIC_BUG_FRAME=y.
> Could you please give me a suggestion how I can work around this
> problem? ( I thought that it would be enough to use default y but
> randconfig can override it ).

You don't want to allow the user to deselect GENERIC_DO_BUG_FRAME. So
you want config ARM to select it:

(arch/arm/Kconfig)
config ARM
    ...
    select GENERIC_DO_BUG_FRAME


(common/Kconfig)
config GENERIC_DO_BUG_FRAME
    bool

> Or is it needed to provide an empty implementation for do_bug_frame()
> GENERIC_BUG_FRAME=n?
> Also I thought about weak function as an option...
> 
> Here is pipeline for generic bug frame feature and the patch ( of
> course not ready for upstream but at least it shows an idea ):
>   *
> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/766581174
>   *
> https://gitlab.com/xen-project/people/olkur/xen/-/commit/6fc6481202852e0aa2c2cb3877f2d71ac0213511
>   
> P.S.: Probably you have some comments ( something that is unacceptable
> even now ) about the patch. I will happy to hear them too.

I will try to have a look next week.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii]

On Fri, 2023-02-03 at 13:23 +0000, Julien Grall wrote:
> 
> 
> On 03/02/2023 13:15, Oleksii wrote:
> > Hi Julien,
> 
> Hi Oleksii,
> 
> > On Wed, 2023-02-01 at 22:11 +0000, Julien Grall wrote:
> > I am trying to introduce GENERIC_BUG_FRAME config ( only for ARM
> > now as
> > some stuff isn't available now for RISC-V such as
> > find_text_region(),
> > printk() and so on... Thereby I will switch to do_bug_frame() to
> > generic one a little bit later for RISCV ) so I added the following
> > to
> > Kconfig:
> > 
> >      config GENERIC_DO_BUG_FRAME
> >         bool "Generic implementation of do_bug_frame()"
> >         default y if ARM
> >         default n
> >         help
> >           ...
> > 
> > But when I pushed the commit to GitLab all ARM randconfig jobs
> > failed
> > because they decided not to set GENERIC_BUG_FRAME=y.
> > Could you please give me a suggestion how I can work around this
> > problem? ( I thought that it would be enough to use default y but
> > randconfig can override it ).
> 
> You don't want to allow the user to deselect GENERIC_DO_BUG_FRAME. So
> you want config ARM to select it:
> 
> (arch/arm/Kconfig)
> config ARM
>     ...
>     select GENERIC_DO_BUG_FRAME
> 
> 
> (common/Kconfig)
> config GENERIC_DO_BUG_FRAME
>     bool
> 
> > Or is it needed to provide an empty implementation for
> > do_bug_frame()
> > GENERIC_BUG_FRAME=n?
> > Also I thought about weak function as an option...
> > 
> > Here is pipeline for generic bug frame feature and the patch ( of
> > course not ready for upstream but at least it shows an idea ):
> >   *
> > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/766581174
> >   *
> > https://gitlab.com/xen-project/people/olkur/xen/-/commit/6fc6481202852e0aa2c2cb3877f2d71ac0213511
> >   
> > P.S.: Probably you have some comments ( something that is
> > unacceptable
> > even now ) about the patch. I will happy to hear them too.
> 
> I will try to have a look next week.
> 
Thanks a lot.

I think that I'll send separate patch series with generic bug.h stuff
today.

> Cheers,
> 

~ Oleksii

Re: [PATCH v2 01/14] xen/riscv: add _zicsr to CFLAGS

[Oleksii]

Hi Jan, 
On Tue, 2023-01-31 at 10:14 +0100, Jan Beulich wrote:
> On 31.01.2023 01:21, Alistair Francis wrote:
> > On Sat, Jan 28, 2023 at 12:00 AM Oleksii Kurochko
> > <oleksii.kurochko@gmail.com> wrote:
> > > 
> > > Work with some registers requires csr command which is part of
> > > Zicsr.
> > > 
> > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > 
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> There is an open aspect Andrew has pointed out on an earlier version.
> I think it would be quite helpful if that could be settled one way or
> another before this patch gets committed (which formally may now be
> possible, depending on whether that open aspect is considered an
> "open" objection, as per ./MAINTAINERS).
> 
I will change it to 'G' as Alistair doesn't see any sense to restrict
ourselves:
https://lore.kernel.org/xen-devel/CAKmqyKOecoz91e-4-KZUdgT5HNhuwuN83tpFR+HmwkUPb2r3ew@mail.gmail.com/

> Jan
~ Oleksii

Re: [PATCH v2 02/14] xen/riscv: add <asm/asm.h> header

[Oleksii]

Hi Andrew,

In the previous version of the patch series you mentioned that it is
possible to re-use some parts from the existing code base.

Could you please provide me with an example?

I assume that __ASM_STR should be present somewhere but other parts of
the header looks like it is arch-specific.

~ Oleksii

On Fri, 2023-01-27 at 15:59 +0200, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
>   - Nothing changed
> ---
>  xen/arch/riscv/include/asm/asm.h | 54
> ++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/asm.h
> 
> diff --git a/xen/arch/riscv/include/asm/asm.h
> b/xen/arch/riscv/include/asm/asm.h
> new file mode 100644
> index 0000000000..6d426ecea7
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/asm.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only) */
> +/*
> + * Copyright (C) 2015 Regents of the University of California
> + */
> +
> +#ifndef _ASM_RISCV_ASM_H
> +#define _ASM_RISCV_ASM_H
> +
> +#ifdef __ASSEMBLY__
> +#define __ASM_STR(x)   x
> +#else
> +#define __ASM_STR(x)   #x
> +#endif
> +
> +#if __riscv_xlen == 64
> +#define __REG_SEL(a, b)        __ASM_STR(a)
> +#elif __riscv_xlen == 32
> +#define __REG_SEL(a, b)        __ASM_STR(b)
> +#else
> +#error "Unexpected __riscv_xlen"
> +#endif
> +
> +#define REG_L          __REG_SEL(ld, lw)
> +#define REG_S          __REG_SEL(sd, sw)
> +
> +#if __SIZEOF_POINTER__ == 8
> +#ifdef __ASSEMBLY__
> +#define RISCV_PTR              .dword
> +#else
> +#define RISCV_PTR              ".dword"
> +#endif
> +#elif __SIZEOF_POINTER__ == 4
> +#ifdef __ASSEMBLY__
> +#define RISCV_PTR              .word
> +#else
> +#define RISCV_PTR              ".word"
> +#endif
> +#else
> +#error "Unexpected __SIZEOF_POINTER__"
> +#endif
> +
> +#if (__SIZEOF_INT__ == 4)
> +#define RISCV_INT              __ASM_STR(.word)
> +#else
> +#error "Unexpected __SIZEOF_INT__"
> +#endif
> +
> +#if (__SIZEOF_SHORT__ == 2)
> +#define RISCV_SHORT            __ASM_STR(.half)
> +#else
> +#error "Unexpected __SIZEOF_SHORT__"
> +#endif
> +
> +#endif /* _ASM_RISCV_ASM_H */

Re: [PATCH v2 07/14] xen/riscv: introduce exception context

[Oleksii]

On Tue, 2023-01-31 at 17:30 -0800, Stefano Stabellini wrote:
> On Mon, 30 Jan 2023, Jan Beulich wrote:
> > On 30.01.2023 12:54, Oleksii wrote:
> > > Hi Jan,
> > > 
> > > On Fri, 2023-01-27 at 15:24 +0100, Jan Beulich wrote:
> > > > On 27.01.2023 14:59, Oleksii Kurochko wrote:
> > > > > --- /dev/null
> > > > > +++ b/xen/arch/riscv/include/asm/processor.h
> > > > > @@ -0,0 +1,82 @@
> > > > > +/* SPDX-License-Identifier: MIT */
> > > > > +/***********************************************************
> > > > > ******
> > > > > *************
> > > > > + *
> > > > > + * Copyright 2019 (C) Alistair Francis
> > > > > <alistair.francis@wdc.com>
> > > > > + * Copyright 2021 (C) Bobby Eshleman
> > > > > <bobby.eshleman@gmail.com>
> > > > > + * Copyright 2023 (C) Vates
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#ifndef _ASM_RISCV_PROCESSOR_H
> > > > > +#define _ASM_RISCV_PROCESSOR_H
> > > > > +
> > > > > +#ifndef __ASSEMBLY__
> > > > > +
> > > > > +/* On stack VCPU state */
> > > > > +struct cpu_user_regs
> > > > > +{
> > > > > +    unsigned long zero;
> > > > > +    unsigned long ra;
> > > > > +    unsigned long sp;
> > > > > +    unsigned long gp;
> > > > > +    unsigned long tp;
> > > > > +    unsigned long t0;
> > > > > +    unsigned long t1;
> > > > > +    unsigned long t2;
> > > > > +    unsigned long s0;
> > > > > +    unsigned long s1;
> > > > > +    unsigned long a0;
> > > > > +    unsigned long a1;
> > > > > +    unsigned long a2;
> > > > > +    unsigned long a3;
> > > > > +    unsigned long a4;
> > > > > +    unsigned long a5;
> > > > > +    unsigned long a6;
> > > > > +    unsigned long a7;
> > > > > +    unsigned long s2;
> > > > > +    unsigned long s3;
> > > > > +    unsigned long s4;
> > > > > +    unsigned long s5;
> > > > > +    unsigned long s6;
> > > > > +    unsigned long s7;
> > > > > +    unsigned long s8;
> > > > > +    unsigned long s9;
> > > > > +    unsigned long s10;
> > > > > +    unsigned long s11;
> > > > > +    unsigned long t3;
> > > > > +    unsigned long t4;
> > > > > +    unsigned long t5;
> > > > > +    unsigned long t6;
> > > > > +    unsigned long sepc;
> > > > > +    unsigned long sstatus;
> > > > > +    /* pointer to previous stack_cpu_regs */
> > > > > +    unsigned long pregs;
> > > > > +};
> > > > 
> > > > Just to restate what I said on the earlier version: We have a
> > > > struct
> > > > of
> > > > this name in the public interface for x86. Besides to confusion
> > > > about
> > > > re-using the name for something private, I'd still like to
> > > > understand
> > > > what the public interface plans are. This is specifically
> > > > because I
> > > > think it would be better to re-use suitable public interface
> > > > structs
> > > > internally where possible. But that of course requires spelling
> > > > out
> > > > such parts of the public interface first.
> > > > 
> > > I am not sure that I get you here.
> > > I greped a little bit and found out that each architecture
> > > declares
> > > this structure inside arch-specific folder.
> > > 
> > > Mostly the usage of the cpu_user_regs is to save/restore current
> > > state
> > > of CPU during traps ( exceptions/interrupts ) and
> > > context_switch().
> > 
> > Arm effectively duplicates the public interface struct
> > vcpu_guest_core_regs
> > and the internal struct cpu_user_regs (and this goes as far as also
> > duplicating the __DECL_REG() helper). Personally I find such
> > duplication
> > odd at the first glance at least; maybe there is a specific reason
> > for this
> > in Arm. But whether the public interface struct can be re-used can
> > likely
> > only be known once it was spelled out.
> 
> struct vcpu_guest_core_regs is used as part of struct
> vcpu_guest_context, which is used for VCPUOP_initialise, which is not
> used on ARM and RISC-V (we use a standard firmware interface
> instead),
> and for save/restore, which also is not going to work any time soon
> on
> ARM and RISC-V.
> 
> This is to say that we are probably not going to need the public
> interface for the next year or two, so it is difficult to tell at
> this
> stage if it aligns well with struct cpu_user_regs. I think we'll have
> to
> cross that bridge when we come to it.
> 
Agree that it will be better to return to the public interface later.
So I'll this part of the patch series as is now.
> 
> > > Also some registers are modified during construction of a domain.
> > > Thereby I prefer here to see the arch-specific register names
> > > instead
> > > of common.
> > 
> > Not sure what meaning of "common" you imply here. Surely register
> > names
> > want to be arch-specific, and hence can't be "common" with other
> > arch-es.
> 
> I think Oleksii misunderstood your request and believed you were
> asking
> him to make struct cpu_user_regs common across arches.
Yeah, that's what I thought at first...

~ Oleksii