Message ID | 1503471212-109371-1-git-send-email-borntraeger@de.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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?
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
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>
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
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
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.
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.
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
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.
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