[Qemu-devel] [PATCH v1] linux-user: elf: ELF_HWCAP for s390x

David Hildenbrand posted 1 patch 4 years, 10 months ago
Test s390x passed
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190604093656.23565-1-david@redhat.com
Maintainers: Riku Voipio <riku.voipio@iki.fi>, Cornelia Huck <cohuck@redhat.com>, Laurent Vivier <laurent@vivier.eu>
include/elf.h        |  1 +
linux-user/elfload.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
[Qemu-devel] [PATCH v1] linux-user: elf: ELF_HWCAP for s390x
Posted by David Hildenbrand 4 years, 10 months ago
Let's add all HWCAPs that we can support under TCG right now, when the
respective CPU facilities are enabled.

Cc: Riku Voipio <riku.voipio@iki.fi>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---

This patch is based on "[PATCH v2 00/22] s390x/tcg: Vector Instruction
Support Part 4". I guess it is best if I just include it in my next
pull request.

---
 include/elf.h        |  1 +
 linux-user/elfload.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/elf.h b/include/elf.h
index ea7708a4ea..3501e0c8d0 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -598,6 +598,7 @@ typedef struct {
 #define HWCAP_S390_ETF3EH       256
 #define HWCAP_S390_HIGH_GPRS    512
 #define HWCAP_S390_TE           1024
+#define HWCAP_S390_VXRS         2048
 
 /* M68K specific definitions. */
 /* We use the top 24 bits to encode information about the
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index a57b7049dd..59b813066c 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1308,6 +1308,45 @@ static inline void init_thread(struct target_pt_regs *regs,
 #define ELF_DATA	ELFDATA2MSB
 #define ELF_ARCH	EM_S390
 
+#include "elf.h"
+
+#define ELF_HWCAP get_elf_hwcap()
+
+static uint32_t get_elf_hwcap(void)
+{
+    uint32_t hwcap = 0;
+
+    if (s390_has_feat(S390_FEAT_ESAN3)) {
+        hwcap |= HWCAP_S390_ESAN3;
+    }
+    if (s390_has_feat(S390_FEAT_ZARCH)) {
+        hwcap |= HWCAP_S390_ZARCH;
+    }
+    if (s390_has_feat(S390_FEAT_STFLE)) {
+        hwcap |= HWCAP_S390_STFLE;
+    }
+    if (s390_has_feat(S390_FEAT_MSA)) {
+        hwcap |= HWCAP_S390_MSA;
+    }
+    if (s390_has_feat(S390_FEAT_LONG_DISPLACEMENT)) {
+        hwcap |= HWCAP_S390_LDISP;
+    }
+    if (s390_has_feat(S390_FEAT_EXTENDED_IMMEDIATE)) {
+        hwcap |= HWCAP_S390_EIMM;
+    }
+    if (s390_has_feat(S390_FEAT_EXTENDED_TRANSLATION_3) &&
+        s390_has_feat(S390_FEAT_ETF3_ENH)) {
+        hwcap |= HWCAP_S390_ETF3EH;
+    }
+    /* 31-bit processes can use 64-bit registers */
+    hwcap |= HWCAP_S390_HIGH_GPRS;
+    if (s390_has_feat(S390_FEAT_VECTOR)) {
+        hwcap |= HWCAP_S390_VXRS;
+    }
+
+    return hwcap;
+}
+
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
     regs->psw.addr = infop->entry;
-- 
2.21.0


Re: [Qemu-devel] [PATCH v1] linux-user: elf: ELF_HWCAP for s390x
Posted by Laurent Vivier 4 years, 10 months ago
Le 04/06/2019 à 11:36, David Hildenbrand a écrit :
> Let's add all HWCAPs that we can support under TCG right now, when the
> respective CPU facilities are enabled.
> 
> Cc: Riku Voipio <riku.voipio@iki.fi>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Laurent Vivier <laurent@vivier.eu>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> 
> This patch is based on "[PATCH v2 00/22] s390x/tcg: Vector Instruction
> Support Part 4". I guess it is best if I just include it in my next
> pull request.
Based-on: <20190603090635.10631-1-david@redhat.com>
          [PATCH v2 00/22] s390x/tcg: Vector Instruction Support Part 4

> 
> ---
>  include/elf.h        |  1 +
>  linux-user/elfload.c | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/include/elf.h b/include/elf.h
> index ea7708a4ea..3501e0c8d0 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -598,6 +598,7 @@ typedef struct {
>  #define HWCAP_S390_ETF3EH       256
>  #define HWCAP_S390_HIGH_GPRS    512
>  #define HWCAP_S390_TE           1024
> +#define HWCAP_S390_VXRS         2048
>  
>  /* M68K specific definitions. */
>  /* We use the top 24 bits to encode information about the
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index a57b7049dd..59b813066c 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1308,6 +1308,45 @@ static inline void init_thread(struct target_pt_regs *regs,
>  #define ELF_DATA	ELFDATA2MSB
>  #define ELF_ARCH	EM_S390
>  
> +#include "elf.h"
> +
> +#define ELF_HWCAP get_elf_hwcap()
> +
> +static uint32_t get_elf_hwcap(void)
> +{
> +    uint32_t hwcap = 0;
> +
> +    if (s390_has_feat(S390_FEAT_ESAN3)) {
> +        hwcap |= HWCAP_S390_ESAN3;
> +    }
> +    if (s390_has_feat(S390_FEAT_ZARCH)) {
> +        hwcap |= HWCAP_S390_ZARCH;
> +    }
> +    if (s390_has_feat(S390_FEAT_STFLE)) {
> +        hwcap |= HWCAP_S390_STFLE;
> +    }
> +    if (s390_has_feat(S390_FEAT_MSA)) {
> +        hwcap |= HWCAP_S390_MSA;
> +    }
> +    if (s390_has_feat(S390_FEAT_LONG_DISPLACEMENT)) {
> +        hwcap |= HWCAP_S390_LDISP;
> +    }
> +    if (s390_has_feat(S390_FEAT_EXTENDED_IMMEDIATE)) {
> +        hwcap |= HWCAP_S390_EIMM;
> +    }
> +    if (s390_has_feat(S390_FEAT_EXTENDED_TRANSLATION_3) &&
> +        s390_has_feat(S390_FEAT_ETF3_ENH)) {
> +        hwcap |= HWCAP_S390_ETF3EH;
> +    }
> +    /* 31-bit processes can use 64-bit registers */
> +    hwcap |= HWCAP_S390_HIGH_GPRS;
> +    if (s390_has_feat(S390_FEAT_VECTOR)) {
> +        hwcap |= HWCAP_S390_VXRS;
> +    }
> +
> +    return hwcap;
> +}
> +
>  static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
>  {
>      regs->psw.addr = infop->entry;
> 

Acked-by: Laurent Vivier <laurent@vivier.eu>

You can also do like the other architectures that use shortcuts like this:

#define GET_FEATURE(feat, hwcap) \
    do { if (s390_has_feat(feat)) { hwcaps |= hwcap; } } while (0)
...
GET_FEATURE(S390_FEAT_VECTOR, HWCAP_S390_VXRS);
...


Thanks,
Laurent

Re: [Qemu-devel] [PATCH v1] linux-user: elf: ELF_HWCAP for s390x
Posted by David Hildenbrand 4 years, 10 months ago
On 04.06.19 14:54, Laurent Vivier wrote:
> Le 04/06/2019 à 11:36, David Hildenbrand a écrit :
>> Let's add all HWCAPs that we can support under TCG right now, when the
>> respective CPU facilities are enabled.
>>
>> Cc: Riku Voipio <riku.voipio@iki.fi>
>> Cc: Laurent Vivier <laurent@vivier.eu>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Laurent Vivier <laurent@vivier.eu>
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>
>> This patch is based on "[PATCH v2 00/22] s390x/tcg: Vector Instruction
>> Support Part 4". I guess it is best if I just include it in my next
>> pull request.
> Based-on: <20190603090635.10631-1-david@redhat.com>
>           [PATCH v2 00/22] s390x/tcg: Vector Instruction Support Part 4
> 
>>
>> ---
>>  include/elf.h        |  1 +
>>  linux-user/elfload.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 40 insertions(+)
>>
>> diff --git a/include/elf.h b/include/elf.h
>> index ea7708a4ea..3501e0c8d0 100644
>> --- a/include/elf.h
>> +++ b/include/elf.h
>> @@ -598,6 +598,7 @@ typedef struct {
>>  #define HWCAP_S390_ETF3EH       256
>>  #define HWCAP_S390_HIGH_GPRS    512
>>  #define HWCAP_S390_TE           1024
>> +#define HWCAP_S390_VXRS         2048
>>  
>>  /* M68K specific definitions. */
>>  /* We use the top 24 bits to encode information about the
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index a57b7049dd..59b813066c 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -1308,6 +1308,45 @@ static inline void init_thread(struct target_pt_regs *regs,
>>  #define ELF_DATA	ELFDATA2MSB
>>  #define ELF_ARCH	EM_S390
>>  
>> +#include "elf.h"
>> +
>> +#define ELF_HWCAP get_elf_hwcap()
>> +
>> +static uint32_t get_elf_hwcap(void)
>> +{
>> +    uint32_t hwcap = 0;
>> +
>> +    if (s390_has_feat(S390_FEAT_ESAN3)) {
>> +        hwcap |= HWCAP_S390_ESAN3;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_ZARCH)) {
>> +        hwcap |= HWCAP_S390_ZARCH;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_STFLE)) {
>> +        hwcap |= HWCAP_S390_STFLE;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_MSA)) {
>> +        hwcap |= HWCAP_S390_MSA;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_LONG_DISPLACEMENT)) {
>> +        hwcap |= HWCAP_S390_LDISP;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_EXTENDED_IMMEDIATE)) {
>> +        hwcap |= HWCAP_S390_EIMM;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_EXTENDED_TRANSLATION_3) &&
>> +        s390_has_feat(S390_FEAT_ETF3_ENH)) {
>> +        hwcap |= HWCAP_S390_ETF3EH;
>> +    }
>> +    /* 31-bit processes can use 64-bit registers */
>> +    hwcap |= HWCAP_S390_HIGH_GPRS;
>> +    if (s390_has_feat(S390_FEAT_VECTOR)) {
>> +        hwcap |= HWCAP_S390_VXRS;
>> +    }
>> +
>> +    return hwcap;
>> +}
>> +
>>  static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
>>  {
>>      regs->psw.addr = infop->entry;
>>
> 
> Acked-by: Laurent Vivier <laurent@vivier.eu>
> 
> You can also do like the other architectures that use shortcuts like this:
> 
> #define GET_FEATURE(feat, hwcap) \
>     do { if (s390_has_feat(feat)) { hwcaps |= hwcap; } } while (0)
> ...
> GET_FEATURE(S390_FEAT_VECTOR, HWCAP_S390_VXRS);
> ...

Thanks, I'll use this.

Cheers!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v1] linux-user: elf: ELF_HWCAP for s390x
Posted by Richard Henderson 4 years, 10 months ago
On 6/4/19 4:36 AM, David Hildenbrand wrote:
> +    if (s390_has_feat(S390_FEAT_ESAN3)) {
> +        hwcap |= HWCAP_S390_ESAN3;
> +    }
> +    if (s390_has_feat(S390_FEAT_ZARCH)) {
> +        hwcap |= HWCAP_S390_ZARCH;
> +    }

While it's nice and symetrical testing these two features, I don't think they
can ever be false.

> +    if (s390_has_feat(S390_FEAT_STFLE)) {
> +        hwcap |= HWCAP_S390_STFLE;
> +    }
> +    if (s390_has_feat(S390_FEAT_MSA)) {
> +        hwcap |= HWCAP_S390_MSA;
> +    }
> +    if (s390_has_feat(S390_FEAT_LONG_DISPLACEMENT)) {
> +        hwcap |= HWCAP_S390_LDISP;
> +    }
> +    if (s390_has_feat(S390_FEAT_EXTENDED_IMMEDIATE)) {
> +        hwcap |= HWCAP_S390_EIMM;
> +    }
> +    if (s390_has_feat(S390_FEAT_EXTENDED_TRANSLATION_3) &&
> +        s390_has_feat(S390_FEAT_ETF3_ENH)) {
> +        hwcap |= HWCAP_S390_ETF3EH;
> +    }
> +    /* 31-bit processes can use 64-bit registers */
> +    hwcap |= HWCAP_S390_HIGH_GPRS;

And certainly this could never be set unless ZARCH, otherwise you have no
64-bit registers.  ;-)

So maybe clearer to just start with

  hwcap = HWCAP_S390_ESAN3 | HWCAP_S390_ZARCH | HWCAP_S390_HIGH_GPRS;

and continue from there.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1] linux-user: elf: ELF_HWCAP for s390x
Posted by David Hildenbrand 4 years, 10 months ago
On 04.06.19 15:00, Richard Henderson wrote:
> On 6/4/19 4:36 AM, David Hildenbrand wrote:
>> +    if (s390_has_feat(S390_FEAT_ESAN3)) {
>> +        hwcap |= HWCAP_S390_ESAN3;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_ZARCH)) {
>> +        hwcap |= HWCAP_S390_ZARCH;
>> +    }
> 
> While it's nice and symetrical testing these two features, I don't think they
> can ever be false.
> 
>> +    if (s390_has_feat(S390_FEAT_STFLE)) {
>> +        hwcap |= HWCAP_S390_STFLE;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_MSA)) {
>> +        hwcap |= HWCAP_S390_MSA;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_LONG_DISPLACEMENT)) {
>> +        hwcap |= HWCAP_S390_LDISP;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_EXTENDED_IMMEDIATE)) {
>> +        hwcap |= HWCAP_S390_EIMM;
>> +    }
>> +    if (s390_has_feat(S390_FEAT_EXTENDED_TRANSLATION_3) &&
>> +        s390_has_feat(S390_FEAT_ETF3_ENH)) {
>> +        hwcap |= HWCAP_S390_ETF3EH;
>> +    }
>> +    /* 31-bit processes can use 64-bit registers */
>> +    hwcap |= HWCAP_S390_HIGH_GPRS;
> 
> And certainly this could never be set unless ZARCH, otherwise you have no
> 64-bit registers.  ;-)
> 
> So maybe clearer to just start with
> 
>   hwcap = HWCAP_S390_ESAN3 | HWCAP_S390_ZARCH | HWCAP_S390_HIGH_GPRS;

Makes sense, and fits into a single line ;)

Thanks!

> 
> and continue from there.
> 
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
> 


-- 

Thanks,

David / dhildenb