| Message ID | 20180801112508.138159-1-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: Make huge pages unavailable in ucontrol vms | expand |
On Wed, 1 Aug 2018 12:25:08 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: s/vms/VMs/ ? (throughout the patch description) Otherwise, I keep wondering about supporting vms on s390 :) > We currently do not notify all gmaps when using gmap_pmdp_xchg(), due > to locking constraints. This makes ucontrol vms, which is the only vm > type that creates multiple gmaps, incompatible with huge pages. > > ucontrol vms are rather exotic and creating a new locking concept is > no easy task. Hence we return EINVAL when trying to active > KVM_CAP_S390_HPAGE_1M and report it as being not available when > checking for it. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index f9d90337e64a..549f38d1baa1 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -481,7 +481,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > break; > case KVM_CAP_S390_HPAGE_1M: > r = 0; > - if (hpage) > + if (hpage && !kvm_is_ucontrol(kvm)) > r = 1; > break; > case KVM_CAP_S390_MEM_OP: > @@ -691,7 +691,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap) > mutex_lock(&kvm->lock); > if (kvm->created_vcpus) > r = -EBUSY; > - else if (!hpage || kvm->arch.use_cmma) > + else if (!hpage || kvm->arch.use_cmma || kvm_is_ucontrol(kvm)) > r = -EINVAL; > else { > r = 0; Update the documentation for the cap as well?
On 01.08.2018 13:25, Janosch Frank wrote: > We currently do not notify all gmaps when using gmap_pmdp_xchg(), due > to locking constraints. This makes ucontrol vms, which is the only vm > type that creates multiple gmaps, incompatible with huge pages. > > ucontrol vms are rather exotic and creating a new locking concept is > no easy task. Hence we return EINVAL when trying to active > KVM_CAP_S390_HPAGE_1M and report it as being not available when > checking for it. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index f9d90337e64a..549f38d1baa1 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -481,7 +481,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) > break; > case KVM_CAP_S390_HPAGE_1M: > r = 0; > - if (hpage) > + if (hpage && !kvm_is_ucontrol(kvm)) > r = 1; > break; > case KVM_CAP_S390_MEM_OP: > @@ -691,7 +691,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap) > mutex_lock(&kvm->lock); > if (kvm->created_vcpus) > r = -EBUSY; > - else if (!hpage || kvm->arch.use_cmma) > + else if (!hpage || kvm->arch.use_cmma || kvm_is_ucontrol(kvm)) > r = -EINVAL; > else { > r = 0; > Reviewed-by: David Hildenbrand <david@redhat.com> Are there still users for ucontrol? Would it make sense to rip it out?
On 01.08.2018 13:42, David Hildenbrand wrote: > On 01.08.2018 13:25, Janosch Frank wrote: >> We currently do not notify all gmaps when using gmap_pmdp_xchg(), due >> to locking constraints. This makes ucontrol vms, which is the only vm >> type that creates multiple gmaps, incompatible with huge pages. >> >> ucontrol vms are rather exotic and creating a new locking concept is >> no easy task. Hence we return EINVAL when trying to active >> KVM_CAP_S390_HPAGE_1M and report it as being not available when >> checking for it. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> arch/s390/kvm/kvm-s390.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index f9d90337e64a..549f38d1baa1 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -481,7 +481,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) >> break; >> case KVM_CAP_S390_HPAGE_1M: >> r = 0; >> - if (hpage) >> + if (hpage && !kvm_is_ucontrol(kvm)) >> r = 1; >> break; >> case KVM_CAP_S390_MEM_OP: >> @@ -691,7 +691,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap) >> mutex_lock(&kvm->lock); >> if (kvm->created_vcpus) >> r = -EBUSY; >> - else if (!hpage || kvm->arch.use_cmma) >> + else if (!hpage || kvm->arch.use_cmma || kvm_is_ucontrol(kvm)) >> r = -EINVAL; >> else { >> r = 0; >> > > Reviewed-by: David Hildenbrand <david@redhat.com> Thanks > > Are there still users for ucontrol? Would it make sense to rip it out? There are still users and they would actually like to use huge pages in the future. Right now this is a cosmetical patch anyway, as the combination is not in use but I might need to find a solution for ucontrol...
On Wed, 1 Aug 2018 12:25:08 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > We currently do not notify all gmaps when using gmap_pmdp_xchg(), due > to locking constraints. This makes ucontrol vms, which is the only vm > type that creates multiple gmaps, incompatible with huge pages. > > ucontrol vms are rather exotic and creating a new locking concept is > no easy task. Hence we return EINVAL when trying to active > KVM_CAP_S390_HPAGE_1M and report it as being not available when > checking for it. makes sense > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index f9d90337e64a..549f38d1baa1 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -481,7 +481,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, > long ext) break; > case KVM_CAP_S390_HPAGE_1M: > r = 0; > - if (hpage) > + if (hpage && !kvm_is_ucontrol(kvm)) > r = 1; > break; > case KVM_CAP_S390_MEM_OP: > @@ -691,7 +691,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm > *kvm, struct kvm_enable_cap *cap) mutex_lock(&kvm->lock); > if (kvm->created_vcpus) > r = -EBUSY; > - else if (!hpage || kvm->arch.use_cmma) > + else if (!hpage || kvm->arch.use_cmma || > kvm_is_ucontrol(kvm)) r = -EINVAL; > else { > r = 0; the patch is rather straightforward Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index f9d90337e64a..549f38d1baa1 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -481,7 +481,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) break; case KVM_CAP_S390_HPAGE_1M: r = 0; - if (hpage) + if (hpage && !kvm_is_ucontrol(kvm)) r = 1; break; case KVM_CAP_S390_MEM_OP: @@ -691,7 +691,7 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap) mutex_lock(&kvm->lock); if (kvm->created_vcpus) r = -EBUSY; - else if (!hpage || kvm->arch.use_cmma) + else if (!hpage || kvm->arch.use_cmma || kvm_is_ucontrol(kvm)) r = -EINVAL; else { r = 0;
We currently do not notify all gmaps when using gmap_pmdp_xchg(), due to locking constraints. This makes ucontrol vms, which is the only vm type that creates multiple gmaps, incompatible with huge pages. ucontrol vms are rather exotic and creating a new locking concept is no easy task. Hence we return EINVAL when trying to active KVM_CAP_S390_HPAGE_1M and report it as being not available when checking for it. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- arch/s390/kvm/kvm-s390.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)