Re: [RFC PATCH] xen/arm: Add emulation of Debug Data Transfer Registers

[Julien Grall <julien@xxxxxxx>]

On 05/12/2023 10:30, Michal Orzel wrote:
> On 05/12/2023 11:01, Julien Grall wrote:
> > On 05/12/2023 09:28, Michal Orzel wrote:
> > > Hi Julien,
> > >
> > > On 04/12/2023 20:55, Julien Grall wrote:
> > > > On 04/12/2023 13:02, Ayan Kumar Halder wrote:
> > > > > On 04/12/2023 10:31, Julien Grall wrote:
> > > > > > Hi Ayan,
> > > > > Hi Julien,
> > > > > > On 01/12/2023 18:50, Ayan Kumar Halder wrote:
> > > > > > > Currently if user enables HVC_DCC config option in Linux, it invokes
> > > > > > > access to debug data transfer registers (ie DBGDTRTX_EL0 on arm64,
> > > > > > > DBGDTRTXINT on arm32). As these registers are not emulated, Xen injects
> > > > > > > an undefined exception to the guest. And Linux crashes.
> > > > > > I am missing some data points here to be able to say whether I would
> > > > > > be ok with emulating the registers. So some questions:
> > > > > >     * As you wrote below, HVC_DCC will return -ENODEV after this
> > > > > > emulation. So may I ask what's the use case to enable it? (e.g. is
> > > > > > there a distro turning this on?)
> > > > > I am not aware of any distro using (or not using) this feature. This
> > > > > issue came to light during our internal testing, when HVC_DCC was
> > > > > enabled to use the debug console. When Linux runs without Xen, then we
> > > > > could observe the logs on the debug console. When Linux was running as a
> > > > > VM, it crashed.
> > > > >
> > > > > My intention here was to do the bare minimum emulation so that the crash
> > > > > could be avoided.
> > > > This reminds me a bit the discussion around "xen/arm64: Decode ldr/str
> > > > post increment operations". I don't want Xen to contain half-backed
> > > > emulation just to please an OS in certain configuration that doesn't
> > > > seem to be often used.
> > > >
> > > > Also, AFAICT, KVM is in the same situation...
> > > Well, KVM is not in the same situation. It emulates all DCC regs as RAZ/WI, so 
> > > there
> > > will be no fault on an attempt to access the DCC.
> > Does this mean a guest will think the JTAG is availabe?
> Yes, it will believe the DCC is available which is not a totally bad idea. Yes, 
> it will not have
> any effect but at least covers the polling loop. The solution proposed here 
> sounds better but does not take
> into account the busy while loop when sending the char. Linux DCC earlycon does 
> not make an initial check that runtime
> driver does and will keep waiting in the loop if TXfull is set. Emulating 
> everything as RAZ/WI solves that.
> As you can see, each solution has its flaws and depends on the OS 
> implementation.
Right, which prove my earlier point. You are providing an emulation just 
to please a specific driver in Linux (not even the whole Linux). This is 
what I was the most concern of.
So ...

> > > In general, I think that if a register is not optional and does not depend on 
> > > other register
> > > to be checked first (e.g. a feature/control register we emulate), which implies 
> > > that it is fully ok for a guest to
> > > access it directly - we should at least try to do something not to crash a 
> > > guest.
> > This is where we have opposing opinion. I view crashing a guest better
> > than providing a wrong emulation because it gives a clear signal that
> > the register they are trying to access will not function properly.
> >
> > We had this exact same discussion a few years ago when Linux started to
> > access GIC*_ACTIVER registers. I know that Stefano was for emulating
> > them as RAZ but this had consequences on the domain side (Linux
> > sometimes need to read them). We settled on printing a warning which is
> > not great but better than claiming we properly emulate the register.
> >
> > > I agree that this feature is not widely used. In fact I can only find it 
> > > implemented in Linux and U-BOOT
> > > and the issue I found in DBGDSCRINT (no access from EL0, even though we emulate 
> > > REXT.UDCCdis as 0) only
> > > proves that. At the same time, it does not cost us much to add this trivial 
> > > support.
> > See above. If we provide an (even basic) emulation, we need to make sure
> > it is correct and doesn't have a side effect on the guest. If we can't
> > guarantee that (e.g. like for set/way when a device is assigned), then
> > the best course of action is to crash the domain.
> >
> > AFAICT, the proposed emulation would be ok.
... I will need to revise this statement. I am now against this patch.

Cheers,

--
Julien Grall