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

[Michal Orzel <michal.orzel@xxxxxxx>]

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.

> 
>>
>> 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. But I want to make clear
> that I would not generally be ok with this approach and the decision
> would need to be on the case by case basis.
This goes without saying. Every issue requires separate investigation.

~Michal