[Qemu-devel] [PATCH v2] register: fix incorrect read mask

Philippe Mathieu-Daudé posted 1 patch 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170211034332.12498-1-f4bug@amsat.org
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
hw/core/register.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH v2] register: fix incorrect read mask
Posted by Philippe Mathieu-Daudé 7 years, 2 months ago
The register_read() function expects a bitmask argument. To avoid duplicated
code, a new inlined function register_enabled_mask() is introduced.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 v2: squashed together both previous commits as suggested by Francis Alistair.

 hw/core/register.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index 4bfbc508de..482e3f827a 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -59,6 +59,15 @@ static inline uint64_t register_read_val(RegisterInfo *reg)
     return 0; /* unreachable */
 }
 
+static inline uint64_t register_enabled_mask(int data_size, unsigned size)
+{
+    if (data_size < size) {
+        size = data_size;
+    }
+
+    return MAKE_64BIT_MASK(0, size * 8);
+}
+
 void register_write(RegisterInfo *reg, uint64_t val, uint64_t we,
                     const char *prefix, bool debug)
 {
@@ -192,11 +201,7 @@ void register_write_memory(void *opaque, hwaddr addr,
     }
 
     /* Generate appropriate write enable mask */
-    if (reg->data_size < size) {
-        we = MAKE_64BIT_MASK(0, reg->data_size * 8);
-    } else {
-        we = MAKE_64BIT_MASK(0, size * 8);
-    }
+    we = register_enabled_mask(reg->data_size, size);
 
     register_write(reg, value, we, reg_array->prefix,
                    reg_array->debug);
@@ -208,6 +213,7 @@ uint64_t register_read_memory(void *opaque, hwaddr addr,
     RegisterInfoArray *reg_array = opaque;
     RegisterInfo *reg = NULL;
     uint64_t read_val;
+    uint64_t re;
     int i;
 
     for (i = 0; i < reg_array->num_elements; i++) {
@@ -223,7 +229,10 @@ uint64_t register_read_memory(void *opaque, hwaddr addr,
         return 0;
     }
 
-    read_val = register_read(reg, size * 8, reg_array->prefix,
+    /* Generate appropriate read enable mask */
+    re = register_enabled_mask(reg->data_size, size);
+
+    read_val = register_read(reg, re, reg_array->prefix,
                              reg_array->debug);
 
     return extract64(read_val, 0, size * 8);
-- 
2.11.0


Re: [Qemu-devel] [PATCH v2] register: fix incorrect read mask
Posted by Alistair Francis 7 years, 2 months ago
On Fri, Feb 10, 2017 at 7:43 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The register_read() function expects a bitmask argument. To avoid duplicated

The register_read() and register_write() functions

> code, a new inlined function register_enabled_mask() is introduced.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
>  v2: squashed together both previous commits as suggested by Francis Alistair.
>
>  hw/core/register.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/register.c b/hw/core/register.c
> index 4bfbc508de..482e3f827a 100644
> --- a/hw/core/register.c
> +++ b/hw/core/register.c
> @@ -59,6 +59,15 @@ static inline uint64_t register_read_val(RegisterInfo *reg)
>      return 0; /* unreachable */
>  }
>
> +static inline uint64_t register_enabled_mask(int data_size, unsigned size)
> +{
> +    if (data_size < size) {
> +        size = data_size;
> +    }
> +
> +    return MAKE_64BIT_MASK(0, size * 8);
> +}
> +
>  void register_write(RegisterInfo *reg, uint64_t val, uint64_t we,
>                      const char *prefix, bool debug)
>  {
> @@ -192,11 +201,7 @@ void register_write_memory(void *opaque, hwaddr addr,
>      }
>
>      /* Generate appropriate write enable mask */
> -    if (reg->data_size < size) {
> -        we = MAKE_64BIT_MASK(0, reg->data_size * 8);
> -    } else {
> -        we = MAKE_64BIT_MASK(0, size * 8);
> -    }
> +    we = register_enabled_mask(reg->data_size, size);
>
>      register_write(reg, value, we, reg_array->prefix,
>                     reg_array->debug);
> @@ -208,6 +213,7 @@ uint64_t register_read_memory(void *opaque, hwaddr addr,
>      RegisterInfoArray *reg_array = opaque;
>      RegisterInfo *reg = NULL;
>      uint64_t read_val;
> +    uint64_t re;
>      int i;
>
>      for (i = 0; i < reg_array->num_elements; i++) {
> @@ -223,7 +229,10 @@ uint64_t register_read_memory(void *opaque, hwaddr addr,
>          return 0;
>      }
>
> -    read_val = register_read(reg, size * 8, reg_array->prefix,
> +    /* Generate appropriate read enable mask */
> +    re = register_enabled_mask(reg->data_size, size);
> +
> +    read_val = register_read(reg, re, reg_array->prefix,
>                               reg_array->debug);
>
>      return extract64(read_val, 0, size * 8);
> --
> 2.11.0
>
>