Re: [PATCH] KVM: s390: Introduce storage key removal facility

Christian Borntraeger
2020-09-07T14:04:19Z

On 07.09.20 15:14, Janosch Frank wrote:
[...]

> @@ -565,7 +566,44 @@ int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu)
> rc = handle_partial_execution(vcpu);
> break;
> case ICPT_KSS:
> - rc = kvm_s390_skey_check_enable(vcpu);
> + if (likely(!test_kvm_facility(vcpu->kvm, 169))) {

please no "likely" unless really necessary. This is for both cases slow path.

> + rc = kvm_s390_skey_check_enable(vcpu);
> + } else {
> + /*
> + * Storage key removal facility emulation.
> + *
> + * KSS is the same priority as instruction
> + * interception. Hence we need handling here
> + * and in the instruction emulation code.
> + *
> + * lpsw(e) needs to store the problematic psw
> + * as the program old psw. Calling the
> + * handlers directly does that without falsely
> + * increasing stat counters.
> + */

I do not understand this comment and the code below. As the fault is suppressing
we should NOT store the program old psw?

> + switch (vcpu->arch.sie_block->ipa) {
> + case 0xb2b2:
> + kvm_s390_forward_psw(vcpu, kvm_s390_get_ilen(vcpu));
> + rc = kvm_s390_handle_b2(vcpu);
> + break;
> + case 0x8200:
+ kvm_s390_forward_psw(vcpu, kvm_s390_get_ilen(vcpu));
> + rc = kvm_s390_handle_lpsw(vcpu);
> + break;
> + case 0:
> + /* Interception caused by exception new PSW key */
> + rc = kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
> + break;
> + default:
> + /*
> + * KSS is nullifying (no psw forward),
> + * SKRF issues suppressing SPECIAL
> + * OPS, so we need to forward by hand.
> + */
> + kvm_s390_forward_psw(vcpu, kvm_s390_get_ilen(vcpu));
> + rc = kvm_s390_inject_program_int(vcpu, PGM_SPECIAL_OPERATION);
> + }
> + } …