[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH MM-PART1 v3 1/8] xen/arm: Don't boot Xen on platform using AIVIVT instruction caches

Gentle ping.

On 20/05/2019 20:53, Julien Grall wrote:

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@xxxxxxx>

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.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.