[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [qemu-xen stable-4.18] target/riscv: fix satp_mode_finalize() when satp_mode.supported = 0
commit 1805e05db3c6d92c750427b1c1371f44fa8e4893 Author: Daniel Henrique Barboza <dbarboza@xxxxxxxxxxxxxxxx> AuthorDate: Thu Aug 17 12:29:03 2023 -0300 Commit: Michael Tokarev <mjt@xxxxxxxxxx> CommitDate: Wed Sep 20 10:18:14 2023 +0300 target/riscv: fix satp_mode_finalize() when satp_mode.supported = 0 In the same emulated RISC-V host, the 'host' KVM CPU takes 4 times longer to boot than the 'rv64' KVM CPU. The reason is an unintended behavior of riscv_cpu_satp_mode_finalize() when satp_mode.supported = 0, i.e. when cpu_init() does not set satp_mode_max_supported(). satp_mode_max_from_map(map) does: 31 - __builtin_clz(map) This means that, if satp_mode.supported = 0, satp_mode_supported_max wil be '31 - 32'. But this is C, so satp_mode_supported_max will gladly set it to UINT_MAX (4294967295). After that, if the user didn't set a satp_mode, set_satp_mode_default_map(cpu) will make cfg.satp_mode.map = cfg.satp_mode.supported So satp_mode.map = 0. And then satp_mode_map_max will be set to satp_mode_max_from_map(cpu->cfg.satp_mode.map), i.e. also UINT_MAX. The guard "satp_mode_map_max > satp_mode_supported_max" doesn't protect us here since both are UINT_MAX. And finally we have 2 loops: for (int i = satp_mode_map_max - 1; i >= 0; --i) { Which are, in fact, 2 loops from UINT_MAX -1 to -1. This is where the extra delay when booting the 'host' CPU is coming from. Commit 43d1de32f8 already set a precedence for satp_mode.supported = 0 in a different manner. We're doing the same here. If supported == 0, interpret as 'the CPU wants the OS to handle satp mode alone' and skip satp_mode_finalize(). We'll also put a guard in satp_mode_max_from_map() to assert out if map is 0 since the function is not ready to deal with it. Cc: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> Fixes: 6f23aaeb9b ("riscv: Allow user to set the satp mode") Signed-off-by: Daniel Henrique Barboza <dbarboza@xxxxxxxxxxxxxxxx> Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx> Message-ID: <20230817152903.694926-1-dbarboza@xxxxxxxxxxxxxxxx> Signed-off-by: Alistair Francis <alistair.francis@xxxxxxx> (cherry picked from commit 3a2fc23563885c219c73c8f24318921daf02f3f2) Signed-off-by: Michael Tokarev <mjt@xxxxxxxxxx> --- target/riscv/cpu.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 1e97473af2..8c0721c715 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -282,6 +282,17 @@ static uint8_t satp_mode_from_str(const char *satp_mode_str) uint8_t satp_mode_max_from_map(uint32_t map) { + /* + * 'map = 0' will make us return (31 - 32), which C will + * happily overflow to UINT_MAX. There's no good result to + * return if 'map = 0' (e.g. returning 0 will be ambiguous + * with the result for 'map = 1'). + * + * Assert out if map = 0. Callers will have to deal with + * it outside of this function. + */ + g_assert(map > 0); + /* map here has at least one bit set, so no problem with clz */ return 31 - __builtin_clz(map); } @@ -1043,9 +1054,15 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp) { bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32; - uint8_t satp_mode_map_max; - uint8_t satp_mode_supported_max = - satp_mode_max_from_map(cpu->cfg.satp_mode.supported); + uint8_t satp_mode_map_max, satp_mode_supported_max; + + /* The CPU wants the OS to decide which satp mode to use */ + if (cpu->cfg.satp_mode.supported == 0) { + return; + } + + satp_mode_supported_max = + satp_mode_max_from_map(cpu->cfg.satp_mode.supported); if (cpu->cfg.satp_mode.map == 0) { if (cpu->cfg.satp_mode.init == 0) { -- generated by git-patchbot for /home/xen/git/qemu-xen.git#stable-4.18
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |