[PATCH 00/11] KVM: s390: Fixes and features for 4.14

[PATCH 00/11] KVM: s390: Fixes and features for 4.14

[Christian Borntraeger]

This is what I have pending for 4.14 in 
  git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  kvm-s390-next-4.14-1

It contains 2 merges (on topic branch from Martin for the tlb flushing
and a back-merge from the s390 fixes for master to avoid merge conflicts
with followup fixes). 

"New" patches are marked (x) below. (patches 5,6,10,11).
Pull request will come later this week.

----------------------------------------------------------------
KVM: s390: Fixes and features for 4.14

- merge of topic branch tlb-flushing from the s390 tree to get the
  no-dat base features
- wire up the no-dat enhancements in KVM
- multiple epoch facility (z14 feature)
- Configuration z/Architecture Mode fixup
- merge of kvm/master to avoid conflicts with additional sthyi fixes
- more sthyi fixes

----------------------------------------------------------------
Christian Borntraeger (3):
      Merge branch 'tlb-flushing' of git://git.kernel.org/.../s390/linux  into kernelorgnext
      KVM: s390: take srcu lock when getting/setting storage keys
      Merge tag 'kvm-s390-master-4.13-2' into kvms390/next

Claudio Imbrenda (1):
(x)   KVM: s390: expose no-DAT to guest and migration support

Collin L. Walling (1):
(x)   KVM: s390: Multiple Epoch Facility support

Heiko Carstens (3):
      KVM: s390: sthyi: fix sthyi inline assembly
      KVM: s390: sthyi: fix specification exception detection
(x)   KVM: s390: sthyi: remove invalid guest write access

Jason J. Herne (1):
(x)   KVM: s390: Support Configuration z/Architecture Mode

Martin Schwidefsky (4):
      s390/mm: tag normal pages vs pages used in page tables
      s390/mm: add no-dat TLB flush optimization
      s390/mm: add guest ASCE TLB flush optimization
      s390/mm,kvm: use nodat PGSTE tag to optimize TLB flushing

 Documentation/virtual/kvm/devices/vm.txt |  14 ++-
 arch/s390/include/asm/kvm_host.h         |   6 +-
 arch/s390/include/asm/page-states.h      |   3 +-
 arch/s390/include/asm/page.h             |   3 +
 arch/s390/include/asm/pgtable.h          |  88 ++++++++++----
 arch/s390/include/asm/setup.h            |   9 +-
 arch/s390/include/asm/tlbflush.h         |   7 +-
 arch/s390/include/uapi/asm/kvm.h         |   6 +
 arch/s390/kernel/suspend.c               |  22 +++-
 arch/s390/kernel/vdso.c                  |   2 +
 arch/s390/kvm/kvm-s390.c                 | 117 +++++++++++++++++-
 arch/s390/kvm/kvm-s390.h                 |   2 +
 arch/s390/kvm/priv.c                     |   6 +-
 arch/s390/kvm/sigp.c                     |  36 +++---
 arch/s390/kvm/sthyi.c                    |  13 +-
 arch/s390/kvm/vsie.c                     |  10 ++
 arch/s390/mm/init.c                      |   2 +
 arch/s390/mm/page-states.c               | 196 ++++++++++++++++++++++++++++---
 arch/s390/mm/pageattr.c                  |   2 +-
 arch/s390/mm/pgalloc.c                   |   2 +
 arch/s390/mm/pgtable.c                   | 160 +++++++++++++++++++------
 arch/s390/tools/gen_facilities.c         |   2 +
 drivers/s390/char/sclp_early.c           |   6 +-
 23 files changed, 595 insertions(+), 119 deletions(-)

[PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

kvm has always supported the concept of starting in z/Arch mode so let's
reflect the feature bit to the guest.

Also, we change sigp set architecture to reject any request to change
architecture modes.

Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
 arch/s390/tools/gen_facilities.c |  1 +
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
index 1a252f5..11d3845 100644
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -155,29 +155,26 @@ static int __sigp_stop_and_store_status(struct kvm_vcpu *vcpu,
 	return rc;
 }
 
-static int __sigp_set_arch(struct kvm_vcpu *vcpu, u32 parameter)
+static int __sigp_set_arch(struct kvm_vcpu *vcpu, u32 parameter,
+			   u64 *status_reg)
 {
-	int rc;
 	unsigned int i;
 	struct kvm_vcpu *v;
+	bool all_stopped = true;
 
-	switch (parameter & 0xff) {
-	case 0:
-		rc = SIGP_CC_NOT_OPERATIONAL;
-		break;
-	case 1:
-	case 2:
-		kvm_for_each_vcpu(i, v, vcpu->kvm) {
-			v->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
-			kvm_clear_async_pf_completion_queue(v);
-		}
-
-		rc = SIGP_CC_ORDER_CODE_ACCEPTED;
-		break;
-	default:
-		rc = -EOPNOTSUPP;
+	kvm_for_each_vcpu(i, v, vcpu->kvm) {
+		if (v == vcpu)
+			continue;
+		if (!is_vcpu_stopped(v))
+			all_stopped = false;
 	}
-	return rc;
+
+	*status_reg &= 0xffffffff00000000ULL;
+
+	/* Reject set arch order, with czam we're always in z/Arch mode. */
+	*status_reg |= (all_stopped ? SIGP_STATUS_INVALID_PARAMETER :
+					SIGP_STATUS_INCORRECT_STATE);
+	return SIGP_CC_STATUS_STORED;
 }
 
 static int __sigp_set_prefix(struct kvm_vcpu *vcpu, struct kvm_vcpu *dst_vcpu,
@@ -446,7 +443,8 @@ int kvm_s390_handle_sigp(struct kvm_vcpu *vcpu)
 	switch (order_code) {
 	case SIGP_SET_ARCHITECTURE:
 		vcpu->stat.instruction_sigp_arch++;
-		rc = __sigp_set_arch(vcpu, parameter);
+		rc = __sigp_set_arch(vcpu, parameter,
+				     &vcpu->run->s.regs.gprs[r1]);
 		break;
 	default:
 		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index 025ea20..181db5b 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
 			78, /* enhanced-DAT 2 */
 			130, /* instruction-execution-protection */
 			131, /* enhanced-SOP 2 and side-effect */
+			138, /* configuration z/architecture mode (czam) */
 			146, /* msa extension 8 */
 			-1  /* END */
 		}
-- 
2.7.4

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Cornelia Huck]

On Mon, 28 Aug 2017 10:07:28 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> kvm has always supported the concept of starting in z/Arch mode so let's
> reflect the feature bit to the guest.
> 
> Also, we change sigp set architecture to reject any request to change
> architecture modes.

What are the implications of this for guests? IIRC, czam is a z14
feature (the updated PoP is not yet out, is it?). We now reject any set
arch sigp, even if we profess to be an older machine. Linux can
probably deal with this, but is it correct?

> 
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
>  arch/s390/tools/gen_facilities.c |  1 +
>  2 files changed, 18 insertions(+), 19 deletions(-)

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 11:07 AM, Cornelia Huck wrote:
> On Mon, 28 Aug 2017 10:07:28 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> kvm has always supported the concept of starting in z/Arch mode so let's
>> reflect the feature bit to the guest.
>>
>> Also, we change sigp set architecture to reject any request to change
>> architecture modes.
> 
> What are the implications of this for guests? IIRC, czam is a z14
> feature (the updated PoP is not yet out, is it?). We now reject any set
> arch sigp, even if we profess to be an older machine. Linux can
> probably deal with this, but is it correct?

Linux can deal with it. Since KVM does not support IPL in ESA mode, we basically
always provided the czam functionality. We now do it like it was architected.
z/VM does it the same way when enabling czam on an older system like z13.

In other words the old code was not compliant (and never was), the new code
is compliant. Since czam is kind of a "negative" facility, we can implement
it also on older systems.
> 
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
>>  arch/s390/tools/gen_facilities.c |  1 +
>>  2 files changed, 18 insertions(+), 19 deletions(-)
> 

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 11:11 AM, Christian Borntraeger wrote:
> 
> 
> On 08/28/2017 11:07 AM, Cornelia Huck wrote:
>> On Mon, 28 Aug 2017 10:07:28 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>>
>>> kvm has always supported the concept of starting in z/Arch mode so let's
>>> reflect the feature bit to the guest.
>>>
>>> Also, we change sigp set architecture to reject any request to change
>>> architecture modes.
>>
>> What are the implications of this for guests? IIRC, czam is a z14
>> feature (the updated PoP is not yet out, is it?). We now reject any set
>> arch sigp, even if we profess to be an older machine. Linux can
>> probably deal with this, but is it correct?
> 
> Linux can deal with it. Since KVM does not support IPL in ESA mode, we basically
> always provided the czam functionality. We now do it like it was architected.
> z/VM does it the same way when enabling czam on an older system like z13.
> 
> In other words the old code was not compliant (and never was), the new code
> is compliant. Since czam is kind of a "negative" facility, we can implement
> it also on older systems.

FWIW, this code will only run when QEMU does not enable the SIGP forwarding. 
All new QEMUs handle this in QEMU. The equivalent fix in QEMU is

commit 075e52b816648f21ca6bf5cc01624dff3993c99e
Author:     Jason J. Herne <jjherne@linux.vnet.ibm.com>
AuthorDate: Thu Sep 1 10:56:49 2016 +0200
Commit:     Christian Borntraeger <borntraeger@de.ibm.com>
CommitDate: Fri Jul 14 12:29:49 2017 +0200

    s390x/cpumodel: we are always in zarchitecture model

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Cornelia Huck]

On Mon, 28 Aug 2017 11:11:15 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 08/28/2017 11:07 AM, Cornelia Huck wrote:
> > On Mon, 28 Aug 2017 10:07:28 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >   
> >> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> >>
> >> kvm has always supported the concept of starting in z/Arch mode so let's
> >> reflect the feature bit to the guest.
> >>
> >> Also, we change sigp set architecture to reject any request to change
> >> architecture modes.  
> > 
> > What are the implications of this for guests? IIRC, czam is a z14
> > feature (the updated PoP is not yet out, is it?). We now reject any set
> > arch sigp, even if we profess to be an older machine. Linux can
> > probably deal with this, but is it correct?  
> 
> Linux can deal with it. Since KVM does not support IPL in ESA mode, we basically
> always provided the czam functionality. We now do it like it was architected.
> z/VM does it the same way when enabling czam on an older system like z13.

OK, this implies any z operating system needs to be able to deal with
czam on older machines.

> In other words the old code was not compliant (and never was), the new code
> is compliant. Since czam is kind of a "negative" facility, we can implement
> it also on older systems.

Looking at this, we should mirror this in qemu in tcg mode, as the
corresponding qemu patch only handles the kvm path (I obviously did not
look at the patch in enough detail...) Added to my list.

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Cornelia Huck]

On Mon, 28 Aug 2017 10:07:28 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> kvm has always supported the concept of starting in z/Arch mode so let's
> reflect the feature bit to the guest.
> 
> Also, we change sigp set architecture to reject any request to change
> architecture modes.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
>  arch/s390/tools/gen_facilities.c |  1 +
>  2 files changed, 18 insertions(+), 19 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

On 28.08.2017 10:07, Christian Borntraeger wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> kvm has always supported the concept of starting in z/Arch mode so let's
> reflect the feature bit to the guest.
> 
> Also, we change sigp set architecture to reject any request to change
> architecture modes.

Hm ... this seems to imply that CZAM is always set, but what about
running on old user space (possibly on old hw)? Old QEMU will not enable
CZAM.

And especially old user space will rely on SET ARCHITECTURE being
handled in the kernel.

I agree, that for QEMU we can always do that change (as we control which
features are indicated to the guest). But I am wondering if we would
have to stick to existing behavior unless test_kvm_facility(vcpu->kvm, 138).

> 
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
>  arch/s390/tools/gen_facilities.c |  1 +
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
> index 1a252f5..11d3845 100644
> --- a/arch/s390/kvm/sigp.c
> +++ b/arch/s390/kvm/sigp.c
> @@ -155,29 +155,26 @@ static int __sigp_stop_and_store_status(struct kvm_vcpu *vcpu,
>  	return rc;
>  }
>  
[...]

>  	}
> -	return rc;
> +
> +	*status_reg &= 0xffffffff00000000ULL;

s/ULL/UL/

> +
> +	/* Reject set arch order, with czam we're always in z/Arch mode. */
> +	*status_reg |= (all_stopped ? SIGP_STATUS_INVALID_PARAMETER :
> +					SIGP_STATUS_INCORRECT_STATE);
> +	return SIGP_CC_STATUS_STORED;
>  }
>  
>  static int __sigp_set_prefix(struct kvm_vcpu *vcpu, struct kvm_vcpu *dst_vcpu,
> @@ -446,7 +443,8 @@ int kvm_s390_handle_sigp(struct kvm_vcpu *vcpu)
>  	switch (order_code) {
>  	case SIGP_SET_ARCHITECTURE:
>  		vcpu->stat.instruction_sigp_arch++;
> -		rc = __sigp_set_arch(vcpu, parameter);
> +		rc = __sigp_set_arch(vcpu, parameter,
> +				     &vcpu->run->s.regs.gprs[r1]);
>  		break;
>  	default:
>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 025ea20..181db5b 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>  			78, /* enhanced-DAT 2 */
>  			130, /* instruction-execution-protection */
>  			131, /* enhanced-SOP 2 and side-effect */
> +			138, /* configuration z/architecture mode (czam) */
>  			146, /* msa extension 8 */
>  			-1  /* END */
>  		}
> 


-- 

Thanks,

David

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 04:06 PM, David Hildenbrand wrote:
> On 28.08.2017 10:07, Christian Borntraeger wrote:
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> kvm has always supported the concept of starting in z/Arch mode so let's
>> reflect the feature bit to the guest.
>>
>> Also, we change sigp set architecture to reject any request to change
>> architecture modes.
> 
> Hm ... this seems to imply that CZAM is always set, but what about
> running on old user space (possibly on old hw)? Old QEMU will not enable
> CZAM.

3 cases.
1. very old QEMU without user sigp
2. old QEMU with user sigp/without CPU model
3. new QEMU with user sigp/cpu model

I think we agree that cases 2 and 3 should not matter at all for this kernel patch
as the sigp is handled by QEMU. 



This is case 1:
> And especially old user space will rely on SET ARCHITECTURE being
> handled in the kernel.


Yes, and it continues to be handled in the kernel. It is just that the guest
will now see a different sigp return code. Before, our sigp implementation lied
to the guest in a way that worked for Linux (we lied by saying "yes, we switched"). 
We now say "sorry, we are already in zarch mode, sigp ignored" which also works
perfectly fine for Linux. And IMHO it is even the better choice even without 
STFLE.138 being set as it matches what an old hardware would do when in zarch mode.


> 
> I agree, that for QEMU we can always do that change (as we control which
> features are indicated to the guest). But I am wondering if we would
> have to stick to existing behavior unless test_kvm_facility(vcpu->kvm, 138).
> 
>>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  arch/s390/kvm/sigp.c             | 36 +++++++++++++++++-------------------
>>  arch/s390/tools/gen_facilities.c |  1 +
>>  2 files changed, 18 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
>> index 1a252f5..11d3845 100644
>> --- a/arch/s390/kvm/sigp.c
>> +++ b/arch/s390/kvm/sigp.c
>> @@ -155,29 +155,26 @@ static int __sigp_stop_and_store_status(struct kvm_vcpu *vcpu,
>>  	return rc;
>>  }
>>  
> [...]
> 
>>  	}
>> -	return rc;
>> +
>> +	*status_reg &= 0xffffffff00000000ULL;
> 
> s/ULL/UL/


Right, will fix.

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

On 28.08.2017 16:24, Christian Borntraeger wrote:
> 
> 
> On 08/28/2017 04:06 PM, David Hildenbrand wrote:
>> On 28.08.2017 10:07, Christian Borntraeger wrote:
>>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>>
>>> kvm has always supported the concept of starting in z/Arch mode so let's
>>> reflect the feature bit to the guest.
>>>
>>> Also, we change sigp set architecture to reject any request to change
>>> architecture modes.
>>
>> Hm ... this seems to imply that CZAM is always set, but what about
>> running on old user space (possibly on old hw)? Old QEMU will not enable
>> CZAM.
> 
> 3 cases.
> 1. very old QEMU without user sigp
> 2. old QEMU with user sigp/without CPU model
> 3. new QEMU with user sigp/cpu model
> 
> I think we agree that cases 2 and 3 should not matter at all for this kernel patch
> as the sigp is handled by QEMU. 
> 
> 
> 
> This is case 1:
>> And especially old user space will rely on SET ARCHITECTURE being
>> handled in the kernel.
> 
> 
> Yes, and it continues to be handled in the kernel. It is just that the guest
> will now see a different sigp return code. Before, our sigp implementation lied
> to the guest in a way that worked for Linux (we lied by saying "yes, we switched"). 
> We now say "sorry, we are already in zarch mode, sigp ignored" which also works
> perfectly fine for Linux. And IMHO it is even the better choice even without 
> STFLE.138 being set as it matches what an old hardware would do when in zarch mode.

Ok, if it worked for relevant Linux versions, than it should indeed be fine.

Thanks!

>> [...]
>>
>>>  	}
>>> -	return rc;
>>> +
>>> +	*status_reg &= 0xffffffff00000000ULL;
>>
>> s/ULL/UL/
> 
> 
> Right, will fix.
> 


-- 

Thanks,

David

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 04:38 PM, David Hildenbrand wrote:
> On 28.08.2017 16:24, Christian Borntraeger wrote:
>>
>>
>> On 08/28/2017 04:06 PM, David Hildenbrand wrote:
>>> On 28.08.2017 10:07, Christian Borntraeger wrote:
>>>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>>>
>>>> kvm has always supported the concept of starting in z/Arch mode so let's
>>>> reflect the feature bit to the guest.
>>>>
>>>> Also, we change sigp set architecture to reject any request to change
>>>> architecture modes.
>>>
>>> Hm ... this seems to imply that CZAM is always set, but what about
>>> running on old user space (possibly on old hw)? Old QEMU will not enable
>>> CZAM.
>>
>> 3 cases.
>> 1. very old QEMU without user sigp
>> 2. old QEMU with user sigp/without CPU model
>> 3. new QEMU with user sigp/cpu model
>>
>> I think we agree that cases 2 and 3 should not matter at all for this kernel patch
>> as the sigp is handled by QEMU. 
>>
>>
>>
>> This is case 1:
>>> And especially old user space will rely on SET ARCHITECTURE being
>>> handled in the kernel.
>>
>>
>> Yes, and it continues to be handled in the kernel. It is just that the guest
>> will now see a different sigp return code. Before, our sigp implementation lied
>> to the guest in a way that worked for Linux (we lied by saying "yes, we switched"). 
>> We now say "sorry, we are already in zarch mode, sigp ignored" which also works
>> perfectly fine for Linux. And IMHO it is even the better choice even without 
>> STFLE.138 being set as it matches what an old hardware would do when in zarch mode.
> 
> Ok, if it worked for relevant Linux versions, than it should indeed be fine.

I checked back to 2006 and Linux never checked the return value for this sigp.

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 025ea20..181db5b 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>  			78, /* enhanced-DAT 2 */
>  			130, /* instruction-execution-protection */
>  			131, /* enhanced-SOP 2 and side-effect */
> +			138, /* configuration z/architecture mode (czam) */
>  			146, /* msa extension 8 */
>  			-1  /* END */
>  		}
> 

Thinking about it, this should be the right thing to do instead of the
last hunk:

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 84c069afc02f..39115f5a38df 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
long type)
        memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
               S390_ARCH_FAC_LIST_SIZE_BYTE);

+       /* we are always in CZAM mode - even on pre z14 machines */
+       set_kvm_facility(kvm->arch.model.fac_mask, 138);
+       set_kvm_facility(kvm->arch.model.fac_list, 138);
+       /* we emulate STHYI in kvm */
        set_kvm_facility(kvm->arch.model.fac_mask, 74);
        set_kvm_facility(kvm->arch.model.fac_list, 74);
        if (MACHINE_HAS_TLB_GUEST) {

That would produce consistent results for very old QEMU.

-- 

Thanks,

David

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 09:27 PM, David Hildenbrand wrote:
> 
>>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>> index 025ea20..181db5b 100644
>> --- a/arch/s390/tools/gen_facilities.c
>> +++ b/arch/s390/tools/gen_facilities.c
>> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>>  			78, /* enhanced-DAT 2 */
>>  			130, /* instruction-execution-protection */
>>  			131, /* enhanced-SOP 2 and side-effect */
>> +			138, /* configuration z/architecture mode (czam) */
>>  			146, /* msa extension 8 */
>>  			-1  /* END */
>>  		}
>>
> 
> Thinking about it, this should be the right thing to do instead of the
> last hunk:
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 84c069afc02f..39115f5a38df 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
> long type)
>         memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>                S390_ARCH_FAC_LIST_SIZE_BYTE);
> 
> +       /* we are always in CZAM mode - even on pre z14 machines */
> +       set_kvm_facility(kvm->arch.model.fac_mask, 138);
> +       set_kvm_facility(kvm->arch.model.fac_list, 138);
> +       /* we emulate STHYI in kvm */
>         set_kvm_facility(kvm->arch.model.fac_mask, 74);
>         set_kvm_facility(kvm->arch.model.fac_list, 74);
>         if (MACHINE_HAS_TLB_GUEST) {
> 
> That would produce consistent results for very old QEMU.
> 

This should be identical as the initial fac_list is populated from the fac_mask,
which is populated by the facility_defs structure.

[..]
       /* Populate the facility list initially. */
        kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
---->   memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
               S390_ARCH_FAC_LIST_SIZE_BYTE);

[,..]

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 09:35 PM, Christian Borntraeger wrote:
> 
> 
> On 08/28/2017 09:27 PM, David Hildenbrand wrote:
>>
>>>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>> index 025ea20..181db5b 100644
>>> --- a/arch/s390/tools/gen_facilities.c
>>> +++ b/arch/s390/tools/gen_facilities.c
>>> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>>>  			78, /* enhanced-DAT 2 */
>>>  			130, /* instruction-execution-protection */
>>>  			131, /* enhanced-SOP 2 and side-effect */
>>> +			138, /* configuration z/architecture mode (czam) */
>>>  			146, /* msa extension 8 */
>>>  			-1  /* END */
>>>  		}
>>>
>>
>> Thinking about it, this should be the right thing to do instead of the
>> last hunk:
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 84c069afc02f..39115f5a38df 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
>> long type)
>>         memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>>                S390_ARCH_FAC_LIST_SIZE_BYTE);
>>
>> +       /* we are always in CZAM mode - even on pre z14 machines */
>> +       set_kvm_facility(kvm->arch.model.fac_mask, 138);
>> +       set_kvm_facility(kvm->arch.model.fac_list, 138);
>> +       /* we emulate STHYI in kvm */
>>         set_kvm_facility(kvm->arch.model.fac_mask, 74);
>>         set_kvm_facility(kvm->arch.model.fac_list, 74);
>>         if (MACHINE_HAS_TLB_GUEST) {
>>
>> That would produce consistent results for very old QEMU.
>>
> 
> This should be identical as the initial fac_list is populated from the fac_mask,
> which is populated by the facility_defs structure.
> 
> [..]
>        /* Populate the facility list initially. */
>         kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
> ---->   memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>                S390_ARCH_FAC_LIST_SIZE_BYTE);
> 
> [,..]

Well not identical. The patch actually enables the czam facility only if the host
hardware while your variant enables it always. Interesting discussion if it matters
if we should enable it on z13, z12 and earlier or not

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

On 28.08.2017 21:38, Christian Borntraeger wrote:
> 
> 
> On 08/28/2017 09:35 PM, Christian Borntraeger wrote:
>>
>>
>> On 08/28/2017 09:27 PM, David Hildenbrand wrote:
>>>
>>>>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>> index 025ea20..181db5b 100644
>>>> --- a/arch/s390/tools/gen_facilities.c
>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>>>>  			78, /* enhanced-DAT 2 */
>>>>  			130, /* instruction-execution-protection */
>>>>  			131, /* enhanced-SOP 2 and side-effect */
>>>> +			138, /* configuration z/architecture mode (czam) */
>>>>  			146, /* msa extension 8 */
>>>>  			-1  /* END */
>>>>  		}
>>>>
>>>
>>> Thinking about it, this should be the right thing to do instead of the
>>> last hunk:
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 84c069afc02f..39115f5a38df 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
>>> long type)
>>>         memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>>>                S390_ARCH_FAC_LIST_SIZE_BYTE);
>>>
>>> +       /* we are always in CZAM mode - even on pre z14 machines */
>>> +       set_kvm_facility(kvm->arch.model.fac_mask, 138);
>>> +       set_kvm_facility(kvm->arch.model.fac_list, 138);
>>> +       /* we emulate STHYI in kvm */
>>>         set_kvm_facility(kvm->arch.model.fac_mask, 74);
>>>         set_kvm_facility(kvm->arch.model.fac_list, 74);
>>>         if (MACHINE_HAS_TLB_GUEST) {
>>>
>>> That would produce consistent results for very old QEMU.
>>>
>>
>> This should be identical as the initial fac_list is populated from the fac_mask,
>> which is populated by the facility_defs structure.
>>
>> [..]
>>        /* Populate the facility list initially. */
>>         kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
>> ---->   memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>>                S390_ARCH_FAC_LIST_SIZE_BYTE);
>>
>> [,..]
> 
> Well not identical. The patch actually enables the czam facility only if the host
> hardware while your variant enables it always. Interesting discussion if it matters
> if we should enable it on z13, z12 and earlier or not
> 

This is also what we do on the QEMU side and what I meant by "consistent".

-- 

Thanks,

David

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/28/2017 09:42 PM, David Hildenbrand wrote:
> On 28.08.2017 21:38, Christian Borntraeger wrote:
>>
>>
>> On 08/28/2017 09:35 PM, Christian Borntraeger wrote:
>>>
>>>
>>> On 08/28/2017 09:27 PM, David Hildenbrand wrote:
>>>>
>>>>>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
>>>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>>>> index 025ea20..181db5b 100644
>>>>> --- a/arch/s390/tools/gen_facilities.c
>>>>> +++ b/arch/s390/tools/gen_facilities.c
>>>>> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>>>>>  			78, /* enhanced-DAT 2 */
>>>>>  			130, /* instruction-execution-protection */
>>>>>  			131, /* enhanced-SOP 2 and side-effect */
>>>>> +			138, /* configuration z/architecture mode (czam) */
>>>>>  			146, /* msa extension 8 */
>>>>>  			-1  /* END */
>>>>>  		}
>>>>>
>>>>
>>>> Thinking about it, this should be the right thing to do instead of the
>>>> last hunk:
>>>>
>>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>>> index 84c069afc02f..39115f5a38df 100644
>>>> --- a/arch/s390/kvm/kvm-s390.c
>>>> +++ b/arch/s390/kvm/kvm-s390.c
>>>> @@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
>>>> long type)
>>>>         memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>>>>                S390_ARCH_FAC_LIST_SIZE_BYTE);
>>>>
>>>> +       /* we are always in CZAM mode - even on pre z14 machines */
>>>> +       set_kvm_facility(kvm->arch.model.fac_mask, 138);
>>>> +       set_kvm_facility(kvm->arch.model.fac_list, 138);
>>>> +       /* we emulate STHYI in kvm */
>>>>         set_kvm_facility(kvm->arch.model.fac_mask, 74);
>>>>         set_kvm_facility(kvm->arch.model.fac_list, 74);
>>>>         if (MACHINE_HAS_TLB_GUEST) {
>>>>
>>>> That would produce consistent results for very old QEMU.
>>>>
>>>
>>> This should be identical as the initial fac_list is populated from the fac_mask,
>>> which is populated by the facility_defs structure.
>>>
>>> [..]
>>>        /* Populate the facility list initially. */
>>>         kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
>>> ---->   memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>>>                S390_ARCH_FAC_LIST_SIZE_BYTE);
>>>
>>> [,..]
>>
>> Well not identical. The patch actually enables the czam facility only if the host
>> hardware while your variant enables it always. Interesting discussion if it matters
>> if we should enable it on z13, z12 and earlier or not
>>
> 
> This is also what we do on the QEMU side and what I meant by "consistent".

Yes, I agree. QEMU will always enable stfle.138. So should the kernel.
Shall I fixup the patch with your change proposal or do you want to send an on-top patch?

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

>> This is also what we do on the QEMU side and what I meant by "consistent".
> 
> Yes, I agree. QEMU will always enable stfle.138. So should the kernel.
> Shall I fixup the patch with your change proposal or do you want to send an on-top patch?
> 

Whatever you prefer!

-- 

Thanks,

David

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[Christian Borntraeger]

On 08/29/2017 02:08 PM, David Hildenbrand wrote:
> 
>>> This is also what we do on the QEMU side and what I meant by "consistent".
>>
>> Yes, I agree. QEMU will always enable stfle.138. So should the kernel.
>> Shall I fixup the patch with your change proposal or do you want to send an on-top patch?
>>
> 
> Whatever you prefer!

Can you send a fixup patch on top?

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

On 29.08.2017 14:21, Christian Borntraeger wrote:
> 
> 
> On 08/29/2017 02:08 PM, David Hildenbrand wrote:
>>
>>>> This is also what we do on the QEMU side and what I meant by "consistent".
>>>
>>> Yes, I agree. QEMU will always enable stfle.138. So should the kernel.
>>> Shall I fixup the patch with your change proposal or do you want to send an on-top patch?
>>>
>>
>> Whatever you prefer!
> 
> Can you send a fixup patch on top?
> 

Will do!

-- 

Thanks,

David

[PATCH] KVM: s390: we are always in czam mode

[David Hildenbrand]

Indepenent of the underlying hardware, kvm will now always handle
SIGP SET ARCHITECTURE as if czam is enabled. Therefore, let's not
only forward that bit but always set it.

While at it, add a comment regarding STHYI.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/s390/kvm/kvm-s390.c         | 4 ++++
 arch/s390/tools/gen_facilities.c | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 84c069afc02f..39115f5a38df 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
 	       S390_ARCH_FAC_LIST_SIZE_BYTE);
 
+	/* we are always in czam mode - even on pre z14 machines */
+	set_kvm_facility(kvm->arch.model.fac_mask, 138);
+	set_kvm_facility(kvm->arch.model.fac_list, 138);
+	/* we emulate STHYI in kvm */
 	set_kvm_facility(kvm->arch.model.fac_mask, 74);
 	set_kvm_facility(kvm->arch.model.fac_list, 74);
 	if (MACHINE_HAS_TLB_GUEST) {
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index 601bfcf99e2a..19f12cef5bdf 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -80,7 +80,6 @@ static struct facility_def facility_defs[] = {
 			78, /* enhanced-DAT 2 */
 			130, /* instruction-execution-protection */
 			131, /* enhanced-SOP 2 and side-effect */
-			138, /* configuration z/architecture mode (czam) */
 			139, /* multiple epoch facility */
 			146, /* msa extension 8 */
 			-1  /* END */
-- 
2.13.5

Re: [PATCH] KVM: s390: we are always in czam mode

[Cornelia Huck]

On Tue, 29 Aug 2017 16:31:08 +0200
David Hildenbrand <david@redhat.com> wrote:

> Indepenent of the underlying hardware, kvm will now always handle

s/Indepenent/Independent/

> SIGP SET ARCHITECTURE as if czam is enabled. Therefore, let's not

s/is/were/, I think?

> only forward that bit but always set it.
> 
> While at it, add a comment regarding STHYI.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>  arch/s390/tools/gen_facilities.c | 1 -
>  2 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH] KVM: s390: we are always in czam mode

[Christian Borntraeger]

applied to next (with Connys language fixes)

On 08/29/2017 04:31 PM, David Hildenbrand wrote:
> Indepenent of the underlying hardware, kvm will now always handle
> SIGP SET ARCHITECTURE as if czam is enabled. Therefore, let's not
> only forward that bit but always set it.
> 
> While at it, add a comment regarding STHYI.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/s390/kvm/kvm-s390.c         | 4 ++++
>  arch/s390/tools/gen_facilities.c | 1 -
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 84c069afc02f..39115f5a38df 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>  	       S390_ARCH_FAC_LIST_SIZE_BYTE);
> 
> +	/* we are always in czam mode - even on pre z14 machines */
> +	set_kvm_facility(kvm->arch.model.fac_mask, 138);
> +	set_kvm_facility(kvm->arch.model.fac_list, 138);
> +	/* we emulate STHYI in kvm */
>  	set_kvm_facility(kvm->arch.model.fac_mask, 74);
>  	set_kvm_facility(kvm->arch.model.fac_list, 74);
>  	if (MACHINE_HAS_TLB_GUEST) {
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 601bfcf99e2a..19f12cef5bdf 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -80,7 +80,6 @@ static struct facility_def facility_defs[] = {
>  			78, /* enhanced-DAT 2 */
>  			130, /* instruction-execution-protection */
>  			131, /* enhanced-SOP 2 and side-effect */
> -			138, /* configuration z/architecture mode (czam) */
>  			139, /* multiple epoch facility */
>  			146, /* msa extension 8 */
>  			-1  /* END */
> 

Re: [PATCH 05/11] KVM: s390: Support Configuration z/Architecture Mode

[David Hildenbrand]

On 28.08.2017 21:35, Christian Borntraeger wrote:
> 
> 
> On 08/28/2017 09:27 PM, David Hildenbrand wrote:
>>
>>>  		rc = handle_sigp_dst(vcpu, order_code, cpu_addr,
>>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>>> index 025ea20..181db5b 100644
>>> --- a/arch/s390/tools/gen_facilities.c
>>> +++ b/arch/s390/tools/gen_facilities.c
>>> @@ -80,6 +80,7 @@ static struct facility_def facility_defs[] = {
>>>  			78, /* enhanced-DAT 2 */
>>>  			130, /* instruction-execution-protection */
>>>  			131, /* enhanced-SOP 2 and side-effect */
>>> +			138, /* configuration z/architecture mode (czam) */
>>>  			146, /* msa extension 8 */
>>>  			-1  /* END */
>>>  		}
>>>
>>
>> Thinking about it, this should be the right thing to do instead of the
>> last hunk:
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 84c069afc02f..39115f5a38df 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -1927,6 +1927,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned
>> long type)
>>         memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>>                S390_ARCH_FAC_LIST_SIZE_BYTE);
>>
>> +       /* we are always in CZAM mode - even on pre z14 machines */
>> +       set_kvm_facility(kvm->arch.model.fac_mask, 138);
>> +       set_kvm_facility(kvm->arch.model.fac_list, 138);
>> +       /* we emulate STHYI in kvm */
>>         set_kvm_facility(kvm->arch.model.fac_mask, 74);
>>         set_kvm_facility(kvm->arch.model.fac_list, 74);
>>         if (MACHINE_HAS_TLB_GUEST) {
>>
>> That would produce consistent results for very old QEMU.
>>
> 
> This should be identical as the initial fac_list is populated from the fac_mask,
> which is populated by the facility_defs structure.
> 
> [..]
>        /* Populate the facility list initially. */
>         kvm->arch.model.fac_list = kvm->arch.sie_page2->fac_list;
> ---->   memcpy(kvm->arch.model.fac_list, kvm->arch.model.fac_mask,
>                S390_ARCH_FAC_LIST_SIZE_BYTE);
> 
> [,..]
> 

This would keep it unset on e.g. a z13, no? (that's why I added the
comment regarding z14)


By always enabling it we would behave exactly like QEMU.

-- 

Thanks,

David

[PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[Christian Borntraeger]

From: "Collin L. Walling" <walling@linux.vnet.ibm.com>

Allow for the enablement of MEF and the support for the extended
epoch in SIE and VSIE for the extended guest TOD-Clock.

A new interface is used for getting/setting a guest's extended
TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
The old method of getting and setting the guest TOD-Clock is
retained and is used when the old ioctls are called.

Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 Documentation/virtual/kvm/devices/vm.txt |  14 ++++-
 arch/s390/include/asm/kvm_host.h         |   6 +-
 arch/s390/include/uapi/asm/kvm.h         |   6 ++
 arch/s390/kvm/kvm-s390.c                 | 101 +++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.h                 |   2 +
 arch/s390/kvm/vsie.c                     |  10 +++
 arch/s390/tools/gen_facilities.c         |   1 +
 7 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
index 903fc92..8a86778 100644
--- a/Documentation/virtual/kvm/devices/vm.txt
+++ b/Documentation/virtual/kvm/devices/vm.txt
@@ -176,7 +176,8 @@ Architectures: s390
 
 3.1. ATTRIBUTE: KVM_S390_VM_TOD_HIGH
 
-Allows user space to set/get the TOD clock extension (u8).
+Allows user space to set/get the TOD clock extension (u8). (superseded by
+KVM_S390_VM_TOD_EXT).
 
 Parameters: address of a buffer in user space to store the data (u8) to
 Returns:    -EFAULT if the given address is not accessible from kernel space
@@ -190,6 +191,17 @@ the POP (u64).
 Parameters: address of a buffer in user space to store the data (u64) to
 Returns:    -EFAULT if the given address is not accessible from kernel space
 
+3.3. ATTRIBUTE: KVM_S390_VM_TOD_EXT
+
+Allows user space to set/get bits 0-63 of the TOD clock register as defined in
+the POP (u64), as well as the TOD clock extension (u8) if supported by the
+host.
+
+Parameters: address of a buffer in user space to store the data
+            (kvm_s390_vm_tod_clock) to
+Returns:    -EFAULT if the given address is not accessible from kernel space
+	    -EINVAL if setting the TOD clock extension to != 0 is not supported
+
 4. GROUP: KVM_S390_VM_CRYPTO
 Architectures: s390
 
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index a409d59..51375e7 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -226,7 +226,9 @@ struct kvm_s390_sie_block {
 #define ECB3_RI  0x01
 	__u8    ecb3;			/* 0x0063 */
 	__u32	scaol;			/* 0x0064 */
-	__u8	reserved68[4];		/* 0x0068 */
+	__u8	reserved68;		/* 0x0068 */
+	__u8    epdx;			/* 0x0069 */
+	__u8    reserved6a[2];		/* 0x006a */
 	__u32	todpr;			/* 0x006c */
 	__u8	reserved70[16];		/* 0x0070 */
 	__u64	mso;			/* 0x0080 */
@@ -265,6 +267,7 @@ struct kvm_s390_sie_block {
 	__u64	cbrlo;			/* 0x01b8 */
 	__u8	reserved1c0[8];		/* 0x01c0 */
 #define ECD_HOSTREGMGMT	0x20000000
+#define ECD_MEF		0x08000000
 	__u32	ecd;			/* 0x01c8 */
 	__u8	reserved1cc[18];	/* 0x01cc */
 	__u64	pp;			/* 0x01de */
@@ -739,6 +742,7 @@ struct kvm_arch{
 	struct kvm_s390_cpu_model model;
 	struct kvm_s390_crypto crypto;
 	struct kvm_s390_vsie vsie;
+	u8 epdx;
 	u64 epoch;
 	struct kvm_s390_migration_state *migration_state;
 	/* subset of available cpu features enabled by user space */
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 69d09c3..cd7359e 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -88,6 +88,12 @@ struct kvm_s390_io_adapter_req {
 /* kvm attributes for KVM_S390_VM_TOD */
 #define KVM_S390_VM_TOD_LOW		0
 #define KVM_S390_VM_TOD_HIGH		1
+#define KVM_S390_VM_TOD_EXT		2
+
+struct kvm_s390_vm_tod_clock {
+	__u8  epoch_idx;
+	__u64 tod;
+};
 
 /* kvm attributes for KVM_S390_VM_CPU_MODEL */
 /* processor related attributes are r/w */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 3f2884e..2547787 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -130,6 +130,12 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ NULL }
 };
 
+struct kvm_s390_tod_clock_ext {
+	__u8 epoch_idx;
+	__u64 tod;
+	__u8 reserved[7];
+} __packed;
+
 /* allow nested virtualization in KVM (if enabled by user space) */
 static int nested;
 module_param(nested, int, S_IRUGO);
@@ -874,6 +880,26 @@ static int kvm_s390_vm_get_migration(struct kvm *kvm,
 	return 0;
 }
 
+static int kvm_s390_set_tod_ext(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	struct kvm_s390_vm_tod_clock gtod;
+
+	if (copy_from_user(&gtod, (void __user *)attr->addr, sizeof(gtod)))
+		return -EFAULT;
+
+	if (test_kvm_facility(kvm, 139))
+		kvm_s390_set_tod_clock_ext(kvm, &gtod);
+	else if (gtod.epoch_idx == 0)
+		kvm_s390_set_tod_clock(kvm, gtod.tod);
+	else
+		return -EINVAL;
+
+	VM_EVENT(kvm, 3, "SET: TOD extension: 0x%x, TOD base: 0x%llx",
+		gtod.epoch_idx, gtod.tod);
+
+	return 0;
+}
+
 static int kvm_s390_set_tod_high(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	u8 gtod_high;
@@ -909,6 +935,9 @@ static int kvm_s390_set_tod(struct kvm *kvm, struct kvm_device_attr *attr)
 		return -EINVAL;
 
 	switch (attr->attr) {
+	case KVM_S390_VM_TOD_EXT:
+		ret = kvm_s390_set_tod_ext(kvm, attr);
+		break;
 	case KVM_S390_VM_TOD_HIGH:
 		ret = kvm_s390_set_tod_high(kvm, attr);
 		break;
@@ -922,6 +951,43 @@ static int kvm_s390_set_tod(struct kvm *kvm, struct kvm_device_attr *attr)
 	return ret;
 }
 
+static void kvm_s390_get_tod_clock_ext(struct kvm *kvm,
+					struct kvm_s390_vm_tod_clock *gtod)
+{
+	struct kvm_s390_tod_clock_ext htod;
+
+	preempt_disable();
+
+	get_tod_clock_ext((char *)&htod);
+
+	gtod->tod = htod.tod + kvm->arch.epoch;
+	gtod->epoch_idx = htod.epoch_idx + kvm->arch.epdx;
+
+	if (gtod->tod < htod.tod)
+		gtod->epoch_idx += 1;
+
+	preempt_enable();
+}
+
+static int kvm_s390_get_tod_ext(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+	struct kvm_s390_vm_tod_clock gtod;
+
+	memset(&gtod, 0, sizeof(gtod));
+
+	if (test_kvm_facility(kvm, 139))
+		kvm_s390_get_tod_clock_ext(kvm, &gtod);
+	else
+		gtod.tod = kvm_s390_get_tod_clock_fast(kvm);
+
+	if (copy_to_user((void __user *)attr->addr, &gtod, sizeof(gtod)))
+		return -EFAULT;
+
+	VM_EVENT(kvm, 3, "QUERY: TOD extension: 0x%x, TOD base: 0x%llx",
+		gtod.epoch_idx, gtod.tod);
+	return 0;
+}
+
 static int kvm_s390_get_tod_high(struct kvm *kvm, struct kvm_device_attr *attr)
 {
 	u8 gtod_high = 0;
@@ -954,6 +1020,9 @@ static int kvm_s390_get_tod(struct kvm *kvm, struct kvm_device_attr *attr)
 		return -EINVAL;
 
 	switch (attr->attr) {
+	case KVM_S390_VM_TOD_EXT:
+		ret = kvm_s390_get_tod_ext(kvm, attr);
+		break;
 	case KVM_S390_VM_TOD_HIGH:
 		ret = kvm_s390_get_tod_high(kvm, attr);
 		break;
@@ -2365,6 +2434,9 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 		vcpu->arch.sie_block->eca |= ECA_VX;
 		vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
 	}
+	if (test_kvm_facility(vcpu->kvm, 139))
+		vcpu->arch.sie_block->ecd |= ECD_MEF;
+
 	vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
 					| SDNXC;
 	vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
@@ -2851,6 +2923,35 @@ static int kvm_s390_handle_requests(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
+void kvm_s390_set_tod_clock_ext(struct kvm *kvm,
+				 const struct kvm_s390_vm_tod_clock *gtod)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_s390_tod_clock_ext htod;
+	int i;
+
+	mutex_lock(&kvm->lock);
+	preempt_disable();
+
+	get_tod_clock_ext((char *)&htod);
+
+	kvm->arch.epoch = gtod->tod - htod.tod;
+	kvm->arch.epdx = gtod->epoch_idx - htod.epoch_idx;
+
+	if (kvm->arch.epoch > gtod->tod)
+		kvm->arch.epdx -= 1;
+
+	kvm_s390_vcpu_block_all(kvm);
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		vcpu->arch.sie_block->epoch = kvm->arch.epoch;
+		vcpu->arch.sie_block->epdx  = kvm->arch.epdx;
+	}
+
+	kvm_s390_vcpu_unblock_all(kvm);
+	preempt_enable();
+	mutex_unlock(&kvm->lock);
+}
+
 void kvm_s390_set_tod_clock(struct kvm *kvm, u64 tod)
 {
 	struct kvm_vcpu *vcpu;
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 6fedc8b..9f8fdd7 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -272,6 +272,8 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu);
 int handle_sthyi(struct kvm_vcpu *vcpu);
 
 /* implemented in kvm-s390.c */
+void kvm_s390_set_tod_clock_ext(struct kvm *kvm,
+				 const struct kvm_s390_vm_tod_clock *gtod);
 void kvm_s390_set_tod_clock(struct kvm *kvm, u64 tod);
 long kvm_arch_fault_in_page(struct kvm_vcpu *vcpu, gpa_t gpa, int writable);
 int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long addr);
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 715c19c..681d06e 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -349,6 +349,9 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 		scb_s->eca |= scb_o->eca & ECA_IB;
 	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_CEI))
 		scb_s->eca |= scb_o->eca & ECA_CEI;
+	/* Epoch Extension */
+	if (test_kvm_facility(vcpu->kvm, 139))
+		scb_s->ecd |= scb_o->ecd & ECD_MEF;
 
 	prepare_ibc(vcpu, vsie_page);
 	rc = shadow_crycb(vcpu, vsie_page);
@@ -919,6 +922,13 @@ static void register_shadow_scb(struct kvm_vcpu *vcpu,
 	 */
 	preempt_disable();
 	scb_s->epoch += vcpu->kvm->arch.epoch;
+
+	if (test_kvm_facility(vcpu->kvm, 139)) {
+		scb_s->epdx += vcpu->kvm->arch.epdx;
+		if (scb_s->epoch < vcpu->kvm->arch.epoch)
+			scb_s->epdx += 1;
+	}
+
 	preempt_enable();
 }
 
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index 181db5b..601bfcf 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -81,6 +81,7 @@ static struct facility_def facility_defs[] = {
 			130, /* instruction-execution-protection */
 			131, /* enhanced-SOP 2 and side-effect */
 			138, /* configuration z/architecture mode (czam) */
+			139, /* multiple epoch facility */
 			146, /* msa extension 8 */
 			-1  /* END */
 		}
-- 
2.7.4

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[Cornelia Huck]

On Mon, 28 Aug 2017 10:07:29 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
> 
> Allow for the enablement of MEF and the support for the extended
> epoch in SIE and VSIE for the extended guest TOD-Clock.
> 
> A new interface is used for getting/setting a guest's extended
> TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
> The old method of getting and setting the guest TOD-Clock is
> retained and is used when the old ioctls are called.
> 
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  Documentation/virtual/kvm/devices/vm.txt |  14 ++++-
>  arch/s390/include/asm/kvm_host.h         |   6 +-
>  arch/s390/include/uapi/asm/kvm.h         |   6 ++
>  arch/s390/kvm/kvm-s390.c                 | 101 +++++++++++++++++++++++++++++++
>  arch/s390/kvm/kvm-s390.h                 |   2 +
>  arch/s390/kvm/vsie.c                     |  10 +++
>  arch/s390/tools/gen_facilities.c         |   1 +
>  7 files changed, 138 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
> index 903fc92..8a86778 100644
> --- a/Documentation/virtual/kvm/devices/vm.txt
> +++ b/Documentation/virtual/kvm/devices/vm.txt
> @@ -176,7 +176,8 @@ Architectures: s390
>  
>  3.1. ATTRIBUTE: KVM_S390_VM_TOD_HIGH
>  
> -Allows user space to set/get the TOD clock extension (u8).
> +Allows user space to set/get the TOD clock extension (u8). (superseded by

s/(u8)./(u8)/

> +KVM_S390_VM_TOD_EXT).
>  
>  Parameters: address of a buffer in user space to store the data (u8) to
>  Returns:    -EFAULT if the given address is not accessible from kernel space
> @@ -190,6 +191,17 @@ the POP (u64).
>  Parameters: address of a buffer in user space to store the data (u64) to
>  Returns:    -EFAULT if the given address is not accessible from kernel space
>  
> +3.3. ATTRIBUTE: KVM_S390_VM_TOD_EXT
> +
> +Allows user space to set/get bits 0-63 of the TOD clock register as defined in
> +the POP (u64), as well as the TOD clock extension (u8) if supported by the
> +host.

I would be a bit more explicit on what happens if the tod clock
extension is not supported. What about:

"Allows user space to set/get bits 0-63 of the TOD clock register as
defined in the POP (u64). If the host supports the TOD clock extension
(u8), it also allows user space to set it. If the host does not support
it, it is stored as 0 and not allowed to be set to a value != 0."

> +
> +Parameters: address of a buffer in user space to store the data
> +            (kvm_s390_vm_tod_clock) to
> +Returns:    -EFAULT if the given address is not accessible from kernel space
> +	    -EINVAL if setting the TOD clock extension to != 0 is not supported
> +
>  4. GROUP: KVM_S390_VM_CRYPTO
>  Architectures: s390
>  

Else, looks good AFAICS.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[Christian Borntraeger]

On 08/28/2017 01:21 PM, Cornelia Huck wrote:
> On Mon, 28 Aug 2017 10:07:29 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
>>
>> Allow for the enablement of MEF and the support for the extended
>> epoch in SIE and VSIE for the extended guest TOD-Clock.
>>
>> A new interface is used for getting/setting a guest's extended
>> TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
>> The old method of getting and setting the guest TOD-Clock is
>> retained and is used when the old ioctls are called.
>>
>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  Documentation/virtual/kvm/devices/vm.txt |  14 ++++-
>>  arch/s390/include/asm/kvm_host.h         |   6 +-
>>  arch/s390/include/uapi/asm/kvm.h         |   6 ++
>>  arch/s390/kvm/kvm-s390.c                 | 101 +++++++++++++++++++++++++++++++
>>  arch/s390/kvm/kvm-s390.h                 |   2 +
>>  arch/s390/kvm/vsie.c                     |  10 +++
>>  arch/s390/tools/gen_facilities.c         |   1 +
>>  7 files changed, 138 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
>> index 903fc92..8a86778 100644
>> --- a/Documentation/virtual/kvm/devices/vm.txt
>> +++ b/Documentation/virtual/kvm/devices/vm.txt
>> @@ -176,7 +176,8 @@ Architectures: s390
>>  
>>  3.1. ATTRIBUTE: KVM_S390_VM_TOD_HIGH
>>  
>> -Allows user space to set/get the TOD clock extension (u8).
>> +Allows user space to set/get the TOD clock extension (u8). (superseded by
> 
> s/(u8)./(u8)/

ok

> 
>> +KVM_S390_VM_TOD_EXT).
>>  
>>  Parameters: address of a buffer in user space to store the data (u8) to
>>  Returns:    -EFAULT if the given address is not accessible from kernel space
>> @@ -190,6 +191,17 @@ the POP (u64).
>>  Parameters: address of a buffer in user space to store the data (u64) to
>>  Returns:    -EFAULT if the given address is not accessible from kernel space
>>  
>> +3.3. ATTRIBUTE: KVM_S390_VM_TOD_EXT
>> +
>> +Allows user space to set/get bits 0-63 of the TOD clock register as defined in
>> +the POP (u64), as well as the TOD clock extension (u8) if supported by the
>> +host.
> 
> I would be a bit more explicit on what happens if the tod clock
> extension is not supported. What about:
> 
> "Allows user space to set/get bits 0-63 of the TOD clock register as
> defined in the POP (u64). If the host supports the TOD clock extension
> (u8), it also allows user space to set it. If the host does not support
> it, it is stored as 0 and not allowed to be set to a value != 0."

We check test_kvm_facility(kvm, 139))
so what about using your text with the following change,

"host" --> "guest cpu model" ?

> 
>> +
>> +Parameters: address of a buffer in user space to store the data
>> +            (kvm_s390_vm_tod_clock) to
>> +Returns:    -EFAULT if the given address is not accessible from kernel space
>> +	    -EINVAL if setting the TOD clock extension to != 0 is not supported
>> +
>>  4. GROUP: KVM_S390_VM_CRYPTO
>>  Architectures: s390
>>  
> 
> Else, looks good AFAICS.
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>


Thanks

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[Cornelia Huck]

On Mon, 28 Aug 2017 13:36:24 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 08/28/2017 01:21 PM, Cornelia Huck wrote:
> > On Mon, 28 Aug 2017 10:07:29 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >   
> >> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
> >>
> >> Allow for the enablement of MEF and the support for the extended
> >> epoch in SIE and VSIE for the extended guest TOD-Clock.
> >>
> >> A new interface is used for getting/setting a guest's extended
> >> TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
> >> The old method of getting and setting the guest TOD-Clock is
> >> retained and is used when the old ioctls are called.
> >>
> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> >> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> >> Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> >> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >> ---
> >>  Documentation/virtual/kvm/devices/vm.txt |  14 ++++-
> >>  arch/s390/include/asm/kvm_host.h         |   6 +-
> >>  arch/s390/include/uapi/asm/kvm.h         |   6 ++
> >>  arch/s390/kvm/kvm-s390.c                 | 101 +++++++++++++++++++++++++++++++
> >>  arch/s390/kvm/kvm-s390.h                 |   2 +
> >>  arch/s390/kvm/vsie.c                     |  10 +++
> >>  arch/s390/tools/gen_facilities.c         |   1 +
> >>  7 files changed, 138 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/virtual/kvm/devices/vm.txt b/Documentation/virtual/kvm/devices/vm.txt
> >> index 903fc92..8a86778 100644
> >> --- a/Documentation/virtual/kvm/devices/vm.txt
> >> +++ b/Documentation/virtual/kvm/devices/vm.txt
> >> @@ -176,7 +176,8 @@ Architectures: s390
> >>  
> >>  3.1. ATTRIBUTE: KVM_S390_VM_TOD_HIGH
> >>  
> >> -Allows user space to set/get the TOD clock extension (u8).
> >> +Allows user space to set/get the TOD clock extension (u8). (superseded by  
> > 
> > s/(u8)./(u8)/  
> 
> ok
> 
> >   
> >> +KVM_S390_VM_TOD_EXT).
> >>  
> >>  Parameters: address of a buffer in user space to store the data (u8) to
> >>  Returns:    -EFAULT if the given address is not accessible from kernel space
> >> @@ -190,6 +191,17 @@ the POP (u64).
> >>  Parameters: address of a buffer in user space to store the data (u64) to
> >>  Returns:    -EFAULT if the given address is not accessible from kernel space
> >>  
> >> +3.3. ATTRIBUTE: KVM_S390_VM_TOD_EXT
> >> +
> >> +Allows user space to set/get bits 0-63 of the TOD clock register as defined in
> >> +the POP (u64), as well as the TOD clock extension (u8) if supported by the
> >> +host.  
> > 
> > I would be a bit more explicit on what happens if the tod clock
> > extension is not supported. What about:
> > 
> > "Allows user space to set/get bits 0-63 of the TOD clock register as
> > defined in the POP (u64). If the host supports the TOD clock extension
> > (u8), it also allows user space to set it. If the host does not support
> > it, it is stored as 0 and not allowed to be set to a value != 0."  
> 
> We check test_kvm_facility(kvm, 139))
> so what about using your text with the following change,
> 
> "host" --> "guest cpu model" ?

Fine with me.

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[David Hildenbrand]

On 28.08.2017 10:07, Christian Borntraeger wrote:
> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
> 
> Allow for the enablement of MEF and the support for the extended
> epoch in SIE and VSIE for the extended guest TOD-Clock.
> 
> A new interface is used for getting/setting a guest's extended
> TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
> The old method of getting and setting the guest TOD-Clock is
> retained and is used when the old ioctls are called.

After talking to Christian, I understand that we have to set it in "one
shot" as we can otherwise run into problems when getting/setting the TOD
as we are looking at a moving target. This should go into the cover letter.

I need some more info regarding the architecture change.

For now, epoch was an unsigned value that is interpreted as an signed value.

a) Is that still true with multiple epoch?
b) What is the format of the epoch index? Can it also be "negative"?

Why I am asking: I can see comparison made with the epoch:

> +	if (test_kvm_facility(vcpu->kvm, 139)) {
> +		scb_s->epdx += vcpu->kvm->arch.epdx;
> +		if (scb_s->epoch < vcpu->kvm->arch.epoch)
> +			scb_s->epdx += 1;
> +	}

or

> +	if (kvm->arch.epoch > gtod->tod)
> +		kvm->arch.epdx -= 1;


If I remember correctly, such comparisons won't work correctly when
having this signed/unsigned duality. But I might be wrong.


> 
> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---

[...]
>  
>  /* implemented in kvm-s390.c */
> +void kvm_s390_set_tod_clock_ext(struct kvm *kvm,
> +				 const struct kvm_s390_vm_tod_clock *gtod);
>  void kvm_s390_set_tod_clock(struct kvm *kvm, u64 tod);
>  long kvm_arch_fault_in_page(struct kvm_vcpu *vcpu, gpa_t gpa, int writable);
>  int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long addr);
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 715c19c..681d06e 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -349,6 +349,9 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  		scb_s->eca |= scb_o->eca & ECA_IB;
>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_CEI))
>  		scb_s->eca |= scb_o->eca & ECA_CEI;
> +	/* Epoch Extension */
> +	if (test_kvm_facility(vcpu->kvm, 139))
> +		scb_s->ecd |= scb_o->ecd & ECD_MEF;
>  
>  	prepare_ibc(vcpu, vsie_page);
>  	rc = shadow_crycb(vcpu, vsie_page);
> @@ -919,6 +922,13 @@ static void register_shadow_scb(struct kvm_vcpu *vcpu,
>  	 */
>  	preempt_disable();
>  	scb_s->epoch += vcpu->kvm->arch.epoch;
> +
> +	if (test_kvm_facility(vcpu->kvm, 139)) {

Although scb_s->epdx won't be interpreted without ECD_MEF, this should
be (so data is only copied if really enabled).

if (scb_s->ecd | ECD_MEF)

> +		scb_s->epdx += vcpu->kvm->arch.epdx;
> +		if (scb_s->epoch < vcpu->kvm->arch.epoch)
> +			scb_s->epdx += 1;
> +	}
> +
>  	preempt_enable();
>  }
>  
> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
> index 181db5b..601bfcf 100644
> --- a/arch/s390/tools/gen_facilities.c
> +++ b/arch/s390/tools/gen_facilities.c
> @@ -81,6 +81,7 @@ static struct facility_def facility_defs[] = {
>  			130, /* instruction-execution-protection */
>  			131, /* enhanced-SOP 2 and side-effect */
>  			138, /* configuration z/architecture mode (czam) */
> +			139, /* multiple epoch facility */
>  			146, /* msa extension 8 */
>  			-1  /* END */
>  		}
> 


-- 

Thanks,

David

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[Christian Borntraeger]

On 08/29/2017 02:24 PM, David Hildenbrand wrote:
> On 28.08.2017 10:07, Christian Borntraeger wrote:
>> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
>>
>> Allow for the enablement of MEF and the support for the extended
>> epoch in SIE and VSIE for the extended guest TOD-Clock.
>>
>> A new interface is used for getting/setting a guest's extended
>> TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
>> The old method of getting and setting the guest TOD-Clock is
>> retained and is used when the old ioctls are called.
> 
> After talking to Christian, I understand that we have to set it in "one
> shot" as we can otherwise run into problems when getting/setting the TOD
> as we are looking at a moving target. This should go into the cover letter.

You mean in the patch description or in the git tag description?

> 
> I need some more info regarding the architecture change.
> 
> For now, epoch was an unsigned value that is interpreted as an signed value.

It is always considered a signed value, (so the guest can be before or after the host)
we just used u64 to make the wraparound defined (doing all the math in the unsigned realm
as signed overflow is undefined)

> 
> a) Is that still true with multiple epoch?
> b) What is the format of the epoch index? Can it also be "negative"?

The concatenation of both is considered a 72bit signed value.

> 
> Why I am asking: I can see comparison made with the epoch:
> 
>> +	if (test_kvm_facility(vcpu->kvm, 139)) {
>> +		scb_s->epdx += vcpu->kvm->arch.epdx;
>> +		if (scb_s->epoch < vcpu->kvm->arch.epoch)

	This checks for an overflow after the addition and

>> +			scb_s->epdx += 1;

	corrects this. Since this is all unsigned math this is
	defined to wrap around and should work unless I missed 
	something

>> +	}
> 
> or
> 
>> +	if (kvm->arch.epoch > gtod->tod)
>> +		kvm->arch.epdx -= 1;
> 
> 
> If I remember correctly, such comparisons won't work correctly when
> having this signed/unsigned duality. But I might be wrong.

As far as I can see we have unsigned data types everywhere.

> 
> 
>>
>> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com>
>> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>> Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
> 
> [...]
>>  
>>  /* implemented in kvm-s390.c */
>> +void kvm_s390_set_tod_clock_ext(struct kvm *kvm,
>> +				 const struct kvm_s390_vm_tod_clock *gtod);
>>  void kvm_s390_set_tod_clock(struct kvm *kvm, u64 tod);
>>  long kvm_arch_fault_in_page(struct kvm_vcpu *vcpu, gpa_t gpa, int writable);
>>  int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long addr);
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index 715c19c..681d06e 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -349,6 +349,9 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  		scb_s->eca |= scb_o->eca & ECA_IB;
>>  	if (test_kvm_cpu_feat(vcpu->kvm, KVM_S390_VM_CPU_FEAT_CEI))
>>  		scb_s->eca |= scb_o->eca & ECA_CEI;
>> +	/* Epoch Extension */
>> +	if (test_kvm_facility(vcpu->kvm, 139))
>> +		scb_s->ecd |= scb_o->ecd & ECD_MEF;
>>  
>>  	prepare_ibc(vcpu, vsie_page);
>>  	rc = shadow_crycb(vcpu, vsie_page);
>> @@ -919,6 +922,13 @@ static void register_shadow_scb(struct kvm_vcpu *vcpu,
>>  	 */
>>  	preempt_disable();
>>  	scb_s->epoch += vcpu->kvm->arch.epoch;
>> +
>> +	if (test_kvm_facility(vcpu->kvm, 139)) {
> 
> Although scb_s->epdx won't be interpreted without ECD_MEF, this should
> be (so data is only copied if really enabled).
> 
> if (scb_s->ecd | ECD_MEF)

As you said, it does not matter but it certainly is clearer. I can fix.


> 
>> +		scb_s->epdx += vcpu->kvm->arch.epdx;
>> +		if (scb_s->epoch < vcpu->kvm->arch.epoch)
>> +			scb_s->epdx += 1;
>> +	}
>> +
>>  	preempt_enable();
>>  }
>>  
>> diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
>> index 181db5b..601bfcf 100644
>> --- a/arch/s390/tools/gen_facilities.c
>> +++ b/arch/s390/tools/gen_facilities.c
>> @@ -81,6 +81,7 @@ static struct facility_def facility_defs[] = {
>>  			130, /* instruction-execution-protection */
>>  			131, /* enhanced-SOP 2 and side-effect */
>>  			138, /* configuration z/architecture mode (czam) */
>> +			139, /* multiple epoch facility */
>>  			146, /* msa extension 8 */
>>  			-1  /* END */
>>  		}
>>
> 
> 

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[David Hildenbrand]

On 29.08.2017 14:46, Christian Borntraeger wrote:
> 
> 
> On 08/29/2017 02:24 PM, David Hildenbrand wrote:
>> On 28.08.2017 10:07, Christian Borntraeger wrote:
>>> From: "Collin L. Walling" <walling@linux.vnet.ibm.com>
>>>
>>> Allow for the enablement of MEF and the support for the extended
>>> epoch in SIE and VSIE for the extended guest TOD-Clock.
>>>
>>> A new interface is used for getting/setting a guest's extended
>>> TOD-Clock that uses a single ioctl invocation, KVM_S390_VM_TOD_EXT.
>>> The old method of getting and setting the guest TOD-Clock is
>>> retained and is used when the old ioctls are called.
>>
>> After talking to Christian, I understand that we have to set it in "one
>> shot" as we can otherwise run into problems when getting/setting the TOD
>> as we are looking at a moving target. This should go into the cover letter.
> 
> You mean in the patch description or in the git tag description?

Patch description. But just my opinion.

> 
>>
>> I need some more info regarding the architecture change.
>>
>> For now, epoch was an unsigned value that is interpreted as an signed value.
> 
> It is always considered a signed value, (so the guest can be before or after the host)
> we just used u64 to make the wraparound defined (doing all the math in the unsigned realm
> as signed overflow is undefined)
> 
>>
>> a) Is that still true with multiple epoch?
>> b) What is the format of the epoch index? Can it also be "negative"?
> 
> The concatenation of both is considered a 72bit signed value.

Ah okay. So subtracting 1 from idx=0, epoch=0 will result in
idx=0xff epoch=0xffffff

> 
>>
>> Why I am asking: I can see comparison made with the epoch:
>>
>>> +	if (test_kvm_facility(vcpu->kvm, 139)) {
>>> +		scb_s->epdx += vcpu->kvm->arch.epdx;
>>> +		if (scb_s->epoch < vcpu->kvm->arch.epoch)
> 
> 	This checks for an overflow after the addition and
> 
>>> +			scb_s->epdx += 1;
> 
> 	corrects this. Since this is all unsigned math this is
> 	defined to wrap around and should work unless I missed 
> 	something

I just had in mind:

-1 < 0

but 0xffffffff is clearly > 0x0

I might be mistaking, but we could get both an overflow or an underflow
here.

But my brain has to consume new information first, so ignore the noise
for now :)


> 
>>> +	}
>>
>> or
>>
>>> +	if (kvm->arch.epoch > gtod->tod)
>>> +		kvm->arch.epdx -= 1;
>>
>>
>> If I remember correctly, such comparisons won't work correctly when
>> having this signed/unsigned duality. But I might be wrong.
> 
> As far as I can see we have unsigned data types everywhere.
> 

-- 

Thanks,

David

Re: [PATCH 06/11] KVM: s390: Multiple Epoch Facility support

[Christian Borntraeger]

On 08/29/2017 02:46 PM, Christian Borntraeger wrote:
>>> +
>>> +	if (test_kvm_facility(vcpu->kvm, 139)) {
>>
>> Although scb_s->epdx won't be interpreted without ECD_MEF, this should
>> be (so data is only copied if really enabled).
>>
>> if (scb_s->ecd | ECD_MEF)
> 
> As you said, it does not matter but it certainly is clearer. I can fix.

FWIW, It will be & and not |

so

if (scb_s->ecd & ECD_MEF)

[PATCH 10/11] KVM: s390: sthyi: remove invalid guest write access

[Christian Borntraeger]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

handle_sthyi() always writes to guest memory if the sthyi function
code is zero in order to fault in the page that later is written to.

However a function code of zero does not necessarily mean that a write
to guest memory happens: if the KVM host is running as a second level
guest under z/VM 6.2 the sthyi instruction is indicated to be
available to the KVM host, however if the instruction is executed it
will always return with a return code that indicates "unsupported
function code".

In such a case handle_sthyi() must not write to guest memory. This
means that the prior write access to fault in the guest page may
result in invalid guest exceptions, and/or invalid data modification.

In order to be architecture compliant simply remove the write_guest()
call.

Given that the guest assumed a write access anyway, this fix does not
qualify for -stable. This just makes sure the sthyi handler is
architecture compliant.

Fixes: 95ca2cb57985 ("KVM: s390: Add sthyi emulation")
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/sthyi.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/s390/kvm/sthyi.c b/arch/s390/kvm/sthyi.c
index a2e5c24..395926b 100644
--- a/arch/s390/kvm/sthyi.c
+++ b/arch/s390/kvm/sthyi.c
@@ -436,14 +436,6 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
 	if (addr & ~PAGE_MASK)
 		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
 
-	/*
-	 * If the page has not yet been faulted in, we want to do that
-	 * now and not after all the expensive calculations.
-	 */
-	r = write_guest(vcpu, addr, reg2, &cc, 1);
-	if (r)
-		return kvm_s390_inject_prog_cond(vcpu, r);
-
 	sctns = (void *)get_zeroed_page(GFP_KERNEL);
 	if (!sctns)
 		return -ENOMEM;
-- 
2.7.4

Re: [PATCH 10/11] KVM: s390: sthyi: remove invalid guest write access

[Cornelia Huck]

On Mon, 28 Aug 2017 10:07:30 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> handle_sthyi() always writes to guest memory if the sthyi function
> code is zero in order to fault in the page that later is written to.
> 
> However a function code of zero does not necessarily mean that a write
> to guest memory happens: if the KVM host is running as a second level
> guest under z/VM 6.2 the sthyi instruction is indicated to be
> available to the KVM host, however if the instruction is executed it
> will always return with a return code that indicates "unsupported
> function code".
> 
> In such a case handle_sthyi() must not write to guest memory. This
> means that the prior write access to fault in the guest page may
> result in invalid guest exceptions, and/or invalid data modification.
> 
> In order to be architecture compliant simply remove the write_guest()
> call.
> 
> Given that the guest assumed a write access anyway, this fix does not
> qualify for -stable. This just makes sure the sthyi handler is
> architecture compliant.
> 
> Fixes: 95ca2cb57985 ("KVM: s390: Add sthyi emulation")
> Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/sthyi.c | 8 --------
>  1 file changed, 8 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[PATCH 11/11] KVM: s390: expose no-DAT to guest and migration support

[Christian Borntraeger]

From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>

The STFLE bit 147 indicates whether the ESSA no-DAT operation code is
valid, the bit is not normally provided to the host; the host is
instead provided with an SCLP bit that indicates whether guests can
support the feature.

This patch:
* enables the STFLE bit in the guest if the corresponding SCLP bit is
  present in the host.
* adds support for migrating the no-DAT bit in the PGSTEs
* fixes the software interpretation of the ESSA instruction that is
  used when migrating, both for the new operation code and for the old
  "set stable", as per specifications.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/page-states.h | 2 +-
 arch/s390/kvm/kvm-s390.c            | 8 ++++++--
 arch/s390/kvm/priv.c                | 6 +++++-
 arch/s390/mm/pgtable.c              | 6 +++++-
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/arch/s390/include/asm/page-states.h b/arch/s390/include/asm/page-states.h
index ca21b28..22b0f49 100644
--- a/arch/s390/include/asm/page-states.h
+++ b/arch/s390/include/asm/page-states.h
@@ -15,6 +15,6 @@
 #define ESSA_SET_STABLE_IF_RESIDENT	6
 #define ESSA_SET_STABLE_NODAT		7
 
-#define ESSA_MAX	ESSA_SET_STABLE_IF_RESIDENT
+#define ESSA_MAX	ESSA_SET_STABLE_NODAT
 
 #endif
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index e65b763..84c069a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1574,7 +1574,7 @@ static int kvm_s390_get_cmma_bits(struct kvm *kvm,
 		if (r < 0)
 			pgstev = 0;
 		/* save the value */
-		res[i++] = (pgstev >> 24) & 0x3;
+		res[i++] = (pgstev >> 24) & 0x43;
 		/*
 		 * if the next bit is too far away, stop.
 		 * if we reached the previous "next", find the next one
@@ -1652,7 +1652,7 @@ static int kvm_s390_set_cmma_bits(struct kvm *kvm,
 
 		pgstev = bits[i];
 		pgstev = pgstev << 24;
-		mask &= _PGSTE_GPS_USAGE_MASK;
+		mask &= _PGSTE_GPS_USAGE_MASK | _PGSTE_GPS_NODAT;
 		set_pgste_bits(kvm->mm, hva, mask, pgstev);
 	}
 	srcu_read_unlock(&kvm->srcu, srcu_idx);
@@ -1929,6 +1929,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 	set_kvm_facility(kvm->arch.model.fac_mask, 74);
 	set_kvm_facility(kvm->arch.model.fac_list, 74);
+	if (MACHINE_HAS_TLB_GUEST) {
+		set_kvm_facility(kvm->arch.model.fac_mask, 147);
+		set_kvm_facility(kvm->arch.model.fac_list, 147);
+	}
 
 	kvm->arch.model.cpuid = kvm_s390_get_initial_cpuid();
 	kvm->arch.model.ibc = sclp.ibc & 0x0fff;
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 8a1dac7..91dc4a8 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -988,6 +988,8 @@ static inline int do_essa(struct kvm_vcpu *vcpu, const int orc)
 		if (pgstev & _PGSTE_GPS_ZERO)
 			res |= 1;
 	}
+	if (pgstev & _PGSTE_GPS_NODAT)
+		res |= 0x20;
 	vcpu->run->s.regs.gprs[r1] = res;
 	/*
 	 * It is possible that all the normal 511 slots were full, in which case
@@ -1027,7 +1029,9 @@ static int handle_essa(struct kvm_vcpu *vcpu)
 		return kvm_s390_inject_program_int(vcpu, PGM_PRIVILEGED_OP);
 	/* Check for invalid operation request code */
 	orc = (vcpu->arch.sie_block->ipb & 0xf0000000) >> 28;
-	if (orc > ESSA_MAX)
+	/* ORCs 0-6 are always valid */
+	if (orc > (test_kvm_facility(vcpu->kvm, 147) ? ESSA_SET_STABLE_NODAT
+						: ESSA_SET_STABLE_IF_RESIDENT))
 		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
 
 	if (likely(!vcpu->kvm->arch.migration_state)) {
diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 8d018c7..459716d 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -919,7 +919,7 @@ int pgste_perform_essa(struct mm_struct *mm, unsigned long hva, int orc,
 	case ESSA_GET_STATE:
 		break;
 	case ESSA_SET_STABLE:
-		pgstev &= ~_PGSTE_GPS_USAGE_MASK;
+		pgstev &= ~(_PGSTE_GPS_USAGE_MASK | _PGSTE_GPS_NODAT);
 		pgstev |= _PGSTE_GPS_USAGE_STABLE;
 		break;
 	case ESSA_SET_UNUSED:
@@ -965,6 +965,10 @@ int pgste_perform_essa(struct mm_struct *mm, unsigned long hva, int orc,
 			pgstev |= _PGSTE_GPS_USAGE_STABLE;
 		}
 		break;
+	case ESSA_SET_STABLE_NODAT:
+		pgstev &= ~_PGSTE_GPS_USAGE_MASK;
+		pgstev |= _PGSTE_GPS_USAGE_STABLE | _PGSTE_GPS_NODAT;
+		break;
 	default:
 		/* we should never get here! */
 		break;
-- 
2.7.4

Re: [PATCH 11/11] KVM: s390: expose no-DAT to guest and migration support

[Cornelia Huck]

On Mon, 28 Aug 2017 10:07:31 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> 
> The STFLE bit 147 indicates whether the ESSA no-DAT operation code is
> valid, the bit is not normally provided to the host; the host is
> instead provided with an SCLP bit that indicates whether guests can
> support the feature.
> 
> This patch:
> * enables the STFLE bit in the guest if the corresponding SCLP bit is
>   present in the host.
> * adds support for migrating the no-DAT bit in the PGSTEs
> * fixes the software interpretation of the ESSA instruction that is
>   used when migrating, both for the new operation code and for the old
>   "set stable", as per specifications.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/include/asm/page-states.h | 2 +-
>  arch/s390/kvm/kvm-s390.c            | 8 ++++++--
>  arch/s390/kvm/priv.c                | 6 +++++-
>  arch/s390/mm/pgtable.c              | 6 +++++-
>  4 files changed, 17 insertions(+), 5 deletions(-)

This is a bit hard to review without the previous patches (I had to
hunt down MACHINE_HAS_TLB_GUEST, for example), and it seems much if not
all of this is not externally documented... but it seems consistent and
reasonable, so

Acked-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 00/11] KVM: s390: Fixes and features for 4.14

[Cornelia Huck]

On Mon, 28 Aug 2017 10:07:27 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> This is what I have pending for 4.14 in 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  kvm-s390-next-4.14-1
> 
> It contains 2 merges (on topic branch from Martin for the tlb flushing

Have those been posted anywhere? Might have made looking at the feature
easier to post them as well so you do not have to track down the branch.

> and a back-merge from the s390 fixes for master to avoid merge conflicts
> with followup fixes). 
> 
> "New" patches are marked (x) below. (patches 5,6,10,11).
> Pull request will come later this week.
> 
> ----------------------------------------------------------------
> KVM: s390: Fixes and features for 4.14
> 
> - merge of topic branch tlb-flushing from the s390 tree to get the
>   no-dat base features
> - wire up the no-dat enhancements in KVM
> - multiple epoch facility (z14 feature)
> - Configuration z/Architecture Mode fixup
> - merge of kvm/master to avoid conflicts with additional sthyi fixes
> - more sthyi fixes
> 
> ----------------------------------------------------------------
> Christian Borntraeger (3):
>       Merge branch 'tlb-flushing' of git://git.kernel.org/.../s390/linux  into kernelorgnext
>       KVM: s390: take srcu lock when getting/setting storage keys
>       Merge tag 'kvm-s390-master-4.13-2' into kvms390/next
> 
> Claudio Imbrenda (1):
> (x)   KVM: s390: expose no-DAT to guest and migration support
> 
> Collin L. Walling (1):
> (x)   KVM: s390: Multiple Epoch Facility support
> 
> Heiko Carstens (3):
>       KVM: s390: sthyi: fix sthyi inline assembly
>       KVM: s390: sthyi: fix specification exception detection
> (x)   KVM: s390: sthyi: remove invalid guest write access
> 
> Jason J. Herne (1):
> (x)   KVM: s390: Support Configuration z/Architecture Mode
> 
> Martin Schwidefsky (4):
>       s390/mm: tag normal pages vs pages used in page tables
>       s390/mm: add no-dat TLB flush optimization
>       s390/mm: add guest ASCE TLB flush optimization
>       s390/mm,kvm: use nodat PGSTE tag to optimize TLB flushing

I have not reviewed Martin's patches (not surprising, I guess), but the
series looks fine.