Re: [PATCH v3 09/11] hw/sd/sdcard: Correctly display the command name in trace events

[Peter Maydell]

On Fri, 5 Jun 2020 at 11:25, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Some ACMD were incorrectly displayed. Fix by remembering if we
> are processing a ACMD (with current_cmd_is_acmd) and add the
> sd_current_cmd_name() helper, which display to correct name
> regardless it is a CMD or ACMD.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sd.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 952be36399..fad34ab184 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -114,6 +114,7 @@ struct SDState {
>      uint8_t pwd[16];
>      uint32_t pwd_len;
>      uint8_t function_group[6];
> +    bool current_cmd_is_acmd;

This is extra state, so strictly speaking it needs to be
migrated (though the only thing we would get wrong is a
possible wrong trace message after a migration load).

>      uint8_t current_cmd;
>      /* True if we will handle the next command as an ACMD. Note that this 
> does
>       * *not* track the APP_CMD status bit!
> @@ -1687,6 +1688,8 @@ int sd_do_command(SDState *sd, SDRequest *req,
>                        req->cmd);
>          req->cmd &= 0x3f;
>      }
> +    sd->current_cmd = req->cmd;
> +    sd->current_cmd_is_acmd = sd->expecting_acmd;

I'm not 100% sure about moving the update of sd->current_cmd
down here -- if it's an illegal command that seems wrong.

>      if (sd->card_status & CARD_IS_LOCKED) {
>          if (!cmd_valid_while_locked(sd, req->cmd)) {
> @@ -1714,7 +1717,6 @@ int sd_do_command(SDState *sd, SDRequest *req,
>          /* Valid command, we can update the 'state before command' bits.
>           * (Do this now so they appear in r1 responses.)
>           */
> -        sd->current_cmd = req->cmd;
>          sd->card_status &= ~CURRENT_STATE;
>          sd->card_status |= (last_state << 9);
>      }
> @@ -1775,6 +1777,15 @@ send_response:
>      return rsplen;
>  }

thanks
-- PMM