On 3/7/26 16:26, Daniel Henrique Barboza wrote:


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.

Yes, this is clearer, thanks!

Reply via email to