[PATCH] hw/sd/sdhci: Report error when guest access protected registers

[PATCH] hw/sd/sdhci: Report error when guest access protected registers

[Philippe Mathieu-Daudé]

The SDHC_SYSAD and SDHC_BLKSIZE can not be accessed while a
transaction is in progress, see 'SD Host Controller Simplified
Specification'

  1.5) SD Command Generation

  The Host Driver should not read the SDMA System Address, Block
  Size and Block Count registers during a data transaction unless
  the transfer is stopped because the value is changing and not
  stable.

Report guest intents as errors.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Based-on: <1613447214-81951-1-git-send-email-bmeng.cn@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sdhci.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 5b8678110b0..98928c18542 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1136,6 +1136,10 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
                     sdhci_sdma_transfer_single_block(s);
                 }
             }
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Transfer already in progress,"
+                          " can not update SYSAD", __func__);
         }
         break;
     case SDHC_BLKSIZE:
@@ -1163,8 +1167,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
             if (blksize != s->blksize) {
                 s->data_count = 0;
             }
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: Transfer already in progress,"
+                          " can not update BLKSIZE", __func__);
         }
-
         break;
     case SDHC_ARGUMENT:
         MASKED_WRITE(s->argument, mask, value);
-- 
2.26.2

Re: [PATCH] hw/sd/sdhci: Report error when guest access protected registers

[John Snow]

On 2/18/21 12:56 PM, Philippe Mathieu-Daudé wrote:
> The SDHC_SYSAD and SDHC_BLKSIZE can not be accessed while a
> transaction is in progress, see 'SD Host Controller Simplified
> Specification'
> 
>    1.5) SD Command Generation
> 
>    The Host Driver should not read the SDMA System Address, Block
>    Size and Block Count registers during a data transaction unless
>    the transfer is stopped because the value is changing and not
>    stable.
> 

Naive question: Is this an RFC2119 "SHOULD NOT"? (i.e., does it have a 
defined behavior that you are simply encouraged to avoid?)

Is it really an error?

> Report guest intents as errors.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Based-on: <1613447214-81951-1-git-send-email-bmeng.cn@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/sd/sdhci.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 5b8678110b0..98928c18542 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1136,6 +1136,10 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>                       sdhci_sdma_transfer_single_block(s);
>                   }
>               }
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: Transfer already in progress,"
> +                          " can not update SYSAD", __func__);
>           }
>           break;
>       case SDHC_BLKSIZE:
> @@ -1163,8 +1167,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>               if (blksize != s->blksize) {
>                   s->data_count = 0;
>               }
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: Transfer already in progress,"
> +                          " can not update BLKSIZE", __func__);
>           }
> -
>           break;
>       case SDHC_ARGUMENT:
>           MASKED_WRITE(s->argument, mask, value);
> 

Re: [PATCH] hw/sd/sdhci: Report error when guest access protected registers

[Bin Meng]

On Fri, Feb 19, 2021 at 1:56 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The SDHC_SYSAD and SDHC_BLKSIZE can not be accessed while a
> transaction is in progress, see 'SD Host Controller Simplified
> Specification'
>
>   1.5) SD Command Generation
>
>   The Host Driver should not read the SDMA System Address, Block
>   Size and Block Count registers during a data transaction unless
>   the transfer is stopped because the value is changing and not
>   stable.
>
> Report guest intents as errors.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Based-on: <1613447214-81951-1-git-send-email-bmeng.cn@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sdhci.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>