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

[Cornelia Huck <cohuck@redhat.com>]

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.