Message ID | 20221123090833.292938-1-thuth@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: s390: vsie: Fix the initialization of the epoch extension (epdx) field | expand |
On 23.11.22 10:08, Thomas Huth wrote: > We recently experienced some weird huge time jumps in nested guests when > rebooting them in certain cases. After adding some debug code to the epoch > handling in vsie.c (thanks to David Hildenbrand for the idea!), it was > obvious that the "epdx" field (the multi-epoch extension) did not get set > to 0xff in case the "epoch" field was negative. > Seems like the code misses to copy the value from the epdx field from > the guest to the shadow control block. By doing so, the weird time > jumps are gone in our scenarios. > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 > Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > arch/s390/kvm/vsie.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 94138f8f0c1c..ace2541ababd 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -546,8 +546,10 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > 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)) > + if (test_kvm_facility(vcpu->kvm, 139)) { > scb_s->ecd |= scb_o->ecd & ECD_MEF; > + scb_s->epdx = scb_o->epdx; > + } > > /* etoken */ > if (test_kvm_facility(vcpu->kvm, 156)) Nice, Acked-by: David Hildenbrand <david@redhat.com>
On Wed, 23 Nov 2022 10:08:33 +0100 Thomas Huth <thuth@redhat.com> wrote: > We recently experienced some weird huge time jumps in nested guests when > rebooting them in certain cases. After adding some debug code to the epoch > handling in vsie.c (thanks to David Hildenbrand for the idea!), it was > obvious that the "epdx" field (the multi-epoch extension) did not get set > to 0xff in case the "epoch" field was negative. > Seems like the code misses to copy the value from the epdx field from > the guest to the shadow control block. By doing so, the weird time > jumps are gone in our scenarios. > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 > Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > arch/s390/kvm/vsie.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 94138f8f0c1c..ace2541ababd 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -546,8 +546,10 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > 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)) > + if (test_kvm_facility(vcpu->kvm, 139)) { > scb_s->ecd |= scb_o->ecd & ECD_MEF; > + scb_s->epdx = scb_o->epdx; looks quite straightforward > + } > > /* etoken */ > if (test_kvm_facility(vcpu->kvm, 156))
Am 23.11.22 um 10:08 schrieb Thomas Huth: > We recently experienced some weird huge time jumps in nested guests when > rebooting them in certain cases. After adding some debug code to the epoch > handling in vsie.c (thanks to David Hildenbrand for the idea!), it was > obvious that the "epdx" field (the multi-epoch extension) did not get set > to 0xff in case the "epoch" field was negative. > Seems like the code misses to copy the value from the epdx field from > the guest to the shadow control block. By doing so, the weird time > jumps are gone in our scenarios. > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 > Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") We might want to add cc stable, just in case. Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > arch/s390/kvm/vsie.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 94138f8f0c1c..ace2541ababd 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -546,8 +546,10 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > 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)) > + if (test_kvm_facility(vcpu->kvm, 139)) { > scb_s->ecd |= scb_o->ecd & ECD_MEF; > + scb_s->epdx = scb_o->epdx; > + } > > /* etoken */ > if (test_kvm_facility(vcpu->kvm, 156))
On 11/23/22 10:41, Christian Borntraeger wrote: > > > Am 23.11.22 um 10:08 schrieb Thomas Huth: >> We recently experienced some weird huge time jumps in nested guests when >> rebooting them in certain cases. After adding some debug code to the epoch >> handling in vsie.c (thanks to David Hildenbrand for the idea!), it was >> obvious that the "epdx" field (the multi-epoch extension) did not get set >> to 0xff in case the "epoch" field was negative. >> Seems like the code misses to copy the value from the epdx field from >> the guest to the shadow control block. By doing so, the weird time >> jumps are gone in our scenarios. >> >> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 >> Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") > > We might want to add cc stable, just in case. I'm pushing this to devel for the CI with the following additions: Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Cc: stable@vger.kernel.org # 4.19+ > > Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> > > >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> arch/s390/kvm/vsie.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index 94138f8f0c1c..ace2541ababd 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -546,8 +546,10 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> 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)) >> + if (test_kvm_facility(vcpu->kvm, 139)) { >> scb_s->ecd |= scb_o->ecd & ECD_MEF; >> + scb_s->epdx = scb_o->epdx; >> + } >> >> /* etoken */ >> if (test_kvm_facility(vcpu->kvm, 156))
On 11/23/22 10:08, Thomas Huth wrote: > We recently experienced some weird huge time jumps in nested guests when > rebooting them in certain cases. After adding some debug code to the epoch > handling in vsie.c (thanks to David Hildenbrand for the idea!), it was > obvious that the "epdx" field (the multi-epoch extension) did not get set > to 0xff in case the "epoch" field was negative. > Seems like the code misses to copy the value from the epdx field from > the guest to the shadow control block. By doing so, the weird time > jumps are gone in our scenarios. > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 > Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") > Signed-off-by: Thomas Huth <thuth@redhat.com> Could you please add a test for this to the KVM unit tests? I'd guess you might already have some code for it from your debugging sessions.
On 24/11/2022 11.27, Janosch Frank wrote: > On 11/23/22 10:08, Thomas Huth wrote: >> We recently experienced some weird huge time jumps in nested guests when >> rebooting them in certain cases. After adding some debug code to the epoch >> handling in vsie.c (thanks to David Hildenbrand for the idea!), it was >> obvious that the "epdx" field (the multi-epoch extension) did not get set >> to 0xff in case the "epoch" field was negative. >> Seems like the code misses to copy the value from the epdx field from >> the guest to the shadow control block. By doing so, the weird time >> jumps are gone in our scenarios. >> >> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 >> Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") >> Signed-off-by: Thomas Huth <thuth@redhat.com> > > Could you please add a test for this to the KVM unit tests? > I'd guess you might already have some code for it from your debugging sessions. I don't have some test code for this yet - I was only testing with the scenario that is described in the bugzilla ticket. But sure, I can have a try to come up with a k-u-t test. Thomas
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 94138f8f0c1c..ace2541ababd 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -546,8 +546,10 @@ static int shadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) 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)) + if (test_kvm_facility(vcpu->kvm, 139)) { scb_s->ecd |= scb_o->ecd & ECD_MEF; + scb_s->epdx = scb_o->epdx; + } /* etoken */ if (test_kvm_facility(vcpu->kvm, 156))
We recently experienced some weird huge time jumps in nested guests when rebooting them in certain cases. After adding some debug code to the epoch handling in vsie.c (thanks to David Hildenbrand for the idea!), it was obvious that the "epdx" field (the multi-epoch extension) did not get set to 0xff in case the "epoch" field was negative. Seems like the code misses to copy the value from the epdx field from the guest to the shadow control block. By doing so, the weird time jumps are gone in our scenarios. Link: https://bugzilla.redhat.com/show_bug.cgi?id=2140899 Fixes: 8fa1696ea781 ("KVM: s390: Multiple Epoch Facility support") Signed-off-by: Thomas Huth <thuth@redhat.com> --- arch/s390/kvm/vsie.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)