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

Message ID 20180502110814.31971-2-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand May 2, 2018, 11:08 a.m. UTC
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(-)

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!

Patch
diff mbox

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;
 		}