| Message ID | 7c2590c4d8cc95cd40bbb05c0d0c5e2b0735a16b.1560145715.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2] KVM: arm64: Implement vq_present() as a macro | expand |
On Mon, Jun 10, 2019 at 11:36:33AM +0530, Viresh Kumar wrote: > This routine is a one-liner and doesn't really need to be function and > should be rather implemented as a macro. > > Suggested-by: Dave Martin <Dave.Martin@arm.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > V1->V2: > - The previous implementation was fixing a compilation error that > occurred only with old compilers (from 2015) due to a bug in the > compiler itself. > > - Dave suggested to rather implement this as a macro which made more > sense. > > arch/arm64/kvm/guest.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 3ae2f82fca46..a429ed36a6a0 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -207,13 +207,7 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) > > #define vq_word(vq) (((vq) - SVE_VQ_MIN) / 64) > #define vq_mask(vq) ((u64)1 << ((vq) - SVE_VQ_MIN) % 64) > - > -static bool vq_present( > - const u64 (*const vqs)[KVM_ARM64_SVE_VLS_WORDS], > - unsigned int vq) > -{ > - return (*vqs)[vq_word(vq)] & vq_mask(vq); > -} > +#define vq_present(vqs, vq) ((*(vqs))[vq_word(vq)] & vq_mask(vq)) You could drop the extra level of indirection on vqs now. The only thing it achieves is to enforce the size of the array via type- checkout, but the macro can't easily do that (unless you can think of another way to do it). Otherwise, looks good. Cheers ---Dave
On 10-06-19, 10:09, Dave Martin wrote: > You could drop the extra level of indirection on vqs now. The only > thing it achieves is to enforce the size of the array via type- > checkout, but the macro can't easily do that (unless you can think > of another way to do it). > > Otherwise, looks good. Below is what I wrote initially this morning and then moved to the current version as I wasn't sure if you would want that :)
On Mon, Jun 10, 2019 at 03:20:30PM +0530, Viresh Kumar wrote: > On 10-06-19, 10:09, Dave Martin wrote: > > You could drop the extra level of indirection on vqs now. The only > > thing it achieves is to enforce the size of the array via type- > > checkout, but the macro can't easily do that (unless you can think > > of another way to do it). > > > > Otherwise, looks good. > > Below is what I wrote initially this morning and then moved to the > current version as I wasn't sure if you would want that :) > > -- > viresh > > -------------------------8<------------------------- > > From be823e68faffc82a6f621c16ce1bd45990d92791 Mon Sep 17 00:00:00 2001 > Message-Id: <be823e68faffc82a6f621c16ce1bd45990d92791.1560160165.git.viresh.kumar@linaro.org> > From: Viresh Kumar <viresh.kumar@linaro.org> > Date: Mon, 10 Jun 2019 11:15:17 +0530 > Subject: [PATCH] KVM: arm64: Implement vq_present() as a macro > > This routine is a one-liner and doesn't really need to be function and > can be implemented as a macro. > > Suggested-by: Dave Martin <Dave.Martin@arm.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > arch/arm64/kvm/guest.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 3ae2f82fca46..ae734fcfd4ea 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -207,13 +207,7 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) > > #define vq_word(vq) (((vq) - SVE_VQ_MIN) / 64) > #define vq_mask(vq) ((u64)1 << ((vq) - SVE_VQ_MIN) % 64) > - > -static bool vq_present( > - const u64 (*const vqs)[KVM_ARM64_SVE_VLS_WORDS], > - unsigned int vq) > -{ > - return (*vqs)[vq_word(vq)] & vq_mask(vq); > -} > +#define vq_present(vqs, vq) ((vqs)[vq_word(vq)] & vq_mask(vq)) > > static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) > { > @@ -258,7 +252,7 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) > > max_vq = 0; > for (vq = SVE_VQ_MIN; vq <= SVE_VQ_MAX; ++vq) > - if (vq_present(&vqs, vq)) > + if (vq_present(vqs, vq)) > max_vq = vq; > > if (max_vq > sve_vq_from_vl(kvm_sve_max_vl)) > @@ -272,7 +266,7 @@ static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) > * maximum: > */ > for (vq = SVE_VQ_MIN; vq <= max_vq; ++vq) > - if (vq_present(&vqs, vq) != sve_vq_available(vq)) > + if (vq_present(vqs, vq) != sve_vq_available(vq)) > return -EINVAL; I think I prefer this version: Reviewed-by: Dave Martin <Dave.Martin@arm.com> Cheers ---Dave
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 3ae2f82fca46..a429ed36a6a0 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -207,13 +207,7 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) #define vq_word(vq) (((vq) - SVE_VQ_MIN) / 64) #define vq_mask(vq) ((u64)1 << ((vq) - SVE_VQ_MIN) % 64) - -static bool vq_present( - const u64 (*const vqs)[KVM_ARM64_SVE_VLS_WORDS], - unsigned int vq) -{ - return (*vqs)[vq_word(vq)] & vq_mask(vq); -} +#define vq_present(vqs, vq) ((*(vqs))[vq_word(vq)] & vq_mask(vq)) static int get_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) {
This routine is a one-liner and doesn't really need to be function and should be rather implemented as a macro. Suggested-by: Dave Martin <Dave.Martin@arm.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V1->V2: - The previous implementation was fixing a compilation error that occurred only with old compilers (from 2015) due to a bug in the compiler itself. - Dave suggested to rather implement this as a macro which made more sense. arch/arm64/kvm/guest.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)