[ot] hw/riscv: dm: Handle haltreqs for unresponsive harts

This patch works around the Debug Module's current non-conformance with
the RISC-V debug specification in how it handles halt requests
(haltreqs) to unresponsive harts (e.g. harts currently in reset).
Several parts of the RISC-V Debug Specification refer to this behaviour
(see sections 3.2, 3.4, 3.5) but section C.1.3 is specifically a bug
fix defined just for this behaviour.

The current DM implementation just ignores incoming halt requests if the
hart is unresponsive. This commit instead latches these requests in a
bitmask, so that when the hart comes out of reset (i.e. starts executing
instructions) it can be checked and used to immediately halt the hart.

Unfortunately, a fully correct implementation of this behaviour would
likely require a direct link between the RISC-V CPU and the DM to allow
the hart to be halted at this point. Such links are not currently
supported by QEMU. This commit introduces a reasonable workaround where
we poll the availability of cores on a `dmstatus` read and halt newly
responsive harts with latched haltreqs there, relying on the fact that
most debuggers interacting with the DM will write the haltreq and then
repeatedly poll `dmstatus` to watch the harts halt (see e.g. section
B.3 of the RISC-V Debug specification).

This is not 100% conformant (if dmstatus is not polled for a while then
the hart will execute some instructions whereas in real HW it would
halt immediately before executing guest code) but is a small workaround
/ hack that can support the majority of practical use cases reasonably
enough.

Signed-off-by: Alex Jones <[email protected]>

Author: AlexJones0

hw/riscv/dm.c

@@ -334,6 +334,7 @@ struct RISCVDMState {
     const char *soc; /* Subsystem name, for debug */
     uint64_t nonexistent_bm; /* Selected harts that are not existent */
     uint64_t unavailable_bm; /* Selected harts that are not available */
+    uint64_t haltreq_bm; /* Selected harts that have a pending halt request */
     uint64_t to_go_bm; /* Harts that have been flagged for debug exec */
     uint32_t address; /* DM register addr: only bADDRESS_BITS..b0 are used */
     uint32_t *regs; /* Debug module register values */
@@ -1335,6 +1336,8 @@ static CmdErr riscv_dm_dmcontrol_write(RISCVDMState *dm, uint32_t value)
                     trace_riscv_dm_unavailable_hart_control(dm->soc, hartsel,
                                                             "halt");
                     ret = CMD_ERR_HALT_RESUME;
+                    /* flag the haltreq as pending while unavailable */
+                    dm->haltreq_bm |= hartbit;
                 } else {
                     riscv_dm_halt_hart(dm, hartsel);
                 }
@@ -1538,6 +1541,27 @@ static CmdErr riscv_dm_dmstatus_read(RISCVDMState *dm, uint32_t *value)
             trace_riscv_dm_status(dm->soc, cs->cpu_index, "became available");
             /* clear the unavailability flag and resume w/ "regular" states */
             dm->unavailable_bm &= ~mask;
+
+            /*
+             * If a pending haltreq exists for this hart, enter debug mode.
+             *
+             * @todo: This is a workaround for the fact that the current
+             * implementation does not conform to the debug specification. See
+             * e.g. section C.1.4: "When a hart comes out of reset and haltreq
+             * is set, the hart will immediately enter Debug Mode". To support
+             * this the CPU would need to query the DM when it begins executing
+             * instructions, and hence would need a reference to the DM, which
+             * is not ideal.
+             *
+             * For now, we can support most real use cases by exploiting the
+             * fact that reasonable SW will usually send a haltreq and then
+             * repeatedly poll `dmstatus` to see the hart(s) halt. Hence, we
+             * latch haltreqs for unresponsive cores and halt any hart that is
+             * polled as newly available/responsive on a `dmstatus` read.
+             */
+            if (dm->haltreq_bm & mask) {
+                riscv_dm_halt_hart(dm, hix);
+            }
         }
         if (hart->resumed) {
             resumeack += 1;
@@ -2412,6 +2436,7 @@ static void riscv_dm_halt_hart(RISCVDMState *dm, unsigned hartsel)
 
     /* Note: NMI are not yet supported */
     cpu_exit(cs);
+    dm->haltreq_bm &= ~(1u << hartsel);
     /* not sure if the real HW clear this flag on halt */
     dm->hart->resumed = false;
     riscv_dm_set_cs(dm, true);
@@ -2573,6 +2598,7 @@ static void riscv_dm_reset_enter(Object *obj, ResetType type)
     /* Hart statuses are updated on reset_exit */
     dm->nonexistent_bm = 0;
     dm->unavailable_bm = 0;
+    dm->haltreq_bm = 0;
     dm->address = 0;
     dm->to_go_bm = 0;
     for (unsigned ix = 0; ix < dm->hart_count; ix++) {