diff mbox

[2/5] ARM: KVM: add missing dsb before invalidating Stage-2 TLBs

Message ID 1371648006-8036-3-git-send-email-marc.zyngier@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marc Zyngier June 19, 2013, 1:20 p.m. UTC
When performing a Stage-2 TLB invalidation, it is necessary to
make sure the write to the page tables is observable by all CPUs.

For this purpose, add a dsb instruction to __kvm_tlb_flush_vmid_ipa
before doing the TLB invalidation itself.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/interrupts.S | 1 +
 1 file changed, 1 insertion(+)

Comments

Christoffer Dall June 20, 2013, 12:05 a.m. UTC | #1
On Wed, Jun 19, 2013 at 02:20:03PM +0100, Marc Zyngier wrote:
> When performing a Stage-2 TLB invalidation, it is necessary to
> make sure the write to the page tables is observable by all CPUs.
> 
> For this purpose, add a dsb instruction to __kvm_tlb_flush_vmid_ipa
> before doing the TLB invalidation itself.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/interrupts.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index d0a8fa3..afa6c04 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -49,6 +49,7 @@ __kvm_hyp_code_start:
>  ENTRY(__kvm_tlb_flush_vmid_ipa)
>  	push	{r2, r3}
>  
> +	dsb

shouldn't this be a dmb then?

thinking that a dsb only ensures completion between load/stores and not
between the preceeding store and the actual invalidate operation?

or are we relying on the fact that the store must complete before the
ldrd below, which must happen before the mcrr, which must happen before
the invalidate (?), and therefore it's all good?

>  	add	r0, r0, #KVM_VTTBR
>  	ldrd	r2, r3, [r0]
>  	mcrr	p15, 6, r2, r3, c2	@ Write VTTBR
> -- 
> 1.8.2.3
> 
> 
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Christoffer Dall June 20, 2013, 12:08 a.m. UTC | #2
On Wed, Jun 19, 2013 at 05:05:22PM -0700, Christoffer Dall wrote:
> On Wed, Jun 19, 2013 at 02:20:03PM +0100, Marc Zyngier wrote:
> > When performing a Stage-2 TLB invalidation, it is necessary to
> > make sure the write to the page tables is observable by all CPUs.
> > 
> > For this purpose, add a dsb instruction to __kvm_tlb_flush_vmid_ipa
> > before doing the TLB invalidation itself.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/kvm/interrupts.S | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> > index d0a8fa3..afa6c04 100644
> > --- a/arch/arm/kvm/interrupts.S
> > +++ b/arch/arm/kvm/interrupts.S
> > @@ -49,6 +49,7 @@ __kvm_hyp_code_start:
> >  ENTRY(__kvm_tlb_flush_vmid_ipa)
> >  	push	{r2, r3}
> >  
> > +	dsb
> 
> shouldn't this be a dmb then?
> 
> thinking that a dsb only ensures completion between load/stores and not
> between the preceeding store and the actual invalidate operation?
> 
> or are we relying on the fact that the store must complete before the
> ldrd below, which must happen before the mcrr, which must happen before
> the invalidate (?), and therefore it's all good?
> 

bah, ignore my crazy rambling, I completely switched the two when I read
the definitions.

> >  	add	r0, r0, #KVM_VTTBR
> >  	ldrd	r2, r3, [r0]
> >  	mcrr	p15, 6, r2, r3, c2	@ Write VTTBR
> > -- 
> > 1.8.2.3
> > 
> > 
> > 
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm@lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Will Deacon June 20, 2013, 10:47 a.m. UTC | #3
On Wed, Jun 19, 2013 at 02:20:03PM +0100, Marc Zyngier wrote:
> When performing a Stage-2 TLB invalidation, it is necessary to
> make sure the write to the page tables is observable by all CPUs.
> 
> For this purpose, add a dsb instruction to __kvm_tlb_flush_vmid_ipa
> before doing the TLB invalidation itself.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/kvm/interrupts.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index d0a8fa3..afa6c04 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -49,6 +49,7 @@ __kvm_hyp_code_start:
>  ENTRY(__kvm_tlb_flush_vmid_ipa)
>  	push	{r2, r3}
>  
> +	dsb

This can be dsb	ish.

Will
diff mbox

Patch

diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
index d0a8fa3..afa6c04 100644
--- a/arch/arm/kvm/interrupts.S
+++ b/arch/arm/kvm/interrupts.S
@@ -49,6 +49,7 @@  __kvm_hyp_code_start:
 ENTRY(__kvm_tlb_flush_vmid_ipa)
 	push	{r2, r3}
 
+	dsb
 	add	r0, r0, #KVM_VTTBR
 	ldrd	r2, r3, [r0]
 	mcrr	p15, 6, r2, r3, c2	@ Write VTTBR