Hi Ayan, On 31/01/2024 13:10, Ayan Kumar Halder wrote: > When user enables HVC_DCC config option in Linux, it invokes access to debug > transfer register (i.e. DBGDTRTXINT). As this register is not emulated, Xen > injects an undefined exception to the guest and Linux crashes. > To prevent this crash, introduce a partial emulation of DBGDTRTXINT as RAZ/WI > and TXfull should be set to 1. So that Linux will see that TXfull is set, and > it will not access DBGDTRTXINT. It will access it at least once to later check if TXfull is set.
> > As a pre-requisite, DBGOSLSR should be emulated in the same way as its AArch64 > variant (ie OSLSR_EL1). > This is to ensure that DBGOSLSR.OSLK is 0, thus MDSCR_EL1.TXfull is treated > as UNK/SBZP. For that you could just emulate it as RAZ. Instead you also set OSLM[1]. So at least I would make it clear that you do that for consistency. > Only MDCCSR_EL0 can be emulated (which is DBGDSCRINT on arm32). DBGDSCRINT I would write: This way, we only need to emulate DBGDSCRINT. > can be accessed at EL0 as DBGDSCREXT is emulated as RAZ (as DBGOSLSR.OSLK > == 0). So, we tool the opportunity to fix the minimum EL for DBGDSCRINT. > DBGDSCRINT.TXfull is set to 1. I'm not sure why you are mixing AArch64 names with AArch32 ones. It reads a bit difficult. Furthermore, fixing lowest EL deserves a separate section. Also s/tool/took. > > Refer ARM DDI 0487J.a ID042523, G8.3.19, DBGDTRTXint > "If TXfull is set to 1, set DTRTX to UNKNOWN". > So, DBGDTR[TR]XINT is emulated as RAZ/WI. > > Thus, any OS is expected to read DBGDSCRINT and check for TXfull before using > DBGDTRTXINT. > > Signed-off-by: Ayan Kumar Halder <[email protected]> > --- > Changes from > > v1 :- 1. DBGDTR_EL0 does not emulate RXfull. This is to avoid giving the OS > any > indication that the RX buffer is full and is waiting to be read. > > 2. In Arm32, DBGOSLSR is emulated. Also DBGDTRTXINT is emulated at EL0 only. > > 3. Fixed the commit message and inline code comments. > > v2 :- 1. Split the patch into two (separate patches for arm64 and arm32). > 2. Fixed in line comments and style related issues. > 3. Updated commit message to mention DBGDSCRINT handling. > > v3 :- 1. The original emulation of DBGDSCRINT is retained when > 'partial_emulation' is false. > > 2. If 'partial_emulation' is false, then access to DBGDTRTXINT will > lead to undefined exception. > > xen/arch/arm/include/asm/cpregs.h | 2 ++ > xen/arch/arm/vcpreg.c | 35 ++++++++++++++++++++++--------- > 2 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/include/asm/cpregs.h > b/xen/arch/arm/include/asm/cpregs.h > index 6b083de204..aec9e8f329 100644 > --- a/xen/arch/arm/include/asm/cpregs.h > +++ b/xen/arch/arm/include/asm/cpregs.h > @@ -75,6 +75,8 @@ > #define DBGDIDR p14,0,c0,c0,0 /* Debug ID Register */ > #define DBGDSCRINT p14,0,c0,c1,0 /* Debug Status and Control Internal > */ > #define DBGDSCREXT p14,0,c0,c2,2 /* Debug Status and Control External > */ > +#define DBGDTRRXINT p14,0,c0,c5,0 /* Debug Data Transfer Register, > Receive */ > +#define DBGDTRTXINT p14,0,c0,c5,0 /* Debug Data Transfer Register, > Transmit */ > #define DBGVCR p14,0,c0,c7,0 /* Vector Catch */ > #define DBGBVR0 p14,0,c0,c0,4 /* Breakpoint Value 0 */ > #define DBGBCR0 p14,0,c0,c0,5 /* Breakpoint Control 0 */ > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c > index a2d0500704..87df4bd238 100644 > --- a/xen/arch/arm/vcpreg.c > +++ b/xen/arch/arm/vcpreg.c > @@ -493,11 +493,12 @@ void do_cp14_32(struct cpu_user_regs *regs, const union > hsr hsr) > * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58 > * > * Unhandled: > - * DBGOSLSR > * DBGPRCR > */ > case HSR_CPREG32(DBGOSLAR): > return handle_wo_wi(regs, regidx, cp32.read, hsr, 1); > + case HSR_CPREG32(DBGOSLSR): > + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1, 1U << 3); > case HSR_CPREG32(DBGOSDLR): > return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); > > @@ -509,8 +510,6 @@ void do_cp14_32(struct cpu_user_regs *regs, const union > hsr hsr) > * > * Unhandled: > * DBGDCCINT > - * DBGDTRRXint > - * DBGDTRTXint > * DBGWFAR > * DBGDTRTXext > * DBGDTRRXext, > @@ -550,10 +549,22 @@ void do_cp14_32(struct cpu_user_regs *regs, const union > hsr hsr) > > case HSR_CPREG32(DBGDSCRINT): > /* > - * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis > - * is set to 0, which we emulated below. > + * Xen doesn't expose a real (or emulated) Debug Communications > + * Channel (DCC) to a domain. Yet the Arm ARM implies this is not an > + * optional feature. So some domains may start to probe it. For > + * instance, the HVC_DCC driver in Linux (since f377775dc083 and at > + * least up to v6.7), will try to write some characters and check if > + * the transmit buffer has emptied. By setting TX status bit to > + * indicate the transmit buffer is full. This we would hint the OS > + * that the DCC is probably not working. I'm a bit suprised that these messages differ. Why not to use the same text as arm64 but with register names updated? Other than that: Reviewed-by: Michal Orzel <[email protected]> ~Michal
