hw/sd/sdcard: Return ILLEGAL for CMD19/CMD23 prior SD spec v3.01

[PATCH] hw/sd/sdcard: Return ILLEGAL for CMD19/CMD23 prior SD spec v3.01

Posted by Philippe Mathieu-Daudé 1 year, 11 months ago

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

CMD19 (SEND_TUNING_BLOCK) and CMD23 (SET_BLOCK_COUNT) were
added in the Physical SD spec v3.01. When earlier spec version
is requested, we should return ILLEGAL.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8e6fa09151..7e3bb12b1a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1263,7 +1263,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            break;
+            goto bad_cmd;
         }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
@@ -1274,7 +1274,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     case 23:    /* CMD23: SET_BLOCK_COUNT */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            break;
+            goto bad_cmd;
         }
         switch (sd->state) {
         case sd_transfer_state:
-- 
2.35.1

Re: [PATCH] hw/sd/sdcard: Return ILLEGAL for CMD19/CMD23 prior SD spec v3.01

Posted by Bin Meng 1 year, 11 months ago

On Mon, May 9, 2022 at 10:13 PM Philippe Mathieu-Daudé
<philippe.mathieu.daude@gmail.com> wrote:
>
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> CMD19 (SEND_TUNING_BLOCK) and CMD23 (SET_BLOCK_COUNT) were
> added in the Physical SD spec v3.01. When earlier spec version

nits: it should be spec v3.00, despite the fact that in QEMU we have
been using a name v3.01 to indicate v3.00.

> is requested, we should return ILLEGAL.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 8e6fa09151..7e3bb12b1a 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1263,7 +1263,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
>          if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
> -            break;
> +            goto bad_cmd;
>          }
>          if (sd->state == sd_transfer_state) {
>              sd->state = sd_sendingdata_state;
> @@ -1274,7 +1274,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>
>      case 23:    /* CMD23: SET_BLOCK_COUNT */
>          if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
> -            break;
> +            goto bad_cmd;
>          }
>          switch (sd->state) {
>          case sd_transfer_state:
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH] hw/sd/sdcard: Return ILLEGAL for CMD19/CMD23 prior SD spec v3.01

Posted by Philippe Mathieu-Daudé via 1 year, 11 months ago

On 9/5/22 16:29, Bin Meng wrote:
> On Mon, May 9, 2022 at 10:13 PM Philippe Mathieu-Daudé
> <philippe.mathieu.daude@gmail.com> wrote:
>>
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> CMD19 (SEND_TUNING_BLOCK) and CMD23 (SET_BLOCK_COUNT) were
>> added in the Physical SD spec v3.01. When earlier spec version
> 
> nits: it should be spec v3.00, despite the fact that in QEMU we have
> been using a name v3.01 to indicate v3.00.

Only the "Physical Layer Simplified Specification"s are public:
https://www.sdcard.org/downloads/pls/archives/

I'll rename to "Physical Layer Simplified Specification v3.01"
so it is clearer.

> 
>> is requested, we should return ILLEGAL.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/sd/sd.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 8e6fa09151..7e3bb12b1a 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -1263,7 +1263,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>>
>>       case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
>>           if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
>> -            break;
>> +            goto bad_cmd;
>>           }
>>           if (sd->state == sd_transfer_state) {
>>               sd->state = sd_sendingdata_state;
>> @@ -1274,7 +1274,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>>
>>       case 23:    /* CMD23: SET_BLOCK_COUNT */
>>           if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
>> -            break;
>> +            goto bad_cmd;
>>           }
>>           switch (sd->state) {
>>           case sd_transfer_state:
>> --
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Thanks! Queued.