Hi Julien,

I expressed my preference below. We don't agree. Is there anything else
you would like me to add to this thread? Do you have a specific
question? The only question I see below is "Users of what?" but I take
it was just rhetorical.


On Mon, 10 Jun 2019, Julien Grall wrote:
> (+ Committers)
> 
> Ping again... I have quite a few patches blocked on this work.
> 
> Cheers,
> 
> On 29/05/2019 17:44, Julien Grall wrote:
> > Gentle ping.
> > 
> > On 20/05/2019 20:53, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 20/05/2019 19:56, Stefano Stabellini wrote:
> > > > On Tue, 14 May 2019, Julien Grall wrote:
> > > > > The AIVIVT is a type of instruction cache available on Armv7. This is
> > > > > the only cache not implementing the IVIPT extension and therefore
> > > > > requiring specific care.
> > > > > 
> > > > > To simplify maintenance requirements, Xen will not boot on platform
> > > > > using AIVIVT cache.
> > > > > 
> > > > > This should not be an issue because Xen Arm32 can only boot on a small
> > > > > number of processors (see arch/arm/arm32/proc-v7.S). All of them are
> > > > > not using AIVIVT cache.
> > > > > 
> > > > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > > 
> > > > As we have already discussed, I am OK with this and neither of us
> > > > foresee any issues. Given that it could be considered as a drop in
> > > > support for something, I think it would be nice to send an email outside
> > > > of the series to say we won't support AIVIVT processors any longer,
> > > > using words easier to understand to users (not necessarily developers).
> > > 
> > > Users of what? Xen upstream will *panic* on every processor not listed in
> > > arch/arm/arm32/proc-v7.S even without this patch.
> > > 
> > > > Would you be able to do that? I can help you with the text.
> > > While in theory this sounds sensible, for reaching the panic added in this
> > > patch, you would need out-of-tree patches. So in practice you are saying
> > > we should care about out-of-tree users.
> > > 
> > > I have already enough to care about Xen upstream itself that out-of-tree
> > > is my last concern. If someone were using out-of-tree then then too bad
> > > they will see the panic.
> > > 
> > > TBH, I am pretty sure we don't currently properly follow the maintenance
> > > requirements... So we are making them a favor to add a panic. Before they
> > > could just see random corruption...
> > > 
> > > Anyway, feel free to send the message yourself.
> > > 
> > > > 
> > > > 
> > > > > ---
> > > > > 
> > > > >      Changes in v3:
> > > > >          - Patch added
> > > > > ---
> > > > >   xen/arch/arm/setup.c            | 5 +++++
> > > > >   xen/include/asm-arm/processor.h | 5 +++++
> > > > >   2 files changed, 10 insertions(+)
> > > > > 
> > > > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > > > > index ccb0f181ea..faaf029b99 100644
> > > > > --- a/xen/arch/arm/setup.c
> > > > > +++ b/xen/arch/arm/setup.c
> > > > > @@ -526,10 +526,15 @@ static void __init setup_mm(unsigned long
> > > > > dtb_paddr, size_t dtb_size)
> > > > >       unsigned long boot_mfn_start, boot_mfn_end;
> > > > >       int i;
> > > > >       void *fdt;
> > > > > +    const uint32_t ctr = READ_CP32(CTR);
> > > > >       if ( !bootinfo.mem.nr_banks )
> > > > >           panic("No memory bank\n");
> > > > > +    /* We only supports instruction caches implementing the IVIPT
> > > > > extension. */
> > > > 
> > > > Please mention that IVIPT can only be implemented by PIPT and VIPT
> > > > caches, not by AIVIVT caches. That should make it straightforward to
> > > > understand the reason for the panic below.
> > > 
> > > I would prefer to add "This is not the case of AIVIVT" rather than
> > > spelling out the other caches.
> > > 
> > > Cheers,
> > > 
> > > 
> > 
> 
> -- 
> Julien Grall
>