[v1,2/2] KVM: s390: vsie: simplify < 8k address checks

Message ID	20180502110814.31971-2-david@redhat.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <kvm-owner@kernel.org> From: David Hildenbrand <david@redhat.com> To: KVM <kvm@vger.kernel.org> Cc: linux-s390@vger.kernel.org, Christian Borntraeger <borntraeger@de.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Janosch Frank <frankja@linux.ibm.com>, Dan Carpenter <dan.carpenter@oracle.com>, David Hildenbrand <david@redhat.com> Subject: [PATCH v1 2/2] KVM: s390: vsie: simplify < 8k address checks Date: Wed, 2 May 2018 13:08:14 +0200 Message-Id: <20180502110814.31971-2-david@redhat.com> In-Reply-To: <20180502110814.31971-1-david@redhat.com> References: <20180502110655.31811-1-david@redhat.com> <20180502110814.31971-1-david@redhat.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk

Message ID

20180502110814.31971-2-david@redhat.com (mailing list archive)

State

New, archived

Headers

From: David Hildenbrand <david@redhat.com>
To: KVM <kvm@vger.kernel.org>
Cc: linux-s390@vger.kernel.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Cornelia Huck <cohuck@redhat.com>, Janosch Frank <frankja@linux.ibm.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	David Hildenbrand <david@redhat.com>
Subject: [PATCH v1 2/2] KVM: s390: vsie: simplify < 8k address checks
Date: Wed,  2 May 2018 13:08:14 +0200
Message-Id: <20180502110814.31971-2-david@redhat.com>
In-Reply-To: <20180502110814.31971-1-david@redhat.com>
References: <20180502110655.31811-1-david@redhat.com>
	<20180502110814.31971-1-david@redhat.com>
Sender: kvm-owner@vger.kernel.org
Precedence: bulk

Comments

Heiko Carstens May 2, 2018, 12:42 p.m. UTC | #1

On Wed, May 02, 2018 at 01:08:14PM +0200, David Hildenbrand wrote:
> This makes it certainly more readable.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/s390/kvm/vsie.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 969882b54266..7c51a9dc0ec8 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -557,7 +557,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
>  		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
>  	if (gpa) {
> -		if (!(gpa & ~0x1fffUL))
> +		if (gpa < 8192)

2 * PAGE_SIZE, please. That's how it is done on various other places too.

David Hildenbrand May 2, 2018, 2:28 p.m. UTC | #2

On 02.05.2018 14:42, Heiko Carstens wrote:
> On Wed, May 02, 2018 at 01:08:14PM +0200, David Hildenbrand wrote:
>> This makes it certainly more readable.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  arch/s390/kvm/vsie.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index 969882b54266..7c51a9dc0ec8 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -557,7 +557,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
>>  		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
>>  	if (gpa) {
>> -		if (!(gpa & ~0x1fffUL))
>> +		if (gpa < 8192)
> 
> 2 * PAGE_SIZE, please. That's how it is done on various other places too.
> 

Christian, does the documentation use the wording "lowcore" or "8192" / 8k?

If it is lowcore, I agree to use 2 * PAGE_SIZE. If not, I prefer it to
directly match documentation.

Anyhow, I leave this decision to Janosch and Christian. Thanks for
having alook!

Christian Borntraeger May 8, 2018, 7:30 a.m. UTC | #3

On 05/02/2018 04:28 PM, David Hildenbrand wrote:
> On 02.05.2018 14:42, Heiko Carstens wrote:
>> On Wed, May 02, 2018 at 01:08:14PM +0200, David Hildenbrand wrote:
>>> This makes it certainly more readable.
>>>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>>  arch/s390/kvm/vsie.c | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>> index 969882b54266..7c51a9dc0ec8 100644
>>> --- a/arch/s390/kvm/vsie.c
>>> +++ b/arch/s390/kvm/vsie.c
>>> @@ -557,7 +557,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
>>>  		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
>>>  	if (gpa) {
>>> -		if (!(gpa & ~0x1fffUL))
>>> +		if (gpa < 8192)
>>
>> 2 * PAGE_SIZE, please. That's how it is done on various other places too.
>>
> 
> Christian, does the documentation use the wording "lowcore" or "8192" / 8k?
> 
> If it is lowcore, I agree to use 2 * PAGE_SIZE. If not, I prefer it to
> directly match documentation.
> 
> Anyhow, I leave this decision to Janosch and Christian. Thanks for
> having alook!
> 
prefix area and reverse prefix area. 
In fact what we check here is actually the reversed prefix area so for completeness we
would need an additional check.

Christian Borntraeger May 8, 2018, 7:32 a.m. UTC | #4

On 05/08/2018 09:30 AM, Christian Borntraeger wrote:
> 
> 
> On 05/02/2018 04:28 PM, David Hildenbrand wrote:
>> On 02.05.2018 14:42, Heiko Carstens wrote:
>>> On Wed, May 02, 2018 at 01:08:14PM +0200, David Hildenbrand wrote:
>>>> This makes it certainly more readable.
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>  arch/s390/kvm/vsie.c | 10 +++++-----
>>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>>> index 969882b54266..7c51a9dc0ec8 100644
>>>> --- a/arch/s390/kvm/vsie.c
>>>> +++ b/arch/s390/kvm/vsie.c
>>>> @@ -557,7 +557,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
>>>>  		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
>>>>  	if (gpa) {
>>>> -		if (!(gpa & ~0x1fffUL))
>>>> +		if (gpa < 8192)
>>>
>>> 2 * PAGE_SIZE, please. That's how it is done on various other places too.
>>>
>>
>> Christian, does the documentation use the wording "lowcore" or "8192" / 8k?
>>
>> If it is lowcore, I agree to use 2 * PAGE_SIZE. If not, I prefer it to
>> directly match documentation.
>>
>> Anyhow, I leave this decision to Janosch and Christian. Thanks for
>> having alook!
>>
> prefix area and reverse prefix area. 
> In fact what we check here is actually the reversed prefix area so for completeness we
> would need an additional check.

We seem to check both for SCA so we are good for SCA.

David Hildenbrand May 8, 2018, 8:40 a.m. UTC | #5

On 08.05.2018 09:32, Christian Borntraeger wrote:
> 
> 
> On 05/08/2018 09:30 AM, Christian Borntraeger wrote:
>>
>>
>> On 05/02/2018 04:28 PM, David Hildenbrand wrote:
>>> On 02.05.2018 14:42, Heiko Carstens wrote:
>>>> On Wed, May 02, 2018 at 01:08:14PM +0200, David Hildenbrand wrote:
>>>>> This makes it certainly more readable.
>>>>>
>>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>>> ---
>>>>>  arch/s390/kvm/vsie.c | 10 +++++-----
>>>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>>>>> index 969882b54266..7c51a9dc0ec8 100644
>>>>> --- a/arch/s390/kvm/vsie.c
>>>>> +++ b/arch/s390/kvm/vsie.c
>>>>> @@ -557,7 +557,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>>>>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
>>>>>  		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
>>>>>  	if (gpa) {
>>>>> -		if (!(gpa & ~0x1fffUL))
>>>>> +		if (gpa < 8192)
>>>>
>>>> 2 * PAGE_SIZE, please. That's how it is done on various other places too.
>>>>
>>>
>>> Christian, does the documentation use the wording "lowcore" or "8192" / 8k?
>>>
>>> If it is lowcore, I agree to use 2 * PAGE_SIZE. If not, I prefer it to
>>> directly match documentation.
>>>
>>> Anyhow, I leave this decision to Janosch and Christian. Thanks for
>>> having alook!
>>>
>> prefix area and reverse prefix area. 
>> In fact what we check here is actually the reversed prefix area so for completeness we
>> would need an additional check.
> 
> We seem to check both for SCA so we are good for SCA.
> 

I remember that this check only had to be performed for the SCA.

I'll resend, using PAGE_SIZE * 2 then. Thanks!

diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 969882b54266..7c51a9dc0ec8 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -557,7 +557,7 @@  static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_64BSCAO))
 		gpa |= (u64) READ_ONCE(scb_o->scaoh) << 32;
 	if (gpa) {
-		if (!(gpa & ~0x1fffUL))
+		if (gpa < 8192)
 			rc = set_validity_icpt(scb_s, 0x0038U);
 		else if ((gpa & ~0x1fffUL) == kvm_s390_get_prefix(vcpu))
 			rc = set_validity_icpt(scb_s, 0x0011U);
@@ -578,7 +578,7 @@  static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 
 	gpa = READ_ONCE(scb_o->itdba) & ~0xffUL;
 	if (gpa && (scb_s->ecb & ECB_TE)) {
-		if (!(gpa & ~0x1fffUL)) {
+		if (gpa < 8192) {
 			rc = set_validity_icpt(scb_s, 0x0080U);
 			goto unpin;
 		}
@@ -594,7 +594,7 @@  static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 
 	gpa = READ_ONCE(scb_o->gvrd) & ~0x1ffUL;
 	if (gpa && (scb_s->eca & ECA_VX) && !(scb_s->ecd & ECD_HOSTREGMGMT)) {
-		if (!(gpa & ~0x1fffUL)) {
+		if (gpa < 8192) {
 			rc = set_validity_icpt(scb_s, 0x1310U);
 			goto unpin;
 		}
@@ -613,7 +613,7 @@  static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 
 	gpa = READ_ONCE(scb_o->riccbd) & ~0x3fUL;
 	if (gpa && (scb_s->ecb3 & ECB3_RI)) {
-		if (!(gpa & ~0x1fffUL)) {
+		if (gpa < 8192) {
 			rc = set_validity_icpt(scb_s, 0x0043U);
 			goto unpin;
 		}
@@ -632,7 +632,7 @@  static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 
 		gpa = READ_ONCE(scb_o->sdnxo) & ~0xfUL;
 		sdnxc = READ_ONCE(scb_o->sdnxo) & 0xfUL;
-		if (!gpa || !(gpa & ~0x1fffUL)) {
+		if (!gpa || gpa < 8192) {
 			rc = set_validity_icpt(scb_s, 0x10b0U);
 			goto unpin;
 		}

[v1,2/2] KVM: s390: vsie: simplify < 8k address checks

Commit Message

Comments

Patch