[ Retrieve as mbox ]
From: Julien Grall <julien.grall@linaro.org> To: Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xen.org Cc: tim@xen.org, stefano.stabellini@eu.citrix.com Subject: Re: [Xen-devel] [PATCH v4 08/15] xen: arm: don't pretend to handle cache maintenance by set/way Date: Fri, 27 Mar 2015 16:36:28 +0000 Message-ID: <5515870C.1080702@linaro.org>
[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]
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.
> 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
> + * 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.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
From: Ian Campbell <ian.campbell@citrix.com> To: Julien Grall <julien.grall@linaro.org> Cc: tim@xen.org, xen-devel@lists.xen.org, stefano.stabellini@eu.citrix.com Subject: Re: [Xen-devel] [PATCH v4 08/15] xen: arm: don't pretend to handle cache maintenance by set/way Date: Fri, 27 Mar 2015 17:05:12 +0000 Message-ID: <1427475912.13935.192.camel@citrix.com>
[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]
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"
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?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
From: Julien Grall <julien.grall@linaro.org> To: Ian Campbell <ian.campbell@citrix.com> Cc: stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org, tim@xen.org Subject: Re: [Xen-devel] [PATCH v4 08/15] xen: arm: don't pretend to handle cache maintenance by set/way Date: Mon, 30 Mar 2015 13:17:04 +0100 Message-ID: <55193EC0.70207@linaro.org>
[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]
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@lists.xen.org
http://lists.xen.org/xen-devel
From: Ian Campbell <ian.campbell@citrix.com> To: Julien Grall <julien.grall@linaro.org> Cc: stefano.stabellini@eu.citrix.com, xen-devel@lists.xen.org, tim@xen.org Subject: Re: [Xen-devel] [PATCH v4 08/15] xen: arm: don't pretend to handle cache maintenance by set/way Date: Mon, 30 Mar 2015 14:30:59 +0100 Message-ID: <1427722259.13935.295.camel@citrix.com>
[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]
create <5515870C.1080702@linaro.org> title it proper handling of set/way cache maintenance instructions by guests on ARM thanks On Mon, 2015-03-30 at 13:17 +0100, Julien Grall wrote: > Would it be worth to open a bug on the tracker and mark as a blocker? Yes, done. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Control reply; (Full Text)