Re: [PATCH v3] hw/pflash: fix block write start
On 15/5/24 10:43, Gerd Hoffmann wrote:
Move the pflash_blk_write_start() call. We need the offset of the
first data write, not the offset for the setup (number-of-bytes)
write. Without this fix u-boot can do block writes to the first
flash block only.
Wow, that is a fast fix :) Thanks!
While being at it drop a leftover FIXME.
Resolves: #2343
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2343
I suppose we also need:
Cc: qemu-sta...@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé
Fixes: fcc79f2e0955 ("hw/pflash: implement update buffer for block writes")
Signed-off-by: Gerd Hoffmann
---
hw/block/pflash_cfi01.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
Re: [PATCH v6 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm
On 28/5/24 12:38, Stefano Garzarella wrote: `memory-backend-shm` can be used with vhost-user devices, so let's add a new test case for it. Acked-by: Thomas Huth Acked-by: Stefan Hajnoczi Reviewed-by: David Hildenbrand Signed-off-by: Stefano Garzarella --- tests/qtest/vhost-user-test.c | 23 +++ 1 file changed, 23 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC PATCH v2 21/22] qapi: Inline and remove QERR_UNSUPPORTED definition
Michael, Konstantin, QERR_UNSUPPORTED is only used by QGA.
Do you mind giving our maintainer's position on Markus
analysis so we can know how to proceed with this definition?
Regards,
Phil.
On 5/10/23 13:22, Markus Armbruster wrote:
Philippe Mathieu-Daudé writes:
Address the comment added in commit 4629ed1e98
("qerror: Finally unused, clean up"), from 2015:
/*
* These macros will go away, please don't use
* in new code, and do not add new ones!
*/
Mechanical transformation using:
$ sed -i -e 's/QERR_UNSUPPORTED/"this feature or command is not currently
supported"/' \
$(git grep -wl QERR_UNSUPPORTED)
then manually removing the definition in include/qapi/qmp/qerror.h.
Signed-off-by: Philippe Mathieu-Daudé
---
RFC: Not sure what is the best way to change the comment
in qga/qapi-schema.json...
---
qga/qapi-schema.json | 5 +++--
include/qapi/qmp/qerror.h | 3 ---
qga/commands-bsd.c| 8 +++
qga/commands-posix.c | 46 +++
qga/commands-win32.c | 22 +--
5 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index b720dd4379..51683f4dc2 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -6,8 +6,9 @@
##
# = General note concerning the use of guest agent interfaces
#
# "unsupported" is a higher-level error than the errors that
# individual commands might document. The caller should always be
-# prepared to receive QERR_UNSUPPORTED, even if the given command
-# doesn't specify it, or doesn't document any failure mode at all.
+# prepared to receive the "this feature or command is not currently supported"
+# message, even if the given command doesn't specify it, or doesn't document
+# any failure mode at all.
##
##
The comment is problematic before the patch. It's a doc comment,
i.e. it goes into the "QEMU Guest Agent Protocol Reference" manual,
where QERR_UNSUPPORTED is meaningless. Back when the comment was added
(commit 9481ecd737b "qga schema: document generic QERR_UNSUPPORTED"), it
was still internal documentation, where QERR_UNSUPPORTED made sense. It
became external documentation four years later (commit 56e8bdd46a8
"build-sys: add qapi doc generation targets")
I'm not sure how useful the comment is.
I guess we could instead simply point out that clients should always be
prepared for errors, even when the command doesn't document any, simply
because commands need not exist in all versions or builds of qemu-ga.
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 840831cc6a..7606f4525d 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -17,7 +17,4 @@
* add new ones!
*/
-#define QERR_UNSUPPORTED \
-"this feature or command is not currently supported"
-
#endif /* QERROR_H */
diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
index 17bddda1cf..11536f148f 100644
--- a/qga/commands-bsd.c
+++ b/qga/commands-bsd.c
@@ -152,25 +152,25 @@ int qmp_guest_fsfreeze_do_thaw(Error **errp)
GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
{
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "this feature or command is not currently supported");
return NULL;
}
GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
{
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "this feature or command is not currently supported");
return NULL;
}
GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp)
{
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "this feature or command is not currently supported");
return NULL;
}
GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
{
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "this feature or command is not currently supported");
return NULL;
}
These are all commands that are not supported by this build of qemu-ga.
We could use the opportunity to improve the error message: scratch
"feature or ". Or maybe change the message to "this command is not
supported in this version of qemu-ga".
More of the same below, marked [*].
Taking a step back... Do we really need to make this a failure of its
own? Why not fail exactly as if the command didn't exist? Why would a
client ever care for the difference between "command doesn't exist in
this build of qemu-ga (but it does in other builds of this version of
qemu-ga)" and "command doesn't exist in this build of qemu-ga (and it
won't in any build of this version of qemu-ga)"?
If clients don't care, we could instead unregister these commands,
i.e. undo qmp_register_command(). The command will the
Re: [RFC PATCH v2 21/22] qapi: Inline and remove QERR_UNSUPPORTED definition
On 12/6/24 15:07, Markus Armbruster wrote: Philippe Mathieu-Daudé writes: Michael, Konstantin, QERR_UNSUPPORTED is only used by QGA. Do you mind giving our maintainer's position on Markus analysis so we can know how to proceed with this definition? Daniel Berrangé recently posted patches that get rid of most instances of QERR_UNSUPPORTED: [PATCH 00/20] qga: clean up command source locations and conditionals https://lore.kernel.org/qemu-devel/20240604134933.220112-1-berra...@redhat.com/ I pointed out a possible opportunity to remove even more. I think we should let the dust settle there, then figure out how to eliminate remaining QERR_UNSUPPORTED, if any. Ah great, thanks for the update :)
Re: [PATCH v8 01/13] qapi: clarify that the default is backend dependent
On 18/6/24 12:00, Stefano Garzarella wrote: The default value of the @share option of the @MemoryBackendProperties really depends on the backend type, so let's document the default values in the same place where we define the option to avoid dispersing the information. Cc: David Hildenbrand Suggested-by: Markus Armbruster Reviewed-by: Markus Armbruster Signed-off-by: Stefano Garzarella --- v2: https://patchew.org/QEMU/20240611130231.83152-1-sgarz...@redhat.com/ v1: https://patchew.org/QEMU/20240523133302.103858-1-sgarz...@redhat.com/ --- qapi/qom.json | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] block: m25p80: Fix heap-buffer-overflow in flash_erase function
Hi Zheyu,
On 18/6/24 17:23, Zheyu Ma wrote:
This patch fixes a heap-buffer-overflow issue in the flash_erase function
of the m25p80 flash memory emulation. The overflow occurs when the
combination of offset and length exceeds the allocated memory for the
storage. The patch adds a check to ensure that the erase length does not
exceed the storage size and adjusts the length accordingly if necessary.
Reproducer:
cat << EOF | qemu-system-aarch64 -display none \
-machine accel=qtest, -m 512M -machine kudo-bmc -qtest stdio
writeq 0xc010 0x6
writel 0xc00c 0x9
writeq 0xc010 0xf27f9412
writeq 0xc00f 0x2b5cdc26
writeq 0xc00c 0x
writeq 0xc00c 0x
writeq 0xc00c 0x
writel 0xc00c 0x9
writeq 0xc00c 0x9
EOF
ASan log:
==2614308==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x7fd3fb7fc000 at pc 0x55aa77a442dc bp 0x7fffaa155900 sp 0x7fffaa1550c8
WRITE of size 65536 at 0x7fd3fb7fc000 thread T0
#0 0x55aa77a442db in __asan_memset
llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:26:3
#1 0x55aa77e7e6b3 in flash_erase hw/block/m25p80.c:631:5
#2 0x55aa77e6f8b1 in complete_collecting_data hw/block/m25p80.c:773:9
#3 0x55aa77e6aaa9 in m25p80_transfer8 hw/block/m25p80.c:1550:13
#4 0x55aa78e9a691 in ssi_transfer_raw_default hw/ssi/ssi.c:92:16
#5 0x55aa78e996c0 in ssi_transfer hw/ssi/ssi.c:165:14
#6 0x55aa78e8d76a in npcm7xx_fiu_uma_transaction hw/ssi/npcm7xx_fiu.c:336:9
#7 0x55aa78e8be4b in npcm7xx_fiu_ctrl_write hw/ssi/npcm7xx_fiu.c:428:13
Signed-off-by: Zheyu Ma
---
hw/block/m25p80.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..e9a59f6616 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
abort();
}
+if (offset + len > s->size) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Erase exceeds storage size, adjusting
length\n");
Usually hardware doesn't "adjust" but report error earlier.
+len = s->size - offset;
+}
+
trace_m25p80_flash_erase(s, offset, len);
if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
Re: [PATCH] block: m25p80: Fix heap-buffer-overflow in flash_erase function
On 18/6/24 21:11, Zheyu Ma wrote:
Thanks for your useful advice!
So how about report the issue and return:
We might report the issue to the user, but there should
be a way the hardware report the issue to the guest software
running. Usually signaled as error condition, irq, ...
We need to figure out what check wasn't properly done.
The spec is the source of truth.
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..2121b43708 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int offset,
FlashCMD cmd)
abort();
}
+ if (offset + len > s->size) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Erase operation exceeds storage size\n");
+ return;
+ }
+
trace_m25p80_flash_erase(s, offset, len);
if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
regards,
Zheyu
On Tue, Jun 18, 2024 at 5:35 PM Philippe Mathieu-Daudé
mailto:phi...@linaro.org>> wrote:
Hi Zheyu,
On 18/6/24 17:23, Zheyu Ma wrote:
> This patch fixes a heap-buffer-overflow issue in the flash_erase
function
> of the m25p80 flash memory emulation. The overflow occurs when the
> combination of offset and length exceeds the allocated memory for the
> storage. The patch adds a check to ensure that the erase length
does not
> exceed the storage size and adjusts the length accordingly if
necessary.
>
> Reproducer:
> cat << EOF | qemu-system-aarch64 -display none \
> -machine accel=qtest, -m 512M -machine kudo-bmc -qtest stdio
> writeq 0xc010 0x6
> writel 0xc00c 0x9
> writeq 0xc010 0xf27f9412
> writeq 0xc00f 0x2b5cdc26
> writeq 0xc00c 0x
> writeq 0xc00c 0x
> writeq 0xc00c 0x
> writel 0xc00c 0x9
> writeq 0xc00c 0x9
> EOF
>
> ASan log:
> ==2614308==ERROR: AddressSanitizer: heap-buffer-overflow on
address 0x7fd3fb7fc000 at pc 0x55aa77a442dc bp 0x7fffaa155900 sp
0x7fffaa1550c8
> WRITE of size 65536 at 0x7fd3fb7fc000 thread T0
> #0 0x55aa77a442db in __asan_memset
llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:26:3
> #1 0x55aa77e7e6b3 in flash_erase hw/block/m25p80.c:631:5
> #2 0x55aa77e6f8b1 in complete_collecting_data
hw/block/m25p80.c:773:9
> #3 0x55aa77e6aaa9 in m25p80_transfer8 hw/block/m25p80.c:1550:13
> #4 0x55aa78e9a691 in ssi_transfer_raw_default hw/ssi/ssi.c:92:16
> #5 0x55aa78e996c0 in ssi_transfer hw/ssi/ssi.c:165:14
> #6 0x55aa78e8d76a in npcm7xx_fiu_uma_transaction
hw/ssi/npcm7xx_fiu.c:336:9
> #7 0x55aa78e8be4b in npcm7xx_fiu_ctrl_write
hw/ssi/npcm7xx_fiu.c:428:13
>
> Signed-off-by: Zheyu Ma mailto:zheyum...@gmail.com>>
> ---
> hw/block/m25p80.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 8dec134832..e9a59f6616 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int
offset, FlashCMD cmd)
> abort();
> }
>
> + if (offset + len > s->size) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "M25P80: Erase exceeds storage size,
adjusting length\n");
Usually hardware doesn't "adjust" but report error earlier.
> + len = s->size - offset;
> + }
> +
> trace_m25p80_flash_erase(s, offset, len);
>
> if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
Re: [PATCH-for-9.0? 0/2] hw/sd/sdcard: Avoid OOB in sd_read_byte()
On 8/4/24 16:17, Philippe Mathieu-Daudé wrote: Since this is Fix day, I went over this old bug: https://gitlab.com/qemu-project/qemu/-/issues/487 It happens to be a QEMU implementation detail not really related to the spec. Philippe Mathieu-Daudé (2): hw/sd/sdcard: Avoid OOB in sd_read_byte() during unexpected CMD switch First patch queued.
[PATCH 03/23] hw/sd/sdcard: Fix typo in SEND_OP_COND command name
There is no SEND_OP_CMD but SEND_OP_COND.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 6 +++---
hw/sd/sdmmc-internal.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index addeb1940f..331cef5779 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1035,7 +1035,7 @@ static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd,
SDRequest req)
return sd_is_spi(sd) ? sd_r1 : sd_r0;
}
-static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
+static sd_rsp_type_t spi_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
{
sd->state = sd_transfer_state;
@@ -2149,7 +2149,7 @@ static const SDProto sd_proto_spi = {
.name = "SPI",
.cmd = {
[0] = sd_cmd_GO_IDLE_STATE,
-[1] = sd_cmd_SEND_OP_CMD,
+[1] = spi_cmd_SEND_OP_COND,
[2 ... 4] = sd_cmd_illegal,
[5] = sd_cmd_illegal,
[7] = sd_cmd_illegal,
@@ -2159,7 +2159,7 @@ static const SDProto sd_proto_spi = {
},
.acmd = {
[6] = sd_cmd_unimplemented,
-[41]= sd_cmd_SEND_OP_CMD,
+[41]= spi_cmd_SEND_OP_COND,
},
};
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 8648a7808d..c1d5508ae6 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -14,7 +14,7 @@
const char *sd_cmd_name(uint8_t cmd)
{
static const char *cmd_abbrev[SDMMC_CMD_MAX] = {
- [0]= "GO_IDLE_STATE", [1]= "SEND_OP_CMD",
+ [0]= "GO_IDLE_STATE", [1]= "SEND_OP_COND",
[2]= "ALL_SEND_CID",[3]= "SEND_RELATIVE_ADDR",
[4]= "SET_DSR", [5]= "IO_SEND_OP_COND",
[6]= "SWITCH_FUNC", [7]= "SELECT/DESELECT_CARD",
--
2.41.0
[PATCH 00/23] hw/sd/sdcard: Accumulation of cleanups and fixes
Various SD card cleanups and fixes accumulated over the years. Various have been useful to help integrating eMMC support (which will come later). Based-on: <20240621075607.17902-1-phi...@linaro.org> st24_be_p() Philippe Mathieu-Daudé (23): hw/sd/sdcard: Correct code indentation hw/sd/sdcard: Rewrite sd_cmd_ALL_SEND_CID using switch case (CMD2) hw/sd/sdcard: Fix typo in SEND_OP_COND command name hw/sd/sdcard: Use HWBLOCK_SHIFT definition instead of magic values hw/sd/sdcard: Use registerfield CSR::CURRENT_STATE definition hw/sd/sdcard: Use Load/Store API to fill some CID/CSD registers hw/sd/sdcard: Remove ACMD6 handler for SPI mode hw/sd/sdcard: Remove explicit entries for illegal commands hw/sd/sdcard: Generate random RCA value hw/sd/sdcard: Track last command used to help logging hw/sd/sdcard: Trace update of block count (CMD23) hw/sd/sdcard: Trace block offset in READ/WRITE data accesses hw/sd/sdcard: Have cmd_valid_while_locked() return a boolean value hw/sd/sdcard: Factor sd_req_get_rca() method out hw/sd/sdcard: Only call sd_req_get_rca() where RCA is used hw/sd/sdcard: Factor sd_req_get_address() method out hw/sd/sdcard: Only call sd_req_get_address() where address is used hw/sd/sdcard: Add sd_invalid_mode_for_cmd to report invalid mode switch hw/sd/sdcard: Inline BLK_READ_BLOCK / BLK_WRITE_BLOCK macros hw/sd/sdcard: Add comments around registers and commands hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30) hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22) hw/sd/sd.c | 278 +++-- hw/sd/sdmmc-internal.c | 2 +- hw/sd/trace-events | 7 +- 3 files changed, 159 insertions(+), 128 deletions(-) -- 2.41.0
[PATCH 02/23] hw/sd/sdcard: Rewrite sd_cmd_ALL_SEND_CID using switch case (CMD2)
Keep this handler style in sync with other handlers by
using a switch() case, which might become handy to
handle other states.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 626e99ecd6..addeb1940f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1044,13 +1044,13 @@ static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
{
-if (sd->state != sd_ready_state) {
+switch (sd->state) {
+case sd_ready_state:
+sd->state = sd_identification_state;
+return sd_r2_i;
+default:
return sd_invalid_state_for_cmd(sd, req);
}
-
-sd->state = sd_identification_state;
-
-return sd_r2_i;
}
static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
--
2.41.0
[PATCH 09/23] hw/sd/sdcard: Generate random RCA value
Rather than using the obscure 0x4567 magic value,
use a real random one.
Signed-off-by: Philippe Mathieu-Daudé
---
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
[PATCH 07/23] hw/sd/sdcard: Remove ACMD6 handler for SPI mode
There is no ACMD6 command in SPI mode, remove the pointless
handler introduced in commit 946897ce18 ("sdcard: handles
more commands in SPI mode"). Keep sd_cmd_unimplemented()
since we'll reuse it later.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b0cd30c657..e9af834a8c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1012,6 +1012,7 @@ static sd_rsp_type_t sd_cmd_illegal(SDState *sd,
SDRequest req)
}
/* Commands that are recognised but not yet implemented. */
+__attribute__((unused))
static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n",
@@ -2153,7 +2154,6 @@ static const SDProto sd_proto_spi = {
[52 ... 54] = sd_cmd_illegal,
},
.acmd = {
-[6] = sd_cmd_unimplemented,
[41]= spi_cmd_SEND_OP_COND,
},
};
--
2.41.0
[PATCH 15/23] hw/sd/sdcard: Only call sd_req_get_rca() where RCA is used
It will be useful later to assert only AC commands
(Addressed point-to-point Commands, defined as the
'sd_ac' enum) extract the RCA value from the command
argument.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bc47ae36bc..cb9d85bb11 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1102,7 +1102,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
{
-uint16_t rca = sd_req_get_rca(sd, req);
+uint16_t rca;
uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
sd->last_cmd_name = sd_cmd_name(req.cmd);
@@ -1160,6 +1160,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 7: /* CMD7: SELECT/DESELECT_CARD */
+rca = sd_req_get_rca(sd, req);
switch (sd->state) {
case sd_standby_state:
if (sd->rca != rca)
@@ -1214,6 +1215,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
return sd_r7;
case 9: /* CMD9: SEND_CSD */
+rca = sd_req_get_rca(sd, req);
switch (sd->state) {
case sd_standby_state:
if (sd->rca != rca)
@@ -1237,6 +1239,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 10: /* CMD10: SEND_CID */
+rca = sd_req_get_rca(sd, req);
switch (sd->state) {
case sd_standby_state:
if (sd->rca != rca)
@@ -1277,6 +1280,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 13: /* CMD13: SEND_STATUS */
+rca = sd_req_get_rca(sd, req);
switch (sd->mode) {
case sd_data_transfer_mode:
if (!sd_is_spi(sd) && sd->rca != rca) {
@@ -1291,6 +1295,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 15: /* CMD15: GO_INACTIVE_STATE */
+rca = sd_req_get_rca(sd, req);
switch (sd->mode) {
case sd_data_transfer_mode:
if (sd->rca != rca)
@@ -1523,6 +1528,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
/* Application specific commands (Class 8) */
case 55: /* CMD55: APP_CMD */
+rca = sd_req_get_rca(sd, req);
switch (sd->state) {
case sd_ready_state:
case sd_identification_state:
--
2.41.0
[PATCH 05/23] hw/sd/sdcard: Use registerfield CSR::CURRENT_STATE definition
Use registerfield-generated definitions to update card_status. Signed-off-by: Philippe Mathieu-Daudé --- 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 c528c30bcf..24415cb9f0 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1788,8 +1788,8 @@ int sd_do_command(SDState *sd, SDRequest *req, * (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); +sd->card_status = FIELD_DP32(sd->card_status, CSR, + CURRENT_STATE, last_state); } send_response: -- 2.41.0
[PATCH 11/23] hw/sd/sdcard: Trace update of block count (CMD23)
Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 1 + hw/sd/trace-events | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 4e378f7cf7..2586d15cbd 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1087,6 +1087,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req) } sd->multi_blk_cnt = req.arg; +trace_sdcard_set_block_count(sd->multi_blk_cnt); return sd_r1; } diff --git a/hw/sd/trace-events b/hw/sd/trace-events index 94a00557b2..724365efc3 100644 --- a/hw/sd/trace-events +++ b/hw/sd/trace-events @@ -43,7 +43,8 @@ sdcard_response(const char *rspdesc, int rsplen) "%s (sz:%d)" sdcard_powerup(void) "" sdcard_inquiry_cmd41(void) "" sdcard_reset(void) "" -sdcard_set_blocklen(uint16_t length) "0x%03x" +sdcard_set_blocklen(uint16_t length) "block len 0x%03x" +sdcard_set_block_count(uint32_t cnt) "block cnt 0x%"PRIx32 sdcard_inserted(bool readonly) "read_only: %u" sdcard_ejected(void) "" sdcard_erase(uint32_t first, uint32_t last) "addr first 0x%" PRIx32" last 0x%" PRIx32 -- 2.41.0
[PATCH 10/23] hw/sd/sdcard: Track last command used to help logging
The command is selected on the I/O lines, and further
processing might be done on the DAT lines via the
sd_read_byte() and sd_write_byte() handlers. Since
these methods can't distinct between normal and APP
commands, keep the name of the current command in
the SDState and use it in the DAT handlers. This
fixes a bug that all normal commands were displayed
as APP commands.
Fixes: 2ed61fb57b ("sdcard: Display command name when tracing CMD/ACMD")
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e1f13e316a..4e378f7cf7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -134,6 +134,7 @@ struct SDState {
uint32_t pwd_len;
uint8_t function_group[6];
uint8_t current_cmd;
+const char *last_cmd_name;
/* True if we will handle the next command as an ACMD. Note that this does
* *not* track the APP_CMD status bit!
*/
@@ -1095,12 +1096,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
uint32_t rca = 0x;
uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
+sd->last_cmd_name = sd_cmd_name(req.cmd);
/* CMD55 precedes an ACMD, so we are not interested in tracing it.
* However there is no ACMD55, so we want to trace this particular case.
*/
if (req.cmd != 55 || sd->expecting_acmd) {
trace_sdcard_normal_command(sd_proto(sd)->name,
-sd_cmd_name(req.cmd), req.cmd,
+sd->last_cmd_name, req.cmd,
req.arg, sd_state_name(sd->state));
}
@@ -1571,7 +1573,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_app_command(SDState *sd,
SDRequest req)
{
-trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
+sd->last_cmd_name = sd_acmd_name(req.cmd);
+trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
req.cmd, req.arg, sd_state_name(sd->state));
sd->card_status |= APP_CMD;
@@ -1863,7 +1866,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
return;
trace_sdcard_write_data(sd_proto(sd)->name,
-sd_acmd_name(sd->current_cmd),
+sd->last_cmd_name,
sd->current_cmd, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
@@ -2019,7 +2022,7 @@ uint8_t sd_read_byte(SDState *sd)
io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
trace_sdcard_read_data(sd_proto(sd)->name,
- sd_acmd_name(sd->current_cmd),
+ sd->last_cmd_name,
sd->current_cmd, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
@@ -2163,6 +2166,7 @@ static void sd_instance_init(Object *obj)
{
SDState *sd = SD_CARD(obj);
+sd->last_cmd_name = "UNSET";
sd->enable = true;
sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
}
--
2.41.0
[PATCH 17/23] hw/sd/sdcard: Only call sd_req_get_address() where address is used
It will be useful later to assert only ADTC commands
(Addressed point-to-point Data Transfer Commands, defined
as the 'sd_adtc' enum) extract the address value from the
command argument.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a0193a46ea..1df16ce6a2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -,7 +,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
{
uint16_t rca;
-uint64_t addr = sd_req_get_address(sd, req);
+uint64_t addr;
sd->last_cmd_name = sd_cmd_name(req.cmd);
/* CMD55 precedes an ACMD, so we are not interested in tracing it.
@@ -1237,7 +1237,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
}
sd->state = sd_sendingdata_state;
memcpy(sd->data, sd->csd, 16);
-sd->data_start = addr;
+sd->data_start = sd_req_get_address(sd, req);
sd->data_offset = 0;
return sd_r1;
@@ -1261,7 +1261,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
}
sd->state = sd_sendingdata_state;
memcpy(sd->data, sd->cid, 16);
-sd->data_start = addr;
+sd->data_start = sd_req_get_address(sd, req);
sd->data_offset = 0;
return sd_r1;
@@ -1337,6 +1337,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
case 17: /* CMD17: READ_SINGLE_BLOCK */
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
+addr = sd_req_get_address(sd, req);
switch (sd->state) {
case sd_transfer_state:
@@ -1357,6 +1358,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
/* Block write commands (Class 4) */
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
case 25: /* CMD25: WRITE_MULTIPLE_BLOCK */
+addr = sd_req_get_address(sd, req);
switch (sd->state) {
case sd_transfer_state:
@@ -1415,7 +1417,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (sd->size > SDSC_MAX_CAPACITY) {
return sd_illegal;
}
-
+addr = sd_req_get_address(sd, req);
switch (sd->state) {
case sd_transfer_state:
if (!address_in_range(sd, "SET_WRITE_PROT", addr, 1)) {
@@ -1437,7 +1439,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (sd->size > SDSC_MAX_CAPACITY) {
return sd_illegal;
}
-
+addr = sd_req_get_address(sd, req);
switch (sd->state) {
case sd_transfer_state:
if (!address_in_range(sd, "CLR_WRITE_PROT", addr, 1)) {
@@ -1459,7 +1461,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (sd->size > SDSC_MAX_CAPACITY) {
return sd_illegal;
}
-
+addr = sd_req_get_address(sd, req);
switch (sd->state) {
case sd_transfer_state:
if (!address_in_range(sd, "SEND_WRITE_PROT",
--
2.41.0
[PATCH 12/23] hw/sd/sdcard: Trace block offset in READ/WRITE data accesses
Useful to detect out of bound accesses.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4 ++--
hw/sd/trace-events | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2586d15cbd..c6cc1bab11 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1868,7 +1868,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
trace_sdcard_write_data(sd_proto(sd)->name,
sd->last_cmd_name,
-sd->current_cmd, value);
+sd->current_cmd, sd->data_offset, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
sd->data[sd->data_offset ++] = value;
@@ -2024,7 +2024,7 @@ uint8_t sd_read_byte(SDState *sd)
trace_sdcard_read_data(sd_proto(sd)->name,
sd->last_cmd_name,
- sd->current_cmd, io_len);
+ sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
ret = sd->data[sd->data_offset ++];
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 724365efc3..0eee98a646 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -52,8 +52,8 @@ sdcard_lock(void) ""
sdcard_unlock(void) ""
sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
-sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint8_t value) "%s %20s/ CMD%02d value 0x%02x"
-sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t length) "%s %20s/ CMD%02d len %" PRIu32
+sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
+sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint32_t length) "%s %20s/ CMD%02d ofs %"PRIu32" len %" PRIu32
sdcard_set_voltage(uint16_t millivolts) "%u mV"
# pxa2xx_mmci.c
--
2.41.0
[PATCH 06/23] hw/sd/sdcard: Use Load/Store API to fill some CID/CSD registers
The ld/st API helps noticing CID or CSD bytes refer to the same field. Multi-bytes fields are stored MSB first in CID / CSD. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 24415cb9f0..b0cd30c657 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -393,10 +393,7 @@ static void sd_set_cid(SDState *sd) sd->cid[6] = PNM[3]; sd->cid[7] = PNM[4]; sd->cid[8] = PRV; /* Fake product revision (PRV) */ -sd->cid[9] = 0xde; /* Fake serial number (PSN) */ -sd->cid[10] = 0xad; -sd->cid[11] = 0xbe; -sd->cid[12] = 0xef; +stl_be_p(&sd->cid[9], 0xdeadbeef); /* Fake serial number (PSN) */ sd->cid[13] = 0x00 |/* Manufacture date (MDT) */ ((MDT_YR - 2000) / 10); sd->cid[14] = ((MDT_YR % 10) << 4) | MDT_MON; @@ -462,9 +459,7 @@ static void sd_set_csd(SDState *sd, uint64_t size) sd->csd[4] = 0x5b; sd->csd[5] = 0x59; sd->csd[6] = 0x00; -sd->csd[7] = (size >> 16) & 0xff; -sd->csd[8] = (size >> 8) & 0xff; -sd->csd[9] = (size & 0xff); +st24_be_p(&sd->csd[7], size); sd->csd[10] = 0x7f; sd->csd[11] = 0x80; sd->csd[12] = 0x0a; -- 2.41.0
[PATCH 21/23] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
RFC: Is it safe to reuse VMSTATE_UNUSED_V() (which happens
to be the same size)?
Cc: Peter Xu
Cc: Fabiano Rosas
---
hw/sd/sd.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 95e23abd30..712fbc0926 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -143,6 +143,8 @@ struct SDState {
uint64_t data_start;
uint32_t data_offset;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -647,6 +649,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -762,7 +765,7 @@ static const VMStateDescription sd_vmstate = {
VMSTATE_UINT64(data_start, SDState),
VMSTATE_UINT32(data_offset, SDState),
VMSTATE_UINT8_ARRAY(data, SDState, 512),
-VMSTATE_UNUSED_V(1, 512),
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
VMSTATE_BOOL(enable, SDState),
VMSTATE_END_OF_LIST()
},
@@ -2019,9 +2022,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset ++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2155,12 +2157,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset ++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH 08/23] hw/sd/sdcard: Remove explicit entries for illegal commands
NULL handler is already handled as illegal, no need to
duplicate (that keeps this array simpler to maintain).
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e9af834a8c..30239b28bc 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2146,12 +2146,6 @@ static const SDProto sd_proto_spi = {
.cmd = {
[0] = sd_cmd_GO_IDLE_STATE,
[1] = spi_cmd_SEND_OP_COND,
-[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,
},
.acmd = {
[41]= spi_cmd_SEND_OP_COND,
@@ -2162,15 +2156,10 @@ static const SDProto sd_proto_sd = {
.name = "SD",
.cmd = {
[0] = sd_cmd_GO_IDLE_STATE,
-[1] = sd_cmd_illegal,
[2] = sd_cmd_ALL_SEND_CID,
[3] = sd_cmd_SEND_RELATIVE_ADDR,
-[5] = sd_cmd_illegal,
[19]= sd_cmd_SEND_TUNING_BLOCK,
[23]= sd_cmd_SET_BLOCK_COUNT,
-[52 ... 54] = sd_cmd_illegal,
-[58]= sd_cmd_illegal,
-[59]= sd_cmd_illegal,
},
};
--
2.41.0
[PATCH 19/23] hw/sd/sdcard: Inline BLK_READ_BLOCK / BLK_WRITE_BLOCK macros
These macros only save 3 chars and make the code harder
to maintain, simply remove them.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8d63a39a54..ca2c903c5b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -816,8 +816,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
}
}
-#define BLK_READ_BLOCK(a, len) sd_blk_read(sd, a, len)
-#define BLK_WRITE_BLOCK(a, len) sd_blk_write(sd, a, len)
#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
#define APP_WRITE_BLOCK(a, len)
@@ -869,7 +867,7 @@ static void sd_erase(SDState *sd)
continue;
}
}
-BLK_WRITE_BLOCK(erase_addr, erase_len);
+sd_blk_write(sd, erase_addr, erase_len);
}
}
@@ -1901,7 +1899,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
if (sd->data_offset >= sd->blk_len) {
/* TODO: Check CRC before committing */
sd->state = sd_programming_state;
-BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd_blk_write(sd, sd->data_start, sd->data_offset);
sd->blk_written ++;
sd->csd[14] |= 0x40;
/* Bzzztt Operation complete. */
@@ -1927,7 +1925,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
if (sd->data_offset >= sd->blk_len) {
/* TODO: Check CRC before committing */
sd->state = sd_programming_state;
-BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd_blk_read(sd, sd->data_start, sd->data_offset);
sd->blk_written++;
sd->data_start += sd->blk_len;
sd->data_offset = 0;
@@ -2075,8 +2073,9 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 17: /* CMD17: READ_SINGLE_BLOCK */
-if (sd->data_offset == 0)
-BLK_READ_BLOCK(sd->data_start, io_len);
+if (sd->data_offset == 0) {
+sd_blk_read(sd, sd->data_start, io_len);
+}
ret = sd->data[sd->data_offset ++];
if (sd->data_offset >= io_len)
@@ -2089,7 +2088,7 @@ uint8_t sd_read_byte(SDState *sd)
sd->data_start, io_len)) {
return 0x00;
}
-BLK_READ_BLOCK(sd->data_start, io_len);
+sd_blk_read(sd, sd->data_start, io_len);
}
ret = sd->data[sd->data_offset ++];
--
2.41.0
[PATCH 20/23] hw/sd/sdcard: Add comments around registers and commands
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 20
1 file changed, 20 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ca2c903c5b..95e23abd30 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -317,6 +317,8 @@ static uint8_t sd_crc7(const void *message, size_t width)
return shift_reg;
}
+/* Operation Conditions register */
+
#define OCR_POWER_DELAY_NS 50 /* 0.5ms */
FIELD(OCR, VDD_VOLTAGE_WINDOW, 0, 24)
@@ -366,6 +368,8 @@ static void sd_set_ocr(SDState *sd)
}
}
+/* SD Configuration register */
+
static void sd_set_scr(SDState *sd)
{
sd->scr[0] = 0 << 4;/* SCR structure version 1.0 */
@@ -388,6 +392,8 @@ static void sd_set_scr(SDState *sd)
sd->scr[7] = 0x00;
}
+/* Card IDentification register */
+
#define MID 0xaa
#define OID "XY"
#define PNM "QEMU!"
@@ -413,6 +419,8 @@ static void sd_set_cid(SDState *sd)
sd->cid[15] = (sd_crc7(sd->cid, 15) << 1) | 1;
}
+/* Card-Specific Data register */
+
#define HWBLOCK_SHIFT 9/* 512 bytes */
#define SECTOR_SHIFT5/* 16 kilobytes */
#define WPGROUP_SHIFT 7/* 2 megs */
@@ -482,6 +490,8 @@ static void sd_set_csd(SDState *sd, uint64_t size)
sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
}
+/* Relative Card Address register */
+
static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
{
if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc) {
@@ -490,6 +500,8 @@ static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
return 0;
}
+/* Card Status register */
+
FIELD(CSR, AKE_SEQ_ERROR, 3, 1)
FIELD(CSR, APP_CMD, 5, 1)
FIELD(CSR, FX_EVENT,6, 1)
@@ -620,6 +632,8 @@ static void sd_reset(DeviceState *dev)
sect = sd_addr_to_wpnum(size) + 1;
sd->state = sd_idle_state;
+
+/* card registers */
sd->rca = 0x;
sd->size = size;
sd_set_ocr(sd);
@@ -1052,6 +1066,7 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
return sd_illegal;
}
+/* CMD0 */
static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
{
if (sd->state != sd_inactive_state) {
@@ -1062,6 +1077,7 @@ static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd,
SDRequest req)
return sd_is_spi(sd) ? sd_r1 : sd_r0;
}
+/* CMD1 */
static sd_rsp_type_t spi_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
{
sd->state = sd_transfer_state;
@@ -1069,6 +1085,7 @@ static sd_rsp_type_t spi_cmd_SEND_OP_COND(SDState *sd,
SDRequest req)
return sd_r1;
}
+/* CMD2 */
static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
{
switch (sd->state) {
@@ -1080,6 +1097,7 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd,
SDRequest req)
}
}
+/* CMD3 */
static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
{
switch (sd->state) {
@@ -1094,6 +1112,7 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState
*sd, SDRequest req)
}
}
+/* CMD19 */
static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
{
if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
@@ -1110,6 +1129,7 @@ static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState
*sd, SDRequest req)
return sd_r1;
}
+/* CMD23 */
static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req)
{
if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
--
2.41.0
[PATCH 18/23] hw/sd/sdcard: Add sd_invalid_mode_for_cmd to report invalid mode switch
Having the mode switch displayed help to track incomplete
command implementations.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 75 +-
1 file changed, 41 insertions(+), 34 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1df16ce6a2..8d63a39a54 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -180,6 +180,17 @@ static const char *sd_version_str(enum
SDPhySpecificationVersion version)
return sdphy_version[version];
}
+static const char *sd_mode_name(enum SDCardModes mode)
+{
+static const char *mode_name[] = {
+[sd_inactive] = "inactive",
+[sd_card_identification_mode] = "identification",
+[sd_data_transfer_mode] = "transfer",
+};
+assert(mode < ARRAY_SIZE(mode_name));
+return mode_name[mode];
+}
+
static const char *sd_state_name(enum SDCardStates state)
{
static const char *state_name[] = {
@@ -1015,6 +1026,15 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState
*sd, SDRequest req)
return sd_illegal;
}
+static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
+{
+qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
+ sd_proto(sd)->name, req.cmd, sd_mode_name(sd->mode),
+ sd_version_str(sd->spec_version));
+
+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 for spec %s\n",
@@ -1154,18 +1174,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 6: /* CMD6: SWITCH_FUNCTION */
-switch (sd->mode) {
-case sd_data_transfer_mode:
-sd_function_switch(sd, req.arg);
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
-
-default:
-break;
+if (sd->mode != sd_data_transfer_mode) {
+return sd_invalid_mode_for_cmd(sd, req);
}
-break;
+sd_function_switch(sd, req.arg);
+sd->state = sd_sendingdata_state;
+sd->data_start = 0;
+sd->data_offset = 0;
+return sd_r1;
case 7: /* CMD7: SELECT/DESELECT_CARD */
rca = sd_req_get_rca(sd, req);
@@ -1289,33 +1305,24 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
case 13: /* CMD13: SEND_STATUS */
rca = sd_req_get_rca(sd, req);
-switch (sd->mode) {
-case sd_data_transfer_mode:
-if (!sd_is_spi(sd) && sd->rca != rca) {
-return sd_r0;
-}
-
-return sd_r1;
-
-default:
-break;
+if (sd->mode != sd_data_transfer_mode) {
+return sd_invalid_mode_for_cmd(sd, req);
}
-break;
+if (!sd_is_spi(sd) && sd->rca != rca) {
+return sd_r0;
+}
+
+return sd_r1;
case 15: /* CMD15: GO_INACTIVE_STATE */
-rca = sd_req_get_rca(sd, req);
-switch (sd->mode) {
-case sd_data_transfer_mode:
-if (sd->rca != rca)
-return sd_r0;
-
-sd->state = sd_inactive_state;
-return sd_r0;
-
-default:
-break;
+if (sd->mode != sd_data_transfer_mode) {
+return sd_invalid_mode_for_cmd(sd, req);
}
-break;
+rca = sd_req_get_rca(sd, req);
+if (sd->rca == rca) {
+sd->state = sd_inactive_state;
+}
+return sd_r0;
/* Block read commands (Class 2) */
case 16: /* CMD16: SET_BLOCKLEN */
--
2.41.0
[PATCH 01/23] hw/sd/sdcard: Correct code indentation
Fix mis-alignment from commits 793d04f495 and 6380cd2052
("Add sd_cmd_SEND_TUNING_BLOCK" and "Add sd_cmd_SET_BLOCK_COUNT").
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 16d8d52a78..626e99ecd6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1069,33 +1069,33 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState
*sd, SDRequest req)
static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
{
-if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-return sd_cmd_illegal(sd, req);
-}
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+return sd_cmd_illegal(sd, req);
+}
-if (sd->state != sd_transfer_state) {
-return sd_invalid_state_for_cmd(sd, req);
-}
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
-sd->state = sd_sendingdata_state;
-sd->data_offset = 0;
+sd->state = sd_sendingdata_state;
+sd->data_offset = 0;
-return sd_r1;
+return sd_r1;
}
static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd, SDRequest req)
{
-if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-return sd_cmd_illegal(sd, req);
-}
+if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+return sd_cmd_illegal(sd, req);
+}
-if (sd->state != sd_transfer_state) {
-return sd_invalid_state_for_cmd(sd, req);
-}
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
-sd->multi_blk_cnt = req.arg;
+sd->multi_blk_cnt = req.arg;
-return sd_r1;
+return sd_r1;
}
static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
--
2.41.0
[PATCH 14/23] hw/sd/sdcard: Factor sd_req_get_rca() method out
Extract sd_req_get_rca() so we can re-use it in various
SDProto handlers. Return a 16-bit value since RCA is 16-bit.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 510784fc82..bc47ae36bc 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -471,6 +471,14 @@ static void sd_set_csd(SDState *sd, uint64_t size)
sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
}
+static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
+{
+if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc) {
+return req.arg >> 16;
+}
+return 0;
+}
+
FIELD(CSR, AKE_SEQ_ERROR, 3, 1)
FIELD(CSR, APP_CMD, 5, 1)
FIELD(CSR, FX_EVENT,6, 1)
@@ -1094,7 +1102,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
{
-uint32_t rca = 0x;
+uint16_t rca = sd_req_get_rca(sd, req);
uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
sd->last_cmd_name = sd_cmd_name(req.cmd);
@@ -1110,11 +1118,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
/* Not interpreting this as an app command */
sd->card_status &= ~APP_CMD;
-if (sd_cmd_type[req.cmd] == sd_ac
-|| sd_cmd_type[req.cmd] == sd_adtc) {
-rca = req.arg >> 16;
-}
-
/* CMD23 (set block count) must be immediately followed by CMD18 or CMD25
* if not, its effects are cancelled */
if (sd->multi_blk_cnt != 0 && !(req.cmd == 18 || req.cmd == 25)) {
--
2.41.0
[PATCH 16/23] hw/sd/sdcard: Factor sd_req_get_address() method out
Extract sd_cmd_get_address() so we can re-use it
in various SDProto handlers. Use CARD_CAPACITY and
HWBLOCK_SHIFT definitions instead of magic values.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index cb9d85bb11..a0193a46ea 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -579,6 +579,14 @@ static void sd_response_r7_make(SDState *sd, uint8_t
*response)
stl_be_p(response, sd->vhs);
}
+static uint64_t sd_req_get_address(SDState *sd, SDRequest req)
+{
+if (FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
+return (uint64_t) req.arg << HWBLOCK_SHIFT;
+}
+return req.arg;
+}
+
static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
{
return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
@@ -1103,7 +,7 @@ static sd_rsp_type_t sd_cmd_SET_BLOCK_COUNT(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
{
uint16_t rca;
-uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
+uint64_t addr = sd_req_get_address(sd, req);
sd->last_cmd_name = sd_cmd_name(req.cmd);
/* CMD55 precedes an ACMD, so we are not interested in tracing it.
--
2.41.0
[PATCH 22/23] hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30)
Per sections 3.6.1 (SD Bus Protocol) and 7.3.2 (Responses): In the CMD line the Most Significant Bit is transmitted first. Use the stl_be_p() helper to store the value in big-endian. Signed-off-by: Philippe Mathieu-Daudé --- RFC because I'm surprised this has been unnoticed for 17 years (commit a1bb27b1e9 "initial SD card emulation", April 2007). Cc: Peter Maydell --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 712fbc0926..601a6908aa 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1498,7 +1498,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) } sd->state = sd_sendingdata_state; -*(uint32_t *) sd->data = sd_wpbits(sd, req.arg); +stl_be_p(sd->data, sd_wpbits(sd, req.arg)); sd->data_start = addr; sd->data_offset = 0; return sd_r1; -- 2.41.0
[PATCH 23/23] hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22)
Per sections 3.6.1 (SD Bus Protocol), 4.3.4 "Data Write"
and 7.3.2 (Responses):
In the CMD line the Most Significant Bit is transmitted first.
Use the stl_be_p() helper to store the value in big-endian.
Signed-off-by: Philippe Mathieu-Daudé
---
RFC because I'm surprised this has been unnoticed for 17 years
(commit a1bb27b1e9 "initial SD card emulation", April 2007).
Cc: Peter Maydell
---
hw/sd/sd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 601a6908aa..5d572ad13c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1659,8 +1659,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
switch (sd->state) {
case sd_transfer_state:
-*(uint32_t *) sd->data = sd->blk_written;
-
+stl_be_p(sd->data, sd->blk_written);
sd->state = sd_sendingdata_state;
sd->data_start = 0;
sd->data_offset = 0;
--
2.41.0
[PATCH 13/23] hw/sd/sdcard: Have cmd_valid_while_locked() return a boolean value
Signed-off-by: Philippe Mathieu-Daudé
---
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 c6cc1bab11..510784fc82 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1716,7 +1716,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
return sd_illegal;
}
-static int cmd_valid_while_locked(SDState *sd, const uint8_t cmd)
+static bool cmd_valid_while_locked(SDState *sd, unsigned cmd)
{
/* Valid commands in locked state:
* basic class (0)
@@ -1730,7 +1730,7 @@ static int cmd_valid_while_locked(SDState *sd, const
uint8_t cmd)
return cmd == 41 || cmd == 42;
}
if (cmd == 16 || cmd == 55) {
-return 1;
+return true;
}
return sd_cmd_class[cmd] == 0 || sd_cmd_class[cmd] == 7;
}
--
2.41.0
[PATCH 04/23] hw/sd/sdcard: Use HWBLOCK_SHIFT definition instead of magic values
From: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 331cef5779..c528c30bcf 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -596,7 +596,7 @@ static void sd_reset(DeviceState *dev)
} else {
sect = 0;
}
-size = sect << 9;
+size = sect << HWBLOCK_SHIFT;
sect = sd_addr_to_wpnum(size) + 1;
@@ -822,8 +822,8 @@ static void sd_erase(SDState *sd)
if (FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
/* High capacity memory card: erase units are 512 byte blocks */
-erase_start *= 512;
-erase_end *= 512;
+erase_start <<= HWBLOCK_SHIFT;
+erase_end <<= HWBLOCK_SHIFT;
sdsc = false;
}
--
2.41.0
Re: [PATCH 09/23] hw/sd/sdcard: Generate random RCA value
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é
---
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, 0x4567, 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, 0x4567, 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 00/23] hw/sd/sdcard: Accumulation of cleanups and fixes
On 21/6/24 10:05, Philippe Mathieu-Daudé wrote: Various SD card cleanups and fixes accumulated over the years. Various have been useful to help integrating eMMC support (which will come later). Based-on: <20240621075607.17902-1-phi...@linaro.org> st24_be_p() Philippe Mathieu-Daudé (23): hw/sd/sdcard: Correct code indentation hw/sd/sdcard: Rewrite sd_cmd_ALL_SEND_CID using switch case (CMD2) hw/sd/sdcard: Fix typo in SEND_OP_COND command name hw/sd/sdcard: Use HWBLOCK_SHIFT definition instead of magic values hw/sd/sdcard: Use registerfield CSR::CURRENT_STATE definition hw/sd/sdcard: Use Load/Store API to fill some CID/CSD registers hw/sd/sdcard: Remove ACMD6 handler for SPI mode hw/sd/sdcard: Remove explicit entries for illegal commands hw/sd/sdcard: Have cmd_valid_while_locked() return a boolean value hw/sd/sdcard: Factor sd_req_get_rca() method out hw/sd/sdcard: Only call sd_req_get_rca() where RCA is used hw/sd/sdcard: Factor sd_req_get_address() method out hw/sd/sdcard: Only call sd_req_get_address() where address is used hw/sd/sdcard: Add sd_invalid_mode_for_cmd to report invalid mode switch hw/sd/sdcard: Inline BLK_READ_BLOCK / BLK_WRITE_BLOCK macros hw/sd/sdcard: Add comments around registers and commands Patches 1-8, 13-20 queued.
[PATCH v2 02/12] hw/sd/sdcard: Generate random RCA value
Rather than using the obscure 0x4567 magic value,
use a real random one.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
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 a48010cfc1..ec58c5e2a6 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"
@@ -490,11 +491,6 @@ static void sd_set_csd(SDState *sd, uint64_t size)
/* Relative Card Address register */
-static void sd_set_rca(SDState *sd)
-{
-sd->rca += 0x4567;
-}
-
static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
{
if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc) {
@@ -1107,7 +1103,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
[PATCH v2 01/12] tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA
Disable tests using 0x4567 hardcoded RCA otherwise when
using random RCA we get:
ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: assertion
failed: (ret == len)
not ok /arm/npcm7xx_sdhci/read_sd -
ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: assertion failed:
(ret == len)
Bail out!
See
https://lore.kernel.org/qemu-devel/37f83be9-deb5-42a1-b704-14984351d...@linaro.org/
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Hao Wu
Cc: Shengtan Mao
Cc: Tyrone Ting
---
tests/qtest/npcm7xx_sdhci-test.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
index 5d68540e52..6a42b142ad 100644
--- a/tests/qtest/npcm7xx_sdhci-test.c
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -44,6 +44,7 @@ static QTestState *setup_sd_card(void)
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4120, 0, (41 << 8));
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
+g_test_skip("hardcoded 0x4567 card address");
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4567, 0,
SDHC_SELECT_DESELECT_CARD);
@@ -76,6 +77,9 @@ static void test_read_sd(void)
{
QTestState *qts = setup_sd_card();
+g_test_skip("hardcoded 0x4567 card address used in setup_sd_card()");
+return;
+
write_sdread(qts, "hello world");
write_sdread(qts, "goodbye");
@@ -108,6 +112,9 @@ static void test_write_sd(void)
{
QTestState *qts = setup_sd_card();
+g_test_skip("hardcoded 0x4567 card address used in setup_sd_card()");
+return;
+
sdwrite_read(qts, "hello world");
sdwrite_read(qts, "goodbye");
--
2.41.0
[PATCH v2 00/12] hw/sd/sdcard: Accumulation of cleanups and fixes
Since v1: - various patches merged, few more added Various SD card cleanups and fixes accumulated over the years. Various have been useful to help integrating eMMC support (which will come later). Philippe Mathieu-Daudé (12): tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA hw/sd/sdcard: Generate random RCA value hw/sd/sdcard: Track last command used to help logging hw/sd/sdcard: Trace block offset in READ/WRITE data accesses hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30) hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22) hw/sd/sdcard: Use READY_FOR_DATA definition instead of magic value hw/sd/sdcard: Assign SDCardStates enum values hw/sd/sdcard: Simplify sd_inactive_state handling hw/sd/sdcard: Restrict SWITCH_FUNCTION to sd_transfer_state (CMD6) hw/sd/sdcard: Add direct reference to SDProto in SDState hw/sd/sd.c | 119 --- tests/qtest/npcm7xx_sdhci-test.c | 7 ++ hw/sd/trace-events | 4 +- 3 files changed, 70 insertions(+), 60 deletions(-) -- 2.41.0
[PATCH v2 05/12] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
RFC: Is it safe to reuse VMSTATE_UNUSED_V() (which happens
to be the same size)?
Cc: Peter Xu
Cc: Fabiano Rosas
---
hw/sd/sd.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e4587a0a37..0f8440efcc 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -143,6 +143,8 @@ struct SDState {
uint64_t data_start;
uint32_t data_offset;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -647,6 +649,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -762,7 +765,7 @@ static const VMStateDescription sd_vmstate = {
VMSTATE_UINT64(data_start, SDState),
VMSTATE_UINT32(data_offset, SDState),
VMSTATE_UINT8_ARRAY(data, SDState, 512),
-VMSTATE_UNUSED_V(1, 512),
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
VMSTATE_BOOL(enable, SDState),
VMSTATE_END_OF_LIST()
},
@@ -2020,9 +2023,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset ++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2156,12 +2158,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset ++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH v2 11/12] hw/sd/sdcard: Restrict SWITCH_FUNCTION to sd_transfer_state (CMD6)
SWITCH_FUNCTION is only allowed in TRANSFER state
(See 4.8 "Card State Transition Table).
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index fce99d655d..3b885ba8a0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1196,6 +1196,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (sd->mode != sd_data_transfer_mode) {
return sd_invalid_mode_for_cmd(sd, req);
}
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
+
sd_function_switch(sd, req.arg);
sd->state = sd_sendingdata_state;
sd->data_start = 0;
--
2.41.0
[PATCH v2 03/12] hw/sd/sdcard: Track last command used to help logging
The command is selected on the I/O lines, and further
processing might be done on the DAT lines via the
sd_read_byte() and sd_write_byte() handlers. Since
these methods can't distinct between normal and APP
commands, keep the name of the current command in
the SDState and use it in the DAT handlers. This
fixes a bug that all normal commands were displayed
as APP commands.
Fixes: 2ed61fb57b ("sdcard: Display command name when tracing CMD/ACMD")
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ec58c5e2a6..14bfcc5d6b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -134,6 +134,7 @@ struct SDState {
uint32_t pwd_len;
uint8_t function_group[6];
uint8_t current_cmd;
+const char *last_cmd_name;
/* True if we will handle the next command as an ACMD. Note that this does
* *not* track the APP_CMD status bit!
*/
@@ -1150,12 +1151,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
uint16_t rca;
uint64_t addr;
+sd->last_cmd_name = sd_cmd_name(req.cmd);
/* CMD55 precedes an ACMD, so we are not interested in tracing it.
* However there is no ACMD55, so we want to trace this particular case.
*/
if (req.cmd != 55 || sd->expecting_acmd) {
trace_sdcard_normal_command(sd_proto(sd)->name,
-sd_cmd_name(req.cmd), req.cmd,
+sd->last_cmd_name, req.cmd,
req.arg, sd_state_name(sd->state));
}
@@ -1616,7 +1618,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_app_command(SDState *sd,
SDRequest req)
{
-trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
+sd->last_cmd_name = sd_acmd_name(req.cmd);
+trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
req.cmd, req.arg, sd_state_name(sd->state));
sd->card_status |= APP_CMD;
@@ -1909,7 +1912,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
return;
trace_sdcard_write_data(sd_proto(sd)->name,
-sd_acmd_name(sd->current_cmd),
+sd->last_cmd_name,
sd->current_cmd, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
@@ -2065,7 +2068,7 @@ uint8_t sd_read_byte(SDState *sd)
io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
trace_sdcard_read_data(sd_proto(sd)->name,
- sd_acmd_name(sd->current_cmd),
+ sd->last_cmd_name,
sd->current_cmd, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
@@ -2210,6 +2213,7 @@ static void sd_instance_init(Object *obj)
{
SDState *sd = SD_CARD(obj);
+sd->last_cmd_name = "UNSET";
sd->enable = true;
sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
}
--
2.41.0
[PATCH v2 07/12] hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22)
Per sections 3.6.1 (SD Bus Protocol), 4.3.4 "Data Write"
and 7.3.2 (Responses):
In the CMD line the Most Significant Bit is transmitted first.
Use the stl_be_p() helper to store the value in big-endian.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
RFC because I'm surprised this has been unnoticed for 17 years
(commit a1bb27b1e9 "initial SD card emulation", April 2007).
Cc: Peter Maydell
---
hw/sd/sd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b604b8e71f..0742ba8b38 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1659,8 +1659,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
switch (sd->state) {
case sd_transfer_state:
-*(uint32_t *) sd->data = sd->blk_written;
-
+stl_be_p(sd->data, sd->blk_written);
sd->state = sd_sendingdata_state;
sd->data_start = 0;
sd->data_offset = 0;
--
2.41.0
[PATCH v2 08/12] hw/sd/sdcard: Use READY_FOR_DATA definition instead of magic value
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0742ba8b38..8816bd6671 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -557,7 +557,7 @@ FIELD(CSR, OUT_OF_RANGE, 31, 1)
static void sd_set_cardstatus(SDState *sd)
{
-sd->card_status = 0x0100;
+sd->card_status = READY_FOR_DATA;
}
static void sd_set_sdstatus(SDState *sd)
--
2.41.0
[PATCH v2 12/12] hw/sd/sdcard: Add direct reference to SDProto in SDState
Keep direct reference to SDProto in SDState,
remove then unnecessary sd_proto().
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3b885ba8a0..6685fba4bb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -117,6 +117,8 @@ struct SDState {
uint8_t spec_version;
BlockBackend *blk;
+const SDProto *proto;
+
/* Runtime changeables */
uint32_t mode;/* current card mode, one of SDCardModes */
@@ -155,18 +157,11 @@ struct SDState {
static void sd_realize(DeviceState *dev, Error **errp);
-static const struct SDProto *sd_proto(SDState *sd)
-{
-SDCardClass *sc = SD_CARD_GET_CLASS(sd);
-
-return sc->proto;
-}
-
static const SDProto sd_proto_spi;
static bool sd_is_spi(SDState *sd)
{
-return sd_proto(sd) == &sd_proto_spi;
+return sd->proto == &sd_proto_spi;
}
static const char *sd_version_str(enum SDPhySpecificationVersion version)
@@ -1035,7 +1030,7 @@ static bool address_in_range(SDState *sd, const char
*desc,
static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong state: %s (spec
%s)\n",
- sd_proto(sd)->name, req.cmd, sd_state_name(sd->state),
+ sd->proto->name, req.cmd, sd_state_name(sd->state),
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1044,7 +1039,7 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState
*sd, SDRequest req)
static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
- sd_proto(sd)->name, req.cmd, sd_mode_name(sd->mode),
+ sd->proto->name, req.cmd, sd_mode_name(sd->mode),
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1053,7 +1048,7 @@ static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i for spec %s\n",
- sd_proto(sd)->name, req.cmd,
+ sd->proto->name, req.cmd,
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1064,7 +1059,7 @@ __attribute__((unused))
static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n",
- sd_proto(sd)->name, req.cmd);
+ sd->proto->name, req.cmd);
return sd_illegal;
}
@@ -1157,7 +1152,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
* However there is no ACMD55, so we want to trace this particular case.
*/
if (req.cmd != 55 || sd->expecting_acmd) {
-trace_sdcard_normal_command(sd_proto(sd)->name,
+trace_sdcard_normal_command(sd->proto->name,
sd->last_cmd_name, req.cmd,
req.arg, sd_state_name(sd->state));
}
@@ -1176,8 +1171,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
return sd_illegal;
}
-if (sd_proto(sd)->cmd[req.cmd]) {
-return sd_proto(sd)->cmd[req.cmd](sd, req);
+if (sd->proto->cmd[req.cmd]) {
+return sd->proto->cmd[req.cmd](sd, req);
}
switch (req.cmd) {
@@ -1623,12 +1618,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
SDRequest req)
{
sd->last_cmd_name = sd_acmd_name(req.cmd);
-trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
+trace_sdcard_app_command(sd->proto->name, sd->last_cmd_name,
req.cmd, req.arg, sd_state_name(sd->state));
sd->card_status |= APP_CMD;
-if (sd_proto(sd)->acmd[req.cmd]) {
-return sd_proto(sd)->acmd[req.cmd](sd, req);
+if (sd->proto->acmd[req.cmd]) {
+return sd->proto->acmd[req.cmd](sd, req);
}
switch (req.cmd) {
@@ -1919,7 +1914,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
return;
-trace_sdcard_write_data(sd_proto(sd)->name,
+trace_sdcard_write_data(sd->proto->name,
sd->last_cmd_name,
sd->current_cmd, sd->data_offset, value);
switch (sd->current_cmd) {
@@ -2074,7 +2069,7 @@ uint8_t sd_read_byte(SDState *sd)
io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
-trace_sdcard_read_data(sd_proto(sd)->name,
+trace_sdcard_read_data(sd->proto->name,
[PATCH v2 04/12] hw/sd/sdcard: Trace block offset in READ/WRITE data accesses
Useful to detect out of bound accesses.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 4 ++--
hw/sd/trace-events | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 14bfcc5d6b..e4587a0a37 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1913,7 +1913,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
trace_sdcard_write_data(sd_proto(sd)->name,
sd->last_cmd_name,
-sd->current_cmd, value);
+sd->current_cmd, sd->data_offset, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
sd->data[sd->data_offset ++] = value;
@@ -2069,7 +2069,7 @@ uint8_t sd_read_byte(SDState *sd)
trace_sdcard_read_data(sd_proto(sd)->name,
sd->last_cmd_name,
- sd->current_cmd, io_len);
+ sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
ret = sd->data[sd->data_offset ++];
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 724365efc3..0eee98a646 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -52,8 +52,8 @@ sdcard_lock(void) ""
sdcard_unlock(void) ""
sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
-sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint8_t value) "%s %20s/ CMD%02d value 0x%02x"
-sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t length) "%s %20s/ CMD%02d len %" PRIu32
+sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
+sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint32_t length) "%s %20s/ CMD%02d ofs %"PRIu32" len %" PRIu32
sdcard_set_voltage(uint16_t millivolts) "%u mV"
# pxa2xx_mmci.c
--
2.41.0
[PATCH v2 09/12] hw/sd/sdcard: Assign SDCardStates enum values
SDCardStates enum values are specified, so assign them
correspondingly. It will be useful later when we add
states from later specs, which might not be continuous.
See CURRENT_STATE bits in section 4.10.1 "Card Status".
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8816bd6671..36955189e8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -76,16 +76,16 @@ enum SDCardModes {
};
enum SDCardStates {
-sd_inactive_state = -1,
-sd_idle_state = 0,
-sd_ready_state,
-sd_identification_state,
-sd_standby_state,
-sd_transfer_state,
-sd_sendingdata_state,
-sd_receivingdata_state,
-sd_programming_state,
-sd_disconnect_state,
+sd_inactive_state = -1,
+sd_idle_state = 0,
+sd_ready_state = 1,
+sd_identification_state = 2,
+sd_standby_state= 3,
+sd_transfer_state = 4,
+sd_sendingdata_state= 5,
+sd_receivingdata_state = 6,
+sd_programming_state= 7,
+sd_disconnect_state = 8,
};
typedef sd_rsp_type_t (*sd_cmd_handler)(SDState *sd, SDRequest req);
--
2.41.0
[PATCH v2 06/12] hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30)
Per sections 3.6.1 (SD Bus Protocol) and 7.3.2 (Responses): In the CMD line the Most Significant Bit is transmitted first. Use the stl_be_p() helper to store the value in big-endian. Signed-off-by: Philippe Mathieu-Daudé Tested-by: Cédric Le Goater --- RFC because I'm surprised this has been unnoticed for 17 years (commit a1bb27b1e9 "initial SD card emulation", April 2007). Cc: Peter Maydell --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 0f8440efcc..b604b8e71f 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1498,7 +1498,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) } sd->state = sd_sendingdata_state; -*(uint32_t *) sd->data = sd_wpbits(sd, req.arg); +stl_be_p(sd->data, sd_wpbits(sd, req.arg)); sd->data_start = addr; sd->data_offset = 0; return sd_r1; -- 2.41.0
[PATCH v2 10/12] hw/sd/sdcard: Simplify sd_inactive_state handling
Card entering sd_inactive_state powers off, and won't respond
anymore. Handle that once when entering sd_do_command().
Remove condition always true in sd_cmd_GO_IDLE_STATE().
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 36955189e8..fce99d655d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1072,10 +1072,8 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
/* CMD0 */
static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
{
-if (sd->state != sd_inactive_state) {
-sd->state = sd_idle_state;
-sd_reset(DEVICE(sd));
-}
+sd->state = sd_idle_state;
+sd_reset(DEVICE(sd));
return sd_is_spi(sd) ? sd_r1 : sd_r0;
}
@@ -1570,7 +1568,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
switch (sd->state) {
case sd_ready_state:
case sd_identification_state:
-case sd_inactive_state:
return sd_illegal;
case sd_idle_state:
if (rca) {
@@ -1791,6 +1788,11 @@ int sd_do_command(SDState *sd, SDRequest *req,
return 0;
}
+if (sd->state == sd_inactive_state) {
+rtype = sd_illegal;
+goto send_response;
+}
+
if (sd_req_crc_validate(req)) {
sd->card_status |= COM_CRC_ERROR;
rtype = sd_illegal;
--
2.41.0
[PATCH 00/11] hw/sd/sd: Introduce sd_cmd_to_sendingdata() and sd_generic_read_byte()
Consolitate reading bytes on the DAT lines by introducing a pair of helpers to reuse in all commands sending data. Based-on: <20240625055354.23273-1-phi...@linaro.org> Philippe Mathieu-Daudé (11): hw/sd/sdcard: Introduce sd_cmd_to_sendingdata and sd_generic_read_byte hw/sd/sdcard: Convert SWITCH_FUNCTION to generic_read_byte (CMD6) hw/sd/sdcard: Convert SEND_CSD/SEND_CID to generic_read_byte (CMD9 & 10) hw/sd/sdcard: Duplicate READ_SINGLE_BLOCK / READ_MULTIPLE_BLOCK cases hw/sd/sdcard: Convert READ_SINGLE_BLOCK to generic_read_byte (CMD17) hw/sd/sdcard: Convert SEND_TUNING_BLOCK to generic_read_byte (CMD19) hw/sd/sdcard: Convert SEND_WRITE_PROT to generic_read_byte (CMD30) hw/sd/sdcard: Convert GEN_CMD to generic_read_byte (CMD56) hw/sd/sdcard: Convert SD_STATUS to generic_read_byte (ACMD13) hw/sd/sdcard: Convert SEND_NUM_WR_BLOCKS to generic_read_byte (ACMD22) hw/sd/sdcard: Convert SEND_SCR to generic_read_byte (ACMD51) hw/sd/sd.c | 223 - 1 file changed, 100 insertions(+), 123 deletions(-) -- 2.41.0
[PATCH 02/11] hw/sd/sdcard: Convert SWITCH_FUNCTION to generic_read_byte (CMD6)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 89006ba1b6..0011aa86da 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1220,10 +1220,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
}
sd_function_switch(sd, req.arg);
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0, NULL, 64);
case 7: /* CMD7: SELECT/DESELECT_CARD */
rca = sd_req_get_rca(sd, req);
@@ -1922,7 +1919,6 @@ send_response:
return rsplen;
}
-__attribute__((unused))
/* Return true when buffer consumed */
static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
{
@@ -2112,10 +2108,7 @@ uint8_t sd_read_byte(SDState *sd)
sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 64)
-sd->state = sd_transfer_state;
+sd_generic_read_byte(sd, &ret);
break;
case 9: /* CMD9: SEND_CSD */
--
2.41.0
[PATCH 03/11] hw/sd/sdcard: Convert SEND_CSD/SEND_CID to generic_read_byte (CMD9 & 10)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0011aa86da..346bd657a7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1290,11 +1290,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (!sd_is_spi(sd)) {
break;
}
-sd->state = sd_sendingdata_state;
-memcpy(sd->data, sd->csd, 16);
-sd->data_start = sd_req_get_address(sd, req);
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
+ sd->csd, 16);
default:
break;
@@ -1314,11 +1311,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (!sd_is_spi(sd)) {
break;
}
-sd->state = sd_sendingdata_state;
-memcpy(sd->data, sd->cid, 16);
-sd->data_start = sd_req_get_address(sd, req);
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
+ sd->cid, 16);
default:
break;
@@ -2108,15 +2102,9 @@ uint8_t sd_read_byte(SDState *sd)
sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
-sd_generic_read_byte(sd, &ret);
-break;
-
case 9: /* CMD9: SEND_CSD */
-case 10: /* CMD10: SEND_CID */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 16)
-sd->state = sd_transfer_state;
+case 10: /* CMD10: SEND_CID */
+sd_generic_read_byte(sd, &ret);
break;
case 13: /* ACMD13: SD_STATUS */
--
2.41.0
[PATCH 01/11] hw/sd/sdcard: Introduce sd_cmd_to_sendingdata and sd_generic_read_byte
All commands switching from TRANSFER state to (sending)DATA
do the same: send stream of data on the DAT lines. Instead
of duplicating the same code many times, introduce 2 helpers:
- sd_cmd_to_sendingdata() on the I/O line setup the data to
be transferred,
- sd_generic_read_byte() on the DAT lines to fetch the data.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6685fba4bb..89006ba1b6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -142,8 +142,10 @@ struct SDState {
*/
bool expecting_acmd;
uint32_t blk_written;
+
uint64_t data_start;
uint32_t data_offset;
+size_t data_size;
uint8_t data[512];
uint8_t vendor_data[512];
@@ -1064,6 +1066,28 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
return sd_illegal;
}
+__attribute__((unused))
+static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
+ uint64_t start,
+ const void *data, size_t size)
+{
+if (sd->state != sd_transfer_state) {
+sd_invalid_state_for_cmd(sd, req);
+}
+
+sd->state = sd_sendingdata_state;
+sd->data_start = start;
+sd->data_offset = 0;
+if (data) {
+assert(size);
+memcpy(sd->data, data, size);
+}
+if (size) {
+sd->data_size = size;
+}
+return sd_r1;
+}
+
/* CMD0 */
static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
{
@@ -1898,6 +1922,20 @@ send_response:
return rsplen;
}
+__attribute__((unused))
+/* Return true when buffer consumed */
+static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
+{
+*value = sd->data[sd->data_offset];
+
+if (++sd->data_offset >= sd->data_size) {
+sd->state = sd_transfer_state;
+return true;
+}
+
+return false;
+}
+
void sd_write_byte(SDState *sd, uint8_t value)
{
int i;
--
2.41.0
[PATCH 04/11] hw/sd/sdcard: Duplicate READ_SINGLE_BLOCK / READ_MULTIPLE_BLOCK cases
In order to modify the READ_SINGLE_BLOCK case in the
next commit, duplicate it first.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 346bd657a7..9aaa78a334 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1376,6 +1376,24 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 17: /* CMD17: READ_SINGLE_BLOCK */
+addr = sd_req_get_address(sd, req);
+switch (sd->state) {
+case sd_transfer_state:
+
+if (!address_in_range(sd, "READ_SINGLE_BLOCK", addr, sd->blk_len))
{
+return sd_r1;
+}
+
+sd->state = sd_sendingdata_state;
+sd->data_start = addr;
+sd->data_offset = 0;
+return sd_r1;
+
+default:
+break;
+}
+break;
+
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
addr = sd_req_get_address(sd, req);
switch (sd->state) {
--
2.41.0
[PATCH 06/11] hw/sd/sdcard: Convert SEND_TUNING_BLOCK to generic_read_byte (CMD19)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 48 +++-
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6af7b3f034..3d341495e9 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -562,6 +562,21 @@ static void sd_set_sdstatus(SDState *sd)
memset(sd->sd_status, 0, 64);
}
+static const uint8_t sd_tuning_block_pattern4[64] = {
+/*
+ * See: Physical Layer Simplified Specification Version 3.01,
+ * Table 4-2.
+ */
+0xff, 0x0f, 0xff, 0x00, 0x0f, 0xfc, 0xc3, 0xcc,
+0xc3, 0x3c, 0xcc, 0xff, 0xfe, 0xff, 0xfe, 0xef,
+0xff, 0xdf, 0xff, 0xdd, 0xff, 0xfb, 0xff, 0xfb,
+0xbf, 0xff, 0x7f, 0xff, 0x77, 0xf7, 0xbd, 0xef,
+0xff, 0xf0, 0xff, 0xf0, 0x0f, 0xfc, 0xcc, 0x3c,
+0xcc, 0x33, 0xcc, 0xcf, 0xff, 0xef, 0xff, 0xee,
+0xff, 0xfd, 0xff, 0xfd, 0xdf, 0xff, 0xbf, 0xff,
+0xbb, 0xff, 0xf7, 0xff, 0xf7, 0x7f, 0x7b, 0xde
+};
+
static int sd_req_crc_validate(SDRequest *req)
{
uint8_t buffer[5];
@@ -1139,14 +1154,9 @@ static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState
*sd, SDRequest req)
return sd_cmd_illegal(sd, req);
}
-if (sd->state != sd_transfer_state) {
-return sd_invalid_state_for_cmd(sd, req);
-}
-
-sd->state = sd_sendingdata_state;
-sd->data_offset = 0;
-
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0,
+ sd_tuning_block_pattern4,
+ sizeof(sd_tuning_block_pattern4));
}
/* CMD23 */
@@ -2078,20 +2088,6 @@ void sd_write_byte(SDState *sd, uint8_t value)
}
}
-#define SD_TUNING_BLOCK_SIZE64
-
-static const uint8_t sd_tuning_block_pattern[SD_TUNING_BLOCK_SIZE] = {
-/* See: Physical Layer Simplified Specification Version 3.01, Table 4-2 */
-0xff, 0x0f, 0xff, 0x00, 0x0f, 0xfc, 0xc3, 0xcc,
-0xc3, 0x3c, 0xcc, 0xff, 0xfe, 0xff, 0xfe, 0xef,
-0xff, 0xdf, 0xff, 0xdd, 0xff, 0xfb, 0xff, 0xfb,
-0xbf, 0xff, 0x7f, 0xff, 0x77, 0xf7, 0xbd, 0xef,
-0xff, 0xf0, 0xff, 0xf0, 0x0f, 0xfc, 0xcc, 0x3c,
-0xcc, 0x33, 0xcc, 0xcf, 0xff, 0xef, 0xff, 0xee,
-0xff, 0xfd, 0xff, 0xfd, 0xdf, 0xff, 0xbf, 0xff,
-0xbb, 0xff, 0xf7, 0xff, 0xf7, 0x7f, 0x7b, 0xde,
-};
-
uint8_t sd_read_byte(SDState *sd)
{
/* TODO: Append CRCs */
@@ -2120,6 +2116,7 @@ uint8_t sd_read_byte(SDState *sd)
case 9: /* CMD9: SEND_CSD */
case 10: /* CMD10: SEND_CID */
case 17: /* CMD17: READ_SINGLE_BLOCK */
+case 19: /* CMD19: SEND_TUNING_BLOCK (SD) */
sd_generic_read_byte(sd, &ret);
break;
@@ -2154,13 +2151,6 @@ uint8_t sd_read_byte(SDState *sd)
}
break;
-case 19:/* CMD19: SEND_TUNING_BLOCK (SD) */
-if (sd->data_offset >= SD_TUNING_BLOCK_SIZE - 1) {
-sd->state = sd_transfer_state;
-}
-ret = sd_tuning_block_pattern[sd->data_offset++];
-break;
-
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
ret = sd->data[sd->data_offset ++];
--
2.41.0
[PATCH 09/11] hw/sd/sdcard: Convert SD_STATUS to generic_read_byte (ACMD13)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bf78f06b5c..2f00184bb3 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1681,10 +1681,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
case 13: /* ACMD13: SD_STATUS */
switch (sd->state) {
case sd_transfer_state:
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0,
+ sd->sd_status,
+ sizeof(sd->sd_status));
default:
break;
@@ -2114,6 +2113,7 @@ uint8_t sd_read_byte(SDState *sd)
case 6: /* CMD6: SWITCH_FUNCTION */
case 9: /* CMD9: SEND_CSD */
case 10: /* CMD10: SEND_CID */
+case 13: /* ACMD13: SD_STATUS */
case 17: /* CMD17: READ_SINGLE_BLOCK */
case 19: /* CMD19: SEND_TUNING_BLOCK (SD) */
case 30: /* CMD30: SEND_WRITE_PROT */
@@ -2121,13 +2121,6 @@ uint8_t sd_read_byte(SDState *sd)
sd_generic_read_byte(sd, &ret);
break;
-case 13: /* ACMD13: SD_STATUS */
-ret = sd->sd_status[sd->data_offset ++];
-
-if (sd->data_offset >= sizeof(sd->sd_status))
-sd->state = sd_transfer_state;
-break;
-
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
if (sd->data_offset == 0) {
if (!address_in_range(sd, "READ_MULTIPLE_BLOCK",
--
2.41.0
[PATCH 07/11] hw/sd/sdcard: Convert SEND_WRITE_PROT to generic_read_byte (CMD30)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 17 -
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3d341495e9..6e6b37bd2d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1180,6 +1180,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
{
uint16_t rca;
uint64_t addr;
+uint32_t data;
sd->last_cmd_name = sd_cmd_name(req.cmd);
/* CMD55 precedes an ACMD, so we are not interested in tracing it.
@@ -1533,12 +1534,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
req.arg, sd->blk_len)) {
return sd_r1;
}
-
-sd->state = sd_sendingdata_state;
-stl_be_p(sd->data, sd_wpbits(sd, req.arg));
-sd->data_start = addr;
-sd->data_offset = 0;
-return sd_r1;
+data = sd_wpbits(sd, req.arg);
+return sd_cmd_to_sendingdata(sd, req, addr, &data, sizeof(data));
default:
break;
@@ -2117,6 +2114,7 @@ uint8_t sd_read_byte(SDState *sd)
case 10: /* CMD10: SEND_CID */
case 17: /* CMD17: READ_SINGLE_BLOCK */
case 19: /* CMD19: SEND_TUNING_BLOCK (SD) */
+case 30: /* CMD30: SEND_WRITE_PROT */
sd_generic_read_byte(sd, &ret);
break;
@@ -2158,13 +2156,6 @@ uint8_t sd_read_byte(SDState *sd)
sd->state = sd_transfer_state;
break;
-case 30: /* CMD30: SEND_WRITE_PROT */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 4)
-sd->state = sd_transfer_state;
-break;
-
case 51: /* ACMD51: SEND_SCR */
ret = sd->scr[sd->data_offset ++];
--
2.41.0
[PATCH 08/11] hw/sd/sdcard: Convert GEN_CMD to generic_read_byte (CMD56)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6e6b37bd2d..bf78f06b5c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1626,10 +1626,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
switch (sd->state) {
case sd_transfer_state:
sd->data_offset = 0;
-if (req.arg & 1)
-sd->state = sd_sendingdata_state;
-else
-sd->state = sd_receivingdata_state;
+if (req.arg & 1) {
+return sd_cmd_to_sendingdata(sd, req, 0,
+ sd->vendor_data,
+ sizeof(sd->vendor_data));
+}
+sd->state = sd_receivingdata_state;
return sd_r1;
default:
@@ -2115,6 +2117,7 @@ uint8_t sd_read_byte(SDState *sd)
case 17: /* CMD17: READ_SINGLE_BLOCK */
case 19: /* CMD19: SEND_TUNING_BLOCK (SD) */
case 30: /* CMD30: SEND_WRITE_PROT */
+case 56: /* CMD56: GEN_CMD */
sd_generic_read_byte(sd, &ret);
break;
@@ -2163,14 +2166,6 @@ uint8_t sd_read_byte(SDState *sd)
sd->state = sd_transfer_state;
break;
-case 56: /* CMD56: GEN_CMD */
-ret = sd->vendor_data[sd->data_offset ++];
-
-if (sd->data_offset >= sizeof(sd->vendor_data)) {
-sd->state = sd_transfer_state;
-}
-break;
-
default:
qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
return 0x00;
--
2.41.0
[PATCH 11/11] hw/sd/sdcard: Convert SEND_SCR to generic_read_byte (ACMD51)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 023fcc63ac..af3a46373f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1763,10 +1763,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
case 51: /* ACMD51: SEND_SCR */
switch (sd->state) {
case sd_transfer_state:
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0, sd->scr, sizeof(sd->scr));
default:
break;
@@ -2116,6 +2113,7 @@ uint8_t sd_read_byte(SDState *sd)
case 19: /* CMD19: SEND_TUNING_BLOCK (SD) */
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
case 30: /* CMD30: SEND_WRITE_PROT */
+case 51: /* ACMD51: SEND_SCR */
case 56: /* CMD56: GEN_CMD */
sd_generic_read_byte(sd, &ret);
break;
@@ -2144,13 +2142,6 @@ uint8_t sd_read_byte(SDState *sd)
}
break;
-case 51: /* ACMD51: SEND_SCR */
-ret = sd->scr[sd->data_offset ++];
-
-if (sd->data_offset >= sizeof(sd->scr))
-sd->state = sd_transfer_state;
-break;
-
default:
qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
return 0x00;
--
2.41.0
[PATCH 10/11] hw/sd/sdcard: Convert SEND_NUM_WR_BLOCKS to generic_read_byte (ACMD22)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2f00184bb3..023fcc63ac 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1693,11 +1693,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
switch (sd->state) {
case sd_transfer_state:
-stl_be_p(sd->data, sd->blk_written);
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0,
+ &sd->blk_written,
+ sizeof(sd->blk_written));
default:
break;
@@ -2116,6 +2114,7 @@ uint8_t sd_read_byte(SDState *sd)
case 13: /* ACMD13: SD_STATUS */
case 17: /* CMD17: READ_SINGLE_BLOCK */
case 19: /* CMD19: SEND_TUNING_BLOCK (SD) */
+case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
case 30: /* CMD30: SEND_WRITE_PROT */
case 56: /* CMD56: GEN_CMD */
sd_generic_read_byte(sd, &ret);
@@ -2145,13 +2144,6 @@ uint8_t sd_read_byte(SDState *sd)
}
break;
-case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 4)
-sd->state = sd_transfer_state;
-break;
-
case 51: /* ACMD51: SEND_SCR */
ret = sd->scr[sd->data_offset ++];
--
2.41.0
[PATCH 05/11] hw/sd/sdcard: Convert READ_SINGLE_BLOCK to generic_read_byte (CMD17)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9aaa78a334..6af7b3f034 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1383,11 +1383,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (!address_in_range(sd, "READ_SINGLE_BLOCK", addr, sd->blk_len))
{
return sd_r1;
}
-
-sd->state = sd_sendingdata_state;
-sd->data_start = addr;
-sd->data_offset = 0;
-return sd_r1;
+sd_blk_read(sd, addr, sd->blk_len);
+return sd_cmd_to_sendingdata(sd, req, addr, NULL, sd->blk_len);
default:
break;
@@ -2122,6 +2119,7 @@ uint8_t sd_read_byte(SDState *sd)
case 6: /* CMD6: SWITCH_FUNCTION */
case 9: /* CMD9: SEND_CSD */
case 10: /* CMD10: SEND_CID */
+case 17: /* CMD17: READ_SINGLE_BLOCK */
sd_generic_read_byte(sd, &ret);
break;
@@ -2132,16 +2130,6 @@ uint8_t sd_read_byte(SDState *sd)
sd->state = sd_transfer_state;
break;
-case 17: /* CMD17: READ_SINGLE_BLOCK */
-if (sd->data_offset == 0) {
-sd_blk_read(sd, sd->data_start, io_len);
-}
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= io_len)
-sd->state = sd_transfer_state;
-break;
-
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
if (sd->data_offset == 0) {
if (!address_in_range(sd, "READ_MULTIPLE_BLOCK",
--
2.41.0
Re: [PATCH 02/11] hw/sd/sdcard: Convert SWITCH_FUNCTION to generic_read_byte (CMD6)
On 25/6/24 08:10, Philippe Mathieu-Daudé wrote:
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 89006ba1b6..0011aa86da 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1220,10 +1220,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
}
sd_function_switch(sd, req.arg);
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0, NULL, 64);
case 7: /* CMD7: SELECT/DESELECT_CARD */
rca = sd_req_get_rca(sd, req);
Oops, missing:
-- >8 --
@@ -1068,3 +1068,2 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState
*sd, SDRequest req)
-__attribute__((unused))
static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
---
@@ -1922,7 +1919,6 @@ send_response:
return rsplen;
}
-__attribute__((unused))
/* Return true when buffer consumed */
static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
{
@@ -2112,10 +2108,7 @@ uint8_t sd_read_byte(SDState *sd)
sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 64)
-sd->state = sd_transfer_state;
+sd_generic_read_byte(sd, &ret);
break;
case 9: /* CMD9: SEND_CSD */
[PATCH 0/7] hw/sd/sd: Introduce sd_cmd_to_receivingdata() / sd_generic_write_byte()
Consolitate writing bytes on the DAT lines by introducing a pair of helpers to reuse in all commands receiving data. Based-on: <20240625061015.24095-1-phi...@linaro.org> Philippe Mathieu-Daudé (7): hw/sd/sdcard: Introduce sd_cmd_to_receivingdata / sd_generic_write_byte hw/sd/sdcard: Duplicate WRITE_SINGLE_BLOCK / WRITE_MULTIPLE_BLOCK cases hw/sd/sdcard: Convert WRITE_SINGLE_BLOCK to generic_write_byte (CMD24) hw/sd/sdcard: Convert PROGRAM_CID to generic_write_byte (CMD26) hw/sd/sdcard: Convert PROGRAM_CSD to generic_write_byte (CMD27) hw/sd/sdcard: Convert LOCK_UNLOCK to generic_write_byte (CMD42) hw/sd/sdcard: Convert GEN_CMD to generic_write_byte (CMD56) hw/sd/sd.c | 108 ++--- 1 file changed, 61 insertions(+), 47 deletions(-) -- 2.41.0
[PATCH 4/7] hw/sd/sdcard: Convert PROGRAM_CID to generic_write_byte (CMD26)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index fff4be3ae2..9db3b32b0b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1491,17 +1491,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 26: /* CMD26: PROGRAM_CID */
-switch (sd->state) {
-case sd_transfer_state:
-sd->state = sd_receivingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
-
-default:
-break;
-}
-break;
+return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
case 27: /* CMD27: PROGRAM_CSD */
switch (sd->state) {
@@ -2064,8 +2054,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 26: /* CMD26: PROGRAM_CID */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sizeof(sd->cid)) {
+if (sd_generic_write_byte(sd, value)) {
/* TODO: Check CRC before committing */
sd->state = sd_programming_state;
for (i = 0; i < sizeof(sd->cid); i ++)
--
2.41.0
[PATCH 1/7] hw/sd/sdcard: Introduce sd_cmd_to_receivingdata / sd_generic_write_byte
All commands switching from TRANSFER state to (receiving)DATA
do the same: receive stream of data from the DAT lines. Instead
of duplicating the same code many times, introduce 2 helpers:
- sd_cmd_to_receivingdata() on the I/O line setup the data to
be received on the data[] buffer,
- sd_generic_write_byte() on the DAT lines to push the data.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 28
1 file changed, 28 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2df3c28cb7..7fea0afb62 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1081,6 +1081,21 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
return sd_illegal;
}
+__attribute__((unused))
+static sd_rsp_type_t sd_cmd_to_receivingdata(SDState *sd, SDRequest req,
+ uint64_t start, size_t size)
+{
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
+sd->state = sd_receivingdata_state;
+sd->data_start = start;
+sd->data_offset = 0;
+/* sd->data[] used as receive buffer */
+sd->data_size = size ?: sizeof(sd->data);
+return sd_r1;
+}
+
static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
uint64_t start,
const void *data, size_t size)
@@ -1930,6 +1945,19 @@ send_response:
return rsplen;
}
+/* Return true when buffer consumed */
+__attribute__((unused))
+static bool sd_generic_write_byte(SDState *sd, uint8_t value)
+{
+sd->data[sd->data_offset] = value;
+
+if (++sd->data_offset >= sd->data_size) {
+sd->state = sd_transfer_state;
+return true;
+}
+return false;
+}
+
/* Return true when buffer consumed */
static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
{
--
2.41.0
[PATCH 6/7] hw/sd/sdcard: Convert LOCK_UNLOCK to generic_write_byte (CMD42)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b0f29034c0..82b44b65e0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1604,17 +1604,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
/* Lock card commands (Class 7) */
case 42: /* CMD42: LOCK_UNLOCK */
-switch (sd->state) {
-case sd_transfer_state:
-sd->state = sd_receivingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
-
-default:
-break;
-}
-break;
+return sd_cmd_to_receivingdata(sd, req, 0, 0);
/* Application specific commands (Class 8) */
case 55: /* CMD55: APP_CMD */
@@ -2085,8 +2075,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 42: /* CMD42: LOCK_UNLOCK */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
+if (sd_generic_write_byte(sd, value)) {
/* TODO: Check CRC before committing */
sd->state = sd_programming_state;
sd_lock_command(sd);
--
2.41.0
[PATCH 7/7] hw/sd/sdcard: Convert GEN_CMD to generic_write_byte (CMD56)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 82b44b65e0..fe2210d65a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1633,14 +1633,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
case 56: /* CMD56: GEN_CMD */
switch (sd->state) {
case sd_transfer_state:
-sd->data_offset = 0;
if (req.arg & 1) {
return sd_cmd_to_sendingdata(sd, req, 0,
sd->vendor_data,
sizeof(sd->vendor_data));
}
-sd->state = sd_receivingdata_state;
-return sd_r1;
+return sd_cmd_to_receivingdata(sd, req, 0,
sizeof(sd->vendor_data));
default:
break;
@@ -2085,9 +2083,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->vendor_data[sd->data_offset ++] = value;
-if (sd->data_offset >= sizeof(sd->vendor_data)) {
-sd->state = sd_transfer_state;
+if (sd_generic_write_byte(sd, value)) {
+memcpy(sd->vendor_data, sd->data, sizeof(sd->vendor_data));
}
break;
--
2.41.0
[PATCH 5/7] hw/sd/sdcard: Convert PROGRAM_CSD to generic_write_byte (CMD27)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9db3b32b0b..b0f29034c0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1494,17 +1494,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
case 27: /* CMD27: PROGRAM_CSD */
-switch (sd->state) {
-case sd_transfer_state:
-sd->state = sd_receivingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
-
-default:
-break;
-}
-break;
+return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->csd));
/* Write protection (Class 6) */
case 28: /* CMD28: SET_WRITE_PROT */
@@ -2072,8 +2062,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 27: /* CMD27: PROGRAM_CSD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sizeof(sd->csd)) {
+if (sd_generic_write_byte(sd, value)) {
/* TODO: Check CRC before committing */
sd->state = sd_programming_state;
for (i = 0; i < sizeof(sd->csd); i ++)
--
2.41.0
[PATCH 2/7] hw/sd/sdcard: Duplicate WRITE_SINGLE_BLOCK / WRITE_MULTIPLE_BLOCK cases
In order to modify the WRITE_SINGLE_BLOCK case in the
next commit, duplicate it first.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7fea0afb62..8c30826754 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1437,6 +1437,35 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
/* Block write commands (Class 4) */
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
+addr = sd_req_get_address(sd, req);
+switch (sd->state) {
+case sd_transfer_state:
+
+if (!address_in_range(sd, "WRITE_SINGLE_BLOCK", addr,
+ sd->blk_len)) {
+return sd_r1;
+}
+
+sd->state = sd_receivingdata_state;
+sd->data_start = addr;
+sd->data_offset = 0;
+
+if (sd->size <= SDSC_MAX_CAPACITY) {
+if (sd_wp_addr(sd, sd->data_start)) {
+sd->card_status |= WP_VIOLATION;
+}
+}
+if (sd->csd[14] & 0x30) {
+sd->card_status |= WP_VIOLATION;
+}
+sd->blk_written = 0;
+return sd_r1;
+
+default:
+break;
+}
+break;
+
case 25: /* CMD25: WRITE_MULTIPLE_BLOCK */
addr = sd_req_get_address(sd, req);
switch (sd->state) {
--
2.41.0
[PATCH 3/7] hw/sd/sdcard: Convert WRITE_SINGLE_BLOCK to generic_write_byte (CMD24)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8c30826754..fff4be3ae2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1081,7 +1081,6 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
return sd_illegal;
}
-__attribute__((unused))
static sd_rsp_type_t sd_cmd_to_receivingdata(SDState *sd, SDRequest req,
uint64_t start, size_t size)
{
@@ -1446,10 +1445,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
return sd_r1;
}
-sd->state = sd_receivingdata_state;
-sd->data_start = addr;
-sd->data_offset = 0;
-
if (sd->size <= SDSC_MAX_CAPACITY) {
if (sd_wp_addr(sd, sd->data_start)) {
sd->card_status |= WP_VIOLATION;
@@ -1459,7 +1454,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
sd->card_status |= WP_VIOLATION;
}
sd->blk_written = 0;
-return sd_r1;
+return sd_cmd_to_receivingdata(sd, req, addr, sd->blk_len);
default:
break;
@@ -1975,7 +1970,6 @@ send_response:
}
/* Return true when buffer consumed */
-__attribute__((unused))
static bool sd_generic_write_byte(SDState *sd, uint8_t value)
{
sd->data[sd->data_offset] = value;
@@ -2021,8 +2015,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
sd->current_cmd, sd->data_offset, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
+if (sd_generic_write_byte(sd, value)) {
/* TODO: Check CRC before committing */
sd->state = sd_programming_state;
sd_blk_write(sd, sd->data_start, sd->data_offset);
--
2.41.0
Re: [PATCH 04/14] spapr: Free stdout path
On 26/6/24 13:06, Akihiko Odaki wrote: This suppresses LeakSanitizer warnings. Signed-off-by: Akihiko Odaki --- hw/ppc/spapr_vof.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 05/14] ppc/vof: Fix unaligned FDT property access
On 26/6/24 13:06, Akihiko Odaki wrote:
FDT properties are aligned by 4 bytes, not 8 bytes.
Signed-off-by: Akihiko Odaki
---
hw/ppc/vof.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index e3b430a81f4f..b5b6514d79fc 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, GArray
*claimed, uint64_t base)
mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
if (sc == 2) {
-mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) *
ac));
+mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);
} else {
mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) *
ac));
OK but please keep API uses consistent, so convert other uses please.
}
Re: [PATCH 01/14] hw/core: Free CPUState allocations
On 26/6/24 13:06, Akihiko Odaki wrote:
This suppresses LeakSanitizer warnings.
Signed-off-by: Akihiko Odaki
---
hw/core/cpu-common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 0f0a247f5642..42f38b01a97f 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -274,6 +274,9 @@ static void cpu_common_finalize(Object *obj)
{
CPUState *cpu = CPU(obj);
+g_free(cpu->thread);
+g_free(cpu->halt_cond);
+g_free(cpu->cpu_ases);
g_array_free(cpu->gdb_regs, TRUE);
qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
qemu_mutex_destroy(&cpu->work_mutex);
Similar patch queued via trivial tree:
https://lore.kernel.org/qemu-devel/3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathb...@quicinc.com/
[PATCH 2/3] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a48010cfc1..d0a1d5db18 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2280,7 +2280,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
/* We do not model the chip select pin, so allow the board to select
* whether card should be in SSI or MMC/SD mode. It is also up to the
--
2.41.0
[PATCH 0/3] hw/sd/sdcard: Deprecate support for spec v1.10
Deprecate SD spec v1.10, use v3.01 as new default. Philippe Mathieu-Daudé (3): hw/sd/sdcard: Deprecate support for spec v1.10 hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Remove support for spec v1.10 docs/about/removed-features.rst | 5 + include/hw/sd/sd.h | 1 - hw/sd/sd.c | 14 +++--- 3 files changed, 8 insertions(+), 12 deletions(-) -- 2.41.0
[PATCH-for-10.0 3/3] hw/sd/sdcard: Remove support for spec v1.10
Support for spec v1.10 was deprecated in QEMU v9.1.
Signed-off-by: Philippe Mathieu-Daudé
---
docs/about/deprecated.rst | 6 --
docs/about/removed-features.rst | 5 +
include/hw/sd/sd.h | 1 -
hw/sd/sd.c | 12 ++--
4 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 02cdef14aa..ff3da68208 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -362,12 +362,6 @@ recommending to switch to their stable counterparts:
- "Zve64f" should be replaced with "zve64f"
- "Zve64d" should be replaced with "zve64d"
-``-device sd-card,spec_version=1`` (since 9.1)
-^^
-
-SD physical layer specification v2.00 supersedes the v1.10 one.
-v2.00 is the default since QEMU 3.0.0.
-
Block device options
''''''''''''''''''''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index fc7b28e637..dfe04b0555 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -1056,6 +1056,11 @@ by using ``-machine graphics=off``.
The 'pvrdma' device and the whole RDMA subsystem have been removed.
+``-device sd-card,spec_version=1`` (since 10.0)
+'''''''''''''''''''''''''''''''''''''''''''''''
+
+SD physical layer specification v2.00 supersedes the v1.10 one.
+
Related binaries
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 2c8748fb9b..362e149360 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -56,7 +56,6 @@
#define AKE_SEQ_ERROR (1 << 3)
enum SDPhySpecificationVersion {
-SD_PHY_SPECv1_10_VERS = 1,
SD_PHY_SPECv2_00_VERS = 2,
SD_PHY_SPECv3_01_VERS = 3,
};
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d0a1d5db18..37a6a989ee 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -168,7 +168,6 @@ static bool sd_is_spi(SDState *sd)
static const char *sd_version_str(enum SDPhySpecificationVersion version)
{
static const char *sdphy_version[] = {
-[SD_PHY_SPECv1_10_VERS] = "v1.10",
[SD_PHY_SPECv2_00_VERS] = "v2.00",
[SD_PHY_SPECv3_01_VERS] = "v3.01",
};
@@ -371,11 +370,7 @@ static void sd_set_ocr(SDState *sd)
static void sd_set_scr(SDState *sd)
{
sd->scr[0] = 0 << 4;/* SCR structure version 1.0 */
-if (sd->spec_version == SD_PHY_SPECv1_10_VERS) {
-sd->scr[0] |= 1;/* Spec Version 1.10 */
-} else {
-sd->scr[0] |= 2;/* Spec Version 2.00 or Version 3.0X */
-}
+sd->scr[0] |= 2;/* Spec Version 2.00 or Version 3.0X */
sd->scr[1] = (2 << 4) /* SDSC Card (Security Version 1.01) */
| 0b0101; /* 1-bit or 4-bit width bus modes */
sd->scr[2] = 0x00; /* Extended Security is not supported. */
@@ -1241,9 +1236,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 8: /* CMD8: SEND_IF_COND */
-if (sd->spec_version < SD_PHY_SPECv2_00_VERS) {
-break;
-}
if (sd->state != sd_idle_state) {
break;
}
@@ -2231,7 +2223,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
int ret;
switch (sd->spec_version) {
-case SD_PHY_SPECv1_10_VERS
+case SD_PHY_SPECv2_00_VERS
... SD_PHY_SPECv3_01_VERS:
break;
default:
--
2.41.0
[PATCH 1/3] hw/sd/sdcard: Deprecate support for spec v1.10
We use the v2.00 spec by default since commit 2f0939c234
("sdcard: Add a 'spec_version' property, default to Spec v2.00").
Time to deprecate the v1.10 which doesn't bring much, and
is not tested.
Signed-off-by: Philippe Mathieu-Daudé
---
docs/about/deprecated.rst | 6 ++
1 file changed, 6 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ff3da68208..02cdef14aa 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -362,6 +362,12 @@ recommending to switch to their stable counterparts:
- "Zve64f" should be replaced with "zve64f"
- "Zve64d" should be replaced with "zve64d"
+``-device sd-card,spec_version=1`` (since 9.1)
+^^
+
+SD physical layer specification v2.00 supersedes the v1.10 one.
+v2.00 is the default since QEMU 3.0.0.
+
Block device options
''''''''''''''''''''
--
2.41.0
Re: [PATCH 05/14] ppc/vof: Fix unaligned FDT property access
On 27/6/24 15:12, Akihiko Odaki wrote:
On 2024/06/26 21:03, Philippe Mathieu-Daudé wrote:
On 26/6/24 13:06, Akihiko Odaki wrote:
FDT properties are aligned by 4 bytes, not 8 bytes.
Signed-off-by: Akihiko Odaki
---
hw/ppc/vof.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index e3b430a81f4f..b5b6514d79fc 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt,
GArray *claimed, uint64_t base)
mem0_reg = fdt_getprop(fdt, offset, "reg", &proplen);
g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
if (sc == 2) {
- mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg +
sizeof(uint32_t) * ac));
+ mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);
} else {
mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg +
sizeof(uint32_t) * ac));
OK but please keep API uses consistent, so convert other uses please.
This is the only unaligned access.
What I mean with consistent API use is either use the be64_to_cpu and
be32_to_cpu API, or ldq_be_p and ldl_be_p. A mix of both makes review
more confusing.
[PATCH v3 00/17] hw/sd/sdcard: Accumulation of cleanups and fixes
Since v2: - Tested-by from Cédric recorded - more patches added :S Since v1: - various patches merged, few more added Various SD card cleanups and fixes accumulated over the years. Various have been useful to help integrating eMMC support (which will come later). Full series for testing: https://gitlab.com/philmd/qemu/-/tags/emmc-v4 Cédric Le Goater (1): hw/sd/sdcard: Introduce definitions for EXT_CSD register Philippe Mathieu-Daudé (16): hw/sd/sdcard: Deprecate support for spec v1.10 hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Track last command used to help logging hw/sd/sdcard: Trace block offset in READ/WRITE data accesses hw/sd/sdcard: Trace requested address computed by sd_req_get_address() hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30) hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22) hw/sd/sdcard: Use READY_FOR_DATA definition instead of magic value hw/sd/sdcard: Assign SDCardStates enum values hw/sd/sdcard: Simplify sd_inactive_state handling hw/sd/sdcard: Restrict SWITCH_FUNCTION to sd_transfer_state (CMD6) hw/sd/sdcard: Add direct reference to SDProto in SDState hw/sd/sdcard: Extract sd_blk_len() helper tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA hw/sd/sdcard: Generate random RCA value docs/about/deprecated.rst| 6 ++ hw/sd/sdmmc-internal.h | 97 + hw/sd/sd.c | 145 ++- tests/qtest/npcm7xx_sdhci-test.c | 7 ++ hw/sd/trace-events | 6 +- 5 files changed, 199 insertions(+), 62 deletions(-) -- 2.41.0
[PATCH v3 02/17] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
hw/sd/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a48010cfc1..d0a1d5db18 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2280,7 +2280,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
/* We do not model the chip select pin, so allow the board to select
* whether card should be in SSI or MMC/SD mode. It is also up to the
--
2.41.0
[PATCH v3 01/17] hw/sd/sdcard: Deprecate support for spec v1.10
We use the v2.00 spec by default since commit 2f0939c234
("sdcard: Add a 'spec_version' property, default to Spec v2.00").
Time to deprecate the v1.10 which doesn't bring much, and
is not tested.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
docs/about/deprecated.rst | 6 ++
1 file changed, 6 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ff3da68208..02cdef14aa 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -362,6 +362,12 @@ recommending to switch to their stable counterparts:
- "Zve64f" should be replaced with "zve64f"
- "Zve64d" should be replaced with "zve64d"
+``-device sd-card,spec_version=1`` (since 9.1)
+^^
+
+SD physical layer specification v2.00 supersedes the v1.10 one.
+v2.00 is the default since QEMU 3.0.0.
+
Block device options
''''''''''''''''''''
--
2.41.0
[PATCH v3 04/17] hw/sd/sdcard: Trace block offset in READ/WRITE data accesses
Useful to detect out of bound accesses.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 4 ++--
hw/sd/trace-events | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bc87807793..090a6fdcdb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1917,7 +1917,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
trace_sdcard_write_data(sd_proto(sd)->name,
sd->last_cmd_name,
-sd->current_cmd, value);
+sd->current_cmd, sd->data_offset, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
sd->data[sd->data_offset ++] = value;
@@ -2073,7 +2073,7 @@ uint8_t sd_read_byte(SDState *sd)
trace_sdcard_read_data(sd_proto(sd)->name,
sd->last_cmd_name,
- sd->current_cmd, io_len);
+ sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
ret = sd->data[sd->data_offset ++];
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 724365efc3..0eee98a646 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -52,8 +52,8 @@ sdcard_lock(void) ""
sdcard_unlock(void) ""
sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
-sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint8_t value) "%s %20s/ CMD%02d value 0x%02x"
-sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t length) "%s %20s/ CMD%02d len %" PRIu32
+sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
+sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint32_t length) "%s %20s/ CMD%02d ofs %"PRIu32" len %" PRIu32
sdcard_set_voltage(uint16_t millivolts) "%u mV"
# pxa2xx_mmci.c
--
2.41.0
[PATCH v3 03/17] hw/sd/sdcard: Track last command used to help logging
The command is selected on the I/O lines, and further
processing might be done on the DAT lines via the
sd_read_byte() and sd_write_byte() handlers. Since
these methods can't distinct between normal and APP
commands, keep the name of the current command in
the SDState and use it in the DAT handlers. This
fixes a bug that all normal commands were displayed
as APP commands.
Fixes: 2ed61fb57b ("sdcard: Display command name when tracing CMD/ACMD")
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d0a1d5db18..bc87807793 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -133,6 +133,7 @@ struct SDState {
uint32_t pwd_len;
uint8_t function_group[6];
uint8_t current_cmd;
+const char *last_cmd_name;
/* True if we will handle the next command as an ACMD. Note that this does
* *not* track the APP_CMD status bit!
*/
@@ -1154,12 +1155,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
uint16_t rca;
uint64_t addr;
+sd->last_cmd_name = sd_cmd_name(req.cmd);
/* CMD55 precedes an ACMD, so we are not interested in tracing it.
* However there is no ACMD55, so we want to trace this particular case.
*/
if (req.cmd != 55 || sd->expecting_acmd) {
trace_sdcard_normal_command(sd_proto(sd)->name,
-sd_cmd_name(req.cmd), req.cmd,
+sd->last_cmd_name, req.cmd,
req.arg, sd_state_name(sd->state));
}
@@ -1620,7 +1622,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_app_command(SDState *sd,
SDRequest req)
{
-trace_sdcard_app_command(sd_proto(sd)->name, sd_acmd_name(req.cmd),
+sd->last_cmd_name = sd_acmd_name(req.cmd);
+trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
req.cmd, req.arg, sd_state_name(sd->state));
sd->card_status |= APP_CMD;
@@ -1913,7 +1916,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
return;
trace_sdcard_write_data(sd_proto(sd)->name,
-sd_acmd_name(sd->current_cmd),
+sd->last_cmd_name,
sd->current_cmd, value);
switch (sd->current_cmd) {
case 24: /* CMD24: WRITE_SINGLE_BLOCK */
@@ -2069,7 +2072,7 @@ uint8_t sd_read_byte(SDState *sd)
io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
trace_sdcard_read_data(sd_proto(sd)->name,
- sd_acmd_name(sd->current_cmd),
+ sd->last_cmd_name,
sd->current_cmd, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
@@ -2214,6 +2217,7 @@ static void sd_instance_init(Object *obj)
{
SDState *sd = SD_CARD(obj);
+sd->last_cmd_name = "UNSET";
sd->enable = true;
sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
}
--
2.41.0
[PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
RFC: Is it safe to reuse VMSTATE_UNUSED_V() (which happens
to be the same size)?
Cc: Peter Xu
Cc: Fabiano Rosas
---
hw/sd/sd.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 464576751a..1f3eea6e84 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -142,6 +142,8 @@ struct SDState {
uint64_t data_start;
uint32_t data_offset;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -656,6 +658,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -771,7 +774,7 @@ static const VMStateDescription sd_vmstate = {
VMSTATE_UINT64(data_start, SDState),
VMSTATE_UINT32(data_offset, SDState),
VMSTATE_UINT8_ARRAY(data, SDState, 512),
-VMSTATE_UNUSED_V(1, 512),
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
VMSTATE_BOOL(enable, SDState),
VMSTATE_END_OF_LIST()
},
@@ -2029,9 +2032,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset ++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2165,12 +2167,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset ++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH v3 05/17] hw/sd/sdcard: Trace requested address computed by sd_req_get_address()
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 9 +++--
hw/sd/trace-events | 1 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 090a6fdcdb..464576751a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -608,10 +608,15 @@ static void sd_response_r7_make(SDState *sd, uint8_t
*response)
static uint64_t sd_req_get_address(SDState *sd, SDRequest req)
{
+uint64_t addr;
+
if (FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
-return (uint64_t) req.arg << HWBLOCK_SHIFT;
+addr = (uint64_t) req.arg << HWBLOCK_SHIFT;
+} else {
+addr = req.arg;
}
-return req.arg;
+trace_sdcard_req_addr(req.arg, addr);
+return addr;
}
static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 0eee98a646..43eaeba149 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -50,6 +50,7 @@ sdcard_ejected(void) ""
sdcard_erase(uint32_t first, uint32_t last) "addr first 0x%" PRIx32" last 0x%"
PRIx32
sdcard_lock(void) ""
sdcard_unlock(void) ""
+sdcard_req_addr(uint32_t req_arg, uint64_t addr) "req 0x%" PRIx32 " addr 0x%"
PRIx64
sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd,
uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
--
2.41.0
[PATCH v3 08/17] hw/sd/sdcard: Send NUM_WR_BLOCKS bits MSB first (ACMD22)
Per sections 3.6.1 (SD Bus Protocol), 4.3.4 "Data Write"
and 7.3.2 (Responses):
In the CMD line the Most Significant Bit is transmitted first.
Use the stl_be_p() helper to store the value in big-endian.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
RFC because I'm surprised this has been unnoticed for 17 years
(commit a1bb27b1e9 "initial SD card emulation", April 2007).
Cc: Peter Maydell
---
hw/sd/sd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4e09640852..1f37d9c93a 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1668,8 +1668,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
switch (sd->state) {
case sd_transfer_state:
-*(uint32_t *) sd->data = sd->blk_written;
-
+stl_be_p(sd->data, sd->blk_written);
sd->state = sd_sendingdata_state;
sd->data_start = 0;
sd->data_offset = 0;
--
2.41.0
[PATCH v3 09/17] hw/sd/sdcard: Use READY_FOR_DATA definition instead of magic value
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1f37d9c93a..135b7d2e23 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -561,7 +561,7 @@ FIELD(CSR, OUT_OF_RANGE, 31, 1)
static void sd_set_cardstatus(SDState *sd)
{
-sd->card_status = 0x0100;
+sd->card_status = READY_FOR_DATA;
}
static void sd_set_sdstatus(SDState *sd)
--
2.41.0
[PATCH v3 07/17] hw/sd/sdcard: Send WRITE_PROT bits MSB first (CMD30)
Per sections 3.6.1 (SD Bus Protocol) and 7.3.2 (Responses): In the CMD line the Most Significant Bit is transmitted first. Use the stl_be_p() helper to store the value in big-endian. Signed-off-by: Philippe Mathieu-Daudé Tested-by: Cédric Le Goater --- RFC because I'm surprised this has been unnoticed for 17 years (commit a1bb27b1e9 "initial SD card emulation", April 2007). Cc: Peter Maydell --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 1f3eea6e84..4e09640852 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -1507,7 +1507,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req) } sd->state = sd_sendingdata_state; -*(uint32_t *) sd->data = sd_wpbits(sd, req.arg); +stl_be_p(sd->data, sd_wpbits(sd, req.arg)); sd->data_start = addr; sd->data_offset = 0; return sd_r1; -- 2.41.0
[PATCH v3 10/17] hw/sd/sdcard: Assign SDCardStates enum values
SDCardStates enum values are specified, so assign them
correspondingly. It will be useful later when we add
states from later specs, which might not be continuous.
See CURRENT_STATE bits in section 4.10.1 "Card Status".
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 135b7d2e23..fbdfafa3a6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -75,16 +75,16 @@ enum SDCardModes {
};
enum SDCardStates {
-sd_inactive_state = -1,
-sd_idle_state = 0,
-sd_ready_state,
-sd_identification_state,
-sd_standby_state,
-sd_transfer_state,
-sd_sendingdata_state,
-sd_receivingdata_state,
-sd_programming_state,
-sd_disconnect_state,
+sd_inactive_state = -1,
+sd_idle_state = 0,
+sd_ready_state = 1,
+sd_identification_state = 2,
+sd_standby_state= 3,
+sd_transfer_state = 4,
+sd_sendingdata_state= 5,
+sd_receivingdata_state = 6,
+sd_programming_state= 7,
+sd_disconnect_state = 8,
};
typedef sd_rsp_type_t (*sd_cmd_handler)(SDState *sd, SDRequest req);
--
2.41.0
[PATCH v3 11/17] hw/sd/sdcard: Simplify sd_inactive_state handling
Card entering sd_inactive_state powers off, and won't respond
anymore. Handle that once when entering sd_do_command().
Remove condition always true in sd_cmd_GO_IDLE_STATE().
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index fbdfafa3a6..7533a78cf6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1081,10 +1081,8 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
/* CMD0 */
static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
{
-if (sd->state != sd_inactive_state) {
-sd->state = sd_idle_state;
-sd_reset(DEVICE(sd));
-}
+sd->state = sd_idle_state;
+sd_reset(DEVICE(sd));
return sd_is_spi(sd) ? sd_r1 : sd_r0;
}
@@ -1579,7 +1577,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
switch (sd->state) {
case sd_ready_state:
case sd_identification_state:
-case sd_inactive_state:
return sd_illegal;
case sd_idle_state:
if (rca) {
@@ -1800,6 +1797,11 @@ int sd_do_command(SDState *sd, SDRequest *req,
return 0;
}
+if (sd->state == sd_inactive_state) {
+rtype = sd_illegal;
+goto send_response;
+}
+
if (sd_req_crc_validate(req)) {
sd->card_status |= COM_CRC_ERROR;
rtype = sd_illegal;
--
2.41.0
[PATCH v3 12/17] hw/sd/sdcard: Restrict SWITCH_FUNCTION to sd_transfer_state (CMD6)
SWITCH_FUNCTION is only allowed in TRANSFER state
(See 4.8 "Card State Transition Table).
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7533a78cf6..8f441e418c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1205,6 +1205,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (sd->mode != sd_data_transfer_mode) {
return sd_invalid_mode_for_cmd(sd, req);
}
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
+
sd_function_switch(sd, req.arg);
sd->state = sd_sendingdata_state;
sd->data_start = 0;
--
2.41.0
[PATCH v3 13/17] hw/sd/sdcard: Add direct reference to SDProto in SDState
Keep direct reference to SDProto in SDState,
remove then unnecessary sd_proto().
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8f441e418c..aaa50ab2c5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -116,6 +116,8 @@ struct SDState {
uint8_t spec_version;
BlockBackend *blk;
+const SDProto *proto;
+
/* Runtime changeables */
uint32_t mode;/* current card mode, one of SDCardModes */
@@ -154,18 +156,11 @@ struct SDState {
static void sd_realize(DeviceState *dev, Error **errp);
-static const struct SDProto *sd_proto(SDState *sd)
-{
-SDCardClass *sc = SD_CARD_GET_CLASS(sd);
-
-return sc->proto;
-}
-
static const SDProto sd_proto_spi;
static bool sd_is_spi(SDState *sd)
{
-return sd_proto(sd) == &sd_proto_spi;
+return sd->proto == &sd_proto_spi;
}
static const char *sd_version_str(enum SDPhySpecificationVersion version)
@@ -1044,7 +1039,7 @@ static bool address_in_range(SDState *sd, const char
*desc,
static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong state: %s (spec
%s)\n",
- sd_proto(sd)->name, req.cmd, sd_state_name(sd->state),
+ sd->proto->name, req.cmd, sd_state_name(sd->state),
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1053,7 +1048,7 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState
*sd, SDRequest req)
static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
- sd_proto(sd)->name, req.cmd, sd_mode_name(sd->mode),
+ sd->proto->name, req.cmd, sd_mode_name(sd->mode),
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1062,7 +1057,7 @@ static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd,
SDRequest req)
static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i for spec %s\n",
- sd_proto(sd)->name, req.cmd,
+ sd->proto->name, req.cmd,
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1073,7 +1068,7 @@ __attribute__((unused))
static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n",
- sd_proto(sd)->name, req.cmd);
+ sd->proto->name, req.cmd);
return sd_illegal;
}
@@ -1166,7 +1161,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
* However there is no ACMD55, so we want to trace this particular case.
*/
if (req.cmd != 55 || sd->expecting_acmd) {
-trace_sdcard_normal_command(sd_proto(sd)->name,
+trace_sdcard_normal_command(sd->proto->name,
sd->last_cmd_name, req.cmd,
req.arg, sd_state_name(sd->state));
}
@@ -1185,8 +1180,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
return sd_illegal;
}
-if (sd_proto(sd)->cmd[req.cmd]) {
-return sd_proto(sd)->cmd[req.cmd](sd, req);
+if (sd->proto->cmd[req.cmd]) {
+return sd->proto->cmd[req.cmd](sd, req);
}
switch (req.cmd) {
@@ -1632,12 +1627,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
SDRequest req)
{
sd->last_cmd_name = sd_acmd_name(req.cmd);
-trace_sdcard_app_command(sd_proto(sd)->name, sd->last_cmd_name,
+trace_sdcard_app_command(sd->proto->name, sd->last_cmd_name,
req.cmd, req.arg, sd_state_name(sd->state));
sd->card_status |= APP_CMD;
-if (sd_proto(sd)->acmd[req.cmd]) {
-return sd_proto(sd)->acmd[req.cmd](sd, req);
+if (sd->proto->acmd[req.cmd]) {
+return sd->proto->acmd[req.cmd](sd, req);
}
switch (req.cmd) {
@@ -1928,7 +1923,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
return;
-trace_sdcard_write_data(sd_proto(sd)->name,
+trace_sdcard_write_data(sd->proto->name,
sd->last_cmd_name,
sd->current_cmd, sd->data_offset, value);
switch (sd->current_cmd) {
@@ -2083,7 +2078,7 @@ uint8_t sd_read_byte(SDState *sd)
io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
-trace_sdcard_read_data(sd_proto(sd)->name,
+trace_sdcard_read_data(sd
[PATCH v3 14/17] hw/sd/sdcard: Extract sd_blk_len() helper
From: Philippe Mathieu-Daudé
Extract sd_blk_len() helper, use definitions instead
of magic values.
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index aaa50ab2c5..5997e13107 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -603,6 +603,14 @@ static void sd_response_r7_make(SDState *sd, uint8_t
*response)
stl_be_p(response, sd->vhs);
}
+static uint32_t sd_blk_len(SDState *sd)
+{
+if (FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
+return 1 << HWBLOCK_SHIFT;
+}
+return sd->blk_len;
+}
+
static uint64_t sd_req_get_address(SDState *sd, SDRequest req)
{
uint64_t addr;
@@ -2076,7 +2084,7 @@ uint8_t sd_read_byte(SDState *sd)
if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
return 0x00;
-io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
+io_len = sd_blk_len(sd);
trace_sdcard_read_data(sd->proto->name,
sd->last_cmd_name,
--
2.41.0
[PATCH v3 16/17] hw/sd/sdcard: Generate random RCA value
Rather than using the obscure 0x4567 magic value,
use a real random one.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
hw/sd/sd.c | 11 ---
hw/sd/trace-events | 1 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5997e13107..d85b2906f4 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"
@@ -488,9 +489,10 @@ static void sd_set_csd(SDState *sd, uint64_t size)
/* Relative Card Address register */
-static void sd_set_rca(SDState *sd)
+static void sd_set_rca(SDState *sd, uint16_t value)
{
-sd->rca += 0x4567;
+trace_sdcard_set_rca(value);
+sd->rca = value;
}
static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
@@ -1113,11 +1115,14 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd,
SDRequest req)
/* CMD3 */
static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
{
+uint16_t random_rca;
+
switch (sd->state) {
case sd_identification_state:
case sd_standby_state:
sd->state = sd_standby_state;
-sd_set_rca(sd);
+qemu_guest_getrandom_nofail(&random_rca, sizeof(random_rca));
+sd_set_rca(sd, random_rca);
return sd_r6;
default:
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 43eaeba149..6a51b0e906 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -43,6 +43,7 @@ sdcard_response(const char *rspdesc, int rsplen) "%s (sz:%d)"
sdcard_powerup(void) ""
sdcard_inquiry_cmd41(void) ""
sdcard_reset(void) ""
+sdcard_set_rca(uint16_t value) "new RCA: 0x%04x"
sdcard_set_blocklen(uint16_t length) "block len 0x%03x"
sdcard_set_block_count(uint32_t cnt) "block cnt 0x%"PRIx32
sdcard_inserted(bool readonly) "read_only: %u"
--
2.41.0
[PATCH v3 15/17] tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA
Disable tests using 0x4567 hardcoded RCA otherwise when
using random RCA we get:
ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: assertion
failed: (ret == len)
not ok /arm/npcm7xx_sdhci/read_sd -
ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: assertion failed:
(ret == len)
Bail out!
See
https://lore.kernel.org/qemu-devel/37f83be9-deb5-42a1-b704-14984351d...@linaro.org/
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Hao Wu
Cc: Shengtan Mao
Cc: Tyrone Ting
---
tests/qtest/npcm7xx_sdhci-test.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
index 5d68540e52..6a42b142ad 100644
--- a/tests/qtest/npcm7xx_sdhci-test.c
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -44,6 +44,7 @@ static QTestState *setup_sd_card(void)
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4120, 0, (41 << 8));
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
+g_test_skip("hardcoded 0x4567 card address");
sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4567, 0,
SDHC_SELECT_DESELECT_CARD);
@@ -76,6 +77,9 @@ static void test_read_sd(void)
{
QTestState *qts = setup_sd_card();
+g_test_skip("hardcoded 0x4567 card address used in setup_sd_card()");
+return;
+
write_sdread(qts, "hello world");
write_sdread(qts, "goodbye");
@@ -108,6 +112,9 @@ static void test_write_sd(void)
{
QTestState *qts = setup_sd_card();
+g_test_skip("hardcoded 0x4567 card address used in setup_sd_card()");
+return;
+
sdwrite_read(qts, "hello world");
sdwrite_read(qts, "goodbye");
--
2.41.0
[PATCH v3 17/17] hw/sd/sdcard: Introduce definitions for EXT_CSD register
From: Cédric Le Goater Signed-off-by: Cédric Le Goater Signed-off-by: Cédric Le Goater Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sdmmc-internal.h | 97 ++ 1 file changed, 97 insertions(+) diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h index d8bf17d204..306ffa7f53 100644 --- a/hw/sd/sdmmc-internal.h +++ b/hw/sd/sdmmc-internal.h @@ -11,6 +11,103 @@ #ifndef SDMMC_INTERNAL_H #define SDMMC_INTERNAL_H +/* + * EXT_CSD fields + */ + +#define EXT_CSD_CMDQ_MODE_EN15 /* R/W */ +#define EXT_CSD_FLUSH_CACHE 32 /* W */ +#define EXT_CSD_CACHE_CTRL 33 /* R/W */ +#define EXT_CSD_POWER_OFF_NOTIFICATION 34 /* R/W */ +#define EXT_CSD_PACKED_FAILURE_INDEX35 /* RO */ +#define EXT_CSD_PACKED_CMD_STATUS 36 /* RO */ +#define EXT_CSD_EXP_EVENTS_STATUS 54 /* RO, 2 bytes */ +#define EXT_CSD_EXP_EVENTS_CTRL 56 /* R/W, 2 bytes */ +#define EXT_CSD_DATA_SECTOR_SIZE61 /* R */ +#define EXT_CSD_GP_SIZE_MULT143 /* R/W */ +#define EXT_CSD_PARTITION_SETTING_COMPLETED 155 /* R/W */ +#define EXT_CSD_PARTITION_ATTRIBUTE 156 /* R/W */ +#define EXT_CSD_PARTITION_SUPPORT 160 /* RO */ +#define EXT_CSD_HPI_MGMT161 /* R/W */ +#define EXT_CSD_RST_N_FUNCTION 162 /* R/W */ +#define EXT_CSD_BKOPS_EN163 /* R/W */ +#define EXT_CSD_BKOPS_START 164 /* W */ +#define EXT_CSD_SANITIZE_START 165 /* W */ +#define EXT_CSD_WR_REL_PARAM166 /* RO */ +#define EXT_CSD_RPMB_MULT 168 /* RO */ +#define EXT_CSD_FW_CONFIG 169 /* R/W */ +#define EXT_CSD_BOOT_WP 173 /* R/W */ +#define EXT_CSD_ERASE_GROUP_DEF 175 /* R/W */ +#define EXT_CSD_PART_CONFIG 179 /* R/W */ +#define EXT_CSD_ERASED_MEM_CONT 181 /* RO */ +#define EXT_CSD_BUS_WIDTH 183 /* R/W */ +#define EXT_CSD_STROBE_SUPPORT 184 /* RO */ +#define EXT_CSD_HS_TIMING 185 /* R/W */ +#define EXT_CSD_POWER_CLASS 187 /* R/W */ +#define EXT_CSD_REV 192 /* RO */ +#define EXT_CSD_STRUCTURE 194 /* RO */ +#define EXT_CSD_CARD_TYPE 196 /* RO */ +#define EXT_CSD_DRIVER_STRENGTH 197 /* RO */ +#define EXT_CSD_OUT_OF_INTERRUPT_TIME 198 /* RO */ +#define EXT_CSD_PART_SWITCH_TIME199 /* RO */ +#define EXT_CSD_PWR_CL_52_195 200 /* RO */ +#define EXT_CSD_PWR_CL_26_195 201 /* RO */ +#define EXT_CSD_PWR_CL_52_360 202 /* RO */ +#define EXT_CSD_PWR_CL_26_360 203 /* RO */ +#define EXT_CSD_SEC_CNT 212 /* RO, 4 bytes */ +#define EXT_CSD_S_A_TIMEOUT 217 /* RO */ +#define EXT_CSD_S_C_VCCQ219 /* RO */ +#define EXT_CSD_S_C_VCC 220 /* RO */ +#define EXT_CSD_REL_WR_SEC_C222 /* RO */ +#define EXT_CSD_HC_WP_GRP_SIZE 221 /* RO */ +#define EXT_CSD_ERASE_TIMEOUT_MULT 223 /* RO */ +#define EXT_CSD_HC_ERASE_GRP_SIZE 224 /* RO */ +#define EXT_CSD_ACC_SIZE225 /* RO */ +#define EXT_CSD_BOOT_MULT 226 /* RO */ +#define EXT_CSD_BOOT_INFO 228 /* RO */ +#define EXT_CSD_SEC_TRIM_MULT 229 /* RO */ +#define EXT_CSD_SEC_ERASE_MULT 230 /* RO */ +#define EXT_CSD_SEC_FEATURE_SUPPORT 231 /* RO */ +#define EXT_CSD_TRIM_MULT 232 /* RO */ +#define EXT_CSD_PWR_CL_200_195 236 /* RO */ +#define EXT_CSD_PWR_CL_200_360 237 /* RO */ +#define EXT_CSD_PWR_CL_DDR_52_195 238 /* RO */ +#define EXT_CSD_PWR_CL_DDR_52_360 239 /* RO */ +#define EXT_CSD_BKOPS_STATUS246 /* RO */ +#define EXT_CSD_POWER_OFF_LONG_TIME 247 /* RO */ +#define EXT_CSD_GENERIC_CMD6_TIME 248 /* RO */ +#define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */ +#define EXT_CSD_PWR_CL_DDR_200_360 253 /* RO */ +#define EXT_CSD_FIRMWARE_VERSION254 /* RO, 8 bytes */ +#define EXT_CSD_PRE_EOL_INFO267 /* RO */ +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A 268 /* RO */ +#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B 269 /* RO */ +#define EXT_CSD_CMDQ_DEPTH 307 /* RO */ +#define EXT_CSD_CMDQ_SUPPORT308 /* RO */ +#define EXT_CSD_SUPPORTED_MODE 493 /* RO */ +#define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */ +#define EXT_CSD_DATA_TAG_SUPPORT499 /* RO */ +#define EXT_CSD_MAX_PACKED_WRITES 500 /* RO */ +#define EXT_CSD_MAX_PACKED_READS501 /* RO */ +#define EXT_CSD_BKOPS_SUPPORT 502 /* RO */ +#define EXT_CSD_HPI_FEATURES503 /* RO */ +#define
[PATCH v2 00/11] hw/sd/sd: Introduce sd_cmd_to_sendingdata() and sd_generic_read_byte()
Consolitate reading bytes on the DAT lines by introducing a pair of helpers to reuse in all commands sending data. Full series for testing: https://gitlab.com/philmd/qemu/-/tags/emmc-v4 Based-on: <20240627162232.80428-1-phi...@linaro.org> Philippe Mathieu-Daudé (11): hw/sd/sdcard: Introduce sd_cmd_to_sendingdata and sd_generic_read_byte hw/sd/sdcard: Convert SWITCH_FUNCTION to generic_read_byte (CMD6) hw/sd/sdcard: Convert SEND_CSD/SEND_CID to generic_read_byte (CMD9 & 10) hw/sd/sdcard: Duplicate READ_SINGLE_BLOCK / READ_MULTIPLE_BLOCK cases hw/sd/sdcard: Convert READ_SINGLE_BLOCK to generic_read_byte (CMD17) hw/sd/sdcard: Convert SEND_TUNING_BLOCK to generic_read_byte (CMD19) hw/sd/sdcard: Convert SEND_WRITE_PROT to generic_read_byte (CMD30) hw/sd/sdcard: Convert GEN_CMD to generic_read_byte (CMD56) hw/sd/sdcard: Convert SD_STATUS to generic_read_byte (ACMD13) hw/sd/sdcard: Convert SEND_NUM_WR_BLOCKS to generic_read_byte (ACMD22) hw/sd/sdcard: Convert SEND_SCR to generic_read_byte (ACMD51) hw/sd/sd.c | 223 - 1 file changed, 100 insertions(+), 123 deletions(-) -- 2.41.0
[PATCH v2 03/11] hw/sd/sdcard: Convert SEND_CSD/SEND_CID to generic_read_byte (CMD9 & 10)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f7735c39a8..8201f3245c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1312,11 +1312,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (!sd_is_spi(sd)) {
break;
}
-sd->state = sd_sendingdata_state;
-memcpy(sd->data, sd->csd, 16);
-sd->data_start = sd_req_get_address(sd, req);
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
+ sd->csd, 16);
default:
break;
@@ -1336,11 +1333,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (!sd_is_spi(sd)) {
break;
}
-sd->state = sd_sendingdata_state;
-memcpy(sd->data, sd->cid, 16);
-sd->data_start = sd_req_get_address(sd, req);
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
+ sd->cid, 16);
default:
break;
@@ -2130,15 +2124,9 @@ uint8_t sd_read_byte(SDState *sd)
sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
-sd_generic_read_byte(sd, &ret);
-break;
-
case 9: /* CMD9: SEND_CSD */
-case 10: /* CMD10: SEND_CID */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 16)
-sd->state = sd_transfer_state;
+case 10: /* CMD10: SEND_CID */
+sd_generic_read_byte(sd, &ret);
break;
case 13: /* ACMD13: SD_STATUS */
--
2.41.0
[PATCH v2 01/11] hw/sd/sdcard: Introduce sd_cmd_to_sendingdata and sd_generic_read_byte
All commands switching from TRANSFER state to (sending)DATA
do the same: send stream of data on the DAT lines. Instead
of duplicating the same code many times, introduce 2 helpers:
- sd_cmd_to_sendingdata() on the I/O line setup the data to
be transferred,
- sd_generic_read_byte() on the DAT lines to fetch the data.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d85b2906f4..1a8d06804d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -142,8 +142,10 @@ struct SDState {
*/
bool expecting_acmd;
uint32_t blk_written;
+
uint64_t data_start;
uint32_t data_offset;
+size_t data_size;
uint8_t data[512];
uint8_t vendor_data[512];
@@ -1083,6 +1085,29 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
return sd_illegal;
}
+/* Configure fields for following sd_generic_read_byte() calls */
+__attribute__((unused))
+static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
+ uint64_t start,
+ const void *data, size_t size)
+{
+if (sd->state != sd_transfer_state) {
+sd_invalid_state_for_cmd(sd, req);
+}
+
+sd->state = sd_sendingdata_state;
+sd->data_start = start;
+sd->data_offset = 0;
+if (data) {
+assert(size);
+memcpy(sd->data, data, size);
+}
+if (size) {
+sd->data_size = size;
+}
+return sd_r1;
+}
+
/* CMD0 */
static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
{
@@ -1920,6 +1945,20 @@ send_response:
return rsplen;
}
+/* Return true when buffer is consumed. Configured by sd_cmd_to_sendingdata()
*/
+__attribute__((unused))
+static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
+{
+*value = sd->data[sd->data_offset];
+
+if (++sd->data_offset >= sd->data_size) {
+sd->state = sd_transfer_state;
+return true;
+}
+
+return false;
+}
+
void sd_write_byte(SDState *sd, uint8_t value)
{
int i;
--
2.41.0
[PATCH v2 02/11] hw/sd/sdcard: Convert SWITCH_FUNCTION to generic_read_byte (CMD6)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1a8d06804d..f7735c39a8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1086,7 +1086,6 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd,
SDRequest req)
}
/* Configure fields for following sd_generic_read_byte() calls */
-__attribute__((unused))
static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
uint64_t start,
const void *data, size_t size)
@@ -1243,10 +1242,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
}
sd_function_switch(sd, req.arg);
-sd->state = sd_sendingdata_state;
-sd->data_start = 0;
-sd->data_offset = 0;
-return sd_r1;
+return sd_cmd_to_sendingdata(sd, req, 0, NULL, 64);
case 7: /* CMD7: SELECT/DESELECT_CARD */
rca = sd_req_get_rca(sd, req);
@@ -1946,7 +1942,6 @@ send_response:
}
/* Return true when buffer is consumed. Configured by sd_cmd_to_sendingdata()
*/
-__attribute__((unused))
static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
{
*value = sd->data[sd->data_offset];
@@ -2135,10 +2130,7 @@ uint8_t sd_read_byte(SDState *sd)
sd->current_cmd, sd->data_offset, io_len);
switch (sd->current_cmd) {
case 6: /* CMD6: SWITCH_FUNCTION */
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= 64)
-sd->state = sd_transfer_state;
+sd_generic_read_byte(sd, &ret);
break;
case 9: /* CMD9: SEND_CSD */
--
2.41.0
[PATCH v2 04/11] hw/sd/sdcard: Duplicate READ_SINGLE_BLOCK / READ_MULTIPLE_BLOCK cases
In order to modify the READ_SINGLE_BLOCK case in the
next commit, duplicate it first.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8201f3245c..dfcb213aa9 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1398,6 +1398,24 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
break;
case 17: /* CMD17: READ_SINGLE_BLOCK */
+addr = sd_req_get_address(sd, req);
+switch (sd->state) {
+case sd_transfer_state:
+
+if (!address_in_range(sd, "READ_SINGLE_BLOCK", addr, sd->blk_len))
{
+return sd_r1;
+}
+
+sd->state = sd_sendingdata_state;
+sd->data_start = addr;
+sd->data_offset = 0;
+return sd_r1;
+
+default:
+break;
+}
+break;
+
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
addr = sd_req_get_address(sd, req);
switch (sd->state) {
--
2.41.0
[PATCH v2 05/11] hw/sd/sdcard: Convert READ_SINGLE_BLOCK to generic_read_byte (CMD17)
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 18 +++---
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index dfcb213aa9..605269163d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1405,11 +1405,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
if (!address_in_range(sd, "READ_SINGLE_BLOCK", addr, sd->blk_len))
{
return sd_r1;
}
-
-sd->state = sd_sendingdata_state;
-sd->data_start = addr;
-sd->data_offset = 0;
-return sd_r1;
+sd_blk_read(sd, addr, sd->blk_len);
+return sd_cmd_to_sendingdata(sd, req, addr, NULL, sd->blk_len);
default:
break;
@@ -2144,6 +2141,7 @@ uint8_t sd_read_byte(SDState *sd)
case 6: /* CMD6: SWITCH_FUNCTION */
case 9: /* CMD9: SEND_CSD */
case 10: /* CMD10: SEND_CID */
+case 17: /* CMD17: READ_SINGLE_BLOCK */
sd_generic_read_byte(sd, &ret);
break;
@@ -2154,16 +2152,6 @@ uint8_t sd_read_byte(SDState *sd)
sd->state = sd_transfer_state;
break;
-case 17: /* CMD17: READ_SINGLE_BLOCK */
-if (sd->data_offset == 0) {
-sd_blk_read(sd, sd->data_start, io_len);
-}
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= io_len)
-sd->state = sd_transfer_state;
-break;
-
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
if (sd->data_offset == 0) {
if (!address_in_range(sd, "READ_MULTIPLE_BLOCK",
--
2.41.0
