| Message ID | 1472132255-23470-19-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Konrad, On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote: > When we are flushing the cache we are most likley also want > to flush the branch predictor too. Hence add this. I think it makes more sense to fold this patch into #18 because ICIALLUIS may flush the branch predictor if the instruction cache is separate (see Table B4-35 in ARM DDI 0406C.c). Therefore we need to be consistent and also flush the branch predictor for unified cache. > > Suggested-by: Julien Grall <julien.grall@arm.com> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > --- > Cc: Julien Grall <julien.grall@arm.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > > v2: First submission > --- > xen/arch/arm/livepatch.c | 3 +++ > xen/include/asm-arm/arm32/page.h | 5 ++++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c > index c290602..cdb8d65 100644 > --- a/xen/arch/arm/livepatch.c > +++ b/xen/arch/arm/livepatch.c > @@ -45,6 +45,9 @@ void arch_livepatch_revive(void) > /* > * Nuke the instruction cache. Data cache has been cleaned before in > * arch_livepatch_apply_jmp. > + * > + * Need to flush the branch predictor for ARMv7 as it may be > + * architecturally visible to the software (see B2.2.4 in ARM DDI 0406C.b). I don't think this comment belongs to livepatch.c. It describes the behavior internal behavior of invalidate_icache. > */ > invalidate_icache(); > > diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h > index 26184ec..6caf596 100644 > --- a/xen/include/asm-arm/arm32/page.h > +++ b/xen/include/asm-arm/arm32/page.h > @@ -32,7 +32,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte) > /* Invalidate all instruction caches in Inner Shareable domain to PoU */ > static inline void invalidate_icache(void) > { > - asm volatile (CMD_CP32(ICIALLUIS)); > + asm volatile ( > + CMD_CP32(ICIALLUIS) /* Flush I-cache. */ > + CMD_CP32(BPIALLIS) /* Flush branch predictor. */ > + : : : "memory"); > } > > /* > Regards,
On Thu, Sep 01, 2016 at 04:13:30PM +0100, Julien Grall wrote: > Hi Konrad, > > On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote: > > When we are flushing the cache we are most likley also want > > to flush the branch predictor too. Hence add this. > > I think it makes more sense to fold this patch into #18 because ICIALLUIS This is #18 :-) > may flush the branch predictor if the instruction cache is separate (see > Table B4-35 in ARM DDI 0406C.c). > > Therefore we need to be consistent and also flush the branch predictor for > unified cache. > > > > > Suggested-by: Julien Grall <julien.grall@arm.com> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > --- > > Cc: Julien Grall <julien.grall@arm.com> > > Cc: Stefano Stabellini <sstabellini@kernel.org> > > > > v2: First submission > > --- > > xen/arch/arm/livepatch.c | 3 +++ > > xen/include/asm-arm/arm32/page.h | 5 ++++- > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c > > index c290602..cdb8d65 100644 > > --- a/xen/arch/arm/livepatch.c > > +++ b/xen/arch/arm/livepatch.c > > @@ -45,6 +45,9 @@ void arch_livepatch_revive(void) > > /* > > * Nuke the instruction cache. Data cache has been cleaned before in > > * arch_livepatch_apply_jmp. > > + * > > + * Need to flush the branch predictor for ARMv7 as it may be > > + * architecturally visible to the software (see B2.2.4 in ARM DDI 0406C.b). > > I don't think this comment belongs to livepatch.c. It describes the behavior > internal behavior of invalidate_icache. <nods> > > > */ > > invalidate_icache(); > > > > diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h > > index 26184ec..6caf596 100644 > > --- a/xen/include/asm-arm/arm32/page.h > > +++ b/xen/include/asm-arm/arm32/page.h > > @@ -32,7 +32,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte) > > /* Invalidate all instruction caches in Inner Shareable domain to PoU */ > > static inline void invalidate_icache(void) > > { > > - asm volatile (CMD_CP32(ICIALLUIS)); > > + asm volatile ( > > + CMD_CP32(ICIALLUIS) /* Flush I-cache. */ > > + CMD_CP32(BPIALLIS) /* Flush branch predictor. */ > > + : : : "memory"); > > } > > > > /* > > > > Regards, > > -- > Julien Grall
On Thu, Sep 01, 2016 at 04:13:30PM +0100, Julien Grall wrote: > Hi Konrad, > > On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote: > > When we are flushing the cache we are most likley also want > > to flush the branch predictor too. Hence add this. > > I think it makes more sense to fold this patch into #18 because ICIALLUIS I squashed it in " xen/arm32: Add an helper to invalidate all instruction caches" > may flush the branch predictor if the instruction cache is separate (see > Table B4-35 in ARM DDI 0406C.c). > > Therefore we need to be consistent and also flush the branch predictor for > unified cache. > > > > > Suggested-by: Julien Grall <julien.grall@arm.com> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > > --- > > Cc: Julien Grall <julien.grall@arm.com> > > Cc: Stefano Stabellini <sstabellini@kernel.org> > > > > v2: First submission > > --- > > xen/arch/arm/livepatch.c | 3 +++ > > xen/include/asm-arm/arm32/page.h | 5 ++++- > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c > > index c290602..cdb8d65 100644 > > --- a/xen/arch/arm/livepatch.c > > +++ b/xen/arch/arm/livepatch.c > > @@ -45,6 +45,9 @@ void arch_livepatch_revive(void) > > /* > > * Nuke the instruction cache. Data cache has been cleaned before in > > * arch_livepatch_apply_jmp. > > + * > > + * Need to flush the branch predictor for ARMv7 as it may be > > + * architecturally visible to the software (see B2.2.4 in ARM DDI 0406C.b). > > I don't think this comment belongs to livepatch.c. It describes the behavior > internal behavior of invalidate_icache. Moved it to be right above invalidate_icache in xen/include/asm-arm/arm32/page.h. Thanks!
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c index c290602..cdb8d65 100644 --- a/xen/arch/arm/livepatch.c +++ b/xen/arch/arm/livepatch.c @@ -45,6 +45,9 @@ void arch_livepatch_revive(void) /* * Nuke the instruction cache. Data cache has been cleaned before in * arch_livepatch_apply_jmp. + * + * Need to flush the branch predictor for ARMv7 as it may be + * architecturally visible to the software (see B2.2.4 in ARM DDI 0406C.b). */ invalidate_icache(); diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h index 26184ec..6caf596 100644 --- a/xen/include/asm-arm/arm32/page.h +++ b/xen/include/asm-arm/arm32/page.h @@ -32,7 +32,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte) /* Invalidate all instruction caches in Inner Shareable domain to PoU */ static inline void invalidate_icache(void) { - asm volatile (CMD_CP32(ICIALLUIS)); + asm volatile ( + CMD_CP32(ICIALLUIS) /* Flush I-cache. */ + CMD_CP32(BPIALLIS) /* Flush branch predictor. */ + : : : "memory"); } /*
When we are flushing the cache we are most likley also want to flush the branch predictor too. Hence add this. Suggested-by: Julien Grall <julien.grall@arm.com> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- Cc: Julien Grall <julien.grall@arm.com> Cc: Stefano Stabellini <sstabellini@kernel.org> v2: First submission --- xen/arch/arm/livepatch.c | 3 +++ xen/include/asm-arm/arm32/page.h | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-)