1. Patchew
  2. QEMU
  3. [PATCH 00/23] hw/sd/sdcard: Accumulation of cleanups and fixes
  4. View patch

[PATCH 09/23] hw/sd/sdcard: Generate random RCA value

Philippe Mathieu-Daudé posted 23 patches 6 months, 1 week ago
There is a newer version of this series
[PATCH 09/23] hw/sd/sdcard: Generate random RCA value
Posted by Philippe Mathieu-Daudé 6 months, 1 week ago 
Rather than using the obscure 0x4567 magic value,
use a real random one.

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 30239b28bc..e1f13e316a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -46,6 +46,7 @@
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
 #include "qemu/log.h"
+#include "qemu/guest-random.h"
 #include "qemu/module.h"
 #include "sdmmc-internal.h"
 #include "trace.h"
@@ -469,11 +470,6 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
 }
 
-static void sd_set_rca(SDState *sd)
-{
-    sd->rca += 0x4567;
-}
-
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
 FIELD(CSR, APP_CMD,                     5,  1)
 FIELD(CSR, FX_EVENT,                    6,  1)
@@ -1055,7 +1051,7 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
     case sd_identification_state:
     case sd_standby_state:
         sd->state = sd_standby_state;
-        sd_set_rca(sd);
+        qemu_guest_getrandom_nofail(&sd->rca, sizeof(sd->rca));
         return sd_r6;
 
     default:
-- 
2.41.0
Re: [PATCH 09/23] hw/sd/sdcard: Generate random RCA value
Posted by Philippe Mathieu-Daudé 6 months, 1 week ago 
On 21/6/24 10:05, Philippe Mathieu-Daudé wrote:
> Rather than using the obscure 0x4567 magic value,
> use a real random one.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sd/sd.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 30239b28bc..e1f13e316a 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -46,6 +46,7 @@
>   #include "qemu/error-report.h"
>   #include "qemu/timer.h"
>   #include "qemu/log.h"
> +#include "qemu/guest-random.h"
>   #include "qemu/module.h"
>   #include "sdmmc-internal.h"
>   #include "trace.h"
> @@ -469,11 +470,6 @@ static void sd_set_csd(SDState *sd, uint64_t size)
>       sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
>   }
>   
> -static void sd_set_rca(SDState *sd)
> -{
> -    sd->rca += 0x4567;
> -}
> -
>   FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
>   FIELD(CSR, APP_CMD,                     5,  1)
>   FIELD(CSR, FX_EVENT,                    6,  1)
> @@ -1055,7 +1051,7 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
>       case sd_identification_state:
>       case sd_standby_state:
>           sd->state = sd_standby_state;
> -        sd_set_rca(sd);
> +        qemu_guest_getrandom_nofail(&sd->rca, sizeof(sd->rca));
>           return sd_r6;

Hmm the NPCM7xx test is failing:

▶  58/450 ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: 
assertion failed: (!memcmp(rmsg, msg, len)) ERROR

But booting various Linux / FreeBSD guests on SD cards works,
so I suppose the test (or TYPE_NPCM7XX_SDHCI model) need some
rework.

$ git grep 0x4567
tests/qtest/npcm7xx_sdhci-test.c:47:    sdhci_cmd_regs(qts, 
NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
tests/qtest/npcm7xx_sdhci-test.c-48- 
SDHC_SELECT_DESELECT_CARD);

In tests/qtest/libqos/sdhci-cmd.h:

/* CMD Reg */
#define SDHC_SEND_RELATIVE_ADDR (3 << 8)
#define SDHC_SELECT_DESELECT_CARD (7 << 8)

IIUC setup_sd_card():

card address is queried ...:

     sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0,
                    SDHC_SEND_RELATIVE_ADDR);

... but then ignored and magic 0x4567 is used instead:

     sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
                    SDHC_SELECT_DESELECT_CARD);

OK, so this test need rework. I see the sdhci_read_cmd()
method but it reads the SDHC_BDATA FIFO register, not sure
this is what should be used to read the RCA from R6 command
response.

Shengtan, Nuvoton folks, what do you think?

Thanks,

Phil.
Re: [PATCH 09/23] hw/sd/sdcard: Generate random RCA value
Posted by Philippe Mathieu-Daudé 5 months, 4 weeks ago 
On 21/6/24 11:44, Philippe Mathieu-Daudé wrote:
> On 21/6/24 10:05, Philippe Mathieu-Daudé wrote:
>> Rather than using the obscure 0x4567 magic value,
>> use a real random one.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/sd/sd.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)


> Hmm the NPCM7xx test is failing:
> 
> ▶  58/450 ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: 
> assertion failed: (!memcmp(rmsg, msg, len)) ERROR
> 
> But booting various Linux / FreeBSD guests on SD cards works,
> so I suppose the test (or TYPE_NPCM7XX_SDHCI model) need some
> rework.
> 
> $ git grep 0x4567
> tests/qtest/npcm7xx_sdhci-test.c:47:    sdhci_cmd_regs(qts, 
> NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
> tests/qtest/npcm7xx_sdhci-test.c-48- SDHC_SELECT_DESELECT_CARD);
> 
> In tests/qtest/libqos/sdhci-cmd.h:
> 
> /* CMD Reg */
> #define SDHC_SEND_RELATIVE_ADDR (3 << 8)
> #define SDHC_SELECT_DESELECT_CARD (7 << 8)
> 
> IIUC setup_sd_card():
> 
> card address is queried ...:
> 
>      sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0,
>                     SDHC_SEND_RELATIVE_ADDR);
> 
> ... but then ignored and magic 0x4567 is used instead:
> 
>      sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
>                     SDHC_SELECT_DESELECT_CARD);
> 
> OK, so this test need rework. I see the sdhci_read_cmd()
> method but it reads the SDHC_BDATA FIFO register, not sure
> this is what should be used to read the RCA from R6 command
> response.
> 
> Shengtan, Nuvoton folks, what do you think?

More than 1 week passed so I'll have a look at it myself.

Regards,

Phil.
Re: [PATCH 09/23] hw/sd/sdcard: Generate random RCA value
Posted by Philippe Mathieu-Daudé 5 months, 4 weeks ago 
On 2/7/24 15:13, Philippe Mathieu-Daudé wrote:
> On 21/6/24 11:44, Philippe Mathieu-Daudé wrote:
>> On 21/6/24 10:05, Philippe Mathieu-Daudé wrote:
>>> Rather than using the obscure 0x4567 magic value,
>>> use a real random one.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/sd/sd.c | 8 ++------
>>>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> 
>> Hmm the NPCM7xx test is failing:
>>
>> ▶  58/450 ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: 
>> assertion failed: (!memcmp(rmsg, msg, len)) ERROR
>>
>> But booting various Linux / FreeBSD guests on SD cards works,
>> so I suppose the test (or TYPE_NPCM7XX_SDHCI model) need some
>> rework.
>>
>> $ git grep 0x4567
>> tests/qtest/npcm7xx_sdhci-test.c:47:    sdhci_cmd_regs(qts, 
>> NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
>> tests/qtest/npcm7xx_sdhci-test.c-48- SDHC_SELECT_DESELECT_CARD);
>>
>> In tests/qtest/libqos/sdhci-cmd.h:
>>
>> /* CMD Reg */
>> #define SDHC_SEND_RELATIVE_ADDR (3 << 8)
>> #define SDHC_SELECT_DESELECT_CARD (7 << 8)
>>
>> IIUC setup_sd_card():
>>
>> card address is queried ...:
>>
>>      sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0,
>>                     SDHC_SEND_RELATIVE_ADDR);
>>
>> ... but then ignored and magic 0x4567 is used instead:
>>
>>      sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
>>                     SDHC_SELECT_DESELECT_CARD);
>>
>> OK, so this test need rework. I see the sdhci_read_cmd()
>> method but it reads the SDHC_BDATA FIFO register, not sure
>> this is what should be used to read the RCA from R6 command
>> response.

Fix: 
https://lore.kernel.org/qemu-devel/20240702140842.54242-4-philmd@linaro.org/

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.