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

[Julien Grall <Julien.Grall@xxxxxxx>]

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

Cheers,


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