[Qemu-devel] [PATCH 0/4] Trivial changes in "registerfields.h"

[Qemu-devel] [PATCH 0/4] Trivial changes in "registerfields.h"

[Philippe Mathieu-Daudé]

Philippe Mathieu-Daudé (4):
  MAINTAINERS: add "hw/registerfields.h" in Register API entry
  hw/registerfields: fix a typo in the FIELD() documentation
  hw/registerfields: add 64-bit extract/deposit macros
  hw/registerfields: add missing include

 include/hw/registerfields.h | 15 ++++++++++++++-
 MAINTAINERS                 |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

-- 
2.15.1

[Qemu-devel] [PATCH 1/4] MAINTAINERS: add "hw/registerfields.h" in Register API entry

[Philippe Mathieu-Daudé]

Orphan since afb3141c660

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0255113470..7edfa25542 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1538,6 +1538,7 @@ M: Alistair Francis <alistair.francis@xilinx.com>
 S: Maintained
 F: hw/core/register.c
 F: include/hw/register.h
+F: include/hw/registerfields.h
 
 SLIRP
 M: Samuel Thibault <samuel.thibault@ens-lyon.org>
-- 
2.15.1

[Qemu-devel] [PATCH 2/4] hw/registerfields: fix a typo in the FIELD() documentation

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/registerfields.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index af101d5ae6..ad9d7a82a3 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -22,7 +22,7 @@
 
 /* Define SHIFT, LENGTH and MASK constants for a field within a register */
 
-/* This macro will define FOO_BAR_MASK, FOO_BAR_SHIFT and FOO_BAR_LENGTH 
+/* This macro will define R_FOO_BAR_MASK, R_FOO_BAR_SHIFT and R_FOO_BAR_LENGTH
  * constants for field BAR in register FOO.
  */
 #define FIELD(reg, field, shift, length)                                  \
-- 
2.15.1

[Qemu-devel] [PATCH 3/4] hw/registerfields: add 64-bit extract/deposit macros

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/registerfields.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index ad9d7a82a3..f59e7f47bd 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -35,6 +35,9 @@
 #define FIELD_EX32(storage, reg, field)                                   \
     extract32((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
               R_ ## reg ## _ ## field ## _LENGTH)
+#define FIELD_EX64(storage, reg, field)                                   \
+    extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,               \
+              R_ ## reg ## _ ## field ## _LENGTH)
 
 /* Extract a field from an array of registers */
 #define ARRAY_FIELD_EX32(regs, reg, field)                                \
@@ -52,6 +55,14 @@
     d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
                   R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
     d; })
+#define FIELD_DP64(storage, reg, field, val) ({                           \
+    struct {                                                              \
+        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;                \
+    } v = { .v = val };                                                   \
+    uint64_t d;                                                           \
+    d = deposit64((storage), R_ ## reg ## _ ## field ## _SHIFT,           \
+                  R_ ## reg ## _ ## field ## _LENGTH, v.v);               \
+    d; })
 
 /* Deposit a field to array of registers.  */
 #define ARRAY_FIELD_DP32(regs, reg, field, val)                           \
-- 
2.15.1

Re: [Qemu-devel] [PATCH 3/4] hw/registerfields: add 64-bit extract/deposit macros

[francisco iglesias]

On 13 December 2017 at 06:17, Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/registerfields.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
> index ad9d7a82a3..f59e7f47bd 100644
> --- a/include/hw/registerfields.h
> +++ b/include/hw/registerfields.h
> @@ -35,6 +35,9 @@
>  #define FIELD_EX32(storage, reg, field)
>  \
>      extract32((storage), R_ ## reg ## _ ## field ## _SHIFT,
>  \
>                R_ ## reg ## _ ## field ## _LENGTH)
> +#define FIELD_EX64(storage, reg, field)
>  \
> +    extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,
>  \
> +              R_ ## reg ## _ ## field ## _LENGTH)
>
>  /* Extract a field from an array of registers */
>  #define ARRAY_FIELD_EX32(regs, reg, field)
> \
> @@ -52,6 +55,14 @@
>      d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,
>  \
>                    R_ ## reg ## _ ## field ## _LENGTH, v.v);
>  \
>      d; })
> +#define FIELD_DP64(storage, reg, field, val) ({
>  \
> +    struct {
> \
> +        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;
> \
>

Hi Phiilppe,

I'm just wondering if 'v' above maybe should be uint64_t? The size of
unsigned int seems to be 'implementation defined' but I would guess 32 bits
is not unsual. Basically wouldn't a field length > 32 above be problematic?

Best regards,
Francisco


> +    } v = { .v = val };
>  \
> +    uint64_t d;
>  \
> +    d = deposit64((storage), R_ ## reg ## _ ## field ## _SHIFT,
>  \
> +                  R_ ## reg ## _ ## field ## _LENGTH, v.v);
>  \
> +    d; })
>
>  /* Deposit a field to array of registers.  */
>  #define ARRAY_FIELD_DP32(regs, reg, field, val)
>  \
> --
> 2.15.1
>
>
>

Re: [Qemu-devel] [PATCH 3/4] hw/registerfields: add 64-bit extract/deposit macros

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

Hi Francisco,

On 12/13/2017 06:37 PM, francisco iglesias wrote:
> On 13 December 2017 at 06:17, Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
[...]
>>  /* Extract a field from an array of registers */
[...]
>> +#define FIELD_DP64(storage, reg, field, val) ({
>>  \
>> +    struct {
>> \
>> +        unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;
>> \
>>
> 
> I'm just wondering if 'v' above maybe should be uint64_t? The size of
> unsigned int seems to be 'implementation defined' but I would guess 32 bits
> is not unsual. Basically wouldn't a field length > 32 above be problematic?

Good remark...

As per the standard bitfields access must be per 'unsigned'.
So with a LP64 data model we have sizeof(unsigned) = 32-bit, I wonder
how the compiler behaves.

I added the debian-win32 docker image to tests this kind of regressions,
but it seems the current toolchain used (mingw-w64) is now only a LLP64
model (I still need to verify).

I remember I somewhere have a Linux/X32 docker suite to test this
series. I'll give it a try (I doubt before this week-end).

Regards,

Phil.

>> +    } v = { .v = val };
>>  \
>> +    uint64_t d;
>>  \
>> +    d = deposit64((storage), R_ ## reg ## _ ## field ## _SHIFT,
>>  \
>> +                  R_ ## reg ## _ ## field ## _LENGTH, v.v);
>>  \
>> +    d; })
>>
>>  /* Deposit a field to array of registers.  */
>>  #define ARRAY_FIELD_DP32(regs, reg, field, val)
>>  \
>> --
>> 2.15.1



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[Qemu-devel] [PATCH 4/4] hw/registerfields: add missing include

[Philippe Mathieu-Daudé]

This allows to use this header in qtests.

This fixes:
    CC      tests/test.o
  include/hw/registerfields.h:32:41: error: implicit declaration of function ‘MAKE_64BIT_MASK’ [-Werror=implicit-function-declaration]
              MAKE_64BIT_MASK(shift, length)};
              ^
  include/hw/registerfields.h:39:5: error: implicit declaration of function ‘extract64’; [-Werror=implicit-function-declaration]
       extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,
       ^

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/registerfields.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index f59e7f47bd..44e0b94edf 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -11,6 +11,8 @@
 #ifndef REGISTERFIELDS_H
 #define REGISTERFIELDS_H
 
+#include <qemu/bitops.h>
+
 /* Define constants for a 32 bit register */
 
 /* This macro will define A_FOO, for the byte address of a register
-- 
2.15.1

Re: [Qemu-devel] [PATCH 0/4] Trivial changes in "registerfields.h"

[Darren Kenny]

Looks good to me, for all patches:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

On Wed, Dec 13, 2017 at 02:17:32AM -0300, Philippe Mathieu-Daudé wrote:
>Philippe Mathieu-Daudé (4):
>  MAINTAINERS: add "hw/registerfields.h" in Register API entry
>  hw/registerfields: fix a typo in the FIELD() documentation
>  hw/registerfields: add 64-bit extract/deposit macros
>  hw/registerfields: add missing include
>
> include/hw/registerfields.h | 15 ++++++++++++++-
> MAINTAINERS                 |  1 +
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
>-- 
>2.15.1
>
>

Re: [Qemu-devel] [PATCH 0/4] Trivial changes in "registerfields.h"

[Alistair Francis]

On Wed, Dec 13, 2017 at 1:06 AM, Darren Kenny <darren.kenny@oracle.com> wrote:
> Looks good to me, for all patches:
>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Same, for all four patches.

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

Alistair

>
> Thanks,
>
> Darren.
>
>
> On Wed, Dec 13, 2017 at 02:17:32AM -0300, Philippe Mathieu-Daudé wrote:
>>
>> Philippe Mathieu-Daudé (4):
>>  MAINTAINERS: add "hw/registerfields.h" in Register API entry
>>  hw/registerfields: fix a typo in the FIELD() documentation
>>  hw/registerfields: add 64-bit extract/deposit macros
>>  hw/registerfields: add missing include
>>
>> include/hw/registerfields.h | 15 ++++++++++++++-
>> MAINTAINERS                 |  1 +
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> --
>> 2.15.1
>>
>>
>

Re: [Qemu-devel] [PATCH 0/4] Trivial changes in "registerfields.h"

[Edgar E. Iglesias]

On Wed, Dec 13, 2017 at 02:17:32AM -0300, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (4):
>   MAINTAINERS: add "hw/registerfields.h" in Register API entry
>   hw/registerfields: fix a typo in the FIELD() documentation
>   hw/registerfields: add 64-bit extract/deposit macros
>   hw/registerfields: add missing include
> 
>  include/hw/registerfields.h | 15 ++++++++++++++-
>  MAINTAINERS                 |  1 +
>  2 files changed, 15 insertions(+), 1 deletion(-)

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>