[Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Christian Borntraeger]

KVM guests on s390 need a different page table layout than normal
processes (2kb page table + 2kb page status extensions vs 2kb page table
only). As of today this has to be enabled via the vm.allocate_pgste
sysctl.

Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
and enable the pgste page table extensions in that case. This makes the
vm.allocate_pgste sysctl unnecessary. We enable this program header for
the s390 system emulation (qemu-system-s390x) if we build on s390
- for s390 system emulation
- the linker supports --s390-pgste (binutils >= 2.29)
- KVM is enabled

This will allow distributions to disable the global vm.allocate_pgste
sysctl, which will improve the page table allocation for non KVM
processes as only 2kb chunks are necessary.

Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Dan Horak <dhorak@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
V1->V2:
	- provide ld_has function
	- use ld_has to replace some open coded variants
	- check target arch and arch for s390
	- check for s390x before calling the linker

 configure | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index dd73cce..0b68b37 100755
--- a/configure
+++ b/configure
@@ -240,6 +240,11 @@ supported_target() {
     return 1
 }
 
+
+ld_has() {
+    $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
+}
+
 # default parameters
 source_path=$(dirname "$0")
 cpu=""
@@ -5043,7 +5048,7 @@ fi
 # Use ASLR, no-SEH and DEP if available
 if test "$mingw32" = "yes" ; then
     for flag in --dynamicbase --no-seh --nxcompat; do
-        if $ld --help 2>/dev/null | grep ".$flag" >/dev/null 2>/dev/null ; then
+        if ld_had $flag ; then
             LDFLAGS="-Wl,$flag $LDFLAGS"
         fi
     done
@@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   ldflags="$ldflags $textseg_ldflags"
 fi
 
+# Newer kernels on s390 check for an S390_PGSTE program header and
+# enable the pgste page table extensions in that case. This makes
+# the vm.allocate_pgste sysctl unnecessary. We enable this program
+# header if
+#  - we build on s390x
+#  - we build the system emulation for s390x (qemu-system-s390x)
+#  - KVM is enabled
+#  - the linker support --s390-pgste
+if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then
+    if ld_has --s390-pgste ; then
+        ldflags="-Wl,--s390-pgste $ldflags"
+    fi
+fi
+
 echo "LDFLAGS+=$ldflags" >> $config_target_mak
 echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Christian Borntraeger]

On 08/23/2017 08:53 AM, Christian Borntraeger wrote:
> KVM guests on s390 need a different page table layout than normal
> processes (2kb page table + 2kb page status extensions vs 2kb page table
> only). As of today this has to be enabled via the vm.allocate_pgste
> sysctl.
> 
> Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
> and enable the pgste page table extensions in that case. This makes the
> vm.allocate_pgste sysctl unnecessary. We enable this program header for
> the s390 system emulation (qemu-system-s390x) if we build on s390
> - for s390 system emulation
> - the linker supports --s390-pgste (binutils >= 2.29)
> - KVM is enabled
> 
> This will allow distributions to disable the global vm.allocate_pgste
> sysctl, which will improve the page table allocation for non KVM
> processes as only 2kb chunks are necessary.
> 
> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Dan Horak <dhorak@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> V1->V2:
> 	- provide ld_has function
> 	- use ld_has to replace some open coded variants
> 	- check target arch and arch for s390
> 	- check for s390x before calling the linker
> 
>  configure | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index dd73cce..0b68b37 100755
> --- a/configure
> +++ b/configure
> @@ -240,6 +240,11 @@ supported_target() {
>      return 1
>  }
> 
> +
> +ld_has() {
> +    $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
> +}
> +
>  # default parameters
>  source_path=$(dirname "$0")
>  cpu=""
> @@ -5043,7 +5048,7 @@ fi
>  # Use ASLR, no-SEH and DEP if available
>  if test "$mingw32" = "yes" ; then
>      for flag in --dynamicbase --no-seh --nxcompat; do
> -        if $ld --help 2>/dev/null | grep ".$flag" >/dev/null 2>/dev/null ; then
> +        if ld_had $flag ; then

	typo, will fix with v3 :-/


>              LDFLAGS="-Wl,$flag $LDFLAGS"
>          fi
>      done
> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
>    ldflags="$ldflags $textseg_ldflags"
>  fi
> 
> +# Newer kernels on s390 check for an S390_PGSTE program header and
> +# enable the pgste page table extensions in that case. This makes
> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
> +# header if
> +#  - we build on s390x
> +#  - we build the system emulation for s390x (qemu-system-s390x)
> +#  - KVM is enabled
> +#  - the linker support --s390-pgste
> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then
> +    if ld_has --s390-pgste ; then
> +        ldflags="-Wl,--s390-pgste $ldflags"
> +    fi
> +fi
> +
>  echo "LDFLAGS+=$ldflags" >> $config_target_mak
>  echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
> 

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Christian Ehrhardt]

On Wed, Aug 23, 2017 at 8:53 AM, Christian Borntraeger <
borntraeger@de.ibm.com> wrote:

> KVM guests on s390 need a different page table layout than normal
> processes (2kb page table + 2kb page status extensions vs 2kb page table
> only). As of today this has to be enabled via the vm.allocate_pgste
> sysctl.
>
> Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
> and enable the pgste page table extensions in that case. This makes the
> vm.allocate_pgste sysctl unnecessary. We enable this program header for
> the s390 system emulation (qemu-system-s390x) if we build on s390
> - for s390 system emulation
> - the linker supports --s390-pgste (binutils >= 2.29)
> - KVM is enabled
>
> This will allow distributions to disable the global vm.allocate_pgste
> sysctl, which will improve the page table allocation for non KVM
> processes as only 2kb chunks are necessary.
>

Hi Christian,
it is great to see context pgste come to life.
Currently vm.allocate_pgste defaults to 0 in the kernel but as you stated
mostly enabled for KVM support in Distros.
So when someone wants to disable it he has to drop the enabling
(e.g. /etc/sysctl.d/10-arch-specific.conf for us).

I want to be sure on the proper phasing of this - we can drop the
"enabling" of global pgste once for a release we:
- do not expect/support a kernel <4.12 to run there
- will have only qemu versions >= the one carrying this change (and have it
properly enabled)
- binutils >= 2.29 to get the linking right

But furthermore if we have a qemu with this enabled, there is no drawback
and we could still run it in:
- former releases with older kernels
- former releases with older build environments
That program header would just be ignored and we just would have to keep
the sysctl enabled there right?

Also for the time we want to check on the proper header, you surely have a
one liner you can share that you run against the binary to check if it was
generated correctly?
Maybe even one that you can run against a pid if the status is correct?

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Thomas Huth]

On 23.08.2017 08:55, Christian Borntraeger wrote:
> 
> 
> On 08/23/2017 08:53 AM, Christian Borntraeger wrote:
>> KVM guests on s390 need a different page table layout than normal
>> processes (2kb page table + 2kb page status extensions vs 2kb page table
>> only). As of today this has to be enabled via the vm.allocate_pgste
>> sysctl.
>>
>> Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
>> and enable the pgste page table extensions in that case. This makes the
>> vm.allocate_pgste sysctl unnecessary. We enable this program header for
>> the s390 system emulation (qemu-system-s390x) if we build on s390
>> - for s390 system emulation
>> - the linker supports --s390-pgste (binutils >= 2.29)
>> - KVM is enabled
>>
>> This will allow distributions to disable the global vm.allocate_pgste
>> sysctl, which will improve the page table allocation for non KVM
>> processes as only 2kb chunks are necessary.
>>
>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Dan Horak <dhorak@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> ---
>> V1->V2:
>> 	- provide ld_has function
>> 	- use ld_has to replace some open coded variants
>> 	- check target arch and arch for s390
>> 	- check for s390x before calling the linker
>>
>>  configure | 21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index dd73cce..0b68b37 100755
>> --- a/configure
>> +++ b/configure
>> @@ -240,6 +240,11 @@ supported_target() {
>>      return 1
>>  }
>>
>> +
>> +ld_has() {
>> +    $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>> +}
>> +
>>  # default parameters
>>  source_path=$(dirname "$0")
>>  cpu=""
>> @@ -5043,7 +5048,7 @@ fi
>>  # Use ASLR, no-SEH and DEP if available
>>  if test "$mingw32" = "yes" ; then
>>      for flag in --dynamicbase --no-seh --nxcompat; do
>> -        if $ld --help 2>/dev/null | grep ".$flag" >/dev/null 2>/dev/null ; then
>> +        if ld_had $flag ; then
> 
> 	typo, will fix with v3 :-/
> 
> 
>>              LDFLAGS="-Wl,$flag $LDFLAGS"
>>          fi
>>      done
>> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
>>    ldflags="$ldflags $textseg_ldflags"
>>  fi
>>
>> +# Newer kernels on s390 check for an S390_PGSTE program header and
>> +# enable the pgste page table extensions in that case. This makes
>> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
>> +# header if
>> +#  - we build on s390x
>> +#  - we build the system emulation for s390x (qemu-system-s390x)
>> +#  - KVM is enabled
>> +#  - the linker support --s390-pgste
>> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then
>> +    if ld_has --s390-pgste ; then
>> +        ldflags="-Wl,--s390-pgste $ldflags"
>> +    fi
>> +fi
>> +
>>  echo "LDFLAGS+=$ldflags" >> $config_target_mak
>>  echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak

Apart from the typo, this looks good to me. Feel free to add my

Reviewed-by: Thomas Huth <thuth@redhat.com>

once you've fixed the typo.

 Thomas

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[David Hildenbrand]

On 23.08.2017 08:53, Christian Borntraeger wrote:
> KVM guests on s390 need a different page table layout than normal
> processes (2kb page table + 2kb page status extensions vs 2kb page table
> only). As of today this has to be enabled via the vm.allocate_pgste
> sysctl.
> 
> Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
> and enable the pgste page table extensions in that case. This makes the
> vm.allocate_pgste sysctl unnecessary. We enable this program header for
> the s390 system emulation (qemu-system-s390x) if we build on s390
> - for s390 system emulation
> - the linker supports --s390-pgste (binutils >= 2.29)
> - KVM is enabled
> 
> This will allow distributions to disable the global vm.allocate_pgste
> sysctl, which will improve the page table allocation for non KVM
> processes as only 2kb chunks are necessary.
> 
> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Dan Horak <dhorak@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
> V1->V2:
> 	- provide ld_has function
> 	- use ld_has to replace some open coded variants
> 	- check target arch and arch for s390
> 	- check for s390x before calling the linker
> 
>  configure | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index dd73cce..0b68b37 100755
> --- a/configure
> +++ b/configure
> @@ -240,6 +240,11 @@ supported_target() {
>      return 1
>  }
>  
> +

I'd drop this new line

> +ld_has() {
> +    $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
> +}
> +
>  # default parameters
>  source_path=$(dirname "$0")
>  cpu=""
> @@ -5043,7 +5048,7 @@ fi
>  # Use ASLR, no-SEH and DEP if available
>  if test "$mingw32" = "yes" ; then
>      for flag in --dynamicbase --no-seh --nxcompat; do
> -        if $ld --help 2>/dev/null | grep ".$flag" >/dev/null 2>/dev/null ; then
> +        if ld_had $flag ; then
>              LDFLAGS="-Wl,$flag $LDFLAGS"
>          fi
>      done
> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
>    ldflags="$ldflags $textseg_ldflags"
>  fi
>  
> +# Newer kernels on s390 check for an S390_PGSTE program header and
> +# enable the pgste page table extensions in that case. This makes
> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
> +# header if
> +#  - we build on s390x
> +#  - we build the system emulation for s390x (qemu-system-s390x)
> +#  - KVM is enabled
> +#  - the linker support --s390-pgste
> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then

Wonder if the "$ARCH" check is really necessary: TARGET_ARCH=s390x with
kvm=yes should only build on s390x.

> +    if ld_has --s390-pgste ; then
> +        ldflags="-Wl,--s390-pgste $ldflags"
> +    fi
> +fi
> +
>  echo "LDFLAGS+=$ldflags" >> $config_target_mak
>  echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
>  
> 

With the typo fixed

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Thomas Huth]

On 23.08.2017 10:00, David Hildenbrand wrote:
> On 23.08.2017 08:53, Christian Borntraeger wrote:
>> KVM guests on s390 need a different page table layout than normal
>> processes (2kb page table + 2kb page status extensions vs 2kb page table
>> only). As of today this has to be enabled via the vm.allocate_pgste
>> sysctl.
>>
>> Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
>> and enable the pgste page table extensions in that case. This makes the
>> vm.allocate_pgste sysctl unnecessary. We enable this program header for
>> the s390 system emulation (qemu-system-s390x) if we build on s390
>> - for s390 system emulation
>> - the linker supports --s390-pgste (binutils >= 2.29)
>> - KVM is enabled
>>
>> This will allow distributions to disable the global vm.allocate_pgste
>> sysctl, which will improve the page table allocation for non KVM
>> processes as only 2kb chunks are necessary.
>>
>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>> Cc: Alexander Graf <agraf@suse.de>
>> Cc: Dan Horak <dhorak@redhat.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> ---
>> V1->V2:
>> 	- provide ld_has function
>> 	- use ld_has to replace some open coded variants
>> 	- check target arch and arch for s390
>> 	- check for s390x before calling the linker
>>
>>  configure | 21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index dd73cce..0b68b37 100755
>> --- a/configure
>> +++ b/configure
>> @@ -240,6 +240,11 @@ supported_target() {
>>      return 1
>>  }
>>  
>> +
> 
> I'd drop this new line
> 
>> +ld_has() {
>> +    $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>> +}
>> +
>>  # default parameters
>>  source_path=$(dirname "$0")
>>  cpu=""
>> @@ -5043,7 +5048,7 @@ fi
>>  # Use ASLR, no-SEH and DEP if available
>>  if test "$mingw32" = "yes" ; then
>>      for flag in --dynamicbase --no-seh --nxcompat; do
>> -        if $ld --help 2>/dev/null | grep ".$flag" >/dev/null 2>/dev/null ; then
>> +        if ld_had $flag ; then
>>              LDFLAGS="-Wl,$flag $LDFLAGS"
>>          fi
>>      done
>> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
>>    ldflags="$ldflags $textseg_ldflags"
>>  fi
>>  
>> +# Newer kernels on s390 check for an S390_PGSTE program header and
>> +# enable the pgste page table extensions in that case. This makes
>> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
>> +# header if
>> +#  - we build on s390x
>> +#  - we build the system emulation for s390x (qemu-system-s390x)
>> +#  - KVM is enabled
>> +#  - the linker support --s390-pgste
>> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then
> 
> Wonder if the "$ARCH" check is really necessary: TARGET_ARCH=s390x with
> kvm=yes should only build on s390x.

Isn't kvm=yes and TARGET_ARCH=s390x also possible on a x86 host, where
only the x86_64 target is built with CONFIG_KVM=y, but the s390x target
with CONFIG_KVM=n ?

 Thomas

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Paolo Bonzini]

On 23/08/2017 10:06, Thomas Huth wrote:
> On 23.08.2017 10:00, David Hildenbrand wrote:
>> On 23.08.2017 08:53, Christian Borntraeger wrote:
>>> KVM guests on s390 need a different page table layout than normal
>>> processes (2kb page table + 2kb page status extensions vs 2kb page table
>>> only). As of today this has to be enabled via the vm.allocate_pgste
>>> sysctl.
>>>
>>> Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
>>> and enable the pgste page table extensions in that case. This makes the
>>> vm.allocate_pgste sysctl unnecessary. We enable this program header for
>>> the s390 system emulation (qemu-system-s390x) if we build on s390
>>> - for s390 system emulation
>>> - the linker supports --s390-pgste (binutils >= 2.29)
>>> - KVM is enabled
>>>
>>> This will allow distributions to disable the global vm.allocate_pgste
>>> sysctl, which will improve the page table allocation for non KVM
>>> processes as only 2kb chunks are necessary.
>>>
>>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Cc: Dan Horak <dhorak@redhat.com>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>>> ---
>>> V1->V2:
>>> 	- provide ld_has function
>>> 	- use ld_has to replace some open coded variants
>>> 	- check target arch and arch for s390
>>> 	- check for s390x before calling the linker
>>>
>>>  configure | 21 ++++++++++++++++++++-
>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index dd73cce..0b68b37 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -240,6 +240,11 @@ supported_target() {
>>>      return 1
>>>  }
>>>  
>>> +
>>
>> I'd drop this new line
>>
>>> +ld_has() {
>>> +    $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>>> +}
>>> +
>>>  # default parameters
>>>  source_path=$(dirname "$0")
>>>  cpu=""
>>> @@ -5043,7 +5048,7 @@ fi
>>>  # Use ASLR, no-SEH and DEP if available
>>>  if test "$mingw32" = "yes" ; then
>>>      for flag in --dynamicbase --no-seh --nxcompat; do
>>> -        if $ld --help 2>/dev/null | grep ".$flag" >/dev/null 2>/dev/null ; then
>>> +        if ld_had $flag ; then
>>>              LDFLAGS="-Wl,$flag $LDFLAGS"
>>>          fi
>>>      done
>>> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
>>>    ldflags="$ldflags $textseg_ldflags"
>>>  fi
>>>  
>>> +# Newer kernels on s390 check for an S390_PGSTE program header and
>>> +# enable the pgste page table extensions in that case. This makes
>>> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
>>> +# header if
>>> +#  - we build on s390x
>>> +#  - we build the system emulation for s390x (qemu-system-s390x)
>>> +#  - KVM is enabled
>>> +#  - the linker support --s390-pgste
>>> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then
>>
>> Wonder if the "$ARCH" check is really necessary: TARGET_ARCH=s390x with
>> kvm=yes should only build on s390x.
> 
> Isn't kvm=yes and TARGET_ARCH=s390x also possible on a x86 host, where
> only the x86_64 target is built with CONFIG_KVM=y, but the s390x target
> with CONFIG_KVM=n ?

Yes.  You could use

  if test "$ARCH" = "s390x" && supported_kvm_target $target; then
    ...
  fi

Or, in the existing "if supported_kvm_target $target" conditional, add

  if test "$ARCH" = s390x && ld_has --s390-pgste; then
    ...
  fi

Paolo

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Cornelia Huck]

On Wed, 23 Aug 2017 10:16:27 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 23/08/2017 10:06, Thomas Huth wrote:
> > On 23.08.2017 10:00, David Hildenbrand wrote:  
> >> On 23.08.2017 08:53, Christian Borntraeger wrote:  

> >>> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
> >>>    ldflags="$ldflags $textseg_ldflags"
> >>>  fi
> >>>  
> >>> +# Newer kernels on s390 check for an S390_PGSTE program header and
> >>> +# enable the pgste page table extensions in that case. This makes
> >>> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
> >>> +# header if
> >>> +#  - we build on s390x
> >>> +#  - we build the system emulation for s390x (qemu-system-s390x)
> >>> +#  - KVM is enabled
> >>> +#  - the linker support --s390-pgste
> >>> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then  
> >>
> >> Wonder if the "$ARCH" check is really necessary: TARGET_ARCH=s390x with
> >> kvm=yes should only build on s390x.  
> > 
> > Isn't kvm=yes and TARGET_ARCH=s390x also possible on a x86 host, where
> > only the x86_64 target is built with CONFIG_KVM=y, but the s390x target
> > with CONFIG_KVM=n ?  
> 
> Yes.  You could use
> 
>   if test "$ARCH" = "s390x" && supported_kvm_target $target; then
>     ...
>   fi
> 
> Or, in the existing "if supported_kvm_target $target" conditional, add
> 
>   if test "$ARCH" = s390x && ld_has --s390-pgste; then
>     ...
>   fi

That conditional is unfortunately before the setup of ldflags; but I
like the idea of using supported_kvm_target.

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Christian Borntraeger]

On 08/23/2017 09:28 AM, Christian Ehrhardt wrote:
> 
> 
> On Wed, Aug 23, 2017 at 8:53 AM, Christian Borntraeger <borntraeger@de.ibm.com <mailto:borntraeger@de.ibm.com>> wrote:
> 
>     KVM guests on s390 need a different page table layout than normal
>     processes (2kb page table + 2kb page status extensions vs 2kb page table
>     only). As of today this has to be enabled via the vm.allocate_pgste
>     sysctl.
> 
>     Newer kernels (>= 4.12) on s390 check for an S390_PGSTE program header
>     and enable the pgste page table extensions in that case. This makes the
>     vm.allocate_pgste sysctl unnecessary. We enable this program header for
>     the s390 system emulation (qemu-system-s390x) if we build on s390
>     - for s390 system emulation
>     - the linker supports --s390-pgste (binutils >= 2.29)
>     - KVM is enabled
> 
>     This will allow distributions to disable the global vm.allocate_pgste
>     sysctl, which will improve the page table allocation for non KVM
>     processes as only 2kb chunks are necessary.
> 
> 
> Hi Christian,
> it is great to see context pgste come to life.
> Currently vm.allocate_pgste defaults to 0 in the kernel but as you stated mostly enabled for KVM support in Distros.
> So when someone wants to disable it he has to drop the enabling (e.g. /etc/sysctl.d/10-arch-specific.conf for us).
> 
> I want to be sure on the proper phasing of this - we can drop the "enabling" of global pgste once for a release we:
> - do not expect/support a kernel <4.12 to run there
> - will have only qemu versions >= the one carrying this change (and have it properly enabled)
> - binutils >= 2.29 to get the linking right

Yes. So I guess that for the Ubuntu case you could remove the sysctl thing for 18.04 assuming that
this will hit qemu 2.11 and 18.04 will use 2.11.


> 
> But furthermore if we have a qemu with this enabled, there is no drawback and we could still run it in:
> - former releases with older kernels

Yes.

> - former releases with older build environments

Yes.

> That program header would just be ignored and we just would have to keep the sysctl enabled there right?

Yes.

> 
> Also for the time we want to check on the proper header, you surely have a one liner you can share that you run against the binary to check if it was generated correctly?
> Maybe even one that you can run against a pid if the status is correct?

readelf -l on the binary
$ readelf -l REPOS/qemu/build/s390x-softmmu/qemu-system-s390x 

Elf file type is EXEC (Executable file)
Entry point 0x101f758
There are 11 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000001000040 0x0000000001000040
                 0x0000000000000268 0x0000000000000268  R E    0x8
  INTERP         0x00000000000002a8 0x00000000010002a8 0x00000000010002a8
                 0x000000000000000f 0x000000000000000f  R      0x1
      [Requesting program interpreter: /lib/ld64.so.1]
  LOAD           0x0000000000000000 0x0000000001000000 0x0000000001000000
                 0x00000000004852f0 0x00000000004852f0  R E    0x1000
  LOAD           0x0000000000485450 0x0000000001486450 0x0000000001486450
                 0x000000000003dcc8 0x0000000000485840  RW     0x1000
  DYNAMIC        0x0000000000485b80 0x0000000001486b80 0x0000000001486b80
                 0x0000000000000480 0x0000000000000480  RW     0x8
  NOTE           0x00000000000002b8 0x00000000010002b8 0x00000000010002b8
                 0x0000000000000044 0x0000000000000044  R      0x4
  TLS            0x0000000000485450 0x0000000001486450 0x0000000001486450
                 0x0000000000000000 0x0000000000000230  R      0x8
  GNU_EH_FRAME   0x00000000003dc638 0x00000000013dc638 0x00000000013dc638
                 0x0000000000017a74 0x0000000000017a74  R      0x4
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     0x10
  GNU_RELRO      0x0000000000485450 0x0000000001486450 0x0000000001486450
                 0x0000000000000bb0 0x0000000000000bb0  R      0x1
  S390_PGSTE     0x0000000000000000 0x0000000000000000 0x0000000000000000   <----
                 0x0000000000000000 0x0000000000000000         0x8	    <----

[...]

Older binutils will report something like

  LOPROC+0       0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000         8

instead of S390_PGSTE.

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Christian Borntraeger]

On 08/23/2017 10:39 AM, Cornelia Huck wrote:
> On Wed, 23 Aug 2017 10:16:27 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> On 23/08/2017 10:06, Thomas Huth wrote:
>>> On 23.08.2017 10:00, David Hildenbrand wrote:  
>>>> On 23.08.2017 08:53, Christian Borntraeger wrote:  
> 
>>>>> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
>>>>>    ldflags="$ldflags $textseg_ldflags"
>>>>>  fi
>>>>>  
>>>>> +# Newer kernels on s390 check for an S390_PGSTE program header and
>>>>> +# enable the pgste page table extensions in that case. This makes
>>>>> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
>>>>> +# header if
>>>>> +#  - we build on s390x
>>>>> +#  - we build the system emulation for s390x (qemu-system-s390x)
>>>>> +#  - KVM is enabled
>>>>> +#  - the linker support --s390-pgste
>>>>> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then  
>>>>
>>>> Wonder if the "$ARCH" check is really necessary: TARGET_ARCH=s390x with
>>>> kvm=yes should only build on s390x.  
>>>
>>> Isn't kvm=yes and TARGET_ARCH=s390x also possible on a x86 host, where
>>> only the x86_64 target is built with CONFIG_KVM=y, but the s390x target
>>> with CONFIG_KVM=n ?  
>>
>> Yes.  You could use
>>
>>   if test "$ARCH" = "s390x" && supported_kvm_target $target; then
>>     ...
>>   fi
>>
>> Or, in the existing "if supported_kvm_target $target" conditional, add
>>
>>   if test "$ARCH" = s390x && ld_has --s390-pgste; then
>>     ...
>>   fi
> 
> That conditional is unfortunately before the setup of ldflags; but I
> like the idea of using supported_kvm_target.

This is now bike-shedding, no? :-)
I think I prefer to write out the the single statements as is.
I would need to test for supported_kvm_target AND s390 anyway, to prevent
checking ld on x86 as well.

So unless there are complains, I will provide a v3 with the typo fixed.

Christian

Re: [Qemu-devel] [PATCH v2] configure: enable --s390-pgste linker option

[Cornelia Huck]

On Wed, 23 Aug 2017 11:05:59 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 08/23/2017 10:39 AM, Cornelia Huck wrote:
> > On Wed, 23 Aug 2017 10:16:27 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> >   
> >> On 23/08/2017 10:06, Thomas Huth wrote:  
> >>> On 23.08.2017 10:00, David Hildenbrand wrote:    
> >>>> On 23.08.2017 08:53, Christian Borntraeger wrote:    
> >   
> >>>>> @@ -6522,6 +6527,20 @@ if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
> >>>>>    ldflags="$ldflags $textseg_ldflags"
> >>>>>  fi
> >>>>>  
> >>>>> +# Newer kernels on s390 check for an S390_PGSTE program header and
> >>>>> +# enable the pgste page table extensions in that case. This makes
> >>>>> +# the vm.allocate_pgste sysctl unnecessary. We enable this program
> >>>>> +# header if
> >>>>> +#  - we build on s390x
> >>>>> +#  - we build the system emulation for s390x (qemu-system-s390x)
> >>>>> +#  - KVM is enabled
> >>>>> +#  - the linker support --s390-pgste
> >>>>> +if test "$TARGET_ARCH" = "s390x" -a "$target_softmmu" = "yes"  -a "$ARCH" = "s390x" -a "$kvm" = "yes"; then    
> >>>>
> >>>> Wonder if the "$ARCH" check is really necessary: TARGET_ARCH=s390x with
> >>>> kvm=yes should only build on s390x.    
> >>>
> >>> Isn't kvm=yes and TARGET_ARCH=s390x also possible on a x86 host, where
> >>> only the x86_64 target is built with CONFIG_KVM=y, but the s390x target
> >>> with CONFIG_KVM=n ?    
> >>
> >> Yes.  You could use
> >>
> >>   if test "$ARCH" = "s390x" && supported_kvm_target $target; then
> >>     ...
> >>   fi
> >>
> >> Or, in the existing "if supported_kvm_target $target" conditional, add
> >>
> >>   if test "$ARCH" = s390x && ld_has --s390-pgste; then
> >>     ...
> >>   fi  
> > 
> > That conditional is unfortunately before the setup of ldflags; but I
> > like the idea of using supported_kvm_target.  
> 
> This is now bike-shedding, no? :-)
> I think I prefer to write out the the single statements as is.
> I would need to test for supported_kvm_target AND s390 anyway, to prevent
> checking ld on x86 as well.

I prefer the shorter variant, but I don't mind the exploded one either.

> 
> So unless there are complains, I will provide a v3 with the typo fixed.

Fine with me as well.