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

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

Hi Julien/All,

On 05/12/2023 11:02, Michal Orzel wrote:
> On 05/12/2023 11:42, Julien Grall wrote:
> > 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.
I have sent out a patch ("[PATCH] tty: hvc: dcc: Check for TXfull 
condition while setting up early console") to fix this.
> > 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.
> Yes, the problem was tricky from the very beginning and I somewhat agree. I 
> prepared a POC with one solution
> that Ayan extended and sent to gather feedback (hence RFC). I think we should 
> still wait for others
> opinion (@Stefano, @Bertrand). I think the thread contains all the necessary 
> information
> to decide what to do:
> - do nothing* (guest crashes)
> - emulate DCC the same way as KVM i.e. RAZ/WI (no crash, no busy loop, guest 
> keeps using DCC with no effect)
> - emulate DCC with TXfull set to 1 (no crash, runtime DCC in Linux returns 
> -ENODEV, earlycon busy loop issue)
>
> * I still think we should fix DBGDSCRINT but I can send a separate patch (not 
> really related to the DCC problem)
Regardless if the linux hvc earlycon is fixed or not

@Julien , would you be ok with option 2 or 3 with a suitable warning ?

Also will wait for Stefano's and Bertrand's opinions.

Crashing the guest would seem quite severe imo if there can be a better 
way (option 2 or 3) to tell that DCC is not available.
- Ayan

> ~Michal