On Wed, 23 Jul 2025 at 13:47, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Ben Dooks <ben.do...@codethink.co.uk> writes:
>
> (Add maintainers to CC)
>
> You should get your patch workflow to use scripts/get_maintainer.pl so
> they get CC'd and reduces the chance of it being missed in the fire-hose
> of qemu-devel.
>
> > If we get "ssi_sd: error: Unexpected response to cmd" then having
> > the bad s->arglen would be useful debug and does not add any complexity
> > to the code.
>
> Generally we should be removing the old-style DPRINTF debug and
> replacing them with tracepoints where they are warranted. The main
> problem with the old style DPRINTF's is the format strings tend to
> bitrot because they are not enabled by default.
>
> >
> > Signed-off-by: Ben Dooks <ben.do...@codethink.co.uk>
> > ---
> > hw/sd/ssi-sd.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > index 6c90a86ab4..f1441d2c97 100644
> > --- a/hw/sd/ssi-sd.c
> > +++ b/hw/sd/ssi-sd.c
> > @@ -183,7 +183,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev,
> > uint32_t val)
> > s->response[0] = 1;
> > memcpy(&s->response[1], longresp, 4);
> > } else if (s->arglen != 4) {
> > - BADF("Unexpected response to cmd %d\n", s->cmd);
> > + BADF("Unexpected response to cmd %d, arglen=%d\n", s->cmd,
> > s->arglen);
>
> That said BADF is defined in both cases (although the exit(1) for the
> debug leg is a bit aggressive). Is this an error of the guest
> miss-programming the device with invalid data?
>
> There could be an argument for using:
>
> qemu_log_mask(LOG_GUEST_ERROR, "Unexpected response to cmd %d,
> arglen=%d\n", s->cmd, s->arglen);
>
> instead.
This unexpected response comes from QEMU's SD card emulation,
so I'm not sure to what extent LOG_GUEST_ERROR is appropriate.
Is this triggered by the guest doing something silly, or by
a bug in QEMU itself?
I agree that the BADF() macro is rather a legacy relic
(you can see it in a handful of other files too). exit(1)
seems unlikely to be very useful behaviour even if you are
trying to debug this code, and we have slowly been weeding
out places where QEMU just barfs on error cases.
thanks
-- PMM
