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

Philippe Mathieu-Daudé posted 4 patches 8 years, 1 month ago
[Qemu-devel] [PATCH 3/4] hw/registerfields: add 64-bit extract/deposit macros
Posted by Philippe Mathieu-Daudé 8 years, 1 month ago
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
Posted by francisco iglesias 8 years, 1 month ago
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
Posted by Philippe Mathieu-Daudé 8 years, 1 month ago
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