| Message ID | 20210322140559.500716-3-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/kvm: VSIE: fix prefixing and MSO for MVPG | expand |
On 22.03.21 15:05, Claudio Imbrenda wrote: > Prefixing needs to be applied to the guest real address to translate it > into a guest absolute address. > > The value of MSO needs to be added to a guest-absolute address in order to > obtain the host-virtual. > > Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when in VSIE") > Cc: stable@vger.kernel.org > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > Reported-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/vsie.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 48aab6290a77..ac86f11e46dc 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, > static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > { > struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; > - unsigned long pei_dest, pei_src, src, dest, mask; > + unsigned long pei_dest, pei_src, dest, src, mask, mso, prefix; > u64 *pei_block = &vsie_page->scb_o->mcic; > int edat, rc_dest, rc_src; > union ctlreg0 cr0; > @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > cr0.val = vcpu->arch.sie_block->gcr[0]; > edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8); > mask = _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); > + mso = scb_s->mso & ~(1UL << 20); > + prefix = scb_s->prefix << GUEST_PREFIX_SHIFT; > > dest = vsie_get_register(vcpu, vsie_page, scb_s->ipb >> 16) & mask; > + dest = _kvm_s390_real_to_abs(prefix, dest) + mso; > src = vsie_get_register(vcpu, vsie_page, scb_s->ipb >> 20) & mask; > + src = _kvm_s390_real_to_abs(prefix, src) + mso; > > rc_dest = kvm_s390_shadow_fault(vcpu, vsie_page->gmap, dest, &pei_dest); > rc_src = kvm_s390_shadow_fault(vcpu, vsie_page->gmap, src, &pei_src); > mso is always confusing, but I think this should be correct. Reviewed-by: David Hildenbrand <david@redhat.com>
On 22.03.21 15:05, Claudio Imbrenda wrote: > Prefixing needs to be applied to the guest real address to translate it > into a guest absolute address. > > The value of MSO needs to be added to a guest-absolute address in order to > obtain the host-virtual. > > Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when in VSIE") > Cc: stable@vger.kernel.org > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > Reported-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/vsie.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index 48aab6290a77..ac86f11e46dc 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, > static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > { > struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; > - unsigned long pei_dest, pei_src, src, dest, mask; > + unsigned long pei_dest, pei_src, dest, src, mask, mso, prefix; > u64 *pei_block = &vsie_page->scb_o->mcic; > int edat, rc_dest, rc_src; > union ctlreg0 cr0; > @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > cr0.val = vcpu->arch.sie_block->gcr[0]; > edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8); > mask = _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); > + mso = scb_s->mso & ~(1UL << 20); shouldnt that be ~((1UL << 20 ) -1)
On 23.03.21 16:07, Christian Borntraeger wrote: > > > On 22.03.21 15:05, Claudio Imbrenda wrote: >> Prefixing needs to be applied to the guest real address to translate it >> into a guest absolute address. >> >> The value of MSO needs to be added to a guest-absolute address in order to >> obtain the host-virtual. >> >> Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when in VSIE") >> Cc: stable@vger.kernel.org >> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >> Reported-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> arch/s390/kvm/vsie.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >> index 48aab6290a77..ac86f11e46dc 100644 >> --- a/arch/s390/kvm/vsie.c >> +++ b/arch/s390/kvm/vsie.c >> @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, >> static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> { >> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; >> - unsigned long pei_dest, pei_src, src, dest, mask; >> + unsigned long pei_dest, pei_src, dest, src, mask, mso, prefix; >> u64 *pei_block = &vsie_page->scb_o->mcic; >> int edat, rc_dest, rc_src; >> union ctlreg0 cr0; >> @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >> cr0.val = vcpu->arch.sie_block->gcr[0]; >> edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8); >> mask = _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); >> + mso = scb_s->mso & ~(1UL << 20); > shouldnt that be ~((1UL << 20 ) -1) > Looking at shadow_scb(), we can simply take scb_s->mso unmodified.
On 23.03.21 16:11, David Hildenbrand wrote: > On 23.03.21 16:07, Christian Borntraeger wrote: >> >> >> On 22.03.21 15:05, Claudio Imbrenda wrote: >>> Prefixing needs to be applied to the guest real address to translate it >>> into a guest absolute address. >>> >>> The value of MSO needs to be added to a guest-absolute address in order to >>> obtain the host-virtual. >>> >>> Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when in VSIE") >>> Cc: stable@vger.kernel.org >>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>> Reported-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> arch/s390/kvm/vsie.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>> index 48aab6290a77..ac86f11e46dc 100644 >>> --- a/arch/s390/kvm/vsie.c >>> +++ b/arch/s390/kvm/vsie.c >>> @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, >>> static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>> { >>> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; >>> - unsigned long pei_dest, pei_src, src, dest, mask; >>> + unsigned long pei_dest, pei_src, dest, src, mask, mso, prefix; >>> u64 *pei_block = &vsie_page->scb_o->mcic; >>> int edat, rc_dest, rc_src; >>> union ctlreg0 cr0; >>> @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>> cr0.val = vcpu->arch.sie_block->gcr[0]; >>> edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8); >>> mask = _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); >>> + mso = scb_s->mso & ~(1UL << 20); >> shouldnt that be ~((1UL << 20 ) -1) >> > > Looking at shadow_scb(), we can simply take scb_s->mso unmodified. Right, I think I can fix this up (and get rid of the local mso variable) when queueing. Or shall Claudio send a new version of the patch?
On 23.03.21 16:16, Christian Borntraeger wrote: > > > On 23.03.21 16:11, David Hildenbrand wrote: >> On 23.03.21 16:07, Christian Borntraeger wrote: >>> >>> >>> On 22.03.21 15:05, Claudio Imbrenda wrote: >>>> Prefixing needs to be applied to the guest real address to translate it >>>> into a guest absolute address. >>>> >>>> The value of MSO needs to be added to a guest-absolute address in order to >>>> obtain the host-virtual. >>>> >>>> Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when in VSIE") >>>> Cc: stable@vger.kernel.org >>>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>>> Reported-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> arch/s390/kvm/vsie.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>> index 48aab6290a77..ac86f11e46dc 100644 >>>> --- a/arch/s390/kvm/vsie.c >>>> +++ b/arch/s390/kvm/vsie.c >>>> @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, >>>> static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>> { >>>> struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; >>>> - unsigned long pei_dest, pei_src, src, dest, mask; >>>> + unsigned long pei_dest, pei_src, dest, src, mask, mso, prefix; >>>> u64 *pei_block = &vsie_page->scb_o->mcic; >>>> int edat, rc_dest, rc_src; >>>> union ctlreg0 cr0; >>>> @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) >>>> cr0.val = vcpu->arch.sie_block->gcr[0]; >>>> edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8); >>>> mask = _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); >>>> + mso = scb_s->mso & ~(1UL << 20); >>> shouldnt that be ~((1UL << 20 ) -1) >>> >> >> Looking at shadow_scb(), we can simply take scb_s->mso unmodified. > > Right, I think I can fix this up (and get rid of the local mso variable) > when queueing. Or shall Claudio send a new version of the patch? > IMHO, you can just fix it up.
On Tue, 23 Mar 2021 16:16:18 +0100 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 23.03.21 16:11, David Hildenbrand wrote: > > On 23.03.21 16:07, Christian Borntraeger wrote: > >> > >> > >> On 22.03.21 15:05, Claudio Imbrenda wrote: > >>> Prefixing needs to be applied to the guest real address to > >>> translate it into a guest absolute address. > >>> > >>> The value of MSO needs to be added to a guest-absolute address in > >>> order to obtain the host-virtual. > >>> > >>> Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when > >>> in VSIE") Cc: stable@vger.kernel.org > >>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > >>> Reported-by: Janosch Frank <frankja@linux.ibm.com> > >>> --- > >>> arch/s390/kvm/vsie.c | 6 +++++- > >>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > >>> index 48aab6290a77..ac86f11e46dc 100644 > >>> --- a/arch/s390/kvm/vsie.c > >>> +++ b/arch/s390/kvm/vsie.c > >>> @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct > >>> kvm_vcpu *vcpu, struct vsie_page *vsie_page, static int > >>> vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page > >>> *vsie_page) { struct kvm_s390_sie_block *scb_s = > >>> &vsie_page->scb_s; > >>> - unsigned long pei_dest, pei_src, src, dest, mask; > >>> + unsigned long pei_dest, pei_src, dest, src, mask, mso, > >>> prefix; u64 *pei_block = &vsie_page->scb_o->mcic; > >>> int edat, rc_dest, rc_src; > >>> union ctlreg0 cr0; > >>> @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct > >>> kvm_vcpu *vcpu, struct vsie_page *vsie_page) cr0.val = > >>> vcpu->arch.sie_block->gcr[0]; edat = cr0.edat && > >>> test_kvm_facility(vcpu->kvm, 8); mask = > >>> _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); > >>> + mso = scb_s->mso & ~(1UL << 20); > >> shouldnt that be ~((1UL << 20 ) -1) oops > > > > Looking at shadow_scb(), we can simply take scb_s->mso unmodified. > > Right, I think I can fix this up (and get rid of the local mso I think that's easier/simpler > variable) when queueing. Or shall Claudio send a new version of the > patch?
On 23.03.21 16:21, Claudio Imbrenda wrote: > On Tue, 23 Mar 2021 16:16:18 +0100 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> On 23.03.21 16:11, David Hildenbrand wrote: >>> On 23.03.21 16:07, Christian Borntraeger wrote: >>>> >>>> >>>> On 22.03.21 15:05, Claudio Imbrenda wrote: >>>>> Prefixing needs to be applied to the guest real address to >>>>> translate it into a guest absolute address. >>>>> >>>>> The value of MSO needs to be added to a guest-absolute address in >>>>> order to obtain the host-virtual. >>>>> >>>>> Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when >>>>> in VSIE") Cc: stable@vger.kernel.org >>>>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >>>>> Reported-by: Janosch Frank <frankja@linux.ibm.com> >>>>> --- >>>>> arch/s390/kvm/vsie.c | 6 +++++- >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c >>>>> index 48aab6290a77..ac86f11e46dc 100644 >>>>> --- a/arch/s390/kvm/vsie.c >>>>> +++ b/arch/s390/kvm/vsie.c >>>>> @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct >>>>> kvm_vcpu *vcpu, struct vsie_page *vsie_page, static int >>>>> vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page >>>>> *vsie_page) { struct kvm_s390_sie_block *scb_s = >>>>> &vsie_page->scb_s; >>>>> - unsigned long pei_dest, pei_src, src, dest, mask; >>>>> + unsigned long pei_dest, pei_src, dest, src, mask, mso, >>>>> prefix; u64 *pei_block = &vsie_page->scb_o->mcic; >>>>> int edat, rc_dest, rc_src; >>>>> union ctlreg0 cr0; >>>>> @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct >>>>> kvm_vcpu *vcpu, struct vsie_page *vsie_page) cr0.val = >>>>> vcpu->arch.sie_block->gcr[0]; edat = cr0.edat && >>>>> test_kvm_facility(vcpu->kvm, 8); mask = >>>>> _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); >>>>> + mso = scb_s->mso & ~(1UL << 20); >>>> shouldnt that be ~((1UL << 20 ) -1) > > oops > >>> >>> Looking at shadow_scb(), we can simply take scb_s->mso unmodified. >> >> Right, I think I can fix this up (and get rid of the local mso > > I think that's easier/simpler > >> variable) when queueing. Or shall Claudio send a new version of the >> patch? > queued (with fixup). Please note: can you use "KVM: s390:" instead of "s390/kvm" for the future patches.
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 48aab6290a77..ac86f11e46dc 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -1002,7 +1002,7 @@ static u64 vsie_get_register(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page, static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) { struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s; - unsigned long pei_dest, pei_src, src, dest, mask; + unsigned long pei_dest, pei_src, dest, src, mask, mso, prefix; u64 *pei_block = &vsie_page->scb_o->mcic; int edat, rc_dest, rc_src; union ctlreg0 cr0; @@ -1010,9 +1010,13 @@ static int vsie_handle_mvpg(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) cr0.val = vcpu->arch.sie_block->gcr[0]; edat = cr0.edat && test_kvm_facility(vcpu->kvm, 8); mask = _kvm_s390_logical_to_effective(&scb_s->gpsw, PAGE_MASK); + mso = scb_s->mso & ~(1UL << 20); + prefix = scb_s->prefix << GUEST_PREFIX_SHIFT; dest = vsie_get_register(vcpu, vsie_page, scb_s->ipb >> 16) & mask; + dest = _kvm_s390_real_to_abs(prefix, dest) + mso; src = vsie_get_register(vcpu, vsie_page, scb_s->ipb >> 20) & mask; + src = _kvm_s390_real_to_abs(prefix, src) + mso; rc_dest = kvm_s390_shadow_fault(vcpu, vsie_page->gmap, dest, &pei_dest); rc_src = kvm_s390_shadow_fault(vcpu, vsie_page->gmap, src, &pei_src);
Prefixing needs to be applied to the guest real address to translate it into a guest absolute address. The value of MSO needs to be added to a guest-absolute address in order to obtain the host-virtual. Fixes: 223ea46de9e79 ("s390/kvm: VSIE: correctly handle MVPG when in VSIE") Cc: stable@vger.kernel.org Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reported-by: Janosch Frank <frankja@linux.ibm.com> --- arch/s390/kvm/vsie.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)