Re: [Xen-devel] [PATCH 3/4] xen/arm: Implement a dummy debug monitor for ARM32

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-05-02 at 14:29 +0100, Julien Grall wrote:
> On 05/02/2014 02:14 PM, Ian Campbell wrote:
> > On Fri, 2014-05-02 at 13:53 +0100, Julien Grall wrote:
> >> On 05/02/2014 12:09 PM, Ian Campbell wrote:
> >>> On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote:
> >>>> XSA-93 (commit 0b18220 "xen/arm: Don't let guess access to Debug and 
> >>>> Performance
> >>>> Monitors registers") disable Debug Registers access.
> >>>>
> >>>> When CONFIG_PERF_EVENTS is enabled in the Linux Kernel, it will try to
> >>>> initialize the debug monitors. If an error occured Linux won't use this
> >>>> feature.
> >>>>
> >>>> The implementation made Xen expose a minimal set of registers which let 
> >>>> think
> >>>> the guest (i.e.) thinks HW debug won't work.
> >>>
> >>> Why only for arm32?
> >>
> >> Because, if I'm not mistaken, you've already implemented a dummy HW
> >> debug for arm64 in commit 0b182202 "xen/arm: Don't let guess access to
> >> Debug and Performance Monitor registers".
> > 
> > That's a RAZ/WI thing, I thought this was something cleverer (returing
> > values to make the guest think nothing was there).

I think we've got a bit sidetracked here (especially in the other
subthread), sorry about that.

> Most of the time RAZ/WI is enough.  The arm32 implementation makes the
> life harder.

Looking at it with fresh eyes this morning I see what you mean now, the
essential difference is that with arm32 DBGDIDR is trapped by
MDCR_EL2.TDA, whereas the the arm64 equivalent (IDAA64DFR*) are not
(it's trapped as part of the ID register group, which we don't bother
trapping). The rest of the registers are RAZ/WI which is consistent with
arm64.

So in the end you've convinced me that this is the right thing to do for
now and to backport to 4.4.

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

I've committed this and the previous (perfc) one but not the next one
(useful debug for coproc traps) which had comments.

I did s/DBGCR/DBGBCR/ to match the name used in both the v7 and v8 ARM
ARMs, I think it was just a typo? Hope that's ok.

Also DBGOSLAR is supposed to be WO but you've implemented it as RAZ/WI,
I didn't think that mattered enough to bother with though.

Sorry that it took so long to get my head around this.

> I didn't check the arm64 implementation as I don't use it every day... I
> need to set up the Foundation Model on my computer.

You made me notice that arm64 doesn't handle OSDLR_EL1 traps. I think
that's harmless (unless a guest kernel tries to use it) but I'll send
out a patch shortly.

Ian.



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