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

Philippe Mathieu-Daudé posted 1 patch 3 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210218175648.1636219-1-f4bug@amsat.org
hw/sd/sdhci.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH] hw/sd/sdhci: Report error when guest access protected registers
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago
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
Posted by John Snow 3 years, 2 months ago
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
Posted by Bin Meng 3 years, 2 months ago
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>