On 7/3/2026 10:58 AM, Philippe Mathieu-Daudé wrote:
On 3/7/26 15:46, Daniel Henrique Barboza wrote:
On 7/3/2026 7:58 AM, Philippe Mathieu-Daudé wrote:
On 2/7/26 15:51, Daniel Henrique Barboza wrote:
The PMP emulation isn't present in the KVM driver.
Signed-off-by: Daniel Henrique Barboza <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Reviewed-by: Chao Liu <[email protected]>
---
target/riscv/machine.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index ba96ceceef..e1cbcaa349 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -20,6 +20,7 @@
#include "cpu.h"
#include "qemu/error-report.h"
#include "system/kvm.h"
+#include "system/tcg.h"
#include "migration/cpu.h"
#include "exec/icount.h"
#include "target/riscv/debug.h"
@@ -29,7 +30,11 @@ static bool pmp_needed(void *opaque)
{
RISCVCPU *cpu = opaque;
- return cpu->cfg.pmp;
+ if (kvm_enabled()) {
+ return false;
+ }
+
+ return tcg_enabled() && cpu->cfg.pmp;
}
static int pmp_post_load(void *opaque, int version_id)
@@ -39,10 +44,12 @@ static int pmp_post_load(void *opaque, int version_id)
int i;
uint8_t pmp_regions = riscv_cpu_cfg(env)->pmp_regions;
Maybe we can use:
if (!pmp_needed(opaque)) {
return 0;
}
Hmmmm I thought that pmp_post_load() already infers pmp_needed() == true.
In migration/main.rst we have:
---
The 'pre_load' and 'post_load' functions on subsections are only
called if the subsection is loaded.
---
Hence we'll only execute the post_load() cb for a section/subsection if
the section was transferred, and that is determined by the .needed cb.
In this particular case we'll only reach pmp_post_load() if
pmp_needed() is already true.
OK.
Or am I wrong? It has been awhile since I last read migration code and
I'm not finding docs w.r.t how .needed and .pre/post_load interact
with each other.
Maybe check 'if (cpu->cfg.pmp)' then? What confuses me is the
'if (tcg_enabled)' since we already checked that in needed().
That's in there because pmp_update_rule_addr() and pmp_update_rule_nums()
are TCG only and it won't build with --disable-tcg.
And now that I said it out loud maybe this would be clearer:
static int pmp_post_load(void *opaque, int version_id)
{
#ifdef CONFIG_TCG
RISCVCPU *cpu = opaque;
CPURISCVState *env = &cpu->env;
int i;
uint8_t pmp_regions = riscv_cpu_cfg(env)->pmp_regions;
for (i = 0; i < pmp_regions; i++) {
pmp_update_rule_addr(env, i);
}
pmp_update_rule_nums(env);
#endif
return 0;
}
I just checked it and it works with --disable-tcg. Let me know
if you're ok with it and I'll change it for v4.
Cheers,
Daniel
- for (i = 0; i < pmp_regions; i++) {
- pmp_update_rule_addr(env, i);
+ if (tcg_enabled()) {
or use it here; also restrict pmp_regions variable scope?
Just moved pmp_regions. Thanks,
Daniel
Anyhow:
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
+ for (i = 0; i < pmp_regions; i++) {
+ pmp_update_rule_addr(env, i);
+ }
+ pmp_update_rule_nums(env);
}
- pmp_update_rule_nums(env);
return 0;
}
Since you are working in multi-accelerators frontier, keep in
mind in the future we might have some targets migrating between
different accelerators 🤯 :)
