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 <address@hidden>
>> Cc: Alexander Graf <address@hidden>
>> Cc: Dan Horak <address@hidden>
>> Cc: David Hildenbrand <address@hidden>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> Acked-by: Janosch Frank <address@hidden>
>> ---
>> 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 <address@hidden>

once you've fixed the typo.

 Thomas