* [Qemu-devel] [PATCH 0/2] register: fix incorrect read mask
@ 2017-02-06 23:39 Philippe Mathieu-Daudé
2017-02-06 23:39 ` [Qemu-devel] [PATCH 1/2] register: inline register_enabled_mask Philippe Mathieu-Daudé
2017-02-06 23:39 ` [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask Philippe Mathieu-Daudé
0 siblings, 2 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-02-06 23:39 UTC (permalink / raw)
To: Peter Crosthwaite, Alistair Francis
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-trivial
Hi,
This serie fix a bug in register_read_memory(). To avoid duplicated code, a new
inlined function register_enabled_mask() is introduced in the first patch.
Philippe Mathieu-Daudé (2):
register: inline register_enabled_mask
register: fix incorrect read mask
hw/core/register.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] register: inline register_enabled_mask
2017-02-06 23:39 [Qemu-devel] [PATCH 0/2] register: fix incorrect read mask Philippe Mathieu-Daudé
@ 2017-02-06 23:39 ` Philippe Mathieu-Daudé
2017-02-06 23:39 ` [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask Philippe Mathieu-Daudé
1 sibling, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-02-06 23:39 UTC (permalink / raw)
To: Peter Crosthwaite, Alistair Francis
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-trivial
This patch prepares for the fix of register_read() incorrect mask.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/core/register.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/core/register.c b/hw/core/register.c
index 4bfbc508de..12f4c1e62d 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);
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask
2017-02-06 23:39 [Qemu-devel] [PATCH 0/2] register: fix incorrect read mask Philippe Mathieu-Daudé
2017-02-06 23:39 ` [Qemu-devel] [PATCH 1/2] register: inline register_enabled_mask Philippe Mathieu-Daudé
@ 2017-02-06 23:39 ` Philippe Mathieu-Daudé
2017-02-10 22:19 ` Alistair Francis
1 sibling, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-02-06 23:39 UTC (permalink / raw)
To: Peter Crosthwaite, Alistair Francis
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-trivial
The register_read() function expects a bitmask argument.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/core/register.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/core/register.c b/hw/core/register.c
index 12f4c1e62d..482e3f827a 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -213,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++) {
@@ -228,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask
2017-02-06 23:39 ` [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask Philippe Mathieu-Daudé
@ 2017-02-10 22:19 ` Alistair Francis
2017-02-11 3:16 ` Philippe Mathieu-Daudé
2017-02-11 3:43 ` [Qemu-devel] [PATCH v2] " Philippe Mathieu-Daudé
0 siblings, 2 replies; 7+ messages in thread
From: Alistair Francis @ 2017-02-10 22:19 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Crosthwaite, Alistair Francis, QEMU Trivial,
qemu-devel@nongnu.org Developers
On Mon, Feb 6, 2017 at 3:39 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> The register_read() function expects a bitmask argument.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
The functionality of these two patches looks good to me.
I think it's weird that we convert the write function in one patch
(when we add the function) and the read in another.
I think both patches should just be squashed together. It's a pretty
small change.
Thanks,
Alistair
> ---
> hw/core/register.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/register.c b/hw/core/register.c
> index 12f4c1e62d..482e3f827a 100644
> --- a/hw/core/register.c
> +++ b/hw/core/register.c
> @@ -213,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++) {
> @@ -228,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
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask
2017-02-10 22:19 ` Alistair Francis
@ 2017-02-11 3:16 ` Philippe Mathieu-Daudé
2017-02-11 3:43 ` [Qemu-devel] [PATCH v2] " Philippe Mathieu-Daudé
1 sibling, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-02-11 3:16 UTC (permalink / raw)
To: Alistair Francis
Cc: Peter Crosthwaite, QEMU Trivial, qemu-devel@nongnu.org Developers
On 02/10/2017 07:19 PM, Alistair Francis wrote:
> On Mon, Feb 6, 2017 at 3:39 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> The register_read() function expects a bitmask argument.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> The functionality of these two patches looks good to me.
>
> I think it's weird that we convert the write function in one patch
> (when we add the function) and the read in another.
>
> I think both patches should just be squashed together. It's a pretty
> small change.
>
> Thanks,
>
> Alistair
Sure, will do in v2.
Thanks!
>
>> ---
>> hw/core/register.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/register.c b/hw/core/register.c
>> index 12f4c1e62d..482e3f827a 100644
>> --- a/hw/core/register.c
>> +++ b/hw/core/register.c
>> @@ -213,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++) {
>> @@ -228,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
>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2] register: fix incorrect read mask
2017-02-10 22:19 ` Alistair Francis
2017-02-11 3:16 ` Philippe Mathieu-Daudé
@ 2017-02-11 3:43 ` Philippe Mathieu-Daudé
2017-02-14 1:20 ` Alistair Francis
1 sibling, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-02-11 3:43 UTC (permalink / raw)
To: Alistair Francis, qemu-trivial; +Cc: Philippe Mathieu-Daudé, qemu-devel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] register: fix incorrect read mask
2017-02-11 3:43 ` [Qemu-devel] [PATCH v2] " Philippe Mathieu-Daudé
@ 2017-02-14 1:20 ` Alistair Francis
0 siblings, 0 replies; 7+ messages in thread
From: Alistair Francis @ 2017-02-14 1:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Alistair Francis, QEMU Trivial, qemu-devel@nongnu.org Developers
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
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-02-14 1:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-06 23:39 [Qemu-devel] [PATCH 0/2] register: fix incorrect read mask Philippe Mathieu-Daudé
2017-02-06 23:39 ` [Qemu-devel] [PATCH 1/2] register: inline register_enabled_mask Philippe Mathieu-Daudé
2017-02-06 23:39 ` [Qemu-devel] [PATCH 2/2] register: fix incorrect read mask Philippe Mathieu-Daudé
2017-02-10 22:19 ` Alistair Francis
2017-02-11 3:16 ` Philippe Mathieu-Daudé
2017-02-11 3:43 ` [Qemu-devel] [PATCH v2] " Philippe Mathieu-Daudé
2017-02-14 1:20 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.