On 10/16/20 3:44 AM, Christian Borntraeger wrote: > > > On 16.10.20 09:34, Janosch Frank wrote: >> On 10/15/20 9:59 PM, Collin Walling wrote: >>> The DIAGNOSE 0x0318 instruction must be reset on a normal and clear >>> reset. However, this was missed for the clear reset case. >>> >>> Let's fix this by resetting the information during a normal reset. >>> Since clear reset is a superset of normal reset, the info will >>> still reset on a clear reset. >> >> The architecture really confuses me here but I think we don't want this >> in the kernel VCPU reset handlers at all. >> >> This needs to be reset per VM *NOT* per VCPU. >> Hence the resets are bound to diag308 and not SIGP. >> >> I.e. we need to clear it in QEMU's VM reset handler. >> It's still early and I have yet to consume my first coffee, am I missing >> something? > > I agree with Janosch. architecture indicates that this should only be reset > for VM-wide resets, e.g. sigp orders 11 and 12 are explicitly mentioned > to NOT reset the value. > A few questions regarding how resets for diag318 should work here: The AR states that any copies retained by the diag318 should be set to 0 on a clear reset and load normal -- I thought both of those resets were implicitly called by diag308 as well? Should the register used to store diag318 info not be set to 0 *by KVM* then? Should the values be set *by QEMU* and a subsequent sync_regs will ensure things are sane on the KVM side? >> >>> >>> Signed-off-by: Collin Walling <walling@xxxxxxxxxxxxx> >>> --- >>> arch/s390/kvm/kvm-s390.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >>> index 6b74b92c1a58..b0cf8367e261 100644 >>> --- a/arch/s390/kvm/kvm-s390.c >>> +++ b/arch/s390/kvm/kvm-s390.c >>> @@ -3516,6 +3516,7 @@ static void kvm_arch_vcpu_ioctl_normal_reset(struct kvm_vcpu *vcpu) >>> vcpu->arch.sie_block->gpsw.mask &= ~PSW_MASK_RI; >>> vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID; >>> memset(vcpu->run->s.regs.riccb, 0, sizeof(vcpu->run->s.regs.riccb)); >>> + vcpu->run->s.regs.diag318 = 0; >>> >>> kvm_clear_async_pf_completion_queue(vcpu); >>> if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm)) >>> @@ -3582,7 +3583,6 @@ static void kvm_arch_vcpu_ioctl_clear_reset(struct kvm_vcpu *vcpu) >>> >>> regs->etoken = 0; >>> regs->etoken_extension = 0; >>> - regs->diag318 = 0; >>> } >>> >>> int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs) >>> >> >> -- Regards, Collin Stay safe and stay healthy