* [PATCH v2 0/1] KVM: s390: vsie: clarifications on setting the APCB
@ 2022-10-25 9:13 Pierre Morel
2022-10-25 9:13 ` [PATCH v2 1/1] " Pierre Morel
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Morel @ 2022-10-25 9:13 UTC (permalink / raw)
To: kvm
Cc: linux-s390, linux-kernel, borntraeger, frankja, cohuck, david,
thuth, imbrenda, hca, gor, svens
Hi,
This is a v2 of the previously called:
[PATCH] KVM: s390: vsie: fix crycb virtual vs physical usage
After David's comment that the patch has not to do with
virtual vs physical, I reworked the commit message.
I resent the patch because I think that even the previous
version is not really buggy, this new version brings clarity.
Regards,
Pierre
Pierre Morel (1):
KVM: s390: vsie: clarifications on setting the APCB
arch/s390/kvm/vsie.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/1] KVM: s390: vsie: clarifications on setting the APCB
2022-10-25 9:13 [PATCH v2 0/1] KVM: s390: vsie: clarifications on setting the APCB Pierre Morel
@ 2022-10-25 9:13 ` Pierre Morel
2022-10-25 9:30 ` David Hildenbrand
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Morel @ 2022-10-25 9:13 UTC (permalink / raw)
To: kvm
Cc: linux-s390, linux-kernel, borntraeger, frankja, cohuck, david,
thuth, imbrenda, hca, gor, svens
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>
---
arch/s390/kvm/vsie.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 94138f8f0c1c..f37851c9b1ab 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -138,9 +138,12 @@ 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_o, struct kvm_s390_apcb1 *apcb_h)
{
struct kvm_s390_apcb0 tmp;
+ unsigned long apcb_o;
+
+ apcb_o = crycb_o + offsetof(struct kvm_s390_crypto_cb, apcb0);
if (read_guest_real(vcpu, apcb_o, &tmp, sizeof(struct kvm_s390_apcb0)))
return -EFAULT;
@@ -157,14 +160,18 @@ 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_o: real guest 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_o, unsigned long *apcb_h)
{
+ unsigned long apcb_o;
+
+ apcb_o = crycb_o + offsetof(struct kvm_s390_crypto_cb, apcb0);
+
if (read_guest_real(vcpu, apcb_o, apcb_s,
sizeof(struct kvm_s390_apcb0)))
return -EFAULT;
@@ -178,15 +185,19 @@ 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_o: real guest 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_o,
unsigned long *apcb_h)
{
+ unsigned long apcb_o;
+
+ apcb_o = crycb_o + offsetof(struct kvm_s390_crypto_cb, apcb1);
+
if (read_guest_real(vcpu, apcb_o, apcb_s,
sizeof(struct kvm_s390_apcb1)))
return -EFAULT;
@@ -200,7 +211,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_o: real 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.
@@ -215,10 +226,6 @@ static int setup_apcb(struct kvm_vcpu *vcpu, struct kvm_s390_crypto_cb *crycb_s,
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))
@@ -226,18 +233,18 @@ static int setup_apcb(struct kvm_vcpu *vcpu, struct kvm_s390_crypto_cb *crycb_s,
if (fmt_h != CRYCB_FORMAT2)
return -EINVAL;
return setup_apcb11(vcpu, (unsigned long *)&crycb_s->apcb1,
- (unsigned long) &crycb->apcb1,
+ crycb_o,
(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_o,
&crycb_h->apcb1);
case CRYCB_FORMAT1:
return setup_apcb00(vcpu,
(unsigned long *) &crycb_s->apcb0,
- (unsigned long) &crycb->apcb0,
+ crycb_o,
(unsigned long *) &crycb_h->apcb0);
}
break;
@@ -248,13 +255,13 @@ static int setup_apcb(struct kvm_vcpu *vcpu, struct kvm_s390_crypto_cb *crycb_s,
switch (fmt_h) {
case CRYCB_FORMAT2:
return setup_apcb10(vcpu, &crycb_s->apcb1,
- (unsigned long) &crycb->apcb0,
+ crycb_o,
&crycb_h->apcb1);
case CRYCB_FORMAT1:
case CRYCB_FORMAT0:
return setup_apcb00(vcpu,
(unsigned long *) &crycb_s->apcb0,
- (unsigned long) &crycb->apcb0,
+ crycb_o,
(unsigned long *) &crycb_h->apcb0);
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] KVM: s390: vsie: clarifications on setting the APCB
2022-10-25 9:13 ` [PATCH v2 1/1] " Pierre Morel
@ 2022-10-25 9:30 ` David Hildenbrand
2022-10-25 13:17 ` Pierre Morel
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2022-10-25 9:30 UTC (permalink / raw)
To: Pierre Morel, kvm
Cc: linux-s390, linux-kernel, borntraeger, frankja, cohuck, thuth,
imbrenda, hca, gor, svens
On 25.10.22 11:13, 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.
>
While at it, can we rename "crycb_o" to "crycb_gpa" and "apcb_o" to
"apcb_gpa".
These are not pointers but guest physical addresses.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] KVM: s390: vsie: clarifications on setting the APCB
2022-10-25 9:30 ` David Hildenbrand
@ 2022-10-25 13:17 ` Pierre Morel
2022-10-25 16:20 ` Claudio Imbrenda
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Morel @ 2022-10-25 13:17 UTC (permalink / raw)
To: David Hildenbrand, kvm
Cc: linux-s390, linux-kernel, borntraeger, frankja, cohuck, thuth,
imbrenda, hca, gor, svens
On 10/25/22 11:30, David Hildenbrand wrote:
> On 25.10.22 11:13, 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.
>>
>
>
> While at it, can we rename "crycb_o" to "crycb_gpa" and "apcb_o" to
> "apcb_gpa".
>
> These are not pointers but guest physical addresses.
>
I can do that.
the _o came from the name in the documentation "origin"
but gpa is more obvious.
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] KVM: s390: vsie: clarifications on setting the APCB
2022-10-25 13:17 ` Pierre Morel
@ 2022-10-25 16:20 ` Claudio Imbrenda
2022-10-26 8:18 ` Pierre Morel
0 siblings, 1 reply; 6+ messages in thread
From: Claudio Imbrenda @ 2022-10-25 16:20 UTC (permalink / raw)
To: Pierre Morel
Cc: David Hildenbrand, kvm, linux-s390, linux-kernel, borntraeger,
frankja, cohuck, thuth, hca, gor, svens
On Tue, 25 Oct 2022 15:17:36 +0200
Pierre Morel <pmorel@linux.ibm.com> wrote:
> On 10/25/22 11:30, David Hildenbrand wrote:
> > On 25.10.22 11:13, 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.
> >>
> >
> >
> > While at it, can we rename "crycb_o" to "crycb_gpa" and "apcb_o" to
> > "apcb_gpa".
> >
> > These are not pointers but guest physical addresses.
> >
>
> I can do that.
> the _o came from the name in the documentation "origin"
> but gpa is more obvious.
>
with that fixed:
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] KVM: s390: vsie: clarifications on setting the APCB
2022-10-25 16:20 ` Claudio Imbrenda
@ 2022-10-26 8:18 ` Pierre Morel
0 siblings, 0 replies; 6+ messages in thread
From: Pierre Morel @ 2022-10-26 8:18 UTC (permalink / raw)
To: Claudio Imbrenda
Cc: David Hildenbrand, kvm, linux-s390, linux-kernel, borntraeger,
frankja, cohuck, thuth, hca, gor, svens
On 10/25/22 18:20, Claudio Imbrenda wrote:
> On Tue, 25 Oct 2022 15:17:36 +0200
> Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> On 10/25/22 11:30, David Hildenbrand wrote:
>>> On 25.10.22 11:13, 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.
>>>>
>>>
>>>
>>> While at it, can we rename "crycb_o" to "crycb_gpa" and "apcb_o" to
>>> "apcb_gpa".
>>>
>>> These are not pointers but guest physical addresses.
>>>
>>
>> I can do that.
>> the _o came from the name in the documentation "origin"
>> but gpa is more obvious.
>>
>
> with that fixed:
>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-26 8:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-25 9:13 [PATCH v2 0/1] KVM: s390: vsie: clarifications on setting the APCB Pierre Morel
2022-10-25 9:13 ` [PATCH v2 1/1] " Pierre Morel
2022-10-25 9:30 ` David Hildenbrand
2022-10-25 13:17 ` Pierre Morel
2022-10-25 16:20 ` Claudio Imbrenda
2022-10-26 8:18 ` Pierre Morel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).