| Message ID | 20210319193354.399587-2-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/kvm: VSIE: fix prefixing and MSO for MVPG | expand |
On 19/03/2021 20.33, Claudio Imbrenda wrote: > A new function _kvm_s390_real_to_abs will apply prefixing to a real address > with a given prefix value. > > The old kvm_s390_real_to_abs becomes now a wrapper around the new function. > > This is needed to avoid code duplication in vSIE. > > Cc: stable@vger.kernel.org > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > arch/s390/kvm/gaccess.h | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h > index daba10f76936..7c72a5e3449f 100644 > --- a/arch/s390/kvm/gaccess.h > +++ b/arch/s390/kvm/gaccess.h > @@ -18,17 +18,14 @@ > > /** > * kvm_s390_real_to_abs - convert guest real address to guest absolute address > - * @vcpu - guest virtual cpu > + * @prefix - guest prefix > * @gra - guest real address > * > * Returns the guest absolute address that corresponds to the passed guest real > - * address @gra of a virtual guest cpu by applying its prefix. > + * address @gra of by applying the given prefix. > */ > -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, > - unsigned long gra) > +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) <bikeshedding> Just a matter of taste, but maybe this could be named differently? kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? </bikeshedding> Anyway: Reviewed-by: Thomas Huth <thuth@redhat.com>
On 20.03.21 05:57, Thomas Huth wrote: > On 19/03/2021 20.33, Claudio Imbrenda wrote: >> A new function _kvm_s390_real_to_abs will apply prefixing to a real address >> with a given prefix value. >> >> The old kvm_s390_real_to_abs becomes now a wrapper around the new function. >> >> This is needed to avoid code duplication in vSIE. >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >> --- >> arch/s390/kvm/gaccess.h | 23 +++++++++++++++++------ >> 1 file changed, 17 insertions(+), 6 deletions(-) >> >> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h >> index daba10f76936..7c72a5e3449f 100644 >> --- a/arch/s390/kvm/gaccess.h >> +++ b/arch/s390/kvm/gaccess.h >> @@ -18,17 +18,14 @@ >> >> /** >> * kvm_s390_real_to_abs - convert guest real address to guest absolute address >> - * @vcpu - guest virtual cpu >> + * @prefix - guest prefix >> * @gra - guest real address >> * >> * Returns the guest absolute address that corresponds to the passed guest real >> - * address @gra of a virtual guest cpu by applying its prefix. >> + * address @gra of by applying the given prefix. >> */ >> -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, >> - unsigned long gra) >> +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) > > <bikeshedding> > Just a matter of taste, but maybe this could be named differently? > kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? > </bikeshedding> +1, I also dislike these "_.*" style functions here. Reviewed-by: David Hildenbrand <david@redhat.com> > > Anyway: > Reviewed-by: Thomas Huth <thuth@redhat.com> >
On Mon, Mar 22, 2021 at 10:53:46AM +0100, David Hildenbrand wrote: > > > diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h > > > index daba10f76936..7c72a5e3449f 100644 > > > --- a/arch/s390/kvm/gaccess.h > > > +++ b/arch/s390/kvm/gaccess.h > > > @@ -18,17 +18,14 @@ > > > /** > > > * kvm_s390_real_to_abs - convert guest real address to guest absolute address > > > - * @vcpu - guest virtual cpu > > > + * @prefix - guest prefix > > > * @gra - guest real address > > > * > > > * Returns the guest absolute address that corresponds to the passed guest real > > > - * address @gra of a virtual guest cpu by applying its prefix. > > > + * address @gra of by applying the given prefix. > > > */ > > > -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, > > > - unsigned long gra) > > > +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) > > > > <bikeshedding> > > Just a matter of taste, but maybe this could be named differently? > > kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? > > </bikeshedding> > > +1, I also dislike these "_.*" style functions here. Yes, let's bikeshed then :) Could you then please try to rename page_to* and everything that looks similar to page2* please? I'm wondering what the response will be..
On 22.03.21 12:12, Heiko Carstens wrote: > On Mon, Mar 22, 2021 at 10:53:46AM +0100, David Hildenbrand wrote: >>>> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h >>>> index daba10f76936..7c72a5e3449f 100644 >>>> --- a/arch/s390/kvm/gaccess.h >>>> +++ b/arch/s390/kvm/gaccess.h >>>> @@ -18,17 +18,14 @@ >>>> /** >>>> * kvm_s390_real_to_abs - convert guest real address to guest absolute address >>>> - * @vcpu - guest virtual cpu >>>> + * @prefix - guest prefix >>>> * @gra - guest real address >>>> * >>>> * Returns the guest absolute address that corresponds to the passed guest real >>>> - * address @gra of a virtual guest cpu by applying its prefix. >>>> + * address @gra of by applying the given prefix. >>>> */ >>>> -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, >>>> - unsigned long gra) >>>> +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) >>> >>> <bikeshedding> >>> Just a matter of taste, but maybe this could be named differently? >>> kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? >>> </bikeshedding> >> >> +1, I also dislike these "_.*" style functions here. > > Yes, let's bikeshed then :) > > Could you then please try to rename page_to* and everything that looks > similar to page2* please? I'm wondering what the response will be.. Oh, we're bikeshedding about anything now? Cool.
On 22.03.21 12:16, David Hildenbrand wrote: > On 22.03.21 12:12, Heiko Carstens wrote: >> On Mon, Mar 22, 2021 at 10:53:46AM +0100, David Hildenbrand wrote: >>>>> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h >>>>> index daba10f76936..7c72a5e3449f 100644 >>>>> --- a/arch/s390/kvm/gaccess.h >>>>> +++ b/arch/s390/kvm/gaccess.h >>>>> @@ -18,17 +18,14 @@ >>>>> /** >>>>> * kvm_s390_real_to_abs - convert guest real address to guest absolute address >>>>> - * @vcpu - guest virtual cpu >>>>> + * @prefix - guest prefix >>>>> * @gra - guest real address >>>>> * >>>>> * Returns the guest absolute address that corresponds to the passed guest real >>>>> - * address @gra of a virtual guest cpu by applying its prefix. >>>>> + * address @gra of by applying the given prefix. >>>>> */ >>>>> -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, >>>>> - unsigned long gra) >>>>> +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) >>>> >>>> <bikeshedding> >>>> Just a matter of taste, but maybe this could be named differently? >>>> kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? >>>> </bikeshedding> >>> >>> +1, I also dislike these "_.*" style functions here. >> >> Yes, let's bikeshed then :) >> >> Could you then please try to rename page_to* and everything that looks >> similar to page2* please? I'm wondering what the response will be.. > > Oh, we're bikeshedding about anything now? Cool. (I agree that real2abs is not such a good idea ;) )
On 22.03.21 12:12, Heiko Carstens wrote: > On Mon, Mar 22, 2021 at 10:53:46AM +0100, David Hildenbrand wrote: >>>> diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h >>>> index daba10f76936..7c72a5e3449f 100644 >>>> --- a/arch/s390/kvm/gaccess.h >>>> +++ b/arch/s390/kvm/gaccess.h >>>> @@ -18,17 +18,14 @@ >>>> /** >>>> * kvm_s390_real_to_abs - convert guest real address to guest absolute address >>>> - * @vcpu - guest virtual cpu >>>> + * @prefix - guest prefix >>>> * @gra - guest real address >>>> * >>>> * Returns the guest absolute address that corresponds to the passed guest real >>>> - * address @gra of a virtual guest cpu by applying its prefix. >>>> + * address @gra of by applying the given prefix. >>>> */ >>>> -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, >>>> - unsigned long gra) >>>> +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) >>> >>> <bikeshedding> >>> Just a matter of taste, but maybe this could be named differently? >>> kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? >>> </bikeshedding> >> >> +1, I also dislike these "_.*" style functions here. > > Yes, let's bikeshed then :) > > Could you then please try to rename page_to* and everything that looks > similar to page2* please? I'm wondering what the response will be.. Given that this is stable material (due to patch 2), can we try to minimize the bikeshedding to everything that his touched by this patch? Claudio, can you respin the series addressing the comments? I will then either add this to next or fold that into the existing next patches. Not sure yet.
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h index daba10f76936..7c72a5e3449f 100644 --- a/arch/s390/kvm/gaccess.h +++ b/arch/s390/kvm/gaccess.h @@ -18,17 +18,14 @@ /** * kvm_s390_real_to_abs - convert guest real address to guest absolute address - * @vcpu - guest virtual cpu + * @prefix - guest prefix * @gra - guest real address * * Returns the guest absolute address that corresponds to the passed guest real - * address @gra of a virtual guest cpu by applying its prefix. + * address @gra of by applying the given prefix. */ -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, - unsigned long gra) +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) { - unsigned long prefix = kvm_s390_get_prefix(vcpu); - if (gra < 2 * PAGE_SIZE) gra += prefix; else if (gra >= prefix && gra < prefix + 2 * PAGE_SIZE) @@ -36,6 +33,20 @@ static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, return gra; } +/** + * kvm_s390_real_to_abs - convert guest real address to guest absolute address + * @vcpu - guest virtual cpu + * @gra - guest real address + * + * Returns the guest absolute address that corresponds to the passed guest real + * address @gra of a virtual guest cpu by applying its prefix. + */ +static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, + unsigned long gra) +{ + return _kvm_s390_real_to_abs(kvm_s390_get_prefix(vcpu), gra); +} + /** * _kvm_s390_logical_to_effective - convert guest logical to effective address * @psw: psw of the guest
A new function _kvm_s390_real_to_abs will apply prefixing to a real address with a given prefix value. The old kvm_s390_real_to_abs becomes now a wrapper around the new function. This is needed to avoid code duplication in vSIE. Cc: stable@vger.kernel.org Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- arch/s390/kvm/gaccess.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)