On 6/24/21 4:22 PM, Philippe Mathieu-Daudé wrote:
> Log illegal commands as GUEST_ERROR.
>
> Note: we are logging back the SDIO commands (CMD5, CMD52-54).
>
> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
> hw/sd/sd.c | 57 ++++++++++++++++++++++--------------------------------
> 1 file changed, 23 insertions(+), 34 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index ce1eec0374f..0215bdb3689 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -965,6 +965,14 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState
> *sd, SDRequest req)
> return sd_illegal;
> }
>
> +static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
> +{
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i\n",
> + sd->proto->name, req.cmd);
> +
> + return sd_illegal;
> +}> static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
> {
> uint32_t rca = 0x0000;
> @@ -1017,15 +1025,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 1: /* CMD1: SEND_OP_CMD */
> - if (!sd->spi)
> - goto bad_cmd;
> -
> sd->state = sd_transfer_state;
> return sd_r1;
>
> case 2: /* CMD2: ALL_SEND_CID */
> - if (sd->spi)
> - goto bad_cmd;
> switch (sd->state) {
> case sd_ready_state:
> sd->state = sd_identification_state;
> @@ -1037,8 +1040,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 3: /* CMD3: SEND_RELATIVE_ADDR */
> - if (sd->spi)
> - goto bad_cmd;
> switch (sd->state) {
> case sd_identification_state:
> case sd_standby_state:
> @@ -1052,8 +1053,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 4: /* CMD4: SEND_DSR */
> - if (sd->spi)
> - goto bad_cmd;
> switch (sd->state) {
> case sd_standby_state:
> break;
> @@ -1063,9 +1062,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> }
> break;
>
> - case 5: /* CMD5: reserved for SDIO cards */
> - return sd_illegal;
> -
> case 6: /* CMD6: SWITCH_FUNCTION */
> switch (sd->mode) {
> case sd_data_transfer_mode:
> @@ -1081,8 +1077,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 7: /* CMD7: SELECT/DESELECT_CARD */
> - if (sd->spi)
> - goto bad_cmd;
> switch (sd->state) {
> case sd_standby_state:
> if (sd->rca != rca)
> @@ -1212,8 +1206,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 15: /* CMD15: GO_INACTIVE_STATE */
> - if (sd->spi)
> - goto bad_cmd;
> switch (sd->mode) {
> case sd_data_transfer_mode:
> if (sd->rca != rca)
> @@ -1320,8 +1312,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 26: /* CMD26: PROGRAM_CID */
> - if (sd->spi)
> - goto bad_cmd;
> switch (sd->state) {
> case sd_transfer_state:
> sd->state = sd_receivingdata_state;
> @@ -1466,15 +1456,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> }
> break;
>
> - case 52 ... 54:
> - /* CMD52, CMD53, CMD54: reserved for SDIO cards
> - * (see the SDIO Simplified Specification V2.0)
> - * Handle as illegal command but do not complain
> - * on stderr, as some OSes may use these in their
> - * probing for presence of an SDIO card.
> - */
> - return sd_illegal;
> -
> /* Application specific commands (Class 8) */
> case 55: /* CMD55: APP_CMD */
> switch (sd->state) {
> @@ -1515,19 +1496,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
> SDRequest req)
> break;
>
> case 58: /* CMD58: READ_OCR (SPI) */
> - if (!sd->spi) {
> - goto bad_cmd;
> - }
> return sd_r3;
>
> case 59: /* CMD59: CRC_ON_OFF (SPI) */
> - if (!sd->spi) {
> - goto bad_cmd;
> - }
> return sd_r1;
>
> default:
> - bad_cmd:
> qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
> return sd_illegal;
> }
> @@ -2114,10 +2088,25 @@ void sd_enable(SDState *sd, bool enable)
>
> static const SDProto sd_proto_spi = {
> .name = "SPI",
> + .cmd = {
> + [2 ... 4] = sd_cmd_illegal,
> + [5] = sd_cmd_illegal,
> + [7] = sd_cmd_illegal,
> + [15] = sd_cmd_illegal,
> + [26] = sd_cmd_illegal,
> + [52 ... 54] = sd_cmd_illegal,
> + },
> };
>
> static const SDProto sd_proto_sd = {
> .name = "SD",
> + .cmd = {
> + [1] = sd_cmd_illegal,
> + [5] = sd_cmd_illegal,
> + [52 ... 54] = sd_cmd_illegal,
> + [58] = sd_cmd_illegal,
> + [59] = sd_cmd_illegal,
> + },
> };
>
> static void sd_instance_init(Object *obj)
>
Looks good.
I would start to move these commands in a sd_cmd.c file or sd_common.c
may be.
Thanks,
C.
