Re: [PULL 00/76] Misc patches for 2024-06-18

[Peter Maydell]

On Tue, 18 Jun 2024 at 21:32, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/18/24 13:15, Philippe Mathieu-Daudé wrote:
> > On 18/6/24 22:06, Richard Henderson wrote:
> >> Fails testing:
> >>
> >> https://gitlab.com/qemu-project/qemu/-/jobs/7129004955
> >>
> >> qemu-system-tricore: ../accel/tcg/cpu-exec.c:1082: tcg_exec_realizefn: 
> >> Assertion
> >> `cpu->cc->tcg_ops->cpu_exec_halt' failed.
> >
> > Doh sorry, I tested but hit a libusb issue on this target and mingw64,
> > tested there and thought this was the only issue and missed that.
> >
> > Hmm cpu_has_work() is defined inlined. I'll just drop the 3 lines
> > I added to Peter's patch and send his unmodified.
>
> No, the assert is exactly correct, and caught a bug in the previous patch 
> (55/76).
> Without your assert, we will just SEGV in cpu_handle_halt instead with patch 
> (56/76).

Yep, we're missing the setup of .cpu_exec_halt for tricore.
I missed this because I used a grep for the setup of .cpu_exec_interrupt
to find all the TCGCPUOps I needed to update, and for some reason
tricore doesn't implement that hook. (This surprises me because
I expected every guest CPU to need to provide handling for
interrupts. But looking at the callsite in accel/tcg it does
indeed allow a target to leave cpu_exec_interrupt NULL.)

Though we won't ever segv without the assert() in patch 3:
tricore doesn't implement a halt state, so we won't ever
try to call the non-existent cpu_exec_halt method. (This is
why my local testing didn't catch the missing method.)

Anyway, I've squashed this in to patch 2 and re-queued the
series to target-arm.next:

--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -169,6 +169,7 @@ static const TCGCPUOps tricore_tcg_ops = {
     .synchronize_from_tb = tricore_cpu_synchronize_from_tb,
     .restore_state_to_opc = tricore_restore_state_to_opc,
     .tlb_fill = tricore_cpu_tlb_fill,
+    .cpu_exec_halt = tricore_cpu_has_work,
 };

-- PMM