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

[Philippe Mathieu-Daudé]

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.