diff mbox

[v4,3/4] arm64: Use __tlbi() macros in KVM code

Message ID 20170125155232.10277-3-cov@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Christopher Covington Jan. 25, 2017, 3:52 p.m. UTC
Refactor the KVM code to use the __tlbi macros, which will allow an errata
workaround that repeats tlbi dsb sequences to only change one location.
This is not intended to change the generated assembly and comparing before
and after vmlinux objdump shows no functional changes.

Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
 arch/arm64/kvm/hyp/tlb.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Christoffer Dall Jan. 25, 2017, 7:39 p.m. UTC | #1
On Wed, Jan 25, 2017 at 10:52:31AM -0500, Christopher Covington wrote:
> Refactor the KVM code to use the __tlbi macros, which will allow an errata
> workaround that repeats tlbi dsb sequences to only change one location.
> This is not intended to change the generated assembly and comparing before
> and after vmlinux objdump shows no functional changes.
> 
> Signed-off-by: Christopher Covington <cov@codeaurora.org>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
Will Deacon Jan. 27, 2017, 1:53 p.m. UTC | #2
On Wed, Jan 25, 2017 at 08:39:43PM +0100, Christoffer Dall wrote:
> On Wed, Jan 25, 2017 at 10:52:31AM -0500, Christopher Covington wrote:
> > Refactor the KVM code to use the __tlbi macros, which will allow an errata
> > workaround that repeats tlbi dsb sequences to only change one location.
> > This is not intended to change the generated assembly and comparing before
> > and after vmlinux objdump shows no functional changes.
> > 
> > Signed-off-by: Christopher Covington <cov@codeaurora.org>
> 
> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

Thanks, I'll queue this one via arm64.

Will
Will Deacon Jan. 27, 2017, 3:03 p.m. UTC | #3
On Wed, Jan 25, 2017 at 10:52:31AM -0500, Christopher Covington wrote:
> Refactor the KVM code to use the __tlbi macros, which will allow an errata
> workaround that repeats tlbi dsb sequences to only change one location.
> This is not intended to change the generated assembly and comparing before
> and after vmlinux objdump shows no functional changes.
> 
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  arch/arm64/kvm/hyp/tlb.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

[...]

> @@ -82,7 +83,7 @@ void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
>  void __hyp_text __kvm_flush_vm_context(void)
>  {
>  	dsb(ishst);
> -	asm volatile("tlbi alle1is	\n"
> -		     "ic ialluis	  ": : );
> +	__tlbi(alle1is);
> +	asm volatile("ic ialluis" : : );
>  	dsb(ish);

Should be a separate patch, but this can now be a __flush_icache_all instead
of the open-coded asm.

Will
Punit Agrawal Feb. 1, 2017, 5:02 p.m. UTC | #4
Will Deacon <will.deacon@arm.com> writes:

> On Wed, Jan 25, 2017 at 08:39:43PM +0100, Christoffer Dall wrote:
>> On Wed, Jan 25, 2017 at 10:52:31AM -0500, Christopher Covington wrote:
>> > Refactor the KVM code to use the __tlbi macros, which will allow an errata
>> > workaround that repeats tlbi dsb sequences to only change one location.
>> > This is not intended to change the generated assembly and comparing before
>> > and after vmlinux objdump shows no functional changes.
>> > 
>> > Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> 
>> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>
> Thanks, I'll queue this one via arm64.

I just noticed this patch but there's been a similar patch from Mark
that I've been carrying as part of the KVM TLB monitoring series[0].

[0] http://www.mail-archive.com/kvmarm@lists.cs.columbia.edu/msg09359.html

>
> Will
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Will Deacon Feb. 1, 2017, 5:08 p.m. UTC | #5
On Wed, Feb 01, 2017 at 05:02:43PM +0000, Punit Agrawal wrote:
> Will Deacon <will.deacon@arm.com> writes:
> 
> > On Wed, Jan 25, 2017 at 08:39:43PM +0100, Christoffer Dall wrote:
> >> On Wed, Jan 25, 2017 at 10:52:31AM -0500, Christopher Covington wrote:
> >> > Refactor the KVM code to use the __tlbi macros, which will allow an errata
> >> > workaround that repeats tlbi dsb sequences to only change one location.
> >> > This is not intended to change the generated assembly and comparing before
> >> > and after vmlinux objdump shows no functional changes.
> >> > 
> >> > Signed-off-by: Christopher Covington <cov@codeaurora.org>
> >> 
> >> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
> >
> > Thanks, I'll queue this one via arm64.
> 
> I just noticed this patch but there's been a similar patch from Mark
> that I've been carrying as part of the KVM TLB monitoring series[0].
> 
> [0] http://www.mail-archive.com/kvmarm@lists.cs.columbia.edu/msg09359.html

Well I've already queued the one from Christopher. It's weird that Mark's
version appears to miss the local VMID case (but it does have the
flush_icache_all I wanted :).

Anyway, I'm not reverting anything, so you'll need to rebase when this
lot lands in mainline.

Will
Punit Agrawal Feb. 1, 2017, 5:14 p.m. UTC | #6
Will Deacon <will.deacon@arm.com> writes:

> On Wed, Feb 01, 2017 at 05:02:43PM +0000, Punit Agrawal wrote:
>> Will Deacon <will.deacon@arm.com> writes:
>> 
>> > On Wed, Jan 25, 2017 at 08:39:43PM +0100, Christoffer Dall wrote:
>> >> On Wed, Jan 25, 2017 at 10:52:31AM -0500, Christopher Covington wrote:
>> >> > Refactor the KVM code to use the __tlbi macros, which will allow an errata
>> >> > workaround that repeats tlbi dsb sequences to only change one location.
>> >> > This is not intended to change the generated assembly and comparing before
>> >> > and after vmlinux objdump shows no functional changes.
>> >> > 
>> >> > Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> >> 
>> >> Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
>> >
>> > Thanks, I'll queue this one via arm64.
>> 
>> I just noticed this patch but there's been a similar patch from Mark
>> that I've been carrying as part of the KVM TLB monitoring series[0].
>> 
>> [0] http://www.mail-archive.com/kvmarm@lists.cs.columbia.edu/msg09359.html
>
> Well I've already queued the one from Christopher. It's weird that Mark's
> version appears to miss the local VMID case (but it does have the
> flush_icache_all I wanted :).
>
> Anyway, I'm not reverting anything, so you'll need to rebase when this
> lot lands in mainline.

Sure. I'll drop the patch the next time around.

The patch is unrelated to that series and should've gone in when
we introduced the __tlbi() macro but got left out somehow.

>
> Will
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
diff mbox

Patch

diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
index 88e2f2b938f0..e8e7ba2bc11f 100644
--- a/arch/arm64/kvm/hyp/tlb.c
+++ b/arch/arm64/kvm/hyp/tlb.c
@@ -16,6 +16,7 @@ 
  */
 
 #include <asm/kvm_hyp.h>
+#include <asm/tlbflush.h>
 
 void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 {
@@ -32,7 +33,7 @@  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	 * whole of Stage-1. Weep...
 	 */
 	ipa >>= 12;
-	asm volatile("tlbi ipas2e1is, %0" : : "r" (ipa));
+	__tlbi(ipas2e1is, ipa);
 
 	/*
 	 * We have to ensure completion of the invalidation at Stage-2,
@@ -41,7 +42,7 @@  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
 	 * the Stage-1 invalidation happened first.
 	 */
 	dsb(ish);
-	asm volatile("tlbi vmalle1is" : : );
+	__tlbi(vmalle1is);
 	dsb(ish);
 	isb();
 
@@ -57,7 +58,7 @@  void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
 	write_sysreg(kvm->arch.vttbr, vttbr_el2);
 	isb();
 
-	asm volatile("tlbi vmalls12e1is" : : );
+	__tlbi(vmalls12e1is);
 	dsb(ish);
 	isb();
 
@@ -72,7 +73,7 @@  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
 	write_sysreg(kvm->arch.vttbr, vttbr_el2);
 	isb();
 
-	asm volatile("tlbi vmalle1" : : );
+	__tlbi(vmalle1);
 	dsb(nsh);
 	isb();
 
@@ -82,7 +83,7 @@  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
 void __hyp_text __kvm_flush_vm_context(void)
 {
 	dsb(ishst);
-	asm volatile("tlbi alle1is	\n"
-		     "ic ialluis	  ": : );
+	__tlbi(alle1is);
+	asm volatile("ic ialluis" : : );
 	dsb(ish);
 }