Re: [Xen-devel] [PATCH v4 08/15] xen: arm: don't pretend to handle cache maintenance by set/way

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 27/03/15 17:05, Ian Campbell wrote:
> On Fri, 2015-03-27 at 16:36 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 27/03/15 14:33, Ian Campbell wrote:
>>> We set HCR_EL2.TSW but only (sort of) handle 32-bit access to DCCISW
>>> but not the other two registers, nor any 64-bit access. Add handlers
>>> for all of these.
>>
>> We don't set HCR_EL2.TSW so DCCISW is not trapped.
> 
> I was completely sure we did, but I was wrong.
> 
>>> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
>>> index c2dcb66..cf3d6cc 100644
>>> --- a/xen/include/public/arch-arm.h
>>> +++ b/xen/include/public/arch-arm.h
>>> @@ -161,6 +161,11 @@
>>>   *
>>>   * - The device tree Xen compatible node is fully described under Linux
>>>   *   at Documentation/devicetree/bindings/arm/xen.txt.
>>> + *
>>> + * - Cache maintenaince operations by set/way ("dc isw|cisw|csw" and
> 
> I've just spotted a typo here, which I have fixed in my tree.
> 
>>> + *   the equivalent cp15 registers) are not available when running
>>> + *   under Xen and will result in an undefined instruction exception
>>> + *   delivered to the guest.
>>>   */
>>
>> set/way operations is used by Linux ARM32 in order to flush all the
>> cache. Injecting an undefined instruction would make guest unusable.
> 
> Yes, that's a shame. AIUI operations by set/way aren't really very
> useful under virt. See ARM v7 B1.14.4.
> 
> AIUI2 the reasons Linux does those flushes are due to bootloader's
> dirtying of cachelines, which we take great care to avoid in our domain
> builder, so I _think_ they probably don't need this under Xen, but have
> no easy way to know that at the point they do them. Perhaps it would be
> needed for a bootloader run under Xen to do this, I think that would
> come under "things to fix when paravirtualaising a bootloader"

Technically we already support PV bootloader such as UEFI.

> 
> So I think we have a few options:
> 
>      1. Continue to not trap set/way operations. Guests will be able to
>         flush the entire host cache by set/way. I don't think this is a
>         good idea to keep allowing as a general principal.
>      2. Trap set/way operations and do one of:
>              1. Inject #undef
>              2. Silently ignore
>              3. Ignore with a debug level print of some sort
>              4. Try to do some sort of useful operation.
>                 
> I don't think #1 is a very good idea, and we've essentially ruled out
> #2.1 (essentially this patch + enable the trap) here I think.
> 
> I've absolutely no idea what #2.4 might be.
> 
> So I think we are down to trap and ignore either with or without
> logging.
> 
> Given the caveats with s/w under a hypervisor knowing about it would be
> nice. The logging would be a few dozen (nr_sets*nr_ways) on each guest
> boot, so would have to be a debug level log, but it wouldn't be terribly
> spammy.
> 
> On the other hand, there is no way for a kernel to know it can not
> bother with these, so those log messages will always be there and any
> problematic uses won't be noticeable anyway.
> 
> So I am probably leaning towards #2.2
> 
> The KVM approach appears to be to flush the entire guest RAM space on
> the first s/w operation and set HCR_EL2.TVM and then ignore all
> subsequent s/w ops until caches are enabled, at which point they disable
> HCR_EL2.TVM and go back to normal until the next s/w op. This catches
> the actual expected use of s/w which is when enabling/disabling caches.
> 
> Something similar might work for us too actually. Maybe we could go with
> #2.2 in the short term and plan to do the more complex thing later?

I'm ok with the #2.2 as a temporary solution. Although I think it should
be properly fixed for Xen 4.6.

Would it be worth to open a bug on the tracker and mark as a blocker?

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel