Re: [Xen-devel] [PATCH v2 4/7] xen/arm: page: Clarify the Xen TLBs helpers name

[Julien Grall <julien.grall@xxxxxxx>]

On 09/05/2019 22:46, Julien Grall wrote:
> Hi,
>
> On 09/05/2019 21:32, Julien Grall wrote:
> > Hi,
> >
> > On 09/05/2019 21:13, Stefano Stabellini wrote:
> > > On Wed, 8 May 2019, Julien Grall wrote:
> > > >   /* Release all __init and __initdata ranges to be reused */
> > > > diff --git a/xen/include/asm-arm/arm32/page.h 
> > > > b/xen/include/asm-arm/arm32/page.h
> > > > index 40a77daa9d..0b41b9214b 100644
> > > > --- a/xen/include/asm-arm/arm32/page.h
> > > > +++ b/xen/include/asm-arm/arm32/page.h
> > > > @@ -61,12 +61,8 @@ static inline void invalidate_icache_local(void)
> > > >       isb();                      /* Synchronize fetched instruction stream. */
> > > >   }
> > > > -/*
> > > > - * Flush all hypervisor mappings from the data TLB of the local
> > > > - * processor. This is not sufficient when changing code mappings or
> > > > - * for self modifying code.
> > > > - */
> > > > -static inline void flush_xen_data_tlb_local(void)
> > > > +/* Flush all hypervisor mappings from the TLB of the local processor. */
> > > I realize that the statement "This is not sufficient when changing code
> > > mappings or for self modifying code" is not quite accurate, but I would
> > > prefer not to remove it completely. It would be good to retain a warning
> > > somewhere about IC been needed when changing Xen's own mappings. Maybe
> > > on top of invalidate_icache_local?
> > Can you please expand in which circumstance you need to invalid the 
> > instruction cache when changing Xen's own mappings?
> Reading the Armv7 (B3.11.2 in ARM DDI 0406C.c) and Armv8 (D5.11.2 in ARM DDI 
> 0487D.a), most of the instruction caches implement the IVIPT extension. This 
> means that instruction cache maintenance is required only after write new data 
> to a PA that holds instructions (see D5-2522 in ARM DDI 0487D.a and B3.11.2 in 
> ARM DDI 0406C.c).
> The only one that differs with that behavior is ASID and VMID tagged VIVT 
> instruction caches which is only present in Armv7 (I can't remember why it was 
> dropped in Armv8). Instruction cache maintenance can be required when changing 
> the translation of a virtual address to a physical address.
I thought about this a bit more and chat with my team at Arm. Xen on Arm only 
support Cortex-A7, Cortex-A15 and Brahma 15 (see the CPU ID check in arm32/head.S).
        
None of them are actually using VIVT instruction caches. In general, VIVT caches 
are more difficult to deal with because they require more flush. So I would be 
more incline to deny booting Xen on platform where the instruction caches don't 
support IVIVT extension.
I don't think that will have a major impact on the user because of my point 
above.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel