* [PATCH V3] KVM: arm64: Implement vq_present() as a macro @ 2019-06-10 10:00 Viresh Kumar 2019-07-03 12:04 ` Zhang, Lei 0 siblings, 1 reply; 3+ messages in thread From: Viresh Kumar @ 2019-06-10 10:00 UTC (permalink / raw) To: Marc Zyngier, James Morse, Julien Thierry, Suzuki K Pouloze, Catalin Marinas, Will Deacon Cc: Vincent Guittot, Viresh Kumar, linux-kernel, kvmarm, Dave Martin, linux-arm-kernel 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> Reviewed-by: Dave Martin <Dave.Martin@arm.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- V2->V3: - Pass "vqs" instead of "&vqs" to vq_present(). - Added Reviewed-by from Dave. 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 | 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; /* Can't run with no vector lengths at all: */ -- 2.21.0.rc0.269.g1a574e7a288b _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH V3] KVM: arm64: Implement vq_present() as a macro 2019-06-10 10:00 [PATCH V3] KVM: arm64: Implement vq_present() as a macro Viresh Kumar @ 2019-07-03 12:04 ` Zhang, Lei 2019-07-03 17:46 ` Dave Martin 0 siblings, 1 reply; 3+ messages in thread From: Zhang, Lei @ 2019-07-03 12:04 UTC (permalink / raw) To: 'Viresh Kumar', Marc Zyngier, James Morse, Julien Thierry, Suzuki K Pouloze, Catalin Marinas, Will Deacon Cc: Okamoto, Takayuki, linux-kernel, kvmarm, Mizuma, Masayoshi, Zhang, Lei, Dave Martin, linux-arm-kernel, Indoh, Takao Hi guys, I can't start up KVM guest os with SVE feature with your patch. The error message is qemu-system-aarch64: kvm_init_vcpu failed: Invalid argument. My test enviroment. kernel linux-5.2-rc6 qemu [Qemu-devel] [PATCH v2 00/14] target/arm/kvm: enable SVE in guests https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04945.html KVM start up option -machine virt,gic-version=host,accel=kvm \ -cpu host \ -machine type=virt \ -nographic \ -smp 16 \ -m 4096 \ -drive if=none,file=/root/image.qcow2,id=hd0,format=qcow2 \ -device virtio-blk-device,drive=hd0 \ -netdev user,id=mynet0,restrict=off,hostfwd=tcp::38001-:22 \ -device virtio-net-device,netdev=mynet0 \ -bios /root/QEMU_EFI.fd sve_vq_available function's return value' type is bool. But vq_present is macro, so the value is not only TRUE, FALSE but also some numbers. So It failed at if (vq_present(vqs, vq) != sve_vq_available(vq)). I think it is nessary to make vq_present macro's value only TRUE and FALSE. arch/arm64/kvm/guest.c static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) for (vq = SVE_VQ_MIN; vq <= max_vq; ++vq) if (vq_present(vqs, vq) != sve_vq_available(vq)) // It failed at here. return -EINVAL; My patch as follows. I have started up KVM guest os successfully with SVE feature with this patch. Could you review and merge my patch? Signed-off-by: Zhang Lei <zhang.lei@jp.fujitsu.com> --- arch/arm64/kvm/guest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index c2afa79..dfd6264 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -208,7 +208,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) -#define vq_present(vqs, vq) ((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) { -- 1.8.3.1 > -----Original Message----- > From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On > Behalf Of Viresh Kumar > Sent: Monday, June 10, 2019 7:00 PM > To: Marc Zyngier <marc.zyngier@arm.com>; James Morse > <james.morse@arm.com>; Julien Thierry <julien.thierry@arm.com>; Suzuki K > Pouloze <suzuki.poulose@arm.com>; Catalin Marinas > <catalin.marinas@arm.com>; Will Deacon <will.deacon@arm.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org>; linux-kernel@vger.kernel.org; > kvmarm@lists.cs.columbia.edu; Dave Martin <Dave.Martin@arm.com>; > linux-arm-kernel@lists.infradead.org > Subject: [PATCH V3] 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> > Reviewed-by: Dave Martin <Dave.Martin@arm.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > V2->V3: > - Pass "vqs" instead of "&vqs" to vq_present(). > - Added Reviewed-by from Dave. > > 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 | 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; > > /* Can't run with no vector lengths at all: */ > -- > 2.21.0.rc0.269.g1a574e7a288b > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH V3] KVM: arm64: Implement vq_present() as a macro 2019-07-03 12:04 ` Zhang, Lei @ 2019-07-03 17:46 ` Dave Martin 0 siblings, 0 replies; 3+ messages in thread From: Dave Martin @ 2019-07-03 17:46 UTC (permalink / raw) To: Zhang, Lei Cc: Okamoto, Takayuki, Marc Zyngier, 'Viresh Kumar', Will Deacon, linux-kernel, Catalin Marinas, Mizuma, Masayoshi, kvmarm, linux-arm-kernel, Indoh, Takao On Wed, Jul 03, 2019 at 12:04:11PM +0000, Zhang, Lei wrote: > Hi guys, > > I can't start up KVM guest os with SVE feature with your patch. > The error message is > qemu-system-aarch64: kvm_init_vcpu failed: Invalid argument. > > My test enviroment. > kernel linux-5.2-rc6 > qemu [Qemu-devel] [PATCH v2 00/14] target/arm/kvm: enable SVE in guests https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04945.html > KVM start up option > -machine virt,gic-version=host,accel=kvm \ > -cpu host \ > -machine type=virt \ > -nographic \ > -smp 16 \ -m 4096 \ > -drive if=none,file=/root/image.qcow2,id=hd0,format=qcow2 \ > -device virtio-blk-device,drive=hd0 \ > -netdev user,id=mynet0,restrict=off,hostfwd=tcp::38001-:22 \ > -device virtio-net-device,netdev=mynet0 \ > -bios /root/QEMU_EFI.fd > > sve_vq_available function's return value' type is bool. > But vq_present is macro, so the value is not only TRUE, FALSE but also some numbers. > So It failed at > if (vq_present(vqs, vq) != sve_vq_available(vq)). > I think it is nessary to make vq_present macro's value only TRUE and FALSE. > > arch/arm64/kvm/guest.c > static int set_sve_vls(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) > for (vq = SVE_VQ_MIN; vq <= max_vq; ++vq) > if (vq_present(vqs, vq) != sve_vq_available(vq)) // It failed at here. > return -EINVAL; > > My patch as follows. > I have started up KVM guest os successfully with SVE feature with this patch. > > Could you review and merge my patch? [...] Thanks for reporting this! It looks like we didn't realise we dropped the implicit cast to bool when the result was returned from the original version of vq_present(). Your fix looks sensible to me. For the future, see Documentation/process/submitting-patches.rst for guidance on how to prepare a patch for submission. However, due to the fact that we're already at -rc7 I've written a commit message for the patch and reposted [1]. Since the fix is yours, I'll keep your authorship and S-o-B. Please retest when you can (though the diff should be the same). Note, your mail seems to be corrupted, but since the diff is a one-line fix, I'm pretty confident I decoded it correctly. If anything looks wrong, please let me know. [...] Cheers ---Dave [1] [PATCH] KVM: arm64/sve: Fix vq_present() macro to yield a bool http://lists.infradead.org/pipermail/linux-arm-kernel/2019-July/664745.html _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-07-03 17:46 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-10 10:00 [PATCH V3] KVM: arm64: Implement vq_present() as a macro Viresh Kumar 2019-07-03 12:04 ` Zhang, Lei 2019-07-03 17:46 ` Dave Martin
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).