[PULL 0/2] riscv-to-apply queue

[PULL 0/2] riscv-to-apply queue

[Alistair Francis]

The following changes since commit 3944e93af06f06eb07316e0bef46b007573e0309:

  Update version for v8.1.0-rc3 release (2023-08-10 12:49:56 -0700)

are available in the Git repository at:

  https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20230811-3

for you to fetch changes up to b274c2388e9fcde75d60c6e7c7d8f888874b61b7:

  hw/riscv/virt.c: change 'aclint' TCG check (2023-08-11 14:16:26 -0400)

----------------------------------------------------------------
Sixth RISC-V PR for 8.1

This is a last minute PR for RISC-V.

The main goal is to fix
https://gitlab.com/qemu-project/qemu/-/issues/1823
which is a regression that means the aclint option
cannot be enabled.

While we are here we also fixup KVM issue.

 * KVM: fix mvendorid size
 * Fixup aclint check

----------------------------------------------------------------
Daniel Henrique Barboza (2):
      target/riscv/kvm.c: fix mvendorid size in vcpu_set_machine_ids()
      hw/riscv/virt.c: change 'aclint' TCG check

 hw/riscv/virt.c    | 20 +++++++++++++-------
 target/riscv/kvm.c |  9 ++++++++-
 2 files changed, 21 insertions(+), 8 deletions(-)

[PULL 1/2] target/riscv/kvm.c: fix mvendorid size in vcpu_set_machine_ids()

[Alistair Francis]

From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

cpu->cfg.mvendorid is a 32 bit field and kvm_set_one_reg() always write
a target_ulong val, i.e. a 64 bit field in a 64 bit host.

Given that we're passing a pointer to the mvendorid field, the reg is
reading 64 bits starting from mvendorid and going 32 bits in the next
field, marchid. Here's an example:

$ ./qemu-system-riscv64 -machine virt,accel=kvm -m 2G -smp 1 \
   -cpu rv64,marchid=0xab,mvendorid=0xcd,mimpid=0xef(...)

(inside the guest)
 # cat /proc/cpuinfo
processor	: 0
hart		: 0
isa		: rv64imafdc_zicbom_zicboz_zihintpause_zbb_sstc
mmu		: sv57
mvendorid	: 0xab000000cd
marchid		: 0xab
mimpid		: 0xef

'mvendorid' was written as a combination of 0xab (the value from the
adjacent field, marchid) and its intended value 0xcd.

Fix it by assigning cpu->cfg.mvendorid to a target_ulong var 'reg' and
use it as input for kvm_set_one_reg(). Here's the result with this patch
applied and using the same QEMU command line:

 # cat /proc/cpuinfo
processor	: 0
hart		: 0
isa		: rv64imafdc_zicbom_zicboz_zihintpause_zbb_sstc
mmu		: sv57
mvendorid	: 0xcd
marchid		: 0xab
mimpid		: 0xef

This bug affects only the generic (rv64) CPUs when running with KVM in a
64 bit env since the 'host' CPU does not allow the machine IDs to be
changed via command line.

Fixes: 1fb5a622f7 ("target/riscv: handle mvendorid/marchid/mimpid for KVM CPUs")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Message-ID: <20230802180058.281385-1-dbarboza@ventanamicro.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/kvm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
index 9d8a8982f9..b1fd2233c0 100644
--- a/target/riscv/kvm.c
+++ b/target/riscv/kvm.c
@@ -852,12 +852,19 @@ void kvm_arch_init_irq_routing(KVMState *s)
 static int kvm_vcpu_set_machine_ids(RISCVCPU *cpu, CPUState *cs)
 {
     CPURISCVState *env = &cpu->env;
+    target_ulong reg;
     uint64_t id;
     int ret;
 
     id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG,
                           KVM_REG_RISCV_CONFIG_REG(mvendorid));
-    ret = kvm_set_one_reg(cs, id, &cpu->cfg.mvendorid);
+    /*
+     * cfg.mvendorid is an uint32 but a target_ulong will
+     * be written. Assign it to a target_ulong var to avoid
+     * writing pieces of other cpu->cfg fields in the reg.
+     */
+    reg = cpu->cfg.mvendorid;
+    ret = kvm_set_one_reg(cs, id, &reg);
     if (ret != 0) {
         return ret;
     }
-- 
2.41.0

[PULL 2/2] hw/riscv/virt.c: change 'aclint' TCG check

[Alistair Francis]

From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

The 'aclint' property is being conditioned with tcg acceleration in
virt_machine_class_init(). But acceleration code starts later than the
class init of the board, meaning that tcg_enabled() will be always be
false during class_init(), and the option is never being declared even
when declaring TCG accel:

$ ./build/qemu-system-riscv64 -M virt,accel=tcg,aclint=on
qemu-system-riscv64: Property 'virt-machine.aclint' not found

Fix it by moving the check from class_init() to machine_init(). Tune the
description to mention that the option is TCG only.

Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Fixes: c0716c81b ("hw/riscv/virt: Restrict ACLINT to TCG")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1823
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230811160224.440697-2-dbarboza@ventanamicro.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/virt.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d90286dc46..99c4e6314b 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1350,6 +1350,11 @@ static void virt_machine_init(MachineState *machine)
         exit(1);
     }
 
+    if (!tcg_enabled() && s->have_aclint) {
+        error_report("'aclint' is only available with TCG acceleration");
+        exit(1);
+    }
+
     /* Initialize sockets */
     mmio_irqchip = virtio_irqchip = pcie_irqchip = NULL;
     for (i = 0; i < socket_count; i++) {
@@ -1683,13 +1688,14 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
 #endif
 
-    if (tcg_enabled()) {
-        object_class_property_add_bool(oc, "aclint", virt_get_aclint,
-                                       virt_set_aclint);
-        object_class_property_set_description(oc, "aclint",
-                                              "Set on/off to enable/disable "
-                                              "emulating ACLINT devices");
-    }
+
+    object_class_property_add_bool(oc, "aclint", virt_get_aclint,
+                                   virt_set_aclint);
+    object_class_property_set_description(oc, "aclint",
+                                          "(TCG only) Set on/off to "
+                                          "enable/disable emulating "
+                                          "ACLINT devices");
+
     object_class_property_add_str(oc, "aia", virt_get_aia,
                                   virt_set_aia);
     object_class_property_set_description(oc, "aia",
-- 
2.41.0

Re: [PULL 0/2] riscv-to-apply queue

[Richard Henderson]

On 8/11/23 11:24, Alistair Francis wrote:
> The following changes since commit 3944e93af06f06eb07316e0bef46b007573e0309:
> 
>    Update version for v8.1.0-rc3 release (2023-08-10 12:49:56 -0700)
> 
> are available in the Git repository at:
> 
>    https://github.com/alistair23/qemu.git  tags/pull-riscv-to-apply-20230811-3
> 
> for you to fetch changes up to b274c2388e9fcde75d60c6e7c7d8f888874b61b7:
> 
>    hw/riscv/virt.c: change 'aclint' TCG check (2023-08-11 14:16:26 -0400)
> 
> ----------------------------------------------------------------
> Sixth RISC-V PR for 8.1
> 
> This is a last minute PR for RISC-V.
> 
> The main goal is to fix
> https://gitlab.com/qemu-project/qemu/-/issues/1823
> which is a regression that means the aclint option
> cannot be enabled.
> 
> While we are here we also fixup KVM issue.
> 
>   * KVM: fix mvendorid size
>   * Fixup aclint check

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~