[PATCH v2 0/1] allow risc-v domains to use CPU definitions

Hi, The bulk of the changes from v1 is in the commit message. See version 1 [1] for more details. Changes from v1: - rewrote the commit message to reflect the actual problem - changed virCPUriscvUpdate() to virCPURiscv64Update() to be in line with the name convention of the driver - v1 link: https://listman.redhat.com/archives/libvir-list/2023-April/239687.html [1] https://listman.redhat.com/archives/libvir-list/2023-April/239687.html Daniel Henrique Barboza (1): cpu_riscv64.c: add update() implementation po/POTFILES | 1 + src/cpu/cpu_riscv64.c | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) -- 2.40.0

At this moment it is not possible to launch a 'riscv64' domain if a CPU definition is presented in the domain. For example, adding this CPU definition: <cpu mode='custom' match='exact' check='none'> <model fallback='forbid'>rv64</model> </cpu> Will trigger the following error: $ sudo ./run tools/virsh start riscv-virt1 error: Failed to start domain 'riscv-virt1' error: this function is not supported by the connection driver: cannot update guest CPU for riscv64 architecture The error comes from virCPUUpdate(), via qemuProcessUpdateGuestCPU(), and it's caused by the absence of the 'update' API in the existing RISC-V driver. Add an 'update' API impl to the RISC-V driver to allow for CPU definitions to be declared in RISC-V domains. This API was copied from the ARM driver (virCPUarmUpdate()) since it's a good enough implementation to get us going. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- po/POTFILES | 1 + src/cpu/cpu_riscv64.c | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/po/POTFILES b/po/POTFILES index b122f02818..5d6ec195b4 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -70,6 +70,7 @@ src/cpu/cpu.c src/cpu/cpu_arm.c src/cpu/cpu_map.c src/cpu/cpu_ppc64.c +src/cpu/cpu_riscv64.c src/cpu/cpu_s390.c src/cpu/cpu_x86.c src/datatypes.c diff --git a/src/cpu/cpu_riscv64.c b/src/cpu/cpu_riscv64.c index c44bdeb291..4cb795f849 100644 --- a/src/cpu/cpu_riscv64.c +++ b/src/cpu/cpu_riscv64.c @@ -46,6 +46,32 @@ virCPURiscv64ValidateFeatures(virCPUDef *cpu G_GNUC_UNUSED) } +static int +virCPURiscv64Update(virCPUDef *guest, + const virCPUDef *host, + bool relative) +{ + g_autoptr(virCPUDef) updated = virCPUDefCopyWithoutModel(guest); + + if (!relative || guest->mode != VIR_CPU_MODE_HOST_MODEL) + return 0; + + if (!host) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown host CPU model")); + return -1; + } + + updated->mode = VIR_CPU_MODE_CUSTOM; + virCPUDefCopyModel(updated, host, true); + + virCPUDefStealModel(guest, updated, false); + guest->mode = VIR_CPU_MODE_CUSTOM; + guest->match = VIR_CPU_MATCH_EXACT; + + return 0; +} + struct cpuArchDriver cpuDriverRiscv64 = { .name = "riscv64", .arch = archs, @@ -54,6 +80,6 @@ struct cpuArchDriver cpuDriverRiscv64 = { .decode = NULL, .encode = NULL, .baseline = NULL, - .update = NULL, + .update = virCPURiscv64Update, .validateFeatures = virCPURiscv64ValidateFeatures, }; -- 2.40.0

On Fri, Apr 28, 2023 at 04:57:28PM -0300, Daniel Henrique Barboza wrote:
At this moment it is not possible to launch a 'riscv64' domain if a CPU definition is presented in the domain. For example, adding this CPU definition:
<cpu mode='custom' match='exact' check='none'> <model fallback='forbid'>rv64</model> </cpu>
Will trigger the following error:
$ sudo ./run tools/virsh start riscv-virt1 error: Failed to start domain 'riscv-virt1' error: this function is not supported by the connection driver: cannot update guest CPU for riscv64 architecture
The error comes from virCPUUpdate(), via qemuProcessUpdateGuestCPU(), and it's caused by the absence of the 'update' API in the existing RISC-V driver.
Add an 'update' API impl to the RISC-V driver to allow for CPU definitions to be declared in RISC-V domains. This API was copied from the ARM driver (virCPUarmUpdate()) since it's a good enough implementation to get us going.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- po/POTFILES | 1 + src/cpu/cpu_riscv64.c | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-)
Using the Arm CPU driver as a base sounds reasonable, as they currently have the same level (read: pretty low) of sophistication. I've tested this against both the current version of QEMU and the upcoming one which implements query-cpu-definitions, and it works as expected. Reviewed-by: Andrea Bolognani <abologna@redhat.com> and pushed. -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani
-
Daniel Henrique Barboza