* [Qemu-devel] [PATCH 01/20] sdcard: Use the ldst API
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
@ 2018-05-04 15:58 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 02/20] sdcard: Extract sd_calc_frame48_crc7() from sd_req_crc_validate() Philippe Mathieu-Daudé
` (19 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:58 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis,
Michael Walle
The load/store API will ease further code movement.
Per the Physical Layer Simplified Spec. "3.6 Bus Protocol":
"In the CMD line the Most Significant Bit (MSB) is transmitted
first, the Least Significant Bit (LSB) is the last."
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/milkymist-memcard.c | 3 +--
hw/sd/ssi-sd.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 5570c1e9a0..ff2b92dc64 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -100,8 +100,7 @@ static void memcard_sd_command(MilkymistMemcardState *s)
SDRequest req;
req.cmd = s->command[0] & 0x3f;
- req.arg = (s->command[1] << 24) | (s->command[2] << 16)
- | (s->command[3] << 8) | s->command[4];
+ req.arg = ldl_be_p(s->command + 1);
req.crc = s->command[5];
s->response[0] = req.cmd;
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index ae04b6641b..0bb26e596d 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -97,8 +97,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
uint8_t longresp[16];
/* FIXME: Check CRC. */
request.cmd = s->cmd;
- request.arg = (s->cmdarg[0] << 24) | (s->cmdarg[1] << 16)
- | (s->cmdarg[2] << 8) | s->cmdarg[3];
+ request.arg = ldl_be_p(s->cmdarg);
DPRINTF("CMD%d arg 0x%08x\n", s->cmd, request.arg);
s->arglen = sdbus_do_command(&s->sdbus, &request, longresp);
if (s->arglen <= 0) {
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 02/20] sdcard: Extract sd_calc_frame48_crc7() from sd_req_crc_validate()
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
2018-05-04 15:58 ` [Qemu-devel] [PATCH 01/20] sdcard: Use the ldst API Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 03/20] sdcard: Rename the SDRequest as SDFrame48 Philippe Mathieu-Daudé
` (18 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Extract calculate() from validate() so we can calculate the CRC
out of the validate() function.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 235e0518d6..861bba197d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -481,13 +481,18 @@ static void sd_set_sdstatus(SDState *sd)
memset(sd->sd_status, 0, 64);
}
-static int sd_req_crc_validate(SDRequest *req)
+static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg)
{
uint8_t buffer[5];
- buffer[0] = 0x40 | req->cmd;
- stl_be_p(&buffer[1], req->arg);
+ buffer[0] = 0x40 | cmd;
+ stl_be_p(&buffer[1], arg);
+ return sd_crc7(buffer, sizeof(buffer));
+}
+
+static int sd_req_crc_validate(SDRequest *req)
+{
return 0;
- return sd_crc7(buffer, 5) != req->crc; /* TODO */
+ return sd_calc_frame48_crc7(req->cmd, req->arg) != req->crc; /* TODO */
}
static void sd_response_r1_make(SDState *sd, uint8_t *response)
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 03/20] sdcard: Rename the SDRequest as SDFrame48
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
2018-05-04 15:58 ` [Qemu-devel] [PATCH 01/20] sdcard: Use the ldst API Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 02/20] sdcard: Extract sd_calc_frame48_crc7() from sd_req_crc_validate() Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 04/20] sdcard: Add sd_prepare_request[_with_crc]() Philippe Mathieu-Daudé
` (17 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
SD requests are 48-bit SD frames, while SD responses can be
48-bit or 136-bit frames. The 48-bit response frames share
the same CRC logic than request frames.
Unify the 48-bit framing to reuse the same CRC logic between
requests and responses.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/sd/sd.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 9bdb3c9285..f0b41232f7 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -76,11 +76,22 @@ typedef enum {
sd_adtc, /* addressed with data transfer */
} sd_cmd_type_t;
-typedef struct {
+/**
+ * SDFrame48: 48 bits commands or responses
+ *
+ * @cmd: request: command
+ * response: mirrored command
+ * @arg: request: address information or parameter
+ * response: status information, OCR register, RCA
+ * @crc: 7-bit CRC checksum
+ */
+typedef struct SDFrame48 {
uint8_t cmd;
uint32_t arg;
uint8_t crc;
-} SDRequest;
+} SDFrame48;
+
+typedef struct SDFrame48 SDRequest;
typedef struct SDState SDState;
typedef struct SDBus SDBus;
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 04/20] sdcard: Add sd_prepare_request[_with_crc]()
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 03/20] sdcard: Rename the SDRequest as SDFrame48 Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 05/20] sdcard: Use the sd_prepare_request*() functions Philippe Mathieu-Daudé
` (16 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
The sd_prepare_request*() functions fill all the request frame members.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/sd/sd.h | 27 +++++++++++++++++++++++++++
hw/sd/sd.c | 37 ++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index f0b41232f7..7d9c906897 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -141,6 +141,33 @@ typedef struct {
void (*set_readonly)(DeviceState *dev, bool readonly);
} SDBusClass;
+/**
+ * sd_prepare_request:
+ * @req: the #SDRequest to be filled
+ * @cmd: the SD command
+ * @arg: the SD command argument
+ * @gen_crc: generates the frame CRC7 if true, else fill with zeroes
+ *
+ * Initialize a SD request buffer.
+ *
+ * If @gen_crc the frame checksum will be calculated
+ * and filled in the request buffer.
+ */
+void sd_prepare_request(SDRequest *req, uint8_t cmd, uint32_t arg,
+ bool gen_crc);
+
+/**
+ * sd_prepare_request_with_crc:
+ * @req: the #SDRequest to be filled
+ * @cmd: the SD command
+ * @arg: the SD command argument
+ * @crc: the calculated request CRC7
+ *
+ * Initialize a SD request buffer.
+ */
+void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
+ uint8_t crc);
+
/* Legacy functions to be used only by non-qdevified callers */
SDState *sd_init(BlockBackend *bs, bool is_spi);
int sd_do_command(SDState *sd, SDRequest *req,
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 861bba197d..be75e118c0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -489,10 +489,45 @@ static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg)
return sd_crc7(buffer, sizeof(buffer));
}
+static bool sd_verify_frame48_checksum(SDFrame48 *frame)
+{
+ uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
+
+ return crc == frame->crc;
+}
+
static int sd_req_crc_validate(SDRequest *req)
{
return 0;
- return sd_calc_frame48_crc7(req->cmd, req->arg) != req->crc; /* TODO */
+ return !sd_verify_frame48_checksum(req); /* TODO */
+}
+
+static void sd_update_frame48_checksum(SDFrame48 *frame)
+{
+ frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
+}
+
+static void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
+ bool gen_crc)
+{
+ frame->cmd = cmd;
+ frame->arg = arg;
+ frame->crc = 0x00;
+ if (gen_crc) {
+ sd_update_frame48_checksum(frame);
+ }
+}
+
+void sd_prepare_request(SDFrame48 *req, uint8_t cmd, uint32_t arg, bool gen_crc)
+{
+ sd_prepare_frame48(req, cmd, arg, gen_crc);
+}
+
+void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
+ uint8_t crc)
+{
+ sd_prepare_frame48(req, cmd, arg, /* gen_crc */ false);
+ req->crc = crc;
}
static void sd_response_r1_make(SDState *sd, uint8_t *response)
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 05/20] sdcard: Use the sd_prepare_request*() functions
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 04/20] sdcard: Add sd_prepare_request[_with_crc]() Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 06/20] sdcard: Add a "validate-crc" property Philippe Mathieu-Daudé
` (15 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis,
Michael Walle, Andrzej Zaborowski, open list:PXA2XX
This guaranties all SDRequest members are initialized.
This silent the Coverity warning:
"Use of an uninitialized variable (CWE-457)"
and fixes the following issues (all "Uninitialized scalar variable"):
- CID1386072 (hw/sd/sdhci.c::sdhci_end_transfer)
- CID1386074 (hw/sd/bcm2835_sdhost.c::bcm2835_sdhost_send_command)
- CID1386076 (hw/sd/sdhci.c::sdhci_send_command)
- CID1390571 (hw/sd/ssi-sd.c::ssi_sd_transfer)
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/bcm2835_sdhost.c | 3 +--
hw/sd/milkymist-memcard.c | 7 +++----
hw/sd/omap_mmc.c | 4 +---
hw/sd/pxa2xx_mmci.c | 4 +---
hw/sd/sdhci.c | 6 ++----
hw/sd/ssi-sd.c | 5 ++---
6 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
index ebf3b926c2..5134d7b5c7 100644
--- a/hw/sd/bcm2835_sdhost.c
+++ b/hw/sd/bcm2835_sdhost.c
@@ -110,8 +110,7 @@ static void bcm2835_sdhost_send_command(BCM2835SDHostState *s)
uint8_t rsp[16];
int rlen;
- request.cmd = s->cmd & SDCMD_CMD_MASK;
- request.arg = s->cmdarg;
+ sd_prepare_request(&request, s->cmd & SDCMD_CMD_MASK, s->cmdarg, false);
rlen = sdbus_do_command(&s->sdbus, &request, rsp);
if (rlen < 0) {
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index ff2b92dc64..d8cbb7b681 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -99,10 +99,9 @@ static void memcard_sd_command(MilkymistMemcardState *s)
{
SDRequest req;
- req.cmd = s->command[0] & 0x3f;
- req.arg = ldl_be_p(s->command + 1);
- req.crc = s->command[5];
-
+ sd_prepare_request_with_crc(&req, s->command[0] & 0x3f,
+ ldl_be_p(s->command + 1),
+ s->command[5]);
s->response[0] = req.cmd;
s->response_len = sdbus_do_command(&s->sdbus, &req, s->response + 1);
s->response_read_ptr = 0;
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index 5b47cadf11..7b71984115 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -135,9 +135,7 @@ static void omap_mmc_command(struct omap_mmc_s *host, int cmd, int dir,
mask = 0;
rspstatus = 0;
- request.cmd = cmd;
- request.arg = host->arg;
- request.crc = 0; /* FIXME */
+ sd_prepare_request(&request, cmd, host->arg, false /* FIXME */);
rsplen = sd_do_command(host->card, &request, response);
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 82f8ec0d50..fd5c1e7686 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -224,9 +224,7 @@ static void pxa2xx_mmci_wakequeues(PXA2xxMMCIState *s)
s->tx_len = 0;
s->cmdreq = 0;
- request.cmd = s->cmd;
- request.arg = s->arg;
- request.crc = 0; /* FIXME */
+ sd_prepare_request(&request, s->cmd, s->arg, false /* FIXME */);
rsplen = sdbus_do_command(&s->sdbus, &request, response);
s->intreq |= INT_END_CMD;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 63c44a4ee8..9260f59e80 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -336,8 +336,7 @@ static void sdhci_send_command(SDHCIState *s)
s->errintsts = 0;
s->acmd12errsts = 0;
- request.cmd = s->cmdreg >> 8;
- request.arg = s->argument;
+ sd_prepare_request(&request, s->cmdreg >> 8, s->argument, false);
trace_sdhci_send_command(request.cmd, request.arg);
rlen = sdbus_do_command(&s->sdbus, &request, response);
@@ -393,8 +392,7 @@ static void sdhci_end_transfer(SDHCIState *s)
SDRequest request;
uint8_t response[16];
- request.cmd = 0x0C;
- request.arg = 0;
+ sd_prepare_request(&request, 12, 0, false);
trace_sdhci_end_transfer(request.cmd, request.arg);
sdbus_do_command(&s->sdbus, &request, response);
/* Auto CMD12 response goes to the upper Response register */
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 0bb26e596d..c22967170c 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -95,9 +95,8 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
if (s->arglen == 4) {
SDRequest request;
uint8_t longresp[16];
- /* FIXME: Check CRC. */
- request.cmd = s->cmd;
- request.arg = ldl_be_p(s->cmdarg);
+ sd_prepare_request(&request, s->cmd,
+ ldl_be_p(s->cmdarg), false /* FIXME */);
DPRINTF("CMD%d arg 0x%08x\n", s->cmd, request.arg);
s->arglen = sdbus_do_command(&s->sdbus, &request, longresp);
if (s->arglen <= 0) {
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 06/20] sdcard: Add a "validate-crc" property
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 05/20] sdcard: Use the sd_prepare_request*() functions Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 23:33 ` Alistair Francis
2018-05-04 15:59 ` [Qemu-devel] [PATCH 07/20] sdcard: Constify sd_crc*()'s message argument Philippe Mathieu-Daudé
` (14 subsequent siblings)
20 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis,
Michael Walle
Since not all modelled controllers use the CRC verification (which is
somehow expensive), let the controller have a configurable property
to enable verification.
So far only the Milkymist controller uses it.
This silent the Coverity warning:
"Code block is unreachable because of the syntactic structure of the code (CWE-561)"
and fixes the following issue:
- CID1005332 (hw/sd/sd.c::sd_req_crc_validate) Structurally dead code
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/milkymist-memcard.c | 1 +
hw/sd/sd.c | 12 ++++++++----
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index d8cbb7b681..04babc092f 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -278,6 +278,7 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
qdev_prop_set_drive(carddev, "drive", blk, &err);
+ object_property_set_bool(OBJECT(carddev), true, "validate-crc", &err);
object_property_set_bool(OBJECT(carddev), true, "realized", &err);
if (err) {
error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index be75e118c0..801ddc2cb5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -93,6 +93,7 @@ struct SDState {
/* Configurable properties */
BlockBackend *blk;
bool spi;
+ bool validate_crc;
uint32_t mode; /* current card mode, one of SDCardModes */
int32_t state; /* current card state, one of SDCardStates */
@@ -496,10 +497,12 @@ static bool sd_verify_frame48_checksum(SDFrame48 *frame)
return crc == frame->crc;
}
-static int sd_req_crc_validate(SDRequest *req)
+static bool sd_req_crc_validate(SDState *sd, SDRequest *req)
{
- return 0;
- return !sd_verify_frame48_checksum(req); /* TODO */
+ if (sd->validate_crc) {
+ return sd_verify_frame48_checksum(req);
+ }
+ return true;
}
static void sd_update_frame48_checksum(SDFrame48 *frame)
@@ -1685,7 +1688,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
return 0;
}
- if (sd_req_crc_validate(req)) {
+ if (!sd_req_crc_validate(sd, req)) {
sd->card_status |= COM_CRC_ERROR;
rtype = sd_illegal;
goto send_response;
@@ -2134,6 +2137,7 @@ static Property sd_properties[] = {
* board to ensure that ssi transfers only occur when the chip select
* is asserted. */
DEFINE_PROP_BOOL("spi", SDState, spi, false),
+ DEFINE_PROP_BOOL("validate-crc", SDState, validate_crc, false),
DEFINE_PROP_END_OF_LIST()
};
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 06/20] sdcard: Add a "validate-crc" property
2018-05-04 15:59 ` [Qemu-devel] [PATCH 06/20] sdcard: Add a "validate-crc" property Philippe Mathieu-Daudé
@ 2018-05-04 23:33 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2018-05-04 23:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Edgar Iglesias, Alistair Francis,
qemu-devel@nongnu.org Developers, Michael Walle, Stefan Hajnoczi,
Paolo Bonzini
On Fri, May 4, 2018 at 9:00 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:
> Since not all modelled controllers use the CRC verification (which is
> somehow expensive), let the controller have a configurable property
> to enable verification.
> So far only the Milkymist controller uses it.
> This silent the Coverity warning:
> "Code block is unreachable because of the syntactic structure of the
code (CWE-561)"
> and fixes the following issue:
> - CID1005332 (hw/sd/sd.c::sd_req_crc_validate) Structurally dead code
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/sd/milkymist-memcard.c | 1 +
> hw/sd/sd.c | 12 ++++++++----
> 2 files changed, 9 insertions(+), 4 deletions(-)
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index d8cbb7b681..04babc092f 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -278,6 +278,7 @@ static void milkymist_memcard_realize(DeviceState
*dev, Error **errp)
> blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> qdev_prop_set_drive(carddev, "drive", blk, &err);
> + object_property_set_bool(OBJECT(carddev), true, "validate-crc",
&err);
> object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> if (err) {
> error_setg(errp, "failed to init SD card: %s",
error_get_pretty(err));
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index be75e118c0..801ddc2cb5 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -93,6 +93,7 @@ struct SDState {
> /* Configurable properties */
> BlockBackend *blk;
> bool spi;
> + bool validate_crc;
> uint32_t mode; /* current card mode, one of SDCardModes */
> int32_t state; /* current card state, one of SDCardStates */
> @@ -496,10 +497,12 @@ static bool sd_verify_frame48_checksum(SDFrame48
*frame)
> return crc == frame->crc;
> }
> -static int sd_req_crc_validate(SDRequest *req)
> +static bool sd_req_crc_validate(SDState *sd, SDRequest *req)
> {
> - return 0;
> - return !sd_verify_frame48_checksum(req); /* TODO */
> + if (sd->validate_crc) {
> + return sd_verify_frame48_checksum(req);
> + }
> + return true;
> }
> static void sd_update_frame48_checksum(SDFrame48 *frame)
> @@ -1685,7 +1688,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
> return 0;
> }
> - if (sd_req_crc_validate(req)) {
> + if (!sd_req_crc_validate(sd, req)) {
> sd->card_status |= COM_CRC_ERROR;
> rtype = sd_illegal;
> goto send_response;
> @@ -2134,6 +2137,7 @@ static Property sd_properties[] = {
> * board to ensure that ssi transfers only occur when the chip select
> * is asserted. */
> DEFINE_PROP_BOOL("spi", SDState, spi, false),
> + DEFINE_PROP_BOOL("validate-crc", SDState, validate_crc, false),
> DEFINE_PROP_END_OF_LIST()
> };
> --
> 2.17.0
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 07/20] sdcard: Constify sd_crc*()'s message argument
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 06/20] sdcard: Add a "validate-crc" property Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 08/20] sdcard: Fix sd_crc*() style Philippe Mathieu-Daudé
` (13 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 801ddc2cb5..3708ec1d72 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -237,11 +237,11 @@ static const int sd_cmd_class[SDMMC_CMD_MAX] = {
7, 7, 10, 7, 9, 9, 9, 8, 8, 10, 8, 8, 8, 8, 8, 8,
};
-static uint8_t sd_crc7(void *message, size_t width)
+static uint8_t sd_crc7(const void *message, size_t width)
{
int i, bit;
uint8_t shift_reg = 0x00;
- uint8_t *msg = (uint8_t *) message;
+ const uint8_t *msg = (const uint8_t *)message;
for (i = 0; i < width; i ++, msg ++)
for (bit = 7; bit >= 0; bit --) {
@@ -253,11 +253,11 @@ static uint8_t sd_crc7(void *message, size_t width)
return shift_reg;
}
-static uint16_t sd_crc16(void *message, size_t width)
+static uint16_t sd_crc16(const void *message, size_t width)
{
int i, bit;
uint16_t shift_reg = 0x0000;
- uint16_t *msg = (uint16_t *) message;
+ const uint16_t *msg = (const uint16_t *)message;
width <<= 1;
for (i = 0; i < width; i ++, msg ++)
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 08/20] sdcard: Fix sd_crc*() style
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 07/20] sdcard: Constify sd_crc*()'s message argument Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 23:34 ` Alistair Francis
2018-05-04 15:59 ` [Qemu-devel] [PATCH 09/20] sdcard: Expose sd_crc*() functions for QTest use Philippe Mathieu-Daudé
` (12 subsequent siblings)
20 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Fix style to keep patchew/checkpatch happy when moving this code
in the next patch.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sd.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3708ec1d72..a28ef8de5e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -243,12 +243,14 @@ static uint8_t sd_crc7(const void *message, size_t width)
uint8_t shift_reg = 0x00;
const uint8_t *msg = (const uint8_t *)message;
- for (i = 0; i < width; i ++, msg ++)
- for (bit = 7; bit >= 0; bit --) {
+ for (i = 0; i < width; i++, msg++) {
+ for (bit = 7; bit >= 0; bit--) {
shift_reg <<= 1;
- if ((shift_reg >> 7) ^ ((*msg >> bit) & 1))
+ if ((shift_reg >> 7) ^ ((*msg >> bit) & 1)) {
shift_reg ^= 0x89;
+ }
}
+ }
return shift_reg;
}
@@ -260,12 +262,14 @@ static uint16_t sd_crc16(const void *message, size_t width)
const uint16_t *msg = (const uint16_t *)message;
width <<= 1;
- for (i = 0; i < width; i ++, msg ++)
- for (bit = 15; bit >= 0; bit --) {
+ for (i = 0; i < width; i++, msg++) {
+ for (bit = 15; bit >= 0; bit--) {
shift_reg <<= 1;
- if ((shift_reg >> 15) ^ ((*msg >> bit) & 1))
+ if ((shift_reg >> 15) ^ ((*msg >> bit) & 1)) {
shift_reg ^= 0x1011;
+ }
}
+ }
return shift_reg;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 08/20] sdcard: Fix sd_crc*() style
2018-05-04 15:59 ` [Qemu-devel] [PATCH 08/20] sdcard: Fix sd_crc*() style Philippe Mathieu-Daudé
@ 2018-05-04 23:34 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2018-05-04 23:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Edgar Iglesias, Paolo Bonzini, Alistair Francis,
Stefan Hajnoczi, qemu-devel@nongnu.org Developers
On Fri, May 4, 2018 at 9:03 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:
> Fix style to keep patchew/checkpatch happy when moving this code
> in the next patch.
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/sd/sd.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 3708ec1d72..a28ef8de5e 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -243,12 +243,14 @@ static uint8_t sd_crc7(const void *message, size_t
width)
> uint8_t shift_reg = 0x00;
> const uint8_t *msg = (const uint8_t *)message;
> - for (i = 0; i < width; i ++, msg ++)
> - for (bit = 7; bit >= 0; bit --) {
> + for (i = 0; i < width; i++, msg++) {
> + for (bit = 7; bit >= 0; bit--) {
> shift_reg <<= 1;
> - if ((shift_reg >> 7) ^ ((*msg >> bit) & 1))
> + if ((shift_reg >> 7) ^ ((*msg >> bit) & 1)) {
> shift_reg ^= 0x89;
> + }
> }
> + }
> return shift_reg;
> }
> @@ -260,12 +262,14 @@ static uint16_t sd_crc16(const void *message,
size_t width)
> const uint16_t *msg = (const uint16_t *)message;
> width <<= 1;
> - for (i = 0; i < width; i ++, msg ++)
> - for (bit = 15; bit >= 0; bit --) {
> + for (i = 0; i < width; i++, msg++) {
> + for (bit = 15; bit >= 0; bit--) {
> shift_reg <<= 1;
> - if ((shift_reg >> 15) ^ ((*msg >> bit) & 1))
> + if ((shift_reg >> 15) ^ ((*msg >> bit) & 1)) {
> shift_reg ^= 0x1011;
> + }
> }
> + }
> return shift_reg;
> }
> --
> 2.17.0
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 09/20] sdcard: Expose sd_crc*() functions for QTest use
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 08/20] sdcard: Fix sd_crc*() style Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 10/20] sdcard: Expose sd_prepare_request*() " Philippe Mathieu-Daudé
` (11 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Take those functions out of hw/sd/sd.c (via "sdmmc-internal.h")
to be able to write QTests for them.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdmmc-internal.h | 22 ++++++++++++++++++++++
hw/sd/sd.c | 37 -------------------------------------
hw/sd/sdmmc-internal.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+), 37 deletions(-)
diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index 9aa04766fc..62c0ff250d 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -36,4 +36,26 @@ const char *sd_cmd_name(uint8_t cmd);
*/
const char *sd_acmd_name(uint8_t cmd);
+/**
+ * sd_crc7:
+ * @data: pointer to the data buffer
+ * @data_len: data length
+ *
+ * Calculate the 7-bit CRC of a SD frame.
+ *
+ * Returns: The frame CRC.
+ */
+uint8_t sd_crc7(const void *data, size_t data_len);
+
+/**
+ * sd_crc16:
+ * @data: pointer to the data buffer
+ * @data_len: data length
+ *
+ * Calculate the 16-bit CRC of a SD data frame.
+ *
+ * Returns: The frame CRC.
+ */
+uint16_t sd_crc16(const void *data, size_t data_len);
+
#endif
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a28ef8de5e..11b4606051 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -237,43 +237,6 @@ static const int sd_cmd_class[SDMMC_CMD_MAX] = {
7, 7, 10, 7, 9, 9, 9, 8, 8, 10, 8, 8, 8, 8, 8, 8,
};
-static uint8_t sd_crc7(const void *message, size_t width)
-{
- int i, bit;
- uint8_t shift_reg = 0x00;
- const uint8_t *msg = (const uint8_t *)message;
-
- for (i = 0; i < width; i++, msg++) {
- for (bit = 7; bit >= 0; bit--) {
- shift_reg <<= 1;
- if ((shift_reg >> 7) ^ ((*msg >> bit) & 1)) {
- shift_reg ^= 0x89;
- }
- }
- }
-
- return shift_reg;
-}
-
-static uint16_t sd_crc16(const void *message, size_t width)
-{
- int i, bit;
- uint16_t shift_reg = 0x0000;
- const uint16_t *msg = (const uint16_t *)message;
- width <<= 1;
-
- for (i = 0; i < width; i++, msg++) {
- for (bit = 15; bit >= 0; bit--) {
- shift_reg <<= 1;
- if ((shift_reg >> 15) ^ ((*msg >> bit) & 1)) {
- shift_reg ^= 0x1011;
- }
- }
- }
-
- return shift_reg;
-}
-
#define OCR_POWER_DELAY_NS 500000 /* 0.5ms */
FIELD(OCR, VDD_VOLTAGE_WINDOW, 0, 24)
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 2053def3f1..69ad0a99e6 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -70,3 +70,41 @@ const char *sd_acmd_name(uint8_t cmd)
return acmd_abbrev[cmd] ? acmd_abbrev[cmd] : "UNKNOWN_ACMD";
}
+
+/* 7 bit CRC with polynomial x^7 + x^3 + 1 */
+uint8_t sd_crc7(const void *message, size_t width)
+{
+ int i, bit;
+ uint8_t shift_reg = 0x00;
+ const uint8_t *msg = (const uint8_t *)message;
+
+ for (i = 0; i < width; i++, msg++) {
+ for (bit = 7; bit >= 0; bit--) {
+ shift_reg <<= 1;
+ if ((shift_reg >> 7) ^ ((*msg >> bit) & 1)) {
+ shift_reg ^= 0x89;
+ }
+ }
+ }
+
+ return shift_reg;
+}
+
+uint16_t sd_crc16(const void *message, size_t width)
+{
+ int i, bit;
+ uint16_t shift_reg = 0x0000;
+ const uint16_t *msg = (const uint16_t *)message;
+ width <<= 1;
+
+ for (i = 0; i < width; i++, msg++) {
+ for (bit = 15; bit >= 0; bit--) {
+ shift_reg <<= 1;
+ if ((shift_reg >> 15) ^ ((*msg >> bit) & 1)) {
+ shift_reg ^= 0x1011;
+ }
+ }
+ }
+
+ return shift_reg;
+}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 10/20] sdcard: Expose sd_prepare_request*() functions for QTest use
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 09/20] sdcard: Expose sd_crc*() functions for QTest use Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 11/20] sdcard: Add test_sd_request_frame_crc7() qtest (request command CRC7) Philippe Mathieu-Daudé
` (10 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Take those functions out of hw/sd/sd.c (via "sdmmc-internal.h")
to be able to write QTests for them.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdmmc-internal.h | 2 ++
include/hw/sd/sd.h | 18 ++++++++++++++++++
hw/sd/sd.c | 43 ------------------------------------------
hw/sd/sdmmc-internal.c | 43 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 63 insertions(+), 43 deletions(-)
diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index 62c0ff250d..d672112200 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -10,6 +10,8 @@
#ifndef SD_INTERNAL_H
#define SD_INTERNAL_H
+#include "hw/sd/sd.h"
+
#define SDMMC_CMD_MAX 64
/**
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 7d9c906897..b65107ffe1 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -168,6 +168,24 @@ void sd_prepare_request(SDRequest *req, uint8_t cmd, uint32_t arg,
void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
uint8_t crc);
+/**
+ * sd_update_frame48_checksum:
+ * @frame: the #SDFrame48 to verify
+ *
+ * Update the 7-bit CRC checksum of a SD 48-bit frame.
+ */
+void sd_update_frame48_checksum(SDFrame48 *frame);
+
+/**
+ * sd_verify_frame48_checksum:
+ * @frame: the #SDFrame48 to verify
+ *
+ * Verify the 7-bit CRC checksum of a SD 48-bit frame.
+ *
+ * Returns: A boolean indicating whether the frame 7-bit CRC is correct.
+ */
+bool sd_verify_frame48_checksum(SDFrame48 *frame);
+
/* Legacy functions to be used only by non-qdevified callers */
SDState *sd_init(BlockBackend *bs, bool is_spi);
int sd_do_command(SDState *sd, SDRequest *req,
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 11b4606051..2d2c31d308 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -449,21 +449,6 @@ static void sd_set_sdstatus(SDState *sd)
memset(sd->sd_status, 0, 64);
}
-static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg)
-{
- uint8_t buffer[5];
- buffer[0] = 0x40 | cmd;
- stl_be_p(&buffer[1], arg);
- return sd_crc7(buffer, sizeof(buffer));
-}
-
-static bool sd_verify_frame48_checksum(SDFrame48 *frame)
-{
- uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
-
- return crc == frame->crc;
-}
-
static bool sd_req_crc_validate(SDState *sd, SDRequest *req)
{
if (sd->validate_crc) {
@@ -472,34 +457,6 @@ static bool sd_req_crc_validate(SDState *sd, SDRequest *req)
return true;
}
-static void sd_update_frame48_checksum(SDFrame48 *frame)
-{
- frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
-}
-
-static void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
- bool gen_crc)
-{
- frame->cmd = cmd;
- frame->arg = arg;
- frame->crc = 0x00;
- if (gen_crc) {
- sd_update_frame48_checksum(frame);
- }
-}
-
-void sd_prepare_request(SDFrame48 *req, uint8_t cmd, uint32_t arg, bool gen_crc)
-{
- sd_prepare_frame48(req, cmd, arg, gen_crc);
-}
-
-void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
- uint8_t crc)
-{
- sd_prepare_frame48(req, cmd, arg, /* gen_crc */ false);
- req->crc = crc;
-}
-
static void sd_response_r1_make(SDState *sd, uint8_t *response)
{
stl_be_p(response, sd->card_status);
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 69ad0a99e6..97f5f71569 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -108,3 +108,46 @@ uint16_t sd_crc16(const void *message, size_t width)
return shift_reg;
}
+
+static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg)
+{
+ uint8_t buffer[5];
+ buffer[0] = 0x40 | cmd;
+ stl_be_p(&buffer[1], arg);
+ return sd_crc7(buffer, sizeof(buffer));
+}
+
+bool sd_verify_frame48_checksum(SDFrame48 *frame)
+{
+ uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
+
+ return crc == frame->crc;
+}
+
+void sd_update_frame48_checksum(SDFrame48 *frame)
+{
+ frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
+}
+
+static void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
+ bool gen_crc)
+{
+ frame->cmd = cmd;
+ frame->arg = arg;
+ frame->crc = 0x00;
+ if (gen_crc) {
+ sd_update_frame48_checksum(frame);
+ }
+}
+
+void sd_prepare_request(SDFrame48 *req, uint8_t cmd, uint32_t arg, bool gen_crc)
+{
+ sd_prepare_frame48(req, cmd, arg, gen_crc);
+}
+
+void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
+ uint8_t crc)
+{
+ sd_prepare_frame48(req, cmd, arg, /* gen_crc */ false);
+ req->crc = crc;
+}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 11/20] sdcard: Add test_sd_request_frame_crc7() qtest (request command CRC7)
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 10/20] sdcard: Expose sd_prepare_request*() " Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 12/20] sdcard: Let sd_frame48_crc7_calc() work on response frames Philippe Mathieu-Daudé
` (9 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/sdcard-test.c | 54 ++++++++++++++++++++++++++++++++++++++++++
tests/Makefile.include | 2 ++
2 files changed, 56 insertions(+)
create mode 100644 tests/sdcard-test.c
diff --git a/tests/sdcard-test.c b/tests/sdcard-test.c
new file mode 100644
index 0000000000..1358ee28c6
--- /dev/null
+++ b/tests/sdcard-test.c
@@ -0,0 +1,54 @@
+/*
+ * QTest testcase for SD protocol and cards
+ *
+ * Examples taken from:
+ *
+ * - Physical Layer Simplified Specification (chap. 4.5: Cyclic Redundancy Code)
+ * - http://wiki.seabright.co.nz/wiki/SdCardProtocol.html
+ *
+ * Tests written by Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/sd/sd.h"
+
+static void sd_prepare_request48(SDFrame48 *frame, uint8_t cmd, uint32_t arg)
+{
+ sd_prepare_request(frame, cmd, arg, /* gen_crc */ true);
+}
+
+static void test_sd_request_frame_crc7(void)
+{
+ SDFrame48 frame;
+
+ /* CMD0 */
+ sd_prepare_request48(&frame, 0, 0);
+ g_assert_cmphex(frame.crc, ==, 0b1001010);
+
+ /* CMD17 */
+ sd_prepare_request48(&frame, 17, 0);
+ g_assert_cmphex(frame.crc, ==, 0b0101010);
+
+ /* APP_CMD */
+ sd_prepare_request48(&frame, 55, 0);
+ g_assert_cmphex(frame.crc, ==, 0x32);
+
+ /* ACMD41 SEND_OP_COND */
+ sd_prepare_request48(&frame, 41, 0x00100000);
+ g_assert_cmphex(frame.crc, ==, 0x5f >> 1);
+
+ /* CMD2 ALL_SEND_CID */
+ sd_prepare_request48(&frame, 2, 0);
+ g_assert_cmphex(frame.crc, ==, 0x4d >> 1);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("sd/req_crc7", test_sd_request_frame_crc7);
+
+ return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e31a2..6a70a1dbab 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -380,6 +380,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
gcov-files-arm-y += hw/timer/arm_mptimer.c
check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y += tests/sdhci-test$(EXESUF)
+check-qtest-arm-y += tests/sdcard-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
@@ -835,6 +836,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
tests/numa-test$(EXESUF): tests/numa-test.o
tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
+tests/sdcard-test$(EXESUF): tests/sdcard-test.o hw/sd/sdmmc-internal.o
tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 12/20] sdcard: Let sd_frame48_crc7_calc() work on response frames
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 11/20] sdcard: Add test_sd_request_frame_crc7() qtest (request command CRC7) Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 13/20] sdcard: Expose sd_prepare_frame48() for QTest use Philippe Mathieu-Daudé
` (8 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Currently sd_calc_frame48_crc7() only works for request frames, having
the bit 6 hardwire to 1.
Extend it to handle response frames, adding a 'is_response' boolean argument.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/sd/sd.h | 6 ++++--
hw/sd/sd.c | 2 +-
hw/sd/sdmmc-internal.c | 20 ++++++++++----------
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index b65107ffe1..b1f865fe20 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -171,20 +171,22 @@ void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
/**
* sd_update_frame48_checksum:
* @frame: the #SDFrame48 to verify
+ * @is_response: whether the frame is a command request or response
*
* Update the 7-bit CRC checksum of a SD 48-bit frame.
*/
-void sd_update_frame48_checksum(SDFrame48 *frame);
+void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
/**
* sd_verify_frame48_checksum:
* @frame: the #SDFrame48 to verify
+ * @is_response: whether the frame is a command request or response
*
* Verify the 7-bit CRC checksum of a SD 48-bit frame.
*
* Returns: A boolean indicating whether the frame 7-bit CRC is correct.
*/
-bool sd_verify_frame48_checksum(SDFrame48 *frame);
+bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response);
/* Legacy functions to be used only by non-qdevified callers */
SDState *sd_init(BlockBackend *bs, bool is_spi);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2d2c31d308..5e0b5c2b87 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -452,7 +452,7 @@ static void sd_set_sdstatus(SDState *sd)
static bool sd_req_crc_validate(SDState *sd, SDRequest *req)
{
if (sd->validate_crc) {
- return sd_verify_frame48_checksum(req);
+ return sd_verify_frame48_checksum(req, false);
}
return true;
}
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 97f5f71569..04da81e665 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -109,45 +109,45 @@ uint16_t sd_crc16(const void *message, size_t width)
return shift_reg;
}
-static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg)
+static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg, bool is_response)
{
uint8_t buffer[5];
- buffer[0] = 0x40 | cmd;
+ buffer[0] = (!is_response << 6) | cmd;
stl_be_p(&buffer[1], arg);
return sd_crc7(buffer, sizeof(buffer));
}
-bool sd_verify_frame48_checksum(SDFrame48 *frame)
+bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response)
{
- uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
+ uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
return crc == frame->crc;
}
-void sd_update_frame48_checksum(SDFrame48 *frame)
+void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
{
- frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg);
+ frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
}
static void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
- bool gen_crc)
+ bool is_response, bool gen_crc)
{
frame->cmd = cmd;
frame->arg = arg;
frame->crc = 0x00;
if (gen_crc) {
- sd_update_frame48_checksum(frame);
+ sd_update_frame48_checksum(frame, is_response);
}
}
void sd_prepare_request(SDFrame48 *req, uint8_t cmd, uint32_t arg, bool gen_crc)
{
- sd_prepare_frame48(req, cmd, arg, gen_crc);
+ sd_prepare_frame48(req, cmd, arg, /* is_response */ false, gen_crc);
}
void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
uint8_t crc)
{
- sd_prepare_frame48(req, cmd, arg, /* gen_crc */ false);
+ sd_prepare_request(req, cmd, arg, /* gen_crc */ false);
req->crc = crc;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 13/20] sdcard: Expose sd_prepare_frame48() for QTest use
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 12/20] sdcard: Let sd_frame48_crc7_calc() work on response frames Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 14/20] sdcard: Add test_sd_response_frame48_crc7 qtest (command response CRC7) Philippe Mathieu-Daudé
` (7 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/sd/sd.h | 16 ++++++++++++++++
hw/sd/sdmmc-internal.c | 4 ++--
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index b1f865fe20..13de1b30c3 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -168,6 +168,22 @@ void sd_prepare_request(SDRequest *req, uint8_t cmd, uint32_t arg,
void sd_prepare_request_with_crc(SDRequest *req, uint8_t cmd, uint32_t arg,
uint8_t crc);
+/**
+ * sd_prepare_response48: Initialize a SD response buffer
+ *
+ * If @gen_crc the frame checksum will be calculated
+ * and filled in the request buffer.
+ *
+ * @req: the #SDRequest to be filled
+ * @cmd: the SD command
+ * @arg: the SD command argument
+ * @is_response: whether the frame is a command request or response
+ * @gen_crc: generates the frame CRC7 if true, else fill with zeroes
+ */
+
+void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
+ bool is_response, bool gen_crc);
+
/**
* sd_update_frame48_checksum:
* @frame: the #SDFrame48 to verify
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 04da81e665..c990cc9e8e 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -129,8 +129,8 @@ void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
}
-static void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
- bool is_response, bool gen_crc)
+void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
+ bool is_response, bool gen_crc)
{
frame->cmd = cmd;
frame->arg = arg;
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 14/20] sdcard: Add test_sd_response_frame48_crc7 qtest (command response CRC7)
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 13/20] sdcard: Expose sd_prepare_frame48() for QTest use Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 15/20] sdcard: Add SDFrame136 struct and 136-bit SD response frames functions Philippe Mathieu-Daudé
` (6 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/sdcard-test.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tests/sdcard-test.c b/tests/sdcard-test.c
index 1358ee28c6..2de0a5a0ee 100644
--- a/tests/sdcard-test.c
+++ b/tests/sdcard-test.c
@@ -19,6 +19,11 @@ static void sd_prepare_request48(SDFrame48 *frame, uint8_t cmd, uint32_t arg)
sd_prepare_request(frame, cmd, arg, /* gen_crc */ true);
}
+static void sd_prepare_response48(SDFrame48 *frame, uint8_t cmd, uint32_t arg)
+{
+ sd_prepare_frame48(frame, cmd, arg, /* is_resp */ true, /* gen_crc */ true);
+}
+
static void test_sd_request_frame_crc7(void)
{
SDFrame48 frame;
@@ -44,11 +49,29 @@ static void test_sd_request_frame_crc7(void)
g_assert_cmphex(frame.crc, ==, 0x4d >> 1);
}
+static void test_sd_response_frame48_crc7(void)
+{
+ SDFrame48 frame;
+
+ /* response to CMD17 */
+ sd_prepare_response48(&frame, 17, 0x00000900);
+ g_assert_cmphex(frame.crc, ==, 0b0110011);
+
+ /* response to the APP_CMD */
+ sd_prepare_response48(&frame, 55, 0x00000120);
+ g_assert_cmphex(frame.crc, ==, 0x41);
+
+ /* response to CMD3 SEND_RELATIVE_ADDR (Relative Card Address is 0xb368) */
+ sd_prepare_response48(&frame, 3, 0xb3680500);
+ g_assert_cmphex(frame.crc, ==, 0x0c);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
qtest_add_func("sd/req_crc7", test_sd_request_frame_crc7);
+ qtest_add_func("sd/resp48_crc7", test_sd_response_frame48_crc7);
return g_test_run();
}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 15/20] sdcard: Add SDFrame136 struct and 136-bit SD response frames functions
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (13 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 14/20] sdcard: Add test_sd_response_frame48_crc7 qtest (command response CRC7) Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 16:57 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 16/20] sdcard: Add test_sd_response_frame136_crc7() qtest Philippe Mathieu-Daudé
` (5 subsequent siblings)
20 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
The 'read CID/CSD register' request returns a 136-bit frame
containing those 128-bit registers.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/sd/sd.h | 29 +++++++++++++++++++++++++++++
hw/sd/sdmmc-internal.c | 15 +++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 13de1b30c3..c76be51b32 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -91,6 +91,17 @@ typedef struct SDFrame48 {
uint8_t crc;
} SDFrame48;
+/**
+ * SDFrame136: 136 bits CID or CSD responses
+ *
+ * @content: CID or CSD register
+ * @crc: 7-bit CRC checksum
+ */
+typedef struct SDFrame136 {
+ uint8_t content[15];
+ uint8_t crc;
+} SDFrame136;
+
typedef struct SDFrame48 SDRequest;
typedef struct SDState SDState;
@@ -193,6 +204,14 @@ void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
*/
void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
+/**
+ * sd_update_frame136_checksum:
+ * @frame: the #SDFrame136 to verify
+ *
+ * Update the 16-bit CRC checksum of a SD 136-bit frame.
+ */
+void sd_update_frame136_checksum(SDFrame136 *frame);
+
/**
* sd_verify_frame48_checksum:
* @frame: the #SDFrame48 to verify
@@ -204,6 +223,16 @@ void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
*/
bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response);
+/**
+ * sd_verify_frame136_checksum:
+ * @frame: the #SDFrame48 to verify
+ *
+ * Verify the 16-bit CRC checksum of a SD 136-bit frame.
+ *
+ * Returns: A boolean indicating whether the frame 16-bit CRC is correct.
+ */
+bool sd_verify_frame136_checksum(SDFrame136 *frame);
+
/* Legacy functions to be used only by non-qdevified callers */
SDState *sd_init(BlockBackend *bs, bool is_spi);
int sd_do_command(SDState *sd, SDRequest *req,
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index c990cc9e8e..68350a2304 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -117,6 +117,11 @@ static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg, bool is_response)
return sd_crc7(buffer, sizeof(buffer));
}
+static uint8_t sd_calc_frame136_crc7(SDFrame136 *frame)
+{
+ return (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
+}
+
bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response)
{
uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
@@ -124,11 +129,21 @@ bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response)
return crc == frame->crc;
}
+bool sd_verify_frame136_checksum(SDFrame136 *frame)
+{
+ return sd_calc_frame136_crc7(frame) == frame->crc;
+}
+
void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
{
frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
}
+void sd_update_frame136_checksum(SDFrame136 *frame)
+{
+ frame->crc = (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
+}
+
void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
bool is_response, bool gen_crc)
{
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 15/20] sdcard: Add SDFrame136 struct and 136-bit SD response frames functions
2018-05-04 15:59 ` [Qemu-devel] [PATCH 15/20] sdcard: Add SDFrame136 struct and 136-bit SD response frames functions Philippe Mathieu-Daudé
@ 2018-05-04 16:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 16:57 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
On 05/04/2018 12:59 PM, Philippe Mathieu-Daudé wrote:
> The 'read CID/CSD register' request returns a 136-bit frame
> containing those 128-bit registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/sd/sd.h | 29 +++++++++++++++++++++++++++++
> hw/sd/sdmmc-internal.c | 15 +++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 13de1b30c3..c76be51b32 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -91,6 +91,17 @@ typedef struct SDFrame48 {
> uint8_t crc;
> } SDFrame48;
>
> +/**
> + * SDFrame136: 136 bits CID or CSD responses
> + *
> + * @content: CID or CSD register
> + * @crc: 7-bit CRC checksum
> + */
> +typedef struct SDFrame136 {
> + uint8_t content[15];
> + uint8_t crc;
> +} SDFrame136;
> +
> typedef struct SDFrame48 SDRequest;
>
> typedef struct SDState SDState;
> @@ -193,6 +204,14 @@ void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
> */
> void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
>
> +/**
> + * sd_update_frame136_checksum:
> + * @frame: the #SDFrame136 to verify
> + *
> + * Update the 16-bit CRC checksum of a SD 136-bit frame.
Oops this should be:
"Update the 7-bit CRC checksum of a SD 136-bit frame."
> + */
> +void sd_update_frame136_checksum(SDFrame136 *frame);
> +
> /**
> * sd_verify_frame48_checksum:
> * @frame: the #SDFrame48 to verify
> @@ -204,6 +223,16 @@ void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
> */
> bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response);
>
> +/**
> + * sd_verify_frame136_checksum:
> + * @frame: the #SDFrame48 to verify
> + *
> + * Verify the 16-bit CRC checksum of a SD 136-bit frame.
And:
"Verify the 7-bit CRC checksum of a SD 136-bit frame."
> + *
> + * Returns: A boolean indicating whether the frame 16-bit CRC is correct.
> + */
> +bool sd_verify_frame136_checksum(SDFrame136 *frame);
> +
> /* Legacy functions to be used only by non-qdevified callers */
> SDState *sd_init(BlockBackend *bs, bool is_spi);
> int sd_do_command(SDState *sd, SDRequest *req,
> diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
> index c990cc9e8e..68350a2304 100644
> --- a/hw/sd/sdmmc-internal.c
> +++ b/hw/sd/sdmmc-internal.c
> @@ -117,6 +117,11 @@ static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg, bool is_response)
> return sd_crc7(buffer, sizeof(buffer));
> }
>
> +static uint8_t sd_calc_frame136_crc7(SDFrame136 *frame)
> +{
> + return (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
> +}
> +
> bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response)
> {
> uint8_t crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
> @@ -124,11 +129,21 @@ bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response)
> return crc == frame->crc;
> }
>
> +bool sd_verify_frame136_checksum(SDFrame136 *frame)
> +{
> + return sd_calc_frame136_crc7(frame) == frame->crc;
> +}
> +
> void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
> {
> frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
> }
>
> +void sd_update_frame136_checksum(SDFrame136 *frame)
> +{
> + frame->crc = (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
> +}
> +
> void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
> bool is_response, bool gen_crc)
> {
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 16/20] sdcard: Add test_sd_response_frame136_crc7() qtest
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (14 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 15/20] sdcard: Add SDFrame136 struct and 136-bit SD response frames functions Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 17/20] sdcard: Add SDFrameData struct and data frame checksum functions Philippe Mathieu-Daudé
` (4 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/sdcard-test.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tests/sdcard-test.c b/tests/sdcard-test.c
index 2de0a5a0ee..efcfbfc033 100644
--- a/tests/sdcard-test.c
+++ b/tests/sdcard-test.c
@@ -66,12 +66,25 @@ static void test_sd_response_frame48_crc7(void)
g_assert_cmphex(frame.crc, ==, 0x0c);
}
+static void test_sd_response_frame136_crc7(void)
+{
+ SDFrame136 frame;
+
+ /* response to CMD2 ALL_SEND_CID */
+ memcpy(&frame.content,
+ "\x1d\x41\x44\x53\x44\x20\x20\x20\x10\xa0\x40\x0b\xc1\x00\x88",
+ sizeof(frame.content));
+ sd_update_frame136_checksum(&frame);
+ g_assert_cmphex(frame.crc, ==, 0xad);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
qtest_add_func("sd/req_crc7", test_sd_request_frame_crc7);
qtest_add_func("sd/resp48_crc7", test_sd_response_frame48_crc7);
+ qtest_add_func("sd/resp136_crc7", test_sd_response_frame136_crc7);
return g_test_run();
}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 17/20] sdcard: Add SDFrameData struct and data frame checksum functions
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (15 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 16/20] sdcard: Add test_sd_response_frame136_crc7() qtest Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-05 2:22 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 18/20] sdcard: Fix sd_crc16() Philippe Mathieu-Daudé
` (3 subsequent siblings)
20 siblings, 1 reply; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
The block transfers data frames are upto 512 bytes and use a 16-bit CRC.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/sd/sd.h | 29 +++++++++++++++++++++++++++++
hw/sd/sdmmc-internal.c | 10 ++++++++++
2 files changed, 39 insertions(+)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index c76be51b32..6bf9daf559 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -102,6 +102,17 @@ typedef struct SDFrame136 {
uint8_t crc;
} SDFrame136;
+/**
+ * SDFrameData: 512 bytes block transfers
+ *
+ * @content: block data
+ * @crc: 16-bit CRC checksum
+ */
+typedef struct SDFrameData {
+ uint8_t content[512];
+ uint16_t crc;
+} SDFrameData;
+
typedef struct SDFrame48 SDRequest;
typedef struct SDState SDState;
@@ -212,6 +223,14 @@ void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
*/
void sd_update_frame136_checksum(SDFrame136 *frame);
+/**
+ * sd_update_framedata_checksum:
+ * @frame: the #SDFrameData to verify
+ *
+ * Update the 16-bit CRC checksum of a SD data frame (up to 512 bytes).
+ */
+void sd_update_framedata_checksum(SDFrameData *frame);
+
/**
* sd_verify_frame48_checksum:
* @frame: the #SDFrame48 to verify
@@ -233,6 +252,16 @@ bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response);
*/
bool sd_verify_frame136_checksum(SDFrame136 *frame);
+/**
+ * sd_verify_framedata_checksum:
+ * @frame: the #SDFrameData to verify
+ *
+ * Verify the 16-bit CRC checksum of a SD data frame.
+ *
+ * Returns: A boolean indicating whether the frame 16-bit CRC is correct.
+ */
+bool sd_verify_framedata_checksum(SDFrameData *frame);
+
/* Legacy functions to be used only by non-qdevified callers */
SDState *sd_init(BlockBackend *bs, bool is_spi);
int sd_do_command(SDState *sd, SDRequest *req,
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 68350a2304..0e82e69d99 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -134,6 +134,11 @@ bool sd_verify_frame136_checksum(SDFrame136 *frame)
return sd_calc_frame136_crc7(frame) == frame->crc;
}
+bool sd_verify_framedata_checksum(SDFrameData *frame)
+{
+ return sd_crc16(frame->content, sizeof(frame->content)) == frame->crc;
+}
+
void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
{
frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
@@ -144,6 +149,11 @@ void sd_update_frame136_checksum(SDFrame136 *frame)
frame->crc = (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
}
+void sd_update_framedata_checksum(SDFrameData *frame)
+{
+ frame->crc = sd_crc16(frame->content, sizeof(frame->content));
+}
+
void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
bool is_response, bool gen_crc)
{
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 17/20] sdcard: Add SDFrameData struct and data frame checksum functions
2018-05-04 15:59 ` [Qemu-devel] [PATCH 17/20] sdcard: Add SDFrameData struct and data frame checksum functions Philippe Mathieu-Daudé
@ 2018-05-05 2:22 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-05 2:22 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
On 05/04/2018 12:59 PM, Philippe Mathieu-Daudé wrote:
> The block transfers data frames are upto 512 bytes and use a 16-bit CRC.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/sd/sd.h | 29 +++++++++++++++++++++++++++++
> hw/sd/sdmmc-internal.c | 10 ++++++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index c76be51b32..6bf9daf559 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -102,6 +102,17 @@ typedef struct SDFrame136 {
> uint8_t crc;
> } SDFrame136;
>
> +/**
> + * SDFrameData: 512 bytes block transfers
> + *
> + * @content: block data
> + * @crc: 16-bit CRC checksum
> + */
> +typedef struct SDFrameData {
> + uint8_t content[512];
> + uint16_t crc;
> +} SDFrameData;
> +
> typedef struct SDFrame48 SDRequest;
>
> typedef struct SDState SDState;
> @@ -212,6 +223,14 @@ void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response);
> */
> void sd_update_frame136_checksum(SDFrame136 *frame);
>
> +/**
> + * sd_update_framedata_checksum:
> + * @frame: the #SDFrameData to verify
> + *
> + * Update the 16-bit CRC checksum of a SD data frame (up to 512 bytes).
> + */
> +void sd_update_framedata_checksum(SDFrameData *frame);
Since the frame size is variable, this function needs a size_t argument.
> +
> /**
> * sd_verify_frame48_checksum:
> * @frame: the #SDFrame48 to verify
> @@ -233,6 +252,16 @@ bool sd_verify_frame48_checksum(SDFrame48 *frame, bool is_response);
> */
> bool sd_verify_frame136_checksum(SDFrame136 *frame);
>
> +/**
> + * sd_verify_framedata_checksum:
> + * @frame: the #SDFrameData to verify
> + *
> + * Verify the 16-bit CRC checksum of a SD data frame.
> + *
> + * Returns: A boolean indicating whether the frame 16-bit CRC is correct.
> + */
> +bool sd_verify_framedata_checksum(SDFrameData *frame);
Ditto.
> +
> /* Legacy functions to be used only by non-qdevified callers */
> SDState *sd_init(BlockBackend *bs, bool is_spi);
> int sd_do_command(SDState *sd, SDRequest *req,
> diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
> index 68350a2304..0e82e69d99 100644
> --- a/hw/sd/sdmmc-internal.c
> +++ b/hw/sd/sdmmc-internal.c
> @@ -134,6 +134,11 @@ bool sd_verify_frame136_checksum(SDFrame136 *frame)
> return sd_calc_frame136_crc7(frame) == frame->crc;
> }
>
> +bool sd_verify_framedata_checksum(SDFrameData *frame)
> +{
> + return sd_crc16(frame->content, sizeof(frame->content)) == frame->crc;
I'll probably replace sizeof(frame->content) by a 'length' argument,
or add a 'length' member to SDFrameData.
> +}
> +
> void sd_update_frame48_checksum(SDFrame48 *frame, bool is_response)
> {
> frame->crc = sd_calc_frame48_crc7(frame->cmd, frame->arg, is_response);
> @@ -144,6 +149,11 @@ void sd_update_frame136_checksum(SDFrame136 *frame)
> frame->crc = (sd_crc7(frame->content, sizeof(frame->content)) << 1) | 1;
> }
>
> +void sd_update_framedata_checksum(SDFrameData *frame)
> +{
> + frame->crc = sd_crc16(frame->content, sizeof(frame->content));
> +}
> +
> void sd_prepare_frame48(SDFrame48 *frame, uint8_t cmd, uint32_t arg,
> bool is_response, bool gen_crc)
> {
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 18/20] sdcard: Fix sd_crc16()
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (16 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 17/20] sdcard: Add SDFrameData struct and data frame checksum functions Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 19/20] sdcard: Add test_sd_data_frame_crc16() qtest Philippe Mathieu-Daudé
` (2 subsequent siblings)
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
The current sd_crc16() function does not pass the Spec test,
change it by a working one.
Code generated with pycrc v0.9.1 (https://pycrc.org) using the configuration:
- Width = 16
- Poly = 0x1021
- XorIn = 0x0000
- ReflectIn = False
- XorOut = 0x0000
- ReflectOut = False
- Algorithm = bit-by-bit-fast
Copyright of the generated source code
======================================
The code generated by pycrc is not considered a substantial portion of the
software, therefore the licence does not cover the generated code, and the
author of pycrc will not claim any copyright on the generated code.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdmmc-internal.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 0e82e69d99..d6f9b3b51c 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -90,23 +90,31 @@ uint8_t sd_crc7(const void *message, size_t width)
return shift_reg;
}
-uint16_t sd_crc16(const void *message, size_t width)
+/* 16 bit XMODEM CRC (polynomial 0x1021) */
+uint16_t sd_crc16(const void *data, size_t data_len)
{
- int i, bit;
- uint16_t shift_reg = 0x0000;
- const uint16_t *msg = (const uint16_t *)message;
- width <<= 1;
-
- for (i = 0; i < width; i++, msg++) {
- for (bit = 15; bit >= 0; bit--) {
- shift_reg <<= 1;
- if ((shift_reg >> 15) ^ ((*msg >> bit) & 1)) {
- shift_reg ^= 0x1011;
+ const unsigned char *d = (const unsigned char *)data;
+ uint16_t crc = 0x0000;
+ unsigned char c;
+ unsigned int i;
+ bool bit;
+
+ while (data_len--) {
+ c = *d++;
+ for (i = 0x80; i > 0; i >>= 1) {
+ bit = crc & 0x8000;
+ if (c & i) {
+ bit = !bit;
+ }
+ crc <<= 1;
+ if (bit) {
+ crc ^= 0x1021;
}
}
+ crc &= 0xffff;
}
- return shift_reg;
+ return crc;
}
static uint8_t sd_calc_frame48_crc7(uint8_t cmd, uint32_t arg, bool is_response)
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 19/20] sdcard: Add test_sd_data_frame_crc16() qtest
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (17 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 18/20] sdcard: Fix sd_crc16() Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-04 15:59 ` [Qemu-devel] [PATCH 20/20] sdcard: Add test_sd_verify_cksum_frame48() qtest Philippe Mathieu-Daudé
2018-05-07 20:09 ` [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/sdcard-test.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tests/sdcard-test.c b/tests/sdcard-test.c
index efcfbfc033..c51e8095b6 100644
--- a/tests/sdcard-test.c
+++ b/tests/sdcard-test.c
@@ -78,6 +78,15 @@ static void test_sd_response_frame136_crc7(void)
g_assert_cmphex(frame.crc, ==, 0xad);
}
+static void test_sd_data_frame_crc16(void)
+{
+ SDFrameData frame;
+
+ memset(frame.content, 0xff, sizeof(frame.content));
+ sd_update_framedata_checksum(&frame);
+ g_assert_cmphex(frame.crc, ==, 0x7fa1);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -85,6 +94,7 @@ int main(int argc, char *argv[])
qtest_add_func("sd/req_crc7", test_sd_request_frame_crc7);
qtest_add_func("sd/resp48_crc7", test_sd_response_frame48_crc7);
qtest_add_func("sd/resp136_crc7", test_sd_response_frame136_crc7);
+ qtest_add_func("sd/data_crc16", test_sd_data_frame_crc16);
return g_test_run();
}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 20/20] sdcard: Add test_sd_verify_cksum_frame48() qtest
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (18 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 19/20] sdcard: Add test_sd_data_frame_crc16() qtest Philippe Mathieu-Daudé
@ 2018-05-04 15:59 ` Philippe Mathieu-Daudé
2018-05-07 20:09 ` [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-04 15:59 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: Philippe Mathieu-Daudé,
qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
tests/sdcard-test.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tests/sdcard-test.c b/tests/sdcard-test.c
index c51e8095b6..fa45188e48 100644
--- a/tests/sdcard-test.c
+++ b/tests/sdcard-test.c
@@ -47,6 +47,8 @@ static void test_sd_request_frame_crc7(void)
/* CMD2 ALL_SEND_CID */
sd_prepare_request48(&frame, 2, 0);
g_assert_cmphex(frame.crc, ==, 0x4d >> 1);
+
+ g_assert_true(sd_verify_frame48_checksum(&frame, false));
}
static void test_sd_response_frame48_crc7(void)
@@ -64,6 +66,8 @@ static void test_sd_response_frame48_crc7(void)
/* response to CMD3 SEND_RELATIVE_ADDR (Relative Card Address is 0xb368) */
sd_prepare_response48(&frame, 3, 0xb3680500);
g_assert_cmphex(frame.crc, ==, 0x0c);
+
+ g_assert_true(sd_verify_frame48_checksum(&frame, true));
}
static void test_sd_response_frame136_crc7(void)
@@ -87,6 +91,17 @@ static void test_sd_data_frame_crc16(void)
g_assert_cmphex(frame.crc, ==, 0x7fa1);
}
+static void test_sd_verify_cksum_frame48(void)
+{
+ SDFrame48 frame;
+
+ sd_prepare_request48(&frame, 42, 0x12345678);
+ g_assert_true(sd_verify_frame48_checksum(&frame, false));
+
+ sd_prepare_response48(&frame, 69, 0x98765432);
+ g_assert_true(sd_verify_frame48_checksum(&frame, true));
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -95,6 +110,7 @@ int main(int argc, char *argv[])
qtest_add_func("sd/resp48_crc7", test_sd_response_frame48_crc7);
qtest_add_func("sd/resp136_crc7", test_sd_response_frame136_crc7);
qtest_add_func("sd/data_crc16", test_sd_data_frame_crc16);
+ qtest_add_func("sd/verify_cksum_frame48", test_sd_verify_cksum_frame48);
return g_test_run();
}
--
2.17.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC
2018-05-04 15:58 [Qemu-devel] [PATCH 00/20] sdcard: proper implementation of CRC Philippe Mathieu-Daudé
` (19 preceding siblings ...)
2018-05-04 15:59 ` [Qemu-devel] [PATCH 20/20] sdcard: Add test_sd_verify_cksum_frame48() qtest Philippe Mathieu-Daudé
@ 2018-05-07 20:09 ` Philippe Mathieu-Daudé
20 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-05-07 20:09 UTC (permalink / raw)
To: Peter Maydell, Edgar E . Iglesias
Cc: qemu-devel, Paolo Bonzini, Stefan Hajnoczi, Alistair Francis
On 05/04/2018 12:58 PM, Philippe Mathieu-Daudé wrote:
> This series emerged after last Coverity scan and Peter suggestion in:
> http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg05046.html
>
> (3) "proper" implementation of CRC, so that an sd controller
> can either (a) mark the SDRequest as "no CRC" and have
> sd_req_crc_validate() always pass, or (b) mark the SDRequest
> as having a CRC and have sd_req_crc_validate() actually
> do the check which it currently stubs out with "return 0"
>
> - Coverity issues fixed (up to patch 6)
> - crc16 now works
> - new functions documented
> - qtests added
>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (20):
> sdcard: Use the ldst API
> sdcard: Extract sd_calc_frame48_crc7() from sd_req_crc_validate()
> sdcard: Rename the SDRequest as SDFrame48
> sdcard: Add sd_prepare_request[_with_crc]()
> sdcard: Use the sd_prepare_request*() functions
> sdcard: Add a "validate-crc" property
> sdcard: Constify sd_crc*()'s message argument
> sdcard: Fix sd_crc*() style
> sdcard: Expose sd_crc*() functions for QTest use
> sdcard: Expose sd_prepare_request*() functions for QTest use
> sdcard: Add test_sd_request_frame_crc7() qtest (request command CRC7)
> sdcard: Let sd_frame48_crc7_calc() work on response frames
> sdcard: Expose sd_prepare_frame48() for QTest use
> sdcard: Add test_sd_response_frame48_crc7 qtest (command response CRC7)
> sdcard: Add SDFrame136 struct and 136-bit SD response frames functions
> sdcard: Add test_sd_response_frame136_crc7() qtest
> sdcard: Add SDFrameData struct and data frame checksum functions
> sdcard: Fix sd_crc16()
> sdcard: Add test_sd_data_frame_crc16() qtest
> sdcard: Add test_sd_verify_cksum_frame48() qtest
Please disregard this series, I'll respin a simpler approach.
^ permalink raw reply [flat|nested] 26+ messages in thread