Messages in this thread | | | Date | Thu, 16 Feb 2023 14:41:22 +0100 | Subject | Re: [PATCH v3 1/1] KVM: s390: vsie: clarifications on setting the APCB | From | Janosch Frank <> |
| |
On 2/14/23 13:28, Pierre Morel wrote: > The APCB is part of the CRYCB. > The calculation of the APCB origin can be done by adding > the APCB offset to the CRYCB origin. > > Current code makes confusing transformations, converting > the CRYCB origin to a pointer to calculate the APCB origin. > > Let's make things simpler and keep the CRYCB origin to make > these calculations. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
LGTM: Acked-by: Janosch Frank <frankja@linux.ibm.com>
> --- > arch/s390/kvm/vsie.c | 50 +++++++++++++++++++++++++------------------- > 1 file changed, 29 insertions(+), 21 deletions(-) > > diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c > index b6a0219e470a..8d6b765abf29 100644 > --- a/arch/s390/kvm/vsie.c > +++ b/arch/s390/kvm/vsie.c > @@ -138,11 +138,15 @@ static int prepare_cpuflags(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) > } > /* Copy to APCB FORMAT1 from APCB FORMAT0 */ > static int setup_apcb10(struct kvm_vcpu *vcpu, struct kvm_s390_apcb1 *apcb_s, > - unsigned long apcb_o, struct kvm_s390_apcb1 *apcb_h) > + unsigned long crycb_gpa, struct kvm_s390_apcb1 *apcb_h) > { > struct kvm_s390_apcb0 tmp; > + unsigned long apcb_gpa; > > - if (read_guest_real(vcpu, apcb_o, &tmp, sizeof(struct kvm_s390_apcb0))) > + apcb_gpa = crycb_gpa + offsetof(struct kvm_s390_crypto_cb, apcb0); > + > + if (read_guest_real(vcpu, apcb_gpa, &tmp, > + sizeof(struct kvm_s390_apcb0))) > return -EFAULT; > > apcb_s->apm[0] = apcb_h->apm[0] & tmp.apm[0]; > @@ -157,15 +161,19 @@ static int setup_apcb10(struct kvm_vcpu *vcpu, struct kvm_s390_apcb1 *apcb_s, > * setup_apcb00 - Copy to APCB FORMAT0 from APCB FORMAT0 > * @vcpu: pointer to the virtual CPU > * @apcb_s: pointer to start of apcb in the shadow crycb > - * @apcb_o: pointer to start of original apcb in the guest2 > + * @crycb_gpa: guest physical address to start of original guest crycb > * @apcb_h: pointer to start of apcb in the guest1 > * > * Returns 0 and -EFAULT on error reading guest apcb > */ > static int setup_apcb00(struct kvm_vcpu *vcpu, unsigned long *apcb_s, > - unsigned long apcb_o, unsigned long *apcb_h) > + unsigned long crycb_gpa, unsigned long *apcb_h) > { > - if (read_guest_real(vcpu, apcb_o, apcb_s, > + unsigned long apcb_gpa; > + > + apcb_gpa = crycb_gpa + offsetof(struct kvm_s390_crypto_cb, apcb0); > + > + if (read_guest_real(vcpu, apcb_gpa, apcb_s, > sizeof(struct kvm_s390_apcb0))) > return -EFAULT; > > @@ -178,16 +186,20 @@ static int setup_apcb00(struct kvm_vcpu *vcpu, unsigned long *apcb_s, > * setup_apcb11 - Copy the FORMAT1 APCB from the guest to the shadow CRYCB > * @vcpu: pointer to the virtual CPU > * @apcb_s: pointer to start of apcb in the shadow crycb > - * @apcb_o: pointer to start of original guest apcb > + * @crycb_gpa: guest physical address to start of original guest crycb > * @apcb_h: pointer to start of apcb in the host > * > * Returns 0 and -EFAULT on error reading guest apcb > */ > static int setup_apcb11(struct kvm_vcpu *vcpu, unsigned long *apcb_s, > - unsigned long apcb_o, > + unsigned long crycb_gpa, > unsigned long *apcb_h) > { > - if (read_guest_real(vcpu, apcb_o, apcb_s, > + unsigned long apcb_gpa; > + > + apcb_gpa = crycb_gpa + offsetof(struct kvm_s390_crypto_cb, apcb1); > + > + if (read_guest_real(vcpu, apcb_gpa, apcb_s, > sizeof(struct kvm_s390_apcb1))) > return -EFAULT; > > @@ -200,7 +212,7 @@ static int setup_apcb11(struct kvm_vcpu *vcpu, unsigned long *apcb_s, > * setup_apcb - Create a shadow copy of the apcb. > * @vcpu: pointer to the virtual CPU > * @crycb_s: pointer to shadow crycb > - * @crycb_o: pointer to original guest crycb > + * @crycb_gpa: guest physical address of original guest crycb > * @crycb_h: pointer to the host crycb > * @fmt_o: format of the original guest crycb. > * @fmt_h: format of the host crycb. > @@ -211,50 +223,46 @@ static int setup_apcb11(struct kvm_vcpu *vcpu, unsigned long *apcb_s, > * Return 0 or an error number if the guest and host crycb are incompatible. > */ > static int setup_apcb(struct kvm_vcpu *vcpu, struct kvm_s390_crypto_cb *crycb_s, > - const u32 crycb_o, > + const u32 crycb_gpa, > struct kvm_s390_crypto_cb *crycb_h, > int fmt_o, int fmt_h) > { > - struct kvm_s390_crypto_cb *crycb; > - > - crycb = (struct kvm_s390_crypto_cb *) (unsigned long)crycb_o; > - > switch (fmt_o) { > case CRYCB_FORMAT2: > - if ((crycb_o & PAGE_MASK) != ((crycb_o + 256) & PAGE_MASK)) > + if ((crycb_gpa & PAGE_MASK) != ((crycb_gpa + 256) & PAGE_MASK)) > return -EACCES; > if (fmt_h != CRYCB_FORMAT2) > return -EINVAL; > return setup_apcb11(vcpu, (unsigned long *)&crycb_s->apcb1, > - (unsigned long) &crycb->apcb1, > + crycb_gpa, > (unsigned long *)&crycb_h->apcb1); > case CRYCB_FORMAT1: > switch (fmt_h) { > case CRYCB_FORMAT2: > return setup_apcb10(vcpu, &crycb_s->apcb1, > - (unsigned long) &crycb->apcb0, > + crycb_gpa, > &crycb_h->apcb1); > case CRYCB_FORMAT1: > return setup_apcb00(vcpu, > (unsigned long *) &crycb_s->apcb0, > - (unsigned long) &crycb->apcb0, > + crycb_gpa, > (unsigned long *) &crycb_h->apcb0); > } > break; > case CRYCB_FORMAT0: > - if ((crycb_o & PAGE_MASK) != ((crycb_o + 32) & PAGE_MASK)) > + if ((crycb_gpa & PAGE_MASK) != ((crycb_gpa + 32) & PAGE_MASK)) > return -EACCES; > > switch (fmt_h) { > case CRYCB_FORMAT2: > return setup_apcb10(vcpu, &crycb_s->apcb1, > - (unsigned long) &crycb->apcb0, > + crycb_gpa, > &crycb_h->apcb1); > case CRYCB_FORMAT1: > case CRYCB_FORMAT0: > return setup_apcb00(vcpu, > (unsigned long *) &crycb_s->apcb0, > - (unsigned long) &crycb->apcb0, > + crycb_gpa, > (unsigned long *) &crycb_h->apcb0); > } > }
| |