[RFC,14/14] KVM: s390: Allow the VSIE to be used with huge pages
diff mbox series

Message ID 20180919084802.183381-15-frankja@linux.ibm.com
State New
Headers show
Series
  • KVM: s390: Huge page splitting and shadowing
Related show

Commit Message

Janosch Frank Sept. 19, 2018, 8:48 a.m. UTC
Now that we have VSIE support for VMs with huge memory backing, let's
make both features usable at the same time.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 Documentation/virtual/kvm/api.txt |  7 +++----
 arch/s390/kvm/kvm-s390.c          | 14 ++------------
 arch/s390/mm/gmap.c               |  1 -
 3 files changed, 5 insertions(+), 17 deletions(-)

Comments

David Hildenbrand Sept. 19, 2018, 8:56 a.m. UTC | #1
Am 19.09.18 um 10:48 schrieb Janosch Frank:
> Now that we have VSIE support for VMs with huge memory backing, let's
> make both features usable at the same time.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  Documentation/virtual/kvm/api.txt |  7 +++----
>  arch/s390/kvm/kvm-s390.c          | 14 ++------------
>  arch/s390/mm/gmap.c               |  1 -
>  3 files changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 8d8a372c8340..9ec6d23f41d5 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -4509,15 +4509,14 @@ Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
>  
>  Architectures: s390
>  Parameters: none
> -Returns: 0 on success, -EINVAL if hpage module parameter was not set
> -	 or cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
> +Returns: 0 on success, -EINVAL if cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
>  	 flag set
>  
>  With this capability the KVM support for memory backing with 1m pages
>  through hugetlbfs can be enabled for a VM. After the capability is
>  enabled, cmma can't be enabled anymore and pfmfi and the storage key
> -interpretation are disabled. If cmma has already been enabled or the
> -hpage module parameter is not set to 1, -EINVAL is returned.
> +interpretation are disabled. If cmma has already been enabled, -EINVAL
> +is returned.
>  
>  While it is generally possible to create a huge page backed VM without
>  this capability, the VM will not be able to run.
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ac5da6b0b862..59f53b7c72d6 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -172,11 +172,6 @@ static int nested;
>  module_param(nested, int, S_IRUGO);
>  MODULE_PARM_DESC(nested, "Nested virtualization support");
>  
> -/* allow 1m huge page guest backing, if !nested */
> -static int hpage;
> -module_param(hpage, int, 0444);
> -MODULE_PARM_DESC(hpage, "1m huge page backing support");
> -


Do we want to keep the parameter for now?

This would allow admins to disable it system-wide as long as it is in
its early stage? (just like nested). We can allow co-existence, though.

>  /*
>   * For now we handle at most 16 double words as this is what the s390 base
>   * kernel handles and stores in the prefix page. If we ever need to go beyond
> @@ -481,7 +476,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		break;
>  	case KVM_CAP_S390_HPAGE_1M:
>  		r = 0;
> -		if (hpage && !kvm_is_ucontrol(kvm))
> +		if (!kvm_is_ucontrol(kvm))
>  			r = 1;
>  		break;
>  	case KVM_CAP_S390_MEM_OP:
> @@ -691,7 +686,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 || kvm_is_ucontrol(kvm))
> +		else if (kvm->arch.use_cmma || kvm_is_ucontrol(kvm))
>  			r = -EINVAL;
>  		else {
>  			r = 0;
> @@ -4196,11 +4191,6 @@ static int __init kvm_s390_init(void)
>  		return -ENODEV;
>  	}
>  
> -	if (nested && hpage) {
> -		pr_info("nested (vSIE) and hpage (huge page backing) can currently not be activated concurrently");
> -		return -EINVAL;
> -	}
> -
>  	for (i = 0; i < 16; i++)
>  		kvm_s390_fac_base[i] |=
>  			S390_lowcore.stfle_fac_list[i] & nonhyp_mask(i);
> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
> index 8d5ce51637eb..928cb5818a21 100644
> --- a/arch/s390/mm/gmap.c
> +++ b/arch/s390/mm/gmap.c
> @@ -1830,7 +1830,6 @@ struct gmap *gmap_shadow(struct gmap *parent, unsigned long asce,
>  	unsigned long limit;
>  	int rc;
>  
> -	BUG_ON(parent->mm->context.allow_gmap_hpage_1m);
>  	BUG_ON(gmap_is_shadow(parent));
>  	spin_lock(&parent->shadow_lock);
>  	sg = gmap_find_shadow(parent, asce, edat_level);
>
Janosch Frank Sept. 19, 2018, 9:50 a.m. UTC | #2
On 9/19/18 10:56 AM, David Hildenbrand wrote:
> Am 19.09.18 um 10:48 schrieb Janosch Frank:
>> Now that we have VSIE support for VMs with huge memory backing, let's
>> make both features usable at the same time.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  Documentation/virtual/kvm/api.txt |  7 +++----
>>  arch/s390/kvm/kvm-s390.c          | 14 ++------------
>>  arch/s390/mm/gmap.c               |  1 -
>>  3 files changed, 5 insertions(+), 17 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 8d8a372c8340..9ec6d23f41d5 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -4509,15 +4509,14 @@ Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
>>  
>>  Architectures: s390
>>  Parameters: none
>> -Returns: 0 on success, -EINVAL if hpage module parameter was not set
>> -	 or cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
>> +Returns: 0 on success, -EINVAL if cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
>>  	 flag set
>>  
>>  With this capability the KVM support for memory backing with 1m pages
>>  through hugetlbfs can be enabled for a VM. After the capability is
>>  enabled, cmma can't be enabled anymore and pfmfi and the storage key
>> -interpretation are disabled. If cmma has already been enabled or the
>> -hpage module parameter is not set to 1, -EINVAL is returned.
>> +interpretation are disabled. If cmma has already been enabled, -EINVAL
>> +is returned.
>>  
>>  While it is generally possible to create a huge page backed VM without
>>  this capability, the VM will not be able to run.
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index ac5da6b0b862..59f53b7c72d6 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -172,11 +172,6 @@ static int nested;
>>  module_param(nested, int, S_IRUGO);
>>  MODULE_PARM_DESC(nested, "Nested virtualization support");
>>  
>> -/* allow 1m huge page guest backing, if !nested */
>> -static int hpage;
>> -module_param(hpage, int, 0444);
>> -MODULE_PARM_DESC(hpage, "1m huge page backing support");
>> -
> 
> 
> Do we want to keep the parameter for now?
> 
> This would allow admins to disable it system-wide as long as it is in
> its early stage? (just like nested). We can allow co-existence, though.

Both options are fine with me and considering the fixes needed for the
enablement this might be the better option.

> 
>>  /*
>>   * For now we handle at most 16 double words as this is what the s390 base
>>   * kernel handles and stores in the prefix page. If we ever need to go beyond
>> @@ -481,7 +476,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  		break;
>>  	case KVM_CAP_S390_HPAGE_1M:
>>  		r = 0;
>> -		if (hpage && !kvm_is_ucontrol(kvm))
>> +		if (!kvm_is_ucontrol(kvm))
>>  			r = 1;
>>  		break;
>>  	case KVM_CAP_S390_MEM_OP:
>> @@ -691,7 +686,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 || kvm_is_ucontrol(kvm))
>> +		else if (kvm->arch.use_cmma || kvm_is_ucontrol(kvm))
>>  			r = -EINVAL;
>>  		else {
>>  			r = 0;
>> @@ -4196,11 +4191,6 @@ static int __init kvm_s390_init(void)
>>  		return -ENODEV;
>>  	}
>>  
>> -	if (nested && hpage) {
>> -		pr_info("nested (vSIE) and hpage (huge page backing) can currently not be activated concurrently");
>> -		return -EINVAL;
>> -	}
>> -
>>  	for (i = 0; i < 16; i++)
>>  		kvm_s390_fac_base[i] |=
>>  			S390_lowcore.stfle_fac_list[i] & nonhyp_mask(i);
>> diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
>> index 8d5ce51637eb..928cb5818a21 100644
>> --- a/arch/s390/mm/gmap.c
>> +++ b/arch/s390/mm/gmap.c
>> @@ -1830,7 +1830,6 @@ struct gmap *gmap_shadow(struct gmap *parent, unsigned long asce,
>>  	unsigned long limit;
>>  	int rc;
>>  
>> -	BUG_ON(parent->mm->context.allow_gmap_hpage_1m);
>>  	BUG_ON(gmap_is_shadow(parent));
>>  	spin_lock(&parent->shadow_lock);
>>  	sg = gmap_find_shadow(parent, asce, edat_level);
>>
>
Christian Borntraeger Sept. 19, 2018, 11:13 a.m. UTC | #3
On 09/19/2018 11:50 AM, Janosch Frank wrote:
> On 9/19/18 10:56 AM, David Hildenbrand wrote:
>> Am 19.09.18 um 10:48 schrieb Janosch Frank:
>>> Now that we have VSIE support for VMs with huge memory backing, let's
>>> make both features usable at the same time.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>>  Documentation/virtual/kvm/api.txt |  7 +++----
>>>  arch/s390/kvm/kvm-s390.c          | 14 ++------------
>>>  arch/s390/mm/gmap.c               |  1 -
>>>  3 files changed, 5 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>>> index 8d8a372c8340..9ec6d23f41d5 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -4509,15 +4509,14 @@ Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
>>>  
>>>  Architectures: s390
>>>  Parameters: none
>>> -Returns: 0 on success, -EINVAL if hpage module parameter was not set
>>> -	 or cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
>>> +Returns: 0 on success, -EINVAL if cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
>>>  	 flag set
>>>  
>>>  With this capability the KVM support for memory backing with 1m pages
>>>  through hugetlbfs can be enabled for a VM. After the capability is
>>>  enabled, cmma can't be enabled anymore and pfmfi and the storage key
>>> -interpretation are disabled. If cmma has already been enabled or the
>>> -hpage module parameter is not set to 1, -EINVAL is returned.
>>> +interpretation are disabled. If cmma has already been enabled, -EINVAL
>>> +is returned.
>>>  
>>>  While it is generally possible to create a huge page backed VM without
>>>  this capability, the VM will not be able to run.
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index ac5da6b0b862..59f53b7c72d6 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -172,11 +172,6 @@ static int nested;
>>>  module_param(nested, int, S_IRUGO);
>>>  MODULE_PARM_DESC(nested, "Nested virtualization support");
>>>  
>>> -/* allow 1m huge page guest backing, if !nested */
>>> -static int hpage;
>>> -module_param(hpage, int, 0444);
>>> -MODULE_PARM_DESC(hpage, "1m huge page backing support");
>>> -
>>
>>
>> Do we want to keep the parameter for now?
>>
>> This would allow admins to disable it system-wide as long as it is in
>> its early stage? (just like nested). We can allow co-existence, though.
> 
> Both options are fine with me and considering the fixes needed for the
> enablement this might be the better option.

I think keeping the hpage parameter is fine, but we can consider changing
the default to 1.

Patch
diff mbox series

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 8d8a372c8340..9ec6d23f41d5 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -4509,15 +4509,14 @@  Do not enable KVM_FEATURE_PV_UNHALT if you disable HLT exits.
 
 Architectures: s390
 Parameters: none
-Returns: 0 on success, -EINVAL if hpage module parameter was not set
-	 or cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
+Returns: 0 on success, -EINVAL if cmma is enabled, or the VM has the KVM_VM_S390_UCONTROL
 	 flag set
 
 With this capability the KVM support for memory backing with 1m pages
 through hugetlbfs can be enabled for a VM. After the capability is
 enabled, cmma can't be enabled anymore and pfmfi and the storage key
-interpretation are disabled. If cmma has already been enabled or the
-hpage module parameter is not set to 1, -EINVAL is returned.
+interpretation are disabled. If cmma has already been enabled, -EINVAL
+is returned.
 
 While it is generally possible to create a huge page backed VM without
 this capability, the VM will not be able to run.
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index ac5da6b0b862..59f53b7c72d6 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -172,11 +172,6 @@  static int nested;
 module_param(nested, int, S_IRUGO);
 MODULE_PARM_DESC(nested, "Nested virtualization support");
 
-/* allow 1m huge page guest backing, if !nested */
-static int hpage;
-module_param(hpage, int, 0444);
-MODULE_PARM_DESC(hpage, "1m huge page backing support");
-
 /*
  * For now we handle at most 16 double words as this is what the s390 base
  * kernel handles and stores in the prefix page. If we ever need to go beyond
@@ -481,7 +476,7 @@  int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		break;
 	case KVM_CAP_S390_HPAGE_1M:
 		r = 0;
-		if (hpage && !kvm_is_ucontrol(kvm))
+		if (!kvm_is_ucontrol(kvm))
 			r = 1;
 		break;
 	case KVM_CAP_S390_MEM_OP:
@@ -691,7 +686,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 || kvm_is_ucontrol(kvm))
+		else if (kvm->arch.use_cmma || kvm_is_ucontrol(kvm))
 			r = -EINVAL;
 		else {
 			r = 0;
@@ -4196,11 +4191,6 @@  static int __init kvm_s390_init(void)
 		return -ENODEV;
 	}
 
-	if (nested && hpage) {
-		pr_info("nested (vSIE) and hpage (huge page backing) can currently not be activated concurrently");
-		return -EINVAL;
-	}
-
 	for (i = 0; i < 16; i++)
 		kvm_s390_fac_base[i] |=
 			S390_lowcore.stfle_fac_list[i] & nonhyp_mask(i);
diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c
index 8d5ce51637eb..928cb5818a21 100644
--- a/arch/s390/mm/gmap.c
+++ b/arch/s390/mm/gmap.c
@@ -1830,7 +1830,6 @@  struct gmap *gmap_shadow(struct gmap *parent, unsigned long asce,
 	unsigned long limit;
 	int rc;
 
-	BUG_ON(parent->mm->context.allow_gmap_hpage_1m);
 	BUG_ON(gmap_is_shadow(parent));
 	spin_lock(&parent->shadow_lock);
 	sg = gmap_find_shadow(parent, asce, edat_level);