qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: qemu-devel@nongnu.org, Peter Maydell <peter.maydell@linaro.org>
Cc: "Guenter Roeck" <linux@roeck-us.net>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
	"Daniel Henrique Barboza" <dbarboza@ventanamicro.com>,
	"Strahinja Jankovic" <strahinja.p.jankovic@gmail.com>,
	qemu-riscv@nongnu.org, qemu-arm@nongnu.org,
	"Bin Meng" <bmeng.cn@gmail.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Beniamino Galvani" <b.galvani@gmail.com>,
	"Ben Dooks" <ben.dooks@codethink.co.uk>,
	"Weiwei Li" <liwei1518@gmail.com>,
	qemu-block@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: [PATCH-for-10.1 02/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command()
Date: Thu, 31 Jul 2025 23:27:57 +0200	[thread overview]
Message-ID: <20250731212807.2706-3-philmd@linaro.org> (raw)
In-Reply-To: <20250731212807.2706-1-philmd@linaro.org>

We provide to sdbus_do_command() a pointer to a buffer to be
filled with a varying number of bytes. By not providing the
buffer size, the callee can not check the buffer is big enough.
Pass the buffer size as argument to follow good practices.

sdbus_do_command() doesn't return any error, only the size filled
in the buffer. Convert the returned type to unsigned and remove
the few unreachable lines in callers.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sd/sd.h       | 23 +++++++++++++++++++++--
 hw/sd/allwinner-sdhost.c |  5 +----
 hw/sd/bcm2835_sdhost.c   |  5 +----
 hw/sd/core.c             |  5 +++--
 hw/sd/omap_mmc.c         |  2 +-
 hw/sd/pl181.c            |  4 +---
 hw/sd/sd.c               |  5 +++--
 hw/sd/sdhci.c            |  4 ++--
 hw/sd/ssi-sd.c           |  8 +++++---
 9 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index d6bad175131..55d363f58fb 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -96,7 +96,17 @@ struct SDCardClass {
     DeviceClass parent_class;
     /*< public >*/
 
-    int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
+    /**
+     * Process a SD command request.
+     * @sd: card
+     * @req: command request
+     * @resp: buffer to receive the command response
+     * @respsz: size of @resp buffer
+     *
+     * Return: size of the response
+     */
+    size_t (*do_command)(SDState *sd, SDRequest *req,
+                         uint8_t *resp, size_t respsz);
     /**
      * Write a byte to a SD card.
      * @sd: card
@@ -153,7 +163,16 @@ struct SDBusClass {
 void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts);
 uint8_t sdbus_get_dat_lines(SDBus *sdbus);
 bool sdbus_get_cmd_line(SDBus *sdbus);
-int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response);
+/**
+ * sdbus_do_command: Process a SD command request
+ * @sd: card
+ * @req: command request
+ * @resp: buffer to receive the command response
+ * @respsz: size of @resp buffer
+ *
+ * Return: size of the response
+ */
+size_t sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *resp, size_t respsz);
 /**
  * Write a byte to a SD bus.
  * @sd: bus
diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c
index b31da5c399c..65bef08fc83 100644
--- a/hw/sd/allwinner-sdhost.c
+++ b/hw/sd/allwinner-sdhost.c
@@ -246,10 +246,7 @@ static void allwinner_sdhost_send_command(AwSdHostState *s)
         request.arg = s->command_arg;
 
         /* Send request to SD bus */
-        rlen = sdbus_do_command(&s->sdbus, &request, resp);
-        if (rlen < 0) {
-            goto error;
-        }
+        rlen = sdbus_do_command(&s->sdbus, &request, resp, sizeof(resp));
 
         /* If the command has a response, store it in the response registers */
         if ((s->command & SD_CMDR_RESPONSE)) {
diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
index 29debdf59e4..2b3160f05f3 100644
--- a/hw/sd/bcm2835_sdhost.c
+++ b/hw/sd/bcm2835_sdhost.c
@@ -118,10 +118,7 @@ static void bcm2835_sdhost_send_command(BCM2835SDHostState *s)
     request.cmd = s->cmd & SDCMD_CMD_MASK;
     request.arg = s->cmdarg;
 
-    rlen = sdbus_do_command(&s->sdbus, &request, rsp);
-    if (rlen < 0) {
-        goto error;
-    }
+    rlen = sdbus_do_command(&s->sdbus, &request, rsp, sizeof(rsp));
     if (!(s->cmd & SDCMD_NO_RESPONSE)) {
         if (rlen == 0 || (rlen == 4 && (s->cmd & SDCMD_LONG_RESPONSE))) {
             goto error;
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 4b30218b520..d3c9017445e 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -90,7 +90,8 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
     }
 }
 
-int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
+size_t sdbus_do_command(SDBus *sdbus, SDRequest *req,
+                        uint8_t *resp, size_t respsz)
 {
     SDState *card = get_card(sdbus);
 
@@ -98,7 +99,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
     if (card) {
         SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
-        return sc->do_command(card, req, response);
+        return sc->do_command(card, req, resp, respsz);
     }
 
     return 0;
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index b7648d41cc5..1520ca87b74 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -157,7 +157,7 @@ static void omap_mmc_command(OMAPMMCState *host, int cmd, int dir,
     request.arg = host->arg;
     request.crc = 0; /* FIXME */
 
-    rsplen = sdbus_do_command(&host->sdbus, &request, response);
+    rsplen = sdbus_do_command(&host->sdbus, &request, response, sizeof(response));
 
     /* TODO: validate CRCs */
     switch (resptype) {
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index b8fc9f86f13..b8072530d65 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -178,9 +178,7 @@ static void pl181_do_command(PL181State *s)
     request.cmd = s->cmd & PL181_CMD_INDEX;
     request.arg = s->cmdarg;
     trace_pl181_command_send(request.cmd, request.arg);
-    rlen = sdbus_do_command(&s->sdbus, &request, response);
-    if (rlen < 0)
-        goto error;
+    rlen = sdbus_do_command(&s->sdbus, &request, response, sizeof(response));
     if (s->cmd & PL181_CMD_RESPONSE) {
         if (rlen == 0 || (rlen == 4 && (s->cmd & PL181_CMD_LONGRESP)))
             goto error;
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0bb385268ed..1d88aee38d5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2139,8 +2139,9 @@ static bool cmd_valid_while_locked(SDState *sd, unsigned cmd)
     return cmd_class == 0 || cmd_class == 7;
 }
 
-static int sd_do_command(SDState *sd, SDRequest *req,
-                         uint8_t *response) {
+static size_t sd_do_command(SDState *sd, SDRequest *req,
+                            uint8_t *response, size_t respsz)
+{
     int last_state;
     sd_rsp_type_t rtype;
     int rsplen;
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 226ff133ff9..f1e149f46f3 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -346,7 +346,7 @@ static void sdhci_send_command(SDHCIState *s)
     request.arg = s->argument;
 
     trace_sdhci_send_command(request.cmd, request.arg);
-    rlen = sdbus_do_command(&s->sdbus, &request, response);
+    rlen = sdbus_do_command(&s->sdbus, &request, response, sizeof(response));
 
     if (s->cmdreg & SDHC_CMD_RESPONSE) {
         if (rlen == 4) {
@@ -400,7 +400,7 @@ static void sdhci_end_transfer(SDHCIState *s)
         request.cmd = 0x0C;
         request.arg = 0;
         trace_sdhci_end_transfer(request.cmd, request.arg);
-        sdbus_do_command(&s->sdbus, &request, response);
+        sdbus_do_command(&s->sdbus, &request, response, sizeof(response));
         /* Auto CMD12 response goes to the upper Response register */
         s->rspreg[3] = ldl_be_p(response);
     }
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 6c90a86ab41..3aba0e08ffe 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -146,7 +146,8 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
             /* manually issue cmd12 to stop the transfer */
             request.cmd = 12;
             request.arg = 0;
-            s->arglen = sdbus_do_command(&s->sdbus, &request, longresp);
+            s->arglen = sdbus_do_command(&s->sdbus, &request,
+                                         longresp, sizeof(longresp));
             if (s->arglen <= 0) {
                 s->arglen = 1;
                 /* a zero value indicates the card is busy */
@@ -171,8 +172,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
             request.cmd = s->cmd;
             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) {
+            s->arglen = sdbus_do_command(&s->sdbus, &request,
+                                         longresp, sizeof(longresp));
+            if (s->arglen == 0) {
                 s->arglen = 1;
                 s->response[0] = 4;
                 DPRINTF("SD command failed\n");
-- 
2.49.0



  parent reply	other threads:[~2025-07-31 21:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-31 21:27 [PATCH-for-10.1 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
2025-07-31 21:27 ` [PATCH-for-10.1 01/11] hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata() Philippe Mathieu-Daudé
2025-08-01  6:40   ` Richard Henderson
2025-07-31 21:27 ` Philippe Mathieu-Daudé [this message]
2025-08-01  7:08   ` [PATCH-for-10.1 02/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command() Richard Henderson
2025-07-31 21:27 ` [PATCH-for-10.1 03/11] hw/sd/sdcard: Propagate response size to sd_response_r*_make() Philippe Mathieu-Daudé
2025-08-01  7:17   ` Richard Henderson
2025-07-31 21:27 ` [PATCH-for-10.1 04/11] hw/sd/sdcard: Fill SPI response bits in card code Philippe Mathieu-Daudé
2025-08-01  7:19   ` Richard Henderson
2025-08-04  9:11     ` Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 05/11] hw/sd/sdcard: Implement SPI R2 return value Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 06/11] hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 07/11] hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 08/11] hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 09/11] hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 10/11] hw/sd/sdcard: Remove SDState::mode field Philippe Mathieu-Daudé
2025-07-31 21:28 ` [PATCH-for-10.1 11/11] tests/functional: Test SD cards in SPI mode (using sifive_u machine) Philippe Mathieu-Daudé
2025-07-31 21:34 ` [PATCH-for-10.1 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250731212807.2706-3-philmd@linaro.org \
    --to=philmd@linaro.org \
    --cc=alistair.francis@wdc.com \
    --cc=b.galvani@gmail.com \
    --cc=ben.dooks@codethink.co.uk \
    --cc=bmeng.cn@gmail.com \
    --cc=dbarboza@ventanamicro.com \
    --cc=linux@roeck-us.net \
    --cc=liwei1518@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=strahinja.p.jankovic@gmail.com \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).