diff mbox

[5/6] KVM: s390: wire up seb feature

Message ID 1516182519-10623-6-git-send-email-schwidefsky@de.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Martin Schwidefsky Jan. 17, 2018, 9:48 a.m. UTC
From: Christian Borntraeger <borntraeger@de.ibm.com>

The new firmware interfaces for branch prediction behaviour changes
are transparently available for the guest. Nevertheless, there is
new state attached that should be migrated and properly resetted.
Provide a mechanism for handling reset, migration and VSIE.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |  3 ++-
 arch/s390/include/uapi/asm/kvm.h |  4 +++-
 arch/s390/kvm/kvm-s390.c         | 11 +++++++++++
 arch/s390/kvm/vsie.c             |  8 ++++++++
 include/uapi/linux/kvm.h         |  1 +
 5 files changed, 25 insertions(+), 2 deletions(-)

Comments

Christian Borntraeger Jan. 17, 2018, 11:18 a.m. UTC | #1
Paolo,

while this is kvm code, my current plan is to submit the "final" version after
review and probably some fixes/renames via Martin together with the other
patches.  Are you ok with that? Right now it seems that the CAP number is still
fine.

Christian

On 01/17/2018 10:48 AM, Martin Schwidefsky wrote:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> The new firmware interfaces for branch prediction behaviour changes
> are transparently available for the guest. Nevertheless, there is
> new state attached that should be migrated and properly resetted.
> Provide a mechanism for handling reset, migration and VSIE.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h |  3 ++-
>  arch/s390/include/uapi/asm/kvm.h |  4 +++-
>  arch/s390/kvm/kvm-s390.c         | 11 +++++++++++
>  arch/s390/kvm/vsie.c             |  8 ++++++++
>  include/uapi/linux/kvm.h         |  1 +
>  5 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index e14f381..a2f6c5d 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -207,7 +207,8 @@ struct kvm_s390_sie_block {
>  	__u16	ipa;			/* 0x0056 */
>  	__u32	ipb;			/* 0x0058 */
>  	__u32	scaoh;			/* 0x005c */
> -	__u8	reserved60;		/* 0x0060 */
> +#define FPF_SEBC 	0x20
> +	__u8	fpf;			/* 0x0060 */
>  #define ECB_GS		0x40
>  #define ECB_TE		0x10
>  #define ECB_SRSI	0x04
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index 38535a57..20b9e9f 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -224,6 +224,7 @@ struct kvm_guest_debug_arch {
>  #define KVM_SYNC_RICCB  (1UL << 7)
>  #define KVM_SYNC_FPRS   (1UL << 8)
>  #define KVM_SYNC_GSCB   (1UL << 9)
> +#define KVM_SYNC_SEBC   (1UL << 10)
>  /* length and alignment of the sdnx as a power of two */
>  #define SDNXC 8
>  #define SDNXL (1UL << SDNXC)
> @@ -247,7 +248,8 @@ struct kvm_sync_regs {
>  	};
>  	__u8  reserved[512];	/* for future vector expansion */
>  	__u32 fpc;		/* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */
> -	__u8 padding1[52];	/* riccb needs to be 64byte aligned */
> +	__u8 sebc:1;		/* spec blocking */
> +	__u8 padding1[51];	/* riccb needs to be 64byte aligned */
>  	__u8 riccb[64];		/* runtime instrumentation controls block */
>  	__u8 padding2[192];	/* sdnx needs to be 256byte aligned */
>  	union {
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 2c93cbb..0c18f73 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -421,6 +421,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_S390_GS:
>  		r = test_facility(133);
>  		break;
> +	case KVM_CAP_S390_SEB:
> +		r = test_facility(82);
> +		break;
>  	default:
>  		r = 0;
>  	}
> @@ -2198,6 +2201,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>  	kvm_s390_set_prefix(vcpu, 0);
>  	if (test_kvm_facility(vcpu->kvm, 64))
>  		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
> +	if (test_kvm_facility(vcpu->kvm, 82))
> +		vcpu->run->kvm_valid_regs |= KVM_SYNC_SEBC;
>  	if (test_kvm_facility(vcpu->kvm, 133))
>  		vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB;
>  	/* fprs can be synchronized via vrs, even if the guest has no vx. With
> @@ -2339,6 +2344,7 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
>  	current->thread.fpu.fpc = 0;
>  	vcpu->arch.sie_block->gbea = 1;
>  	vcpu->arch.sie_block->pp = 0;
> +	vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
>  	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
>  	kvm_clear_async_pf_completion_queue(vcpu);
>  	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
> @@ -3298,6 +3304,10 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  		vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
>  		vcpu->arch.gs_enabled = 1;
>  	}
> +	if (kvm_run->kvm_dirty_regs & KVM_SYNC_SEBC) {
> +		vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
> +		vcpu->arch.sie_block->fpf |= kvm_run->s.regs.sebc ? FPF_SEBC : 0;
> +	}
>  	save_access_regs(vcpu->arch.host_acrs);
>  	restore_access_regs(vcpu->run->s.regs.acrs);
>  	/* save host (userspace) fprs/vrs */
> @@ -3344,6 +3354,7 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  	kvm_run->s.regs.pft = vcpu->arch.pfault_token;
>  	kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
>  	kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
> +	kvm_run->s.regs.sebc = (vcpu->arch.sie_block->fpf & FPF_SEBC) == FPF_SEBC;
>  	save_access_regs(vcpu->run->s.regs.acrs);
>  	restore_access_regs(vcpu->arch.host_acrs);
>  	/* Save guest register state */
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 5d6ae03..10ea208 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -223,6 +223,10 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	memcpy(scb_o->gcr, scb_s->gcr, 128);
>  	scb_o->pp = scb_s->pp;
> 
> +	/* speculative blocking */
> +	scb_o->fpf &= ~FPF_SEBC;
> +	scb_o->fpf |= scb_s->fpf & FPF_SEBC;
> +
>  	/* interrupt intercept */
>  	switch (scb_s->icptcode) {
>  	case ICPT_PROGI:
> @@ -265,6 +269,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	scb_s->ecb3 = 0;
>  	scb_s->ecd = 0;
>  	scb_s->fac = 0;
> +	scb_s->fpf = 0;
> 
>  	rc = prepare_cpuflags(vcpu, vsie_page);
>  	if (rc)
> @@ -324,6 +329,9 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  			prefix_unmapped(vsie_page);
>  		scb_s->ecb |= scb_o->ecb & ECB_TE;
>  	}
> +	/* speculative blocking */
> +	if (test_kvm_facility(vcpu->kvm, 82))
> +		scb_s->fpf |= scb_o->fpf & FPF_SEBC;
>  	/* SIMD */
>  	if (test_kvm_facility(vcpu->kvm, 129)) {
>  		scb_s->eca |= scb_o->eca & ECA_VX;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 496e59a..066d7ff 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -932,6 +932,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_HYPERV_SYNIC2 148
>  #define KVM_CAP_HYPERV_VP_INDEX 149
>  #define KVM_CAP_S390_AIS_MIGRATION 150
> +#define KVM_CAP_S390_SEB 151
> 
>  #ifdef KVM_CAP_IRQ_ROUTING
>
Paolo Bonzini Jan. 17, 2018, 11:22 a.m. UTC | #2
> while this is kvm code, my current plan is to submit the "final"
> version after review and probably some fixes/renames via Martin
> together with the other patches.  Are you ok with that? Right now it
> seems that the CAP number is still fine.
Sure, though there will be a capability introduced by PPC for similar
purposes, so check for conflicts.

On 17/01/2018 12:18, Christian Borntraeger wrote:
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 2c93cbb..0c18f73 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -421,6 +421,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_S390_GS:
>  		r = test_facility(133);
>  		break;
> +	case KVM_CAP_S390_SEB:
> +		r = test_facility(82);
> +		break;
>  	default:
>  		r = 0;

Can you add a generic "test facility" capability and ioctl?

Paolo
Christian Borntraeger Jan. 17, 2018, 11:28 a.m. UTC | #3
On 01/17/2018 12:22 PM, Paolo Bonzini wrote:
>> while this is kvm code, my current plan is to submit the "final"
>> version after review and probably some fixes/renames via Martin
>> together with the other patches.  Are you ok with that? Right now it
>> seems that the CAP number is still fine.
> Sure, though there will be a capability introduced by PPC for similar
> purposes, so check for conflicts.
> 
> On 17/01/2018 12:18, Christian Borntraeger wrote:
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 2c93cbb..0c18f73 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -421,6 +421,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_S390_GS:
>>  		r = test_facility(133);
>>  		break;
>> +	case KVM_CAP_S390_SEB:
>> +		r = test_facility(82);
>> +		break;
>>  	default:
>>  		r = 0;
> 
> Can you add a generic "test facility" capability and ioctl?

The problem is not that I announce the facility, I in fact announce that the
programmatic interface is available (the sebc sync reg and the usage of that field).
(So the CAP is part of this patch to have both in lockstep)
A non-existing facility will then just disable that programmatic interface.
Christian Borntraeger Jan. 17, 2018, 11:29 a.m. UTC | #4
On 01/17/2018 12:28 PM, Christian Borntraeger wrote:
> 
> 
> On 01/17/2018 12:22 PM, Paolo Bonzini wrote:
>>> while this is kvm code, my current plan is to submit the "final"
>>> version after review and probably some fixes/renames via Martin
>>> together with the other patches.  Are you ok with that? Right now it
>>> seems that the CAP number is still fine.
>> Sure, though there will be a capability introduced by PPC for similar
>> purposes, so check for conflicts.
>>
>> On 17/01/2018 12:18, Christian Borntraeger wrote:
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 2c93cbb..0c18f73 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -421,6 +421,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>>  	case KVM_CAP_S390_GS:
>>>  		r = test_facility(133);
>>>  		break;
>>> +	case KVM_CAP_S390_SEB:
>>> +		r = test_facility(82);
>>> +		break;
>>>  	default:
>>>  		r = 0;
>>
>> Can you add a generic "test facility" capability and ioctl?
> 
> The problem is not that I announce the facility, I in fact announce that the
> programmatic interface is available (the sebc sync reg and the usage of that field).
> (So the CAP is part of this patch to have both in lockstep)
> A non-existing facility will then just disable that programmatic interface.

To put it differently. CAP_S390_GS and CAP_S390_SEB could also just
do a 

return 1;

and the QEMU has to check both (which it probably does anyway)

Christian
Paolo Bonzini Jan. 17, 2018, 11:32 a.m. UTC | #5
On 17/01/2018 12:29, Christian Borntraeger wrote:
>> The problem is not that I announce the facility, I in fact announce that the
>> programmatic interface is available (the sebc sync reg and the usage of that field).
>> (So the CAP is part of this patch to have both in lockstep)
>> A non-existing facility will then just disable that programmatic interface.
> To put it differently. CAP_S390_GS and CAP_S390_SEB could also just
> do a 
> 
> return 1;
> 
> and the QEMU has to check both (which it probably does anyway)

I see.  Thanks for the explanation!

Paolo
David Hildenbrand Jan. 17, 2018, 11:33 a.m. UTC | #6
>  #define ECB_GS		0x40
>  #define ECB_TE		0x10
>  #define ECB_SRSI	0x04
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index 38535a57..20b9e9f 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -224,6 +224,7 @@ struct kvm_guest_debug_arch {
>  #define KVM_SYNC_RICCB  (1UL << 7)
>  #define KVM_SYNC_FPRS   (1UL << 8)
>  #define KVM_SYNC_GSCB   (1UL << 9)
> +#define KVM_SYNC_SEBC   (1UL << 10)
>  /* length and alignment of the sdnx as a power of two */
>  #define SDNXC 8
>  #define SDNXL (1UL << SDNXC)
> @@ -247,7 +248,8 @@ struct kvm_sync_regs {
>  	};
>  	__u8  reserved[512];	/* for future vector expansion */
>  	__u32 fpc;		/* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */
> -	__u8 padding1[52];	/* riccb needs to be 64byte aligned */
> +	__u8 sebc:1;		/* spec blocking */

do you want to define the unused bits as reserved? Nicer to read IMHO

(especially also using spaces "sebc : 1")

> +	__u8 padding1[51];	/* riccb needs to be 64byte aligned */
>  	__u8 riccb[64];		/* runtime instrumentation controls block */
>  	__u8 padding2[192];	/* sdnx needs to be 256byte aligned */
>  	union {
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 2c93cbb..0c18f73 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -421,6 +421,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_S390_GS:
>  		r = test_facility(133);
>  		break;
> +	case KVM_CAP_S390_SEB:
> +		r = test_facility(82);
> +		break;
>  	default:
>  		r = 0;
>  	}
> @@ -2198,6 +2201,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>  	kvm_s390_set_prefix(vcpu, 0);
>  	if (test_kvm_facility(vcpu->kvm, 64))
>  		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
> +	if (test_kvm_facility(vcpu->kvm, 82))
> +		vcpu->run->kvm_valid_regs |= KVM_SYNC_SEBC;
>  	if (test_kvm_facility(vcpu->kvm, 133))
>  		vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB;
>  	/* fprs can be synchronized via vrs, even if the guest has no vx. With
> @@ -2339,6 +2344,7 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
>  	current->thread.fpu.fpc = 0;
>  	vcpu->arch.sie_block->gbea = 1;
>  	vcpu->arch.sie_block->pp = 0;
> +	vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
>  	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
>  	kvm_clear_async_pf_completion_queue(vcpu);
>  	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
> @@ -3298,6 +3304,10 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  		vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
>  		vcpu->arch.gs_enabled = 1;
>  	}
> +	if (kvm_run->kvm_dirty_regs & KVM_SYNC_SEBC) {

We should test for test_facility(82). Otherwise user space can enable
undefined bits in the SCB on machines with !facility 82.

> +		vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
> +		vcpu->arch.sie_block->fpf |= kvm_run->s.regs.sebc ? FPF_SEBC : 0;
> +	}
>  	save_access_regs(vcpu->arch.host_acrs);
>  	restore_access_regs(vcpu->run->s.regs.acrs);
>  	/* save host (userspace) fprs/vrs */
> @@ -3344,6 +3354,7 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>  	kvm_run->s.regs.pft = vcpu->arch.pfault_token;
>  	kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
>  	kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
> +	kvm_run->s.regs.sebc = (vcpu->arch.sie_block->fpf & FPF_SEBC) == FPF_SEBC;
>  	save_access_regs(vcpu->run->s.regs.acrs);
>  	restore_access_regs(vcpu->arch.host_acrs);
>  	/* Save guest register state */
> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 5d6ae03..10ea208 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -223,6 +223,10 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	memcpy(scb_o->gcr, scb_s->gcr, 128);
>  	scb_o->pp = scb_s->pp;
>  
> +	/* speculative blocking */

This field should only be written back with test_kvm_facility(vcpu->kvm, 82)

(no public documentation, this looks like the SIE can modify this field?
Triggered by which instruction?)

> +	scb_o->fpf &= ~FPF_SEBC;
> +	scb_o->fpf |= scb_s->fpf & FPF_SEBC;
> +
>  	/* interrupt intercept */
>  	switch (scb_s->icptcode) {
>  	case ICPT_PROGI:
> @@ -265,6 +269,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  	scb_s->ecb3 = 0;
>  	scb_s->ecd = 0;
>  	scb_s->fac = 0;
> +	scb_s->fpf = 0;
>  
>  	rc = prepare_cpuflags(vcpu, vsie_page);
>  	if (rc)
> @@ -324,6 +329,9 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>  			prefix_unmapped(vsie_page);
>  		scb_s->ecb |= scb_o->ecb & ECB_TE;
>  	}
> +	/* speculative blocking */
> +	if (test_kvm_facility(vcpu->kvm, 82))
> +		scb_s->fpf |= scb_o->fpf & FPF_SEBC;
>  	/* SIMD */
>  	if (test_kvm_facility(vcpu->kvm, 129)) {
>  		scb_s->eca |= scb_o->eca & ECA_VX;
Christian Borntraeger Jan. 17, 2018, 11:39 a.m. UTC | #7
On 01/17/2018 12:33 PM, David Hildenbrand wrote:
> 
>>  #define ECB_GS		0x40
>>  #define ECB_TE		0x10
>>  #define ECB_SRSI	0x04
>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>> index 38535a57..20b9e9f 100644
>> --- a/arch/s390/include/uapi/asm/kvm.h
>> +++ b/arch/s390/include/uapi/asm/kvm.h
>> @@ -224,6 +224,7 @@ struct kvm_guest_debug_arch {
>>  #define KVM_SYNC_RICCB  (1UL << 7)
>>  #define KVM_SYNC_FPRS   (1UL << 8)
>>  #define KVM_SYNC_GSCB   (1UL << 9)
>> +#define KVM_SYNC_SEBC   (1UL << 10)
>>  /* length and alignment of the sdnx as a power of two */
>>  #define SDNXC 8
>>  #define SDNXL (1UL << SDNXC)
>> @@ -247,7 +248,8 @@ struct kvm_sync_regs {
>>  	};
>>  	__u8  reserved[512];	/* for future vector expansion */
>>  	__u32 fpc;		/* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */
>> -	__u8 padding1[52];	/* riccb needs to be 64byte aligned */
>> +	__u8 sebc:1;		/* spec blocking */
> 
> do you want to define the unused bits as reserved? Nicer to read IMHO

I certainly want to have these bits for future use. So maybe a 
__u8 reserved : 7; 
after that makes a lot of sense. Also the sebc : 1; (spaces)

(FWIW, I will rename that to bpbc for other reasons).


> 
> (especially also using spaces "sebc : 1")
> 
>> +	__u8 padding1[51];	/* riccb needs to be 64byte aligned */
>>  	__u8 riccb[64];		/* runtime instrumentation controls block */
>>  	__u8 padding2[192];	/* sdnx needs to be 256byte aligned */
>>  	union {
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 2c93cbb..0c18f73 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -421,6 +421,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>  	case KVM_CAP_S390_GS:
>>  		r = test_facility(133);
>>  		break;
>> +	case KVM_CAP_S390_SEB:
>> +		r = test_facility(82);
>> +		break;
>>  	default:
>>  		r = 0;
>>  	}
>> @@ -2198,6 +2201,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
>>  	kvm_s390_set_prefix(vcpu, 0);
>>  	if (test_kvm_facility(vcpu->kvm, 64))
>>  		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
>> +	if (test_kvm_facility(vcpu->kvm, 82))
>> +		vcpu->run->kvm_valid_regs |= KVM_SYNC_SEBC;
>>  	if (test_kvm_facility(vcpu->kvm, 133))
>>  		vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB;
>>  	/* fprs can be synchronized via vrs, even if the guest has no vx. With
>> @@ -2339,6 +2344,7 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
>>  	current->thread.fpu.fpc = 0;
>>  	vcpu->arch.sie_block->gbea = 1;
>>  	vcpu->arch.sie_block->pp = 0;
>> +	vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
>>  	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
>>  	kvm_clear_async_pf_completion_queue(vcpu);
>>  	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
>> @@ -3298,6 +3304,10 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  		vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
>>  		vcpu->arch.gs_enabled = 1;
>>  	}
>> +	if (kvm_run->kvm_dirty_regs & KVM_SYNC_SEBC) {
> 
> We should test for test_facility(82). Otherwise user space can enable
> undefined bits in the SCB on machines with !facility 82.

Agreed, will fix.

> 
>> +		vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
>> +		vcpu->arch.sie_block->fpf |= kvm_run->s.regs.sebc ? FPF_SEBC : 0;
>> +	}
>>  	save_access_regs(vcpu->arch.host_acrs);
>>  	restore_access_regs(vcpu->run->s.regs.acrs);
>>  	/* save host (userspace) fprs/vrs */
>> @@ -3344,6 +3354,7 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  	kvm_run->s.regs.pft = vcpu->arch.pfault_token;
>>  	kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
>>  	kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
>> +	kvm_run->s.regs.sebc = (vcpu->arch.sie_block->fpf & FPF_SEBC) == FPF_SEBC;
>>  	save_access_regs(vcpu->run->s.regs.acrs);
>>  	restore_access_regs(vcpu->arch.host_acrs);
>>  	/* Save guest register state */
>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
>> index 5d6ae03..10ea208 100644
>> --- a/arch/s390/kvm/vsie.c
>> +++ b/arch/s390/kvm/vsie.c
>> @@ -223,6 +223,10 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  	memcpy(scb_o->gcr, scb_s->gcr, 128);
>>  	scb_o->pp = scb_s->pp;
>>  
>> +	/* speculative blocking */
> 
> This field should only be written back with test_kvm_facility(vcpu->kvm, 82)

Agreed. 

> 
> (no public documentation, this looks like the SIE can modify this field?
> Triggered by which instruction?)

The instruction from patch 3.
> 
>> +	scb_o->fpf &= ~FPF_SEBC;
>> +	scb_o->fpf |= scb_s->fpf & FPF_SEBC;
>> +
>>  	/* interrupt intercept */
>>  	switch (scb_s->icptcode) {
>>  	case ICPT_PROGI:
>> @@ -265,6 +269,7 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  	scb_s->ecb3 = 0;
>>  	scb_s->ecd = 0;
>>  	scb_s->fac = 0;
>> +	scb_s->fpf = 0;
>>  
>>  	rc = prepare_cpuflags(vcpu, vsie_page);
>>  	if (rc)
>> @@ -324,6 +329,9 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
>>  			prefix_unmapped(vsie_page);
>>  		scb_s->ecb |= scb_o->ecb & ECB_TE;
>>  	}
>> +	/* speculative blocking */
>> +	if (test_kvm_facility(vcpu->kvm, 82))
>> +		scb_s->fpf |= scb_o->fpf & FPF_SEBC;
>>  	/* SIMD */
>>  	if (test_kvm_facility(vcpu->kvm, 129)) {
>>  		scb_s->eca |= scb_o->eca & ECA_VX;
> 

Thanks for the quick review.
diff mbox

Patch

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index e14f381..a2f6c5d 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -207,7 +207,8 @@  struct kvm_s390_sie_block {
 	__u16	ipa;			/* 0x0056 */
 	__u32	ipb;			/* 0x0058 */
 	__u32	scaoh;			/* 0x005c */
-	__u8	reserved60;		/* 0x0060 */
+#define FPF_SEBC 	0x20
+	__u8	fpf;			/* 0x0060 */
 #define ECB_GS		0x40
 #define ECB_TE		0x10
 #define ECB_SRSI	0x04
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 38535a57..20b9e9f 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -224,6 +224,7 @@  struct kvm_guest_debug_arch {
 #define KVM_SYNC_RICCB  (1UL << 7)
 #define KVM_SYNC_FPRS   (1UL << 8)
 #define KVM_SYNC_GSCB   (1UL << 9)
+#define KVM_SYNC_SEBC   (1UL << 10)
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8
 #define SDNXL (1UL << SDNXC)
@@ -247,7 +248,8 @@  struct kvm_sync_regs {
 	};
 	__u8  reserved[512];	/* for future vector expansion */
 	__u32 fpc;		/* valid on KVM_SYNC_VRS or KVM_SYNC_FPRS */
-	__u8 padding1[52];	/* riccb needs to be 64byte aligned */
+	__u8 sebc:1;		/* spec blocking */
+	__u8 padding1[51];	/* riccb needs to be 64byte aligned */
 	__u8 riccb[64];		/* runtime instrumentation controls block */
 	__u8 padding2[192];	/* sdnx needs to be 256byte aligned */
 	union {
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 2c93cbb..0c18f73 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -421,6 +421,9 @@  int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_S390_GS:
 		r = test_facility(133);
 		break;
+	case KVM_CAP_S390_SEB:
+		r = test_facility(82);
+		break;
 	default:
 		r = 0;
 	}
@@ -2198,6 +2201,8 @@  int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	kvm_s390_set_prefix(vcpu, 0);
 	if (test_kvm_facility(vcpu->kvm, 64))
 		vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
+	if (test_kvm_facility(vcpu->kvm, 82))
+		vcpu->run->kvm_valid_regs |= KVM_SYNC_SEBC;
 	if (test_kvm_facility(vcpu->kvm, 133))
 		vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB;
 	/* fprs can be synchronized via vrs, even if the guest has no vx. With
@@ -2339,6 +2344,7 @@  static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
 	current->thread.fpu.fpc = 0;
 	vcpu->arch.sie_block->gbea = 1;
 	vcpu->arch.sie_block->pp = 0;
+	vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
 	vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
 	kvm_clear_async_pf_completion_queue(vcpu);
 	if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
@@ -3298,6 +3304,10 @@  static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		vcpu->arch.sie_block->ecd |= ECD_HOSTREGMGMT;
 		vcpu->arch.gs_enabled = 1;
 	}
+	if (kvm_run->kvm_dirty_regs & KVM_SYNC_SEBC) {
+		vcpu->arch.sie_block->fpf &= ~FPF_SEBC;
+		vcpu->arch.sie_block->fpf |= kvm_run->s.regs.sebc ? FPF_SEBC : 0;
+	}
 	save_access_regs(vcpu->arch.host_acrs);
 	restore_access_regs(vcpu->run->s.regs.acrs);
 	/* save host (userspace) fprs/vrs */
@@ -3344,6 +3354,7 @@  static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	kvm_run->s.regs.pft = vcpu->arch.pfault_token;
 	kvm_run->s.regs.pfs = vcpu->arch.pfault_select;
 	kvm_run->s.regs.pfc = vcpu->arch.pfault_compare;
+	kvm_run->s.regs.sebc = (vcpu->arch.sie_block->fpf & FPF_SEBC) == FPF_SEBC;
 	save_access_regs(vcpu->run->s.regs.acrs);
 	restore_access_regs(vcpu->arch.host_acrs);
 	/* Save guest register state */
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 5d6ae03..10ea208 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -223,6 +223,10 @@  static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	memcpy(scb_o->gcr, scb_s->gcr, 128);
 	scb_o->pp = scb_s->pp;
 
+	/* speculative blocking */
+	scb_o->fpf &= ~FPF_SEBC;
+	scb_o->fpf |= scb_s->fpf & FPF_SEBC;
+
 	/* interrupt intercept */
 	switch (scb_s->icptcode) {
 	case ICPT_PROGI:
@@ -265,6 +269,7 @@  static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	scb_s->ecb3 = 0;
 	scb_s->ecd = 0;
 	scb_s->fac = 0;
+	scb_s->fpf = 0;
 
 	rc = prepare_cpuflags(vcpu, vsie_page);
 	if (rc)
@@ -324,6 +329,9 @@  static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 			prefix_unmapped(vsie_page);
 		scb_s->ecb |= scb_o->ecb & ECB_TE;
 	}
+	/* speculative blocking */
+	if (test_kvm_facility(vcpu->kvm, 82))
+		scb_s->fpf |= scb_o->fpf & FPF_SEBC;
 	/* SIMD */
 	if (test_kvm_facility(vcpu->kvm, 129)) {
 		scb_s->eca |= scb_o->eca & ECA_VX;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 496e59a..066d7ff 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -932,6 +932,7 @@  struct kvm_ppc_resize_hpt {
 #define KVM_CAP_HYPERV_SYNIC2 148
 #define KVM_CAP_HYPERV_VP_INDEX 149
 #define KVM_CAP_S390_AIS_MIGRATION 150
+#define KVM_CAP_S390_SEB 151
 
 #ifdef KVM_CAP_IRQ_ROUTING