[PATCH 17/23] hw/sd/sdcard: Only call sd_req_get_address() where address is used

Philippe Mathieu-Daudé posted 23 patches 5 months ago
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Bin Meng <bmeng.cn@gmail.com>
There is a newer version of this series
[PATCH 17/23] hw/sd/sdcard: Only call sd_req_get_address() where address is used
Posted by Philippe Mathieu-Daudé 5 months ago
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é <philmd@linaro.org>
---
 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
@@ -1111,7 +1111,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


Re: [PATCH 17/23] hw/sd/sdcard: Only call sd_req_get_address() where address is used
Posted by Cédric Le Goater 5 months ago
On 6/21/24 10:05 AM, Philippe Mathieu-Daudé wrote:
> 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é <philmd@linaro.org>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   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
> @@ -1111,7 +1111,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",