Add arch-specific makefiles and configs needed to build for
riscv64. Also add a minimal head.S that is a simple infinite loop.
head.o can be built with
$ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen -C xen TARGET=head.o
No other TARGET is supported at the moment.
Signed-off-by: Connor Davis <connojdavis@gmail.com>
---
config/riscv64.mk | 5 ++
xen/Makefile | 8 +-
xen/arch/riscv/Kconfig | 52 +++++++++++
xen/arch/riscv/Kconfig.debug | 0
xen/arch/riscv/Makefile | 0
xen/arch/riscv/Rules.mk | 0
xen/arch/riscv/arch.mk | 16 ++++
xen/arch/riscv/asm-offsets.c | 0
xen/arch/riscv/configs/riscv64_defconfig | 12 +++
xen/arch/riscv/head.S | 6 ++
xen/include/asm-riscv/config.h | 110 +++++++++++++++++++++++
11 files changed, 207 insertions(+), 2 deletions(-)
create mode 100644 config/riscv64.mk
create mode 100644 xen/arch/riscv/Kconfig
create mode 100644 xen/arch/riscv/Kconfig.debug
create mode 100644 xen/arch/riscv/Makefile
create mode 100644 xen/arch/riscv/Rules.mk
create mode 100644 xen/arch/riscv/arch.mk
create mode 100644 xen/arch/riscv/asm-offsets.c
create mode 100644 xen/arch/riscv/configs/riscv64_defconfig
create mode 100644 xen/arch/riscv/head.S
create mode 100644 xen/include/asm-riscv/config.h
diff --git a/config/riscv64.mk b/config/riscv64.mk
new file mode 100644
index 0000000000..a5a21e5fa2
--- /dev/null
+++ b/config/riscv64.mk
@@ -0,0 +1,5 @@
+CONFIG_RISCV := y
+CONFIG_RISCV_64 := y
+CONFIG_RISCV_$(XEN_OS) := y
+
+CONFIG_XEN_INSTALL_SUFFIX :=
diff --git a/xen/Makefile b/xen/Makefile
index 9f3be7766d..60de4cc6cd 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -26,7 +26,9 @@ MAKEFLAGS += -rR
EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
ARCH=$(XEN_TARGET_ARCH)
-SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
+SRCARCH=$(shell echo $(ARCH) | \
+ sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
+ -e s'/riscv.*/riscv/g')
# Don't break if the build process wasn't called from the top level
# we need XEN_TARGET_ARCH to generate the proper config
@@ -35,7 +37,8 @@ include $(XEN_ROOT)/Config.mk
# Set ARCH/SUBARCH appropriately.
export TARGET_SUBARCH := $(XEN_TARGET_ARCH)
export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \
- sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
+ sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \
+ -e s'/riscv.*/riscv/g')
# Allow someone to change their config file
export KCONFIG_CONFIG ?= .config
@@ -335,6 +338,7 @@ _clean: delete-unfresh-files
$(MAKE) $(clean) xsm
$(MAKE) $(clean) crypto
$(MAKE) $(clean) arch/arm
+ $(MAKE) $(clean) arch/riscv
$(MAKE) $(clean) arch/x86
$(MAKE) $(clean) test
$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig
new file mode 100644
index 0000000000..d4bbd4294e
--- /dev/null
+++ b/xen/arch/riscv/Kconfig
@@ -0,0 +1,52 @@
+config 64BIT
+ bool
+
+config RISCV_64
+ bool
+ depends on 64BIT
+
+config RISCV
+ def_bool y
+
+config ARCH_DEFCONFIG
+ string
+ default "arch/riscv/configs/riscv64_defconfig" if RISCV_64
+
+menu "Architecture Features"
+
+source "arch/Kconfig"
+
+endmenu
+
+menu "ISA Selection"
+
+choice
+ prompt "Base ISA"
+ default RISCV_ISA_RV64IMA
+ help
+ This selects the base ISA extensions that Xen will target.
+
+config RISCV_ISA_RV64IMA
+ bool "RV64IMA"
+ select 64BIT
+ select RISCV_64
+ help
+ Use the RV64I base ISA, plus the "M" and "A" extensions
+ for integer multiply/divide and atomic instructions, respectively.
+
+endchoice
+
+config RISCV_ISA_C
+ bool "Compressed extension"
+ help
+ Add "C" to the ISA subsets that the toolchain is allowed
+ to emit when building Xen, which results in compressed
+ instructions in the Xen binary.
+
+ If unsure, say N.
+
+endmenu
+
+source "common/Kconfig"
+
+source "drivers/Kconfig"
diff --git a/xen/arch/riscv/Kconfig.debug b/xen/arch/riscv/Kconfig.debug
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/riscv/Rules.mk b/xen/arch/riscv/Rules.mk
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
new file mode 100644
index 0000000000..10229c5440
--- /dev/null
+++ b/xen/arch/riscv/arch.mk
@@ -0,0 +1,16 @@
+########################################
+# RISCV-specific definitions
+
+ifeq ($(CONFIG_RISCV_64),y)
+ CFLAGS += -mabi=lp64
+endif
+
+riscv-march-$(CONFIG_RISCV_ISA_RV64IMA) := rv64ima
+riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c
+
+# Note that -mcmodel=medany is used so that Xen can be mapped
+# 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 += -I$(BASEDIR)/include
diff --git a/xen/arch/riscv/asm-offsets.c b/xen/arch/riscv/asm-offsets.c
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/xen/arch/riscv/configs/riscv64_defconfig b/xen/arch/riscv/configs/riscv64_defconfig
new file mode 100644
index 0000000000..664a5d2378
--- /dev/null
+++ b/xen/arch/riscv/configs/riscv64_defconfig
@@ -0,0 +1,12 @@
+# CONFIG_SCHED_CREDIT is not set
+# CONFIG_SCHED_RTDS is not set
+# CONFIG_SCHED_NULL is not set
+# CONFIG_SCHED_ARINC653 is not set
+# CONFIG_TRACEBUFFER is not set
+# CONFIG_DEBUG is not set
+# CONFIG_DEBUG_INFO is not set
+# CONFIG_HYPFS is not set
+# CONFIG_GRANT_TABLE is not set
+# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set
+
+CONFIG_EXPERT=y
diff --git a/xen/arch/riscv/head.S b/xen/arch/riscv/head.S
new file mode 100644
index 0000000000..0dbc27ba75
--- /dev/null
+++ b/xen/arch/riscv/head.S
@@ -0,0 +1,6 @@
+#include <asm/config.h>
+
+ .text
+
+ENTRY(start)
+ j start
diff --git a/xen/include/asm-riscv/config.h b/xen/include/asm-riscv/config.h
new file mode 100644
index 0000000000..84cb436dc1
--- /dev/null
+++ b/xen/include/asm-riscv/config.h
@@ -0,0 +1,110 @@
+/******************************************************************************
+ * config.h
+ *
+ * A Linux-style configuration list.
+ */
+
+#ifndef __RISCV_CONFIG_H__
+#define __RISCV_CONFIG_H__
+
+#if defined(CONFIG_RISCV_64)
+# define LONG_BYTEORDER 3
+# define ELFSIZE 64
+#else
+# error "Unsupported RISCV variant"
+#endif
+
+#define BYTES_PER_LONG (1 << LONG_BYTEORDER)
+#define BITS_PER_LONG (BYTES_PER_LONG << 3)
+#define POINTER_ALIGN BYTES_PER_LONG
+
+#define BITS_PER_LLONG 64
+
+/* xen_ulong_t is always 64 bits */
+#define BITS_PER_XEN_ULONG 64
+
+#define CONFIG_RISCV 1
+#define CONFIG_RISCV_L1_CACHE_SHIFT 6
+
+#define CONFIG_PAGEALLOC_MAX_ORDER 18
+#define CONFIG_DOMU_MAX_ORDER 9
+#define CONFIG_HWDOM_MAX_ORDER 10
+
+#define OPT_CONSOLE_STR "dtuart"
+
+#ifdef CONFIG_RISCV_64
+#define MAX_VIRT_CPUS 128u
+#else
+#error "Unsupported RISCV variant"
+#endif
+
+#define INVALID_VCPU_ID MAX_VIRT_CPUS
+
+/* Linkage for RISCV */
+#ifdef __ASSEMBLY__
+#define ALIGN .align 2
+
+#define ENTRY(name) \
+ .globl name; \
+ ALIGN; \
+ name:
+#endif
+
+#include <xen/const.h>
+
+#ifdef CONFIG_RISCV_64
+
+/*
+ * RISC-V Layout:
+ * 0x0000000000000000 - 0x0000003fffffffff (256GB, L2 slots [0-255])
+ * Unmapped
+ * 0x0000004000000000 - 0xffffffbfffffffff
+ * Inaccessible: sv39 only supports 39-bit sign-extended VAs.
+ * 0xffffffc000000000 - 0xffffffc0001fffff (2MB, L2 slot [256])
+ * Unmapped
+ * 0xffffffc000200000 - 0xffffffc0003fffff (2MB, L2 slot [256])
+ * Xen text, data, bss
+ * 0xffffffc000400000 - 0xffffffc0005fffff (2MB, L2 slot [256])
+ * Fixmap: special-purpose 4K mapping slots
+ * 0xffffffc000600000 - 0xffffffc0009fffff (4MB, L2 slot [256])
+ * Early boot mapping of FDT
+ * 0xffffffc000a00000 - 0xffffffc000bfffff (2MB, L2 slot [256])
+ * Early relocation address, used when relocating Xen and later
+ * for livepatch vmap (if compiled in)
+ * 0xffffffc040000000 - 0xffffffc07fffffff (1GB, L2 slot [257])
+ * VMAP: ioremap and early_ioremap
+ * 0xffffffc080000000 - 0xffffffc13fffffff (3GB, L2 slots [258..260])
+ * Unmapped
+ * 0xffffffc140000000 - 0xffffffc1bfffffff (2GB, L2 slots [261..262])
+ * Frametable: 48 bytes per page for 133GB of RAM
+ * 0xffffffc1c0000000 - 0xffffffe1bfffffff (128GB, L2 slots [263..390])
+ * 1:1 direct mapping of RAM
+ * 0xffffffe1c0000000 - 0xffffffffffffffff (121GB, L2 slots [391..511])
+ * Unmapped
+ */
+
+#define L2_ENTRY_BITS 30
+#define L2_ENTRY_BYTES (_AC(1,UL) << L2_ENTRY_BITS)
+#define L2_ADDR(_slot) \
+ (((_AC(_slot, UL) >> 8) * _AC(0xffffff8000000000,UL)) | \
+ (_AC(_slot, UL) << L2_ENTRY_BITS))
+
+#define XEN_VIRT_START _AT(vaddr_t, L2_ADDR(256) + MB(2))
+#define HYPERVISOR_VIRT_START XEN_VIRT_START
+
+#define FRAMETABLE_VIRT_START _AT(vaddr_t, L2_ADDR(261))
+
+#endif /* CONFIG_RISCV_64 */
+
+#define STACK_ORDER 3
+#define STACK_SIZE (PAGE_SIZE << STACK_ORDER)
+
+#endif /* __RISCV_CONFIG_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.31.1
On 5/14/21 11:53 AM, Connor Davis wrote: > Add arch-specific makefiles and configs needed to build for > riscv64. Also add a minimal head.S that is a simple infinite loop. > head.o can be built with > > $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen -C xen TARGET=head.o > I recently realized that the Linux kernel build uses `ARCH=riscv` with 32 vs 64 being differentiated by Kconfig opts (and __riscv_xlen). I think `riscv64` was inherited by `arm64`. This is something I'd like to eventually change the Xen build to (i.e., `XEN_TARGET_ARCH=riscv make`) would it be possible for us to get that in this series? ... > diff --git a/xen/include/asm-riscv/config.h b/xen/include/asm-riscv/config.h > new file mode 100644 > index 0000000000..84cb436dc1 > --- /dev/null > +++ b/xen/include/asm-riscv/config.h > @@ -0,0 +1,110 @@ > +/****************************************************************************** > + * config.h > + * > + * A Linux-style configuration list. > + */ > + > +#ifndef __RISCV_CONFIG_H__ > +#define __RISCV_CONFIG_H__ > + ... > + > +#ifdef CONFIG_RISCV_64 > + > +/* > + * RISC-V Layout: > + * 0x0000000000000000 - 0x0000003fffffffff (256GB, L2 slots [0-255]) > + * Unmapped > + * 0x0000004000000000 - 0xffffffbfffffffff > + * Inaccessible: sv39 only supports 39-bit sign-extended VAs. > + * 0xffffffc000000000 - 0xffffffc0001fffff (2MB, L2 slot [256]) > + * Unmapped > + * 0xffffffc000200000 - 0xffffffc0003fffff (2MB, L2 slot [256]) > + * Xen text, data, bss > + * 0xffffffc000400000 - 0xffffffc0005fffff (2MB, L2 slot [256]) > + * Fixmap: special-purpose 4K mapping slots > + * 0xffffffc000600000 - 0xffffffc0009fffff (4MB, L2 slot [256]) > + * Early boot mapping of FDT > + * 0xffffffc000a00000 - 0xffffffc000bfffff (2MB, L2 slot [256]) > + * Early relocation address, used when relocating Xen and later > + * for livepatch vmap (if compiled in) > + * 0xffffffc040000000 - 0xffffffc07fffffff (1GB, L2 slot [257]) > + * VMAP: ioremap and early_ioremap > + * 0xffffffc080000000 - 0xffffffc13fffffff (3GB, L2 slots [258..260]) > + * Unmapped > + * 0xffffffc140000000 - 0xffffffc1bfffffff (2GB, L2 slots [261..262]) > + * Frametable: 48 bytes per page for 133GB of RAM > + * 0xffffffc1c0000000 - 0xffffffe1bfffffff (128GB, L2 slots [263..390]) > + * 1:1 direct mapping of RAM > + * 0xffffffe1c0000000 - 0xffffffffffffffff (121GB, L2 slots [391..511]) > + * Unmapped > + */ > + What is the benefit of moving the layout up to 0xffffffc000000000? -- Bobby Eshleman SE at Vates SAS
On 5/14/21 3:53 PM, Bob Eshleman wrote: > On 5/14/21 11:53 AM, Connor Davis wrote: >> Add arch-specific makefiles and configs needed to build for >> riscv64. Also add a minimal head.S that is a simple infinite loop. >> head.o can be built with >> >> $ make XEN_TARGET_ARCH=riscv64 SUBSYSTEMS=xen -C xen TARGET=head.o >> > I recently realized that the Linux kernel build uses `ARCH=riscv` > with 32 vs 64 being differentiated by Kconfig opts (and __riscv_xlen). > I think `riscv64` was inherited by `arm64`. This is something I'd > like to eventually change the Xen build to (i.e., > `XEN_TARGET_ARCH=riscv make`) would it be possible for us to get that > in this series? Sure, I can do that > >> diff --git a/xen/include/asm-riscv/config.h b/xen/include/asm-riscv/config.h >> new file mode 100644 >> index 0000000000..84cb436dc1 >> --- /dev/null >> +++ b/xen/include/asm-riscv/config.h >> @@ -0,0 +1,110 @@ >> +/****************************************************************************** >> + * config.h >> + * >> + * A Linux-style configuration list. >> + */ >> + >> +#ifndef __RISCV_CONFIG_H__ >> +#define __RISCV_CONFIG_H__ >> + > ... > >> + >> +#ifdef CONFIG_RISCV_64 >> + >> +/* >> + * RISC-V Layout: >> + * 0x0000000000000000 - 0x0000003fffffffff (256GB, L2 slots [0-255]) >> + * Unmapped >> + * 0x0000004000000000 - 0xffffffbfffffffff >> + * Inaccessible: sv39 only supports 39-bit sign-extended VAs. >> + * 0xffffffc000000000 - 0xffffffc0001fffff (2MB, L2 slot [256]) >> + * Unmapped >> + * 0xffffffc000200000 - 0xffffffc0003fffff (2MB, L2 slot [256]) >> + * Xen text, data, bss >> + * 0xffffffc000400000 - 0xffffffc0005fffff (2MB, L2 slot [256]) >> + * Fixmap: special-purpose 4K mapping slots >> + * 0xffffffc000600000 - 0xffffffc0009fffff (4MB, L2 slot [256]) >> + * Early boot mapping of FDT >> + * 0xffffffc000a00000 - 0xffffffc000bfffff (2MB, L2 slot [256]) >> + * Early relocation address, used when relocating Xen and later >> + * for livepatch vmap (if compiled in) >> + * 0xffffffc040000000 - 0xffffffc07fffffff (1GB, L2 slot [257]) >> + * VMAP: ioremap and early_ioremap >> + * 0xffffffc080000000 - 0xffffffc13fffffff (3GB, L2 slots [258..260]) >> + * Unmapped >> + * 0xffffffc140000000 - 0xffffffc1bfffffff (2GB, L2 slots [261..262]) >> + * Frametable: 48 bytes per page for 133GB of RAM >> + * 0xffffffc1c0000000 - 0xffffffe1bfffffff (128GB, L2 slots [263..390]) >> + * 1:1 direct mapping of RAM >> + * 0xffffffe1c0000000 - 0xffffffffffffffff (121GB, L2 slots [391..511]) >> + * Unmapped >> + */ >> + > What is the benefit of moving the layout up to 0xffffffc000000000? I thought it made the most sense to use the upper half since Xen is privileged and privileged code is typically mapped in the upper half, at least on x86. I'm happy to move it down if that would be preferred. Thanks, Connor
On 5/14/21 4:47 PM, Connor Davis wrote: > > On 5/14/21 3:53 PM, Bob Eshleman wrote: >> On 5/14/21 11:53 AM, Connor Davis wrote: >> >>> + >>> +#ifdef CONFIG_RISCV_64 >>> + >>> +/* >>> + * RISC-V Layout: >>> + * 0x0000000000000000 - 0x0000003fffffffff (256GB, L2 slots [0-255]) >>> + * Unmapped >>> + * 0x0000004000000000 - 0xffffffbfffffffff >>> + * Inaccessible: sv39 only supports 39-bit sign-extended VAs. >>> + * 0xffffffc000000000 - 0xffffffc0001fffff (2MB, L2 slot [256]) >>> + * Unmapped >>> + * 0xffffffc000200000 - 0xffffffc0003fffff (2MB, L2 slot [256]) >>> + * Xen text, data, bss >>> + * 0xffffffc000400000 - 0xffffffc0005fffff (2MB, L2 slot [256]) >>> + * Fixmap: special-purpose 4K mapping slots >>> + * 0xffffffc000600000 - 0xffffffc0009fffff (4MB, L2 slot [256]) >>> + * Early boot mapping of FDT >>> + * 0xffffffc000a00000 - 0xffffffc000bfffff (2MB, L2 slot [256]) >>> + * Early relocation address, used when relocating Xen and later >>> + * for livepatch vmap (if compiled in) >>> + * 0xffffffc040000000 - 0xffffffc07fffffff (1GB, L2 slot [257]) >>> + * VMAP: ioremap and early_ioremap >>> + * 0xffffffc080000000 - 0xffffffc13fffffff (3GB, L2 slots [258..260]) >>> + * Unmapped >>> + * 0xffffffc140000000 - 0xffffffc1bfffffff (2GB, L2 slots [261..262]) >>> + * Frametable: 48 bytes per page for 133GB of RAM >>> + * 0xffffffc1c0000000 - 0xffffffe1bfffffff (128GB, L2 slots [263..390]) >>> + * 1:1 direct mapping of RAM >>> + * 0xffffffe1c0000000 - 0xffffffffffffffff (121GB, L2 slots [391..511]) >>> + * Unmapped >>> + */ >>> + >> What is the benefit of moving the layout up to 0xffffffc000000000? > > I thought it made the most sense to use the upper half since Xen is privileged > > and privileged code is typically mapped in the upper half, at least on x86. I'm happy to > > move it down if that would be preferred. > > I do like the idea of following norms, but I think I prefer following the ARM norm over the x86 norm unless there is a technical reason not to. Just due to ARM and RISC-V having much more overlap than x86 and RISC-V. In this case, all things being equal, I'd prefer following the ARM model and use the lower half. I definitely like adding the note on the inaccessible region. Thanks, -- Bobby Eshleman SE at Vates SAS
On 5/17/21 7:43 PM, Bob Eshleman wrote: > On 5/14/21 4:47 PM, Connor Davis wrote: >> On 5/14/21 3:53 PM, Bob Eshleman wrote: >>> On 5/14/21 11:53 AM, Connor Davis wrote: >>> >>>> + >>>> +#ifdef CONFIG_RISCV_64 >>>> + >>>> +/* >>>> + * RISC-V Layout: >>>> + * 0x0000000000000000 - 0x0000003fffffffff (256GB, L2 slots [0-255]) >>>> + * Unmapped >>>> + * 0x0000004000000000 - 0xffffffbfffffffff >>>> + * Inaccessible: sv39 only supports 39-bit sign-extended VAs. >>>> + * 0xffffffc000000000 - 0xffffffc0001fffff (2MB, L2 slot [256]) >>>> + * Unmapped >>>> + * 0xffffffc000200000 - 0xffffffc0003fffff (2MB, L2 slot [256]) >>>> + * Xen text, data, bss >>>> + * 0xffffffc000400000 - 0xffffffc0005fffff (2MB, L2 slot [256]) >>>> + * Fixmap: special-purpose 4K mapping slots >>>> + * 0xffffffc000600000 - 0xffffffc0009fffff (4MB, L2 slot [256]) >>>> + * Early boot mapping of FDT >>>> + * 0xffffffc000a00000 - 0xffffffc000bfffff (2MB, L2 slot [256]) >>>> + * Early relocation address, used when relocating Xen and later >>>> + * for livepatch vmap (if compiled in) >>>> + * 0xffffffc040000000 - 0xffffffc07fffffff (1GB, L2 slot [257]) >>>> + * VMAP: ioremap and early_ioremap >>>> + * 0xffffffc080000000 - 0xffffffc13fffffff (3GB, L2 slots [258..260]) >>>> + * Unmapped >>>> + * 0xffffffc140000000 - 0xffffffc1bfffffff (2GB, L2 slots [261..262]) >>>> + * Frametable: 48 bytes per page for 133GB of RAM >>>> + * 0xffffffc1c0000000 - 0xffffffe1bfffffff (128GB, L2 slots [263..390]) >>>> + * 1:1 direct mapping of RAM >>>> + * 0xffffffe1c0000000 - 0xffffffffffffffff (121GB, L2 slots [391..511]) >>>> + * Unmapped >>>> + */ >>>> + >>> What is the benefit of moving the layout up to 0xffffffc000000000? >> I thought it made the most sense to use the upper half since Xen is privileged >> >> and privileged code is typically mapped in the upper half, at least on x86. I'm happy to >> >> move it down if that would be preferred. >> >> > I do like the idea of following norms, but I think I prefer following the ARM norm > over the x86 norm unless there is a technical reason not to. Just due to > ARM and RISC-V having much more overlap than x86 and RISC-V. In this case, > all things being equal, I'd prefer following the ARM model and use the lower half. > I definitely like adding the note on the inaccessible region. Sounds good, I will move it down. Thanks, Connor
On 14.05.2021 20:53, Connor Davis wrote: > --- /dev/null > +++ b/config/riscv64.mk > @@ -0,0 +1,5 @@ > +CONFIG_RISCV := y > +CONFIG_RISCV_64 := y > +CONFIG_RISCV_$(XEN_OS) := y I wonder whether the last one actually gets used anywhere, but I do realize that other architectures have similar definitions. > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -26,7 +26,9 @@ MAKEFLAGS += -rR > EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi > > ARCH=$(XEN_TARGET_ARCH) > -SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g') > +SRCARCH=$(shell echo $(ARCH) | \ > + sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \ > + -e s'/riscv.*/riscv/g') I think in makefiles tab indentation would better be restricted to rules. While here it's a minor nit, ... > @@ -35,7 +37,8 @@ include $(XEN_ROOT)/Config.mk > # Set ARCH/SUBARCH appropriately. > export TARGET_SUBARCH := $(XEN_TARGET_ARCH) > export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \ > - sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g') > + sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \ > + -e s'/riscv.*/riscv/g') ... here you're actually introducing locally inconsistent indentation. > --- /dev/null > +++ b/xen/arch/riscv/Kconfig > @@ -0,0 +1,52 @@ > +config 64BIT > + bool I'm afraid this is stale now - the option now lives in xen/arch/Kconfig, available to all architectures. > +config RISCV_64 > + bool > + depends on 64BIT Such a "depends on" is relatively pointless - they're more important to have when there is a prompt. > +config ARCH_DEFCONFIG > + string > + default "arch/riscv/configs/riscv64_defconfig" if RISCV_64 For the RISCV_64 dependency to be really useful (at least with the command line kconfig), you want its selection to live above the use. > +menu "Architecture Features" > + > +source "arch/Kconfig" > + > +endmenu > + > +menu "ISA Selection" > + > +choice > + prompt "Base ISA" > + default RISCV_ISA_RV64IMA > + help > + This selects the base ISA extensions that Xen will target. > + > +config RISCV_ISA_RV64IMA > + bool "RV64IMA" > + select 64BIT > + select RISCV_64 I think you want only the latter here, and the former done by RISCV_64 (or select the former here, and have "default y if 64BIT" at RISCV_64). That way, not every party wanting 64-bit has to select both. > + help > + Use the RV64I base ISA, plus the "M" and "A" extensions > + for integer multiply/divide and atomic instructions, respectively. > + > +endchoice > + > +config RISCV_ISA_C > + bool "Compressed extension" > + help > + Add "C" to the ISA subsets that the toolchain is allowed > + to emit when building Xen, which results in compressed > + instructions in the Xen binary. > + > + If unsure, say N. For all of the above, please adjust indentation to be consistent with (the bulk of) what we have elsewhere. > --- /dev/null > +++ b/xen/arch/riscv/arch.mk > @@ -0,0 +1,16 @@ > +######################################## > +# RISCV-specific definitions > + > +ifeq ($(CONFIG_RISCV_64),y) > + CFLAGS += -mabi=lp64 > +endif Wherever possible I think we should prefer the list approach: CFLAGS-$(CONFIG_RISCV_64) += -mabi=lp64 > --- /dev/null > +++ b/xen/arch/riscv/configs/riscv64_defconfig > @@ -0,0 +1,12 @@ > +# CONFIG_SCHED_CREDIT is not set > +# CONFIG_SCHED_RTDS is not set > +# CONFIG_SCHED_NULL is not set > +# CONFIG_SCHED_ARINC653 is not set > +# CONFIG_TRACEBUFFER is not set > +# CONFIG_DEBUG is not set > +# CONFIG_DEBUG_INFO is not set > +# CONFIG_HYPFS is not set > +# CONFIG_GRANT_TABLE is not set > +# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set > + > +CONFIG_EXPERT=y These are rather odd defaults, more like for a special purpose config than a general purpose one. None of what you turn off here will guarantee to be off for people actually trying to build things, so it's not clear to me what the idea here is. As a specific remark, especially during bringup work I think it is quite important to not default DEBUG to off: You definitely want to see whether any assertions trigger. > --- /dev/null > +++ b/xen/include/asm-riscv/config.h > @@ -0,0 +1,110 @@ > +/****************************************************************************** > + * config.h > + * > + * A Linux-style configuration list. May I suggest to not further replicate this now inapplicable comment? It was already somewhat bogus for Arm to clone from x86. > + */ > + > +#ifndef __RISCV_CONFIG_H__ > +#define __RISCV_CONFIG_H__ > + > +#if defined(CONFIG_RISCV_64) > +# define LONG_BYTEORDER 3 > +# define ELFSIZE 64 > +#else > +# error "Unsupported RISCV variant" > +#endif > + > +#define BYTES_PER_LONG (1 << LONG_BYTEORDER) > +#define BITS_PER_LONG (BYTES_PER_LONG << 3) > +#define POINTER_ALIGN BYTES_PER_LONG > + > +#define BITS_PER_LLONG 64 > + > +/* xen_ulong_t is always 64 bits */ > +#define BITS_PER_XEN_ULONG 64 > + > +#define CONFIG_RISCV 1 This duplicates a "real" Kconfig setting, doesn't it? > +#define CONFIG_RISCV_L1_CACHE_SHIFT 6 > + > +#define CONFIG_PAGEALLOC_MAX_ORDER 18 > +#define CONFIG_DOMU_MAX_ORDER 9 > +#define CONFIG_HWDOM_MAX_ORDER 10 > + > +#define OPT_CONSOLE_STR "dtuart" > + > +#ifdef CONFIG_RISCV_64 > +#define MAX_VIRT_CPUS 128u > +#else > +#error "Unsupported RISCV variant" > +#endif Could this be folded with the other similar construct further up? Jan
On 5/17/21 5:51 AM, Jan Beulich wrote: > On 14.05.2021 20:53, Connor Davis wrote: >> --- /dev/null >> +++ b/config/riscv64.mk >> @@ -0,0 +1,5 @@ >> +CONFIG_RISCV := y >> +CONFIG_RISCV_64 := y >> +CONFIG_RISCV_$(XEN_OS) := y > I wonder whether the last one actually gets used anywhere, but I do > realize that other architectures have similar definitions. > >> --- a/xen/Makefile >> +++ b/xen/Makefile >> @@ -26,7 +26,9 @@ MAKEFLAGS += -rR >> EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi >> >> ARCH=$(XEN_TARGET_ARCH) >> -SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g') >> +SRCARCH=$(shell echo $(ARCH) | \ >> + sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \ >> + -e s'/riscv.*/riscv/g') > I think in makefiles tab indentation would better be restricted to > rules. While here it's a minor nit, ... > >> @@ -35,7 +37,8 @@ include $(XEN_ROOT)/Config.mk >> # Set ARCH/SUBARCH appropriately. >> export TARGET_SUBARCH := $(XEN_TARGET_ARCH) >> export TARGET_ARCH := $(shell echo $(XEN_TARGET_ARCH) | \ >> - sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g') >> + sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g' \ >> + -e s'/riscv.*/riscv/g') > ... here you're actually introducing locally inconsistent indentation. > >> --- /dev/null >> +++ b/xen/arch/riscv/Kconfig >> @@ -0,0 +1,52 @@ >> +config 64BIT >> + bool > I'm afraid this is stale now - the option now lives in xen/arch/Kconfig, > available to all architectures. > >> +config RISCV_64 >> + bool >> + depends on 64BIT > Such a "depends on" is relatively pointless - they're more important to > have when there is a prompt. > >> +config ARCH_DEFCONFIG >> + string >> + default "arch/riscv/configs/riscv64_defconfig" if RISCV_64 > For the RISCV_64 dependency to be really useful (at least with the > command line kconfig), you want its selection to live above the use. > >> +menu "Architecture Features" >> + >> +source "arch/Kconfig" >> + >> +endmenu >> + >> +menu "ISA Selection" >> + >> +choice >> + prompt "Base ISA" >> + default RISCV_ISA_RV64IMA >> + help >> + This selects the base ISA extensions that Xen will target. >> + >> +config RISCV_ISA_RV64IMA >> + bool "RV64IMA" >> + select 64BIT >> + select RISCV_64 > I think you want only the latter here, and the former done by RISCV_64 > (or select the former here, and have "default y if 64BIT" at RISCV_64). > That way, not every party wanting 64-bit has to select both. > >> + help >> + Use the RV64I base ISA, plus the "M" and "A" extensions >> + for integer multiply/divide and atomic instructions, respectively. >> + >> +endchoice >> + >> +config RISCV_ISA_C >> + bool "Compressed extension" >> + help >> + Add "C" to the ISA subsets that the toolchain is allowed >> + to emit when building Xen, which results in compressed >> + instructions in the Xen binary. >> + >> + If unsure, say N. > For all of the above, please adjust indentation to be consistent with > (the bulk of) what we have elsewhere. Will do >> --- /dev/null >> +++ b/xen/arch/riscv/arch.mk >> @@ -0,0 +1,16 @@ >> +######################################## >> +# RISCV-specific definitions >> + >> +ifeq ($(CONFIG_RISCV_64),y) >> + CFLAGS += -mabi=lp64 >> +endif > Wherever possible I think we should prefer the list approach: > > CFLAGS-$(CONFIG_RISCV_64) += -mabi=lp64 > >> --- /dev/null >> +++ b/xen/arch/riscv/configs/riscv64_defconfig >> @@ -0,0 +1,12 @@ >> +# CONFIG_SCHED_CREDIT is not set >> +# CONFIG_SCHED_RTDS is not set >> +# CONFIG_SCHED_NULL is not set >> +# CONFIG_SCHED_ARINC653 is not set >> +# CONFIG_TRACEBUFFER is not set >> +# CONFIG_DEBUG is not set >> +# CONFIG_DEBUG_INFO is not set >> +# CONFIG_HYPFS is not set >> +# CONFIG_GRANT_TABLE is not set >> +# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set >> + >> +CONFIG_EXPERT=y > These are rather odd defaults, more like for a special purpose > config than a general purpose one. None of what you turn off here > will guarantee to be off for people actually trying to build > things, so it's not clear to me what the idea here is. As a > specific remark, especially during bringup work I think it is > quite important to not default DEBUG to off: You definitely want > to see whether any assertions trigger. The idea was to turn off as much stuff as possible to get a minimal build (involving xen/common) working. Although now that we're focused on only a few files at a time, they could be enabled without adding any undue burden (at least for now). Perhaps it would be best to rename the file to include "tiny" or something, and then add a normal defconfig once things are actually running? At any rate, agreed on the DEBUG setting, I will enable that. >> --- /dev/null >> +++ b/xen/include/asm-riscv/config.h >> @@ -0,0 +1,110 @@ >> +/****************************************************************************** >> + * config.h >> + * >> + * A Linux-style configuration list. > May I suggest to not further replicate this now inapplicable > comment? It was already somewhat bogus for Arm to clone from x86. Sure thing. > >> + */ >> + >> +#ifndef __RISCV_CONFIG_H__ >> +#define __RISCV_CONFIG_H__ >> + >> +#if defined(CONFIG_RISCV_64) >> +# define LONG_BYTEORDER 3 >> +# define ELFSIZE 64 >> +#else >> +# error "Unsupported RISCV variant" >> +#endif >> + >> +#define BYTES_PER_LONG (1 << LONG_BYTEORDER) >> +#define BITS_PER_LONG (BYTES_PER_LONG << 3) >> +#define POINTER_ALIGN BYTES_PER_LONG >> + >> +#define BITS_PER_LLONG 64 >> + >> +/* xen_ulong_t is always 64 bits */ >> +#define BITS_PER_XEN_ULONG 64 >> + >> +#define CONFIG_RISCV 1 > This duplicates a "real" Kconfig setting, doesn't it? Yes, will remove, thanks > >> +#define CONFIG_RISCV_L1_CACHE_SHIFT 6 >> + >> +#define CONFIG_PAGEALLOC_MAX_ORDER 18 >> +#define CONFIG_DOMU_MAX_ORDER 9 >> +#define CONFIG_HWDOM_MAX_ORDER 10 >> + >> +#define OPT_CONSOLE_STR "dtuart" >> + >> +#ifdef CONFIG_RISCV_64 >> +#define MAX_VIRT_CPUS 128u >> +#else >> +#error "Unsupported RISCV variant" >> +#endif > Could this be folded with the other similar construct further up? Yep, will do. Thanks, Connor
On 18.05.2021 06:58, Connor Davis wrote: > On 5/17/21 5:51 AM, Jan Beulich wrote: >> On 14.05.2021 20:53, Connor Davis wrote: >>> --- /dev/null >>> +++ b/xen/arch/riscv/configs/riscv64_defconfig >>> @@ -0,0 +1,12 @@ >>> +# CONFIG_SCHED_CREDIT is not set >>> +# CONFIG_SCHED_RTDS is not set >>> +# CONFIG_SCHED_NULL is not set >>> +# CONFIG_SCHED_ARINC653 is not set >>> +# CONFIG_TRACEBUFFER is not set >>> +# CONFIG_DEBUG is not set >>> +# CONFIG_DEBUG_INFO is not set >>> +# CONFIG_HYPFS is not set >>> +# CONFIG_GRANT_TABLE is not set >>> +# CONFIG_SPECULATIVE_HARDEN_ARRAY is not set >>> + >>> +CONFIG_EXPERT=y >> These are rather odd defaults, more like for a special purpose >> config than a general purpose one. None of what you turn off here >> will guarantee to be off for people actually trying to build >> things, so it's not clear to me what the idea here is. As a >> specific remark, especially during bringup work I think it is >> quite important to not default DEBUG to off: You definitely want >> to see whether any assertions trigger. > The idea was to turn off as much stuff as possible to get a minimal > build (involving xen/common) working. Although now that we're focused on > only a few files at a time, they could be enabled without adding any > undue burden (at least for now). > > Perhaps it would be best to rename the file to include "tiny" or something, > and then add a normal defconfig once things are actually running? Yes please. Jan
© 2016 - 2026 Red Hat, Inc.