| Message ID | 20231218203242.1393268-1-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] xen/arm: Add emulation of Debug Data Transfer Registers | expand |
> On 18 Dec 2023, at 20:32, Michal.Orzel@amd.com wrote: > > From: Michal Orzel <michal.orzel@amd.com> > > Currently if user enables HVC_DCC config option in Linux, it invokes > access to debug data transfer registers (ie MDCCSR_EL0 on arm64, > DBGDTRTXINT on arm32). As these registers are not emulated, Xen injects > an undefined exception to the guest. And Linux crashes. > > We wish to avoid this crash by adding a "partial" emulation. DBGDTR_EL0 > is emulated as TXfull. > Refer ARM DDI 0487I.a ID081822, D17.3.8, DBGDTRTX_EL0 > "If TXfull is set to 1, set DTRRX and DTRTX to UNKNOWN" > > Thus, any OS is expected to read DBGDTR_EL0 and check for TXfull > before using DBGDTRTX_EL0. Linux does it via hvc_dcc_init() ---> > hvc_dcc_check() , it returns -ENODEV. In this way, we are preventing > the guest to be aborted. > > We also emulate DBGDTRTX_EL0 as RAZ/WI. > > We have added emulation for AArch32 variant of these registers as well. > > Also in the case of AArch32, DBGOSLSR is 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. > Thus only MDCCSR_EL0 can be emulated (which is DBGDTRTXINT on arm32). > So, DBGDTR[TR]XINT is emulated as RAZ/WI. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > 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. > > xen/arch/arm/arm64/vsysreg.c | 26 +++++++++++++++---- > xen/arch/arm/include/asm/arm64/hsr.h | 3 +++ > xen/arch/arm/include/asm/cpregs.h | 2 ++ > xen/arch/arm/include/asm/traps.h | 4 +++ > xen/arch/arm/traps.c | 18 +++++++++++++ > xen/arch/arm/vcpreg.c | 38 +++++++++++++++++++++++----- > 6 files changed, 79 insertions(+), 12 deletions(-) > > diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c > index b5d54c569b..ebeb83dd65 100644 > --- a/xen/arch/arm/arm64/vsysreg.c > +++ b/xen/arch/arm/arm64/vsysreg.c > @@ -159,9 +159,6 @@ void do_sysreg(struct cpu_user_regs *regs, > * > * Unhandled: > * MDCCINT_EL1 > - * DBGDTR_EL0 > - * DBGDTRRX_EL0 > - * DBGDTRTX_EL0 > * OSDTRRX_EL1 > * OSDTRTX_EL1 > * OSECCR_EL1 > @@ -172,11 +169,29 @@ void do_sysreg(struct cpu_user_regs *regs, > case HSR_SYSREG_MDSCR_EL1: > return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); > case HSR_SYSREG_MDCCSR_EL0: > + { > /* > + * 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. > + * > + * Bit 29: TX full > + * > * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that > * register as RAZ/WI above. So RO at both EL0 and EL1. > */ > - return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0); > + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 0, > + 1U << 29); > + } > + case HSR_SYSREG_DBGDTR_EL0: > + /* DBGDTR[TR]X_EL0 share the same encoding */ > + case HSR_SYSREG_DBGDTRTX_EL0: > + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 0); > HSR_SYSREG_DBG_CASES(DBGBVR): > HSR_SYSREG_DBG_CASES(DBGBCR): > HSR_SYSREG_DBG_CASES(DBGWVR): > @@ -393,7 +408,8 @@ void do_sysreg(struct cpu_user_regs *regs, > * > * And all other unknown registers. > */ > - default: > + default: goto fail; > + fail: > { > const struct hsr_sysreg sysreg = hsr.sysreg; > > diff --git a/xen/arch/arm/include/asm/arm64/hsr.h b/xen/arch/arm/include/asm/arm64/hsr.h > index e691d41c17..1495ccddea 100644 > --- a/xen/arch/arm/include/asm/arm64/hsr.h > +++ b/xen/arch/arm/include/asm/arm64/hsr.h > @@ -47,6 +47,9 @@ > #define HSR_SYSREG_OSDLR_EL1 HSR_SYSREG(2,0,c1,c3,4) > #define HSR_SYSREG_DBGPRCR_EL1 HSR_SYSREG(2,0,c1,c4,4) > #define HSR_SYSREG_MDCCSR_EL0 HSR_SYSREG(2,3,c0,c1,0) > +#define HSR_SYSREG_DBGDTR_EL0 HSR_SYSREG(2,3,c0,c4,0) > +#define HSR_SYSREG_DBGDTRTX_EL0 HSR_SYSREG(2,3,c0,c5,0) > +#define HSR_SYSREG_DBGDTRRX_EL0 HSR_SYSREG(2,3,c0,c5,0) > > #define HSR_SYSREG_DBGBVRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,4) > #define HSR_SYSREG_DBGBCRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,5) > 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/include/asm/traps.h b/xen/arch/arm/include/asm/traps.h > index 883dae368e..a2692722d5 100644 > --- a/xen/arch/arm/include/asm/traps.h > +++ b/xen/arch/arm/include/asm/traps.h > @@ -56,6 +56,10 @@ void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read, > void handle_ro_read_val(struct cpu_user_regs *regs, int regidx, bool read, > const union hsr hsr, int min_el, register_t val); > > +/* Read only as value provided with 'val' argument, write ignore */ > +void handle_read_val_wi(struct cpu_user_regs *regs, int regidx, > + const union hsr hsr, int min_el, register_t val); > + > /* Co-processor registers emulation (see arch/arm/vcpreg.c). */ > void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr); > void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr); > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 3784e8276e..f5ab555b19 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1676,6 +1676,24 @@ void handle_ro_read_val(struct cpu_user_regs *regs, > advance_pc(regs, hsr); > } > > +/* Read as value provided with 'val' argument of this function, write ignore */ > +void handle_read_val_wi(struct cpu_user_regs *regs, > + int regidx, > + const union hsr hsr, > + int min_el, > + register_t val) > +{ > + ASSERT((min_el == 0) || (min_el == 1)); > + > + if ( min_el > 0 && regs_mode_is_user(regs) ) > + return inject_undef_exception(regs, hsr); > + > + set_user_reg(regs, regidx, val); > + > + advance_pc(regs, hsr); > +} > + > + > /* Read only as read as zero */ > void handle_ro_raz(struct cpu_user_regs *regs, > int regidx, > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c > index 39aeda9dab..5087125111 100644 > --- a/xen/arch/arm/vcpreg.c > +++ b/xen/arch/arm/vcpreg.c > @@ -549,18 +549,41 @@ 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. > + * > + * Bit 29: TX full > + * > + * Accessible by EL0 if DBGDSCRext.UDCCdis is set to 0, which we emulate > + * as RAZ/WI in the next case. > */ > - return handle_ro_raz(regs, regidx, cp32.read, hsr, 1); > + > + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 0, 1 << 29); > + } > > case HSR_CPREG32(DBGDSCREXT): > + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); > + > + case HSR_CPREG32(DBGOSLSR): > + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1, 1 << 3); > + > + case HSR_CPREG32(DBGDTRTXINT): > + { > /* > - * Implement debug status and control register as RAZ/WI. > - * The OS won't use Hardware debug if MDBGen not set. > + * As DBGDSCRINT is emulated which is architecturally mapped to AArch64 > + * register MDCCSR_EL0. MDSCR_EL1 is not emulated. So DBGDTR[TR]XINT can > + * only be accessed as EL0 level. > */ > - return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); > + return handle_raz_wi(regs, regidx, cp32.read, hsr, 0); > + } > > case HSR_CPREG32(DBGVCR): > case HSR_CPREG32(DBGBVR0): > @@ -590,7 +613,8 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) > * > * And all other unknown registers. > */ > - default: > + default: goto fail; I think this label doesn’t belong here, it is used only in patch 3. > + fail: > gdprintk(XENLOG_ERR, > "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n", > cp32.read ? "mrc" : "mcr", > -- > 2.25.1 > >
Hi Ayan, On 18/12/2023 21:32, michal.orzel@amd.com wrote: > From: Michal Orzel <michal.orzel@amd.com> > > Currently if user enables HVC_DCC config option in Linux, it invokes > access to debug data transfer registers (ie MDCCSR_EL0 on arm64, you mention status reg on arm64 and TX reg on arm32. Instead, it should be DBGDTRTX_EL0 on arm64. > DBGDTRTXINT on arm32). As these registers are not emulated, Xen injects > an undefined exception to the guest. And Linux crashes. no need for a full stop before And > > We wish to avoid this crash by adding a "partial" emulation. DBGDTR_EL0 MDCCSR_EL0 (not DBGDTR_EL0) > is emulated as TXfull. > Refer ARM DDI 0487I.a ID081822, D17.3.8, DBGDTRTX_EL0 NIT: best practice is to refer to the latest version of a doc (i.e. J.a) > "If TXfull is set to 1, set DTRRX and DTRTX to UNKNOWN" > > Thus, any OS is expected to read DBGDTR_EL0 and check for TXfull MDCCSR_EL0 (not DBGDTR_EL0) > before using DBGDTRTX_EL0. Linux does it via hvc_dcc_init() ---> > hvc_dcc_check() , it returns -ENODEV. In this way, we are preventing > the guest to be aborted. > > We also emulate DBGDTRTX_EL0 as RAZ/WI. s/DBGDTRTX_EL0/DBGDTR[TR]X_EL0/ > > We have added emulation for AArch32 variant of these registers as well. > > Also in the case of AArch32, DBGOSLSR is 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. > Thus only MDCCSR_EL0 can be emulated (which is DBGDTRTXINT on arm32). > So, DBGDTR[TR]XINT is emulated as RAZ/WI. I can see in the code that you followed my suggestion to fix DBGDSCRINT, so that it can be accessed at EL0 (because DBGDSCREXT is RAZ). However, you do not mention it anywhere. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > 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. > > xen/arch/arm/arm64/vsysreg.c | 26 +++++++++++++++---- > xen/arch/arm/include/asm/arm64/hsr.h | 3 +++ > xen/arch/arm/include/asm/cpregs.h | 2 ++ > xen/arch/arm/include/asm/traps.h | 4 +++ > xen/arch/arm/traps.c | 18 +++++++++++++ > xen/arch/arm/vcpreg.c | 38 +++++++++++++++++++++++----- > 6 files changed, 79 insertions(+), 12 deletions(-) > > diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c > index b5d54c569b..ebeb83dd65 100644 > --- a/xen/arch/arm/arm64/vsysreg.c > +++ b/xen/arch/arm/arm64/vsysreg.c > @@ -159,9 +159,6 @@ void do_sysreg(struct cpu_user_regs *regs, > * > * Unhandled: > * MDCCINT_EL1 > - * DBGDTR_EL0 > - * DBGDTRRX_EL0 > - * DBGDTRTX_EL0 > * OSDTRRX_EL1 > * OSDTRTX_EL1 > * OSECCR_EL1 > @@ -172,11 +169,29 @@ void do_sysreg(struct cpu_user_regs *regs, > case HSR_SYSREG_MDSCR_EL1: > return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); > case HSR_SYSREG_MDCCSR_EL0: > + { > /* > + * 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 ... is full, we would hint... > + * working. > + * > + * Bit 29: TX full > + * > * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that > * register as RAZ/WI above. So RO at both EL0 and EL1. > */ > - return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0); > + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 0, > + 1U << 29); > + } > + case HSR_SYSREG_DBGDTR_EL0: > + /* DBGDTR[TR]X_EL0 share the same encoding */ > + case HSR_SYSREG_DBGDTRTX_EL0: > + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 0); > HSR_SYSREG_DBG_CASES(DBGBVR): > HSR_SYSREG_DBG_CASES(DBGBCR): > HSR_SYSREG_DBG_CASES(DBGWVR): > @@ -393,7 +408,8 @@ void do_sysreg(struct cpu_user_regs *regs, > * > * And all other unknown registers. > */ > - default: > + default: goto fail; > + fail: What is going on here? I see no use so please remove. > { > const struct hsr_sysreg sysreg = hsr.sysreg; > > diff --git a/xen/arch/arm/include/asm/arm64/hsr.h b/xen/arch/arm/include/asm/arm64/hsr.h > index e691d41c17..1495ccddea 100644 > --- a/xen/arch/arm/include/asm/arm64/hsr.h > +++ b/xen/arch/arm/include/asm/arm64/hsr.h > @@ -47,6 +47,9 @@ > #define HSR_SYSREG_OSDLR_EL1 HSR_SYSREG(2,0,c1,c3,4) > #define HSR_SYSREG_DBGPRCR_EL1 HSR_SYSREG(2,0,c1,c4,4) > #define HSR_SYSREG_MDCCSR_EL0 HSR_SYSREG(2,3,c0,c1,0) > +#define HSR_SYSREG_DBGDTR_EL0 HSR_SYSREG(2,3,c0,c4,0) > +#define HSR_SYSREG_DBGDTRTX_EL0 HSR_SYSREG(2,3,c0,c5,0) > +#define HSR_SYSREG_DBGDTRRX_EL0 HSR_SYSREG(2,3,c0,c5,0) > > #define HSR_SYSREG_DBGBVRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,4) > #define HSR_SYSREG_DBGBCRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,5) > 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/include/asm/traps.h b/xen/arch/arm/include/asm/traps.h > index 883dae368e..a2692722d5 100644 > --- a/xen/arch/arm/include/asm/traps.h > +++ b/xen/arch/arm/include/asm/traps.h > @@ -56,6 +56,10 @@ void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read, > void handle_ro_read_val(struct cpu_user_regs *regs, int regidx, bool read, > const union hsr hsr, int min_el, register_t val); > > +/* Read only as value provided with 'val' argument, write ignore */ > +void handle_read_val_wi(struct cpu_user_regs *regs, int regidx, > + const union hsr hsr, int min_el, register_t val); > + Leftover change? Please remove this prototype. > /* Co-processor registers emulation (see arch/arm/vcpreg.c). */ > void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr); > void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr); > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 3784e8276e..f5ab555b19 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1676,6 +1676,24 @@ void handle_ro_read_val(struct cpu_user_regs *regs, > advance_pc(regs, hsr); > } > > +/* Read as value provided with 'val' argument of this function, write ignore */ > +void handle_read_val_wi(struct cpu_user_regs *regs, > + int regidx, > + const union hsr hsr, > + int min_el, > + register_t val) > +{ > + ASSERT((min_el == 0) || (min_el == 1)); > + > + if ( min_el > 0 && regs_mode_is_user(regs) ) > + return inject_undef_exception(regs, hsr); > + > + set_user_reg(regs, regidx, val); > + > + advance_pc(regs, hsr); > +} Leftover change? Please remove this function. > + > + > /* Read only as read as zero */ > void handle_ro_raz(struct cpu_user_regs *regs, > int regidx, > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c > index 39aeda9dab..5087125111 100644 > --- a/xen/arch/arm/vcpreg.c > +++ b/xen/arch/arm/vcpreg.c > @@ -549,18 +549,41 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) > } > Above DBGDIDR, there is a comment listing unhandled registers. It looks like you forgot to update it. > 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. > + * > + * Bit 29: TX full > + * > + * Accessible by EL0 if DBGDSCRext.UDCCdis is set to 0, which we emulate > + * as RAZ/WI in the next case. > */ > - return handle_ro_raz(regs, regidx, cp32.read, hsr, 1); > + > + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 0, 1 << 29); Why not 1U << 29 like in AArch64 case? > + } > > case HSR_CPREG32(DBGDSCREXT): I would keep the comment that was here about MDBGen. > + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); > + > + case HSR_CPREG32(DBGOSLSR): > + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1, 1 << 3); This should be moved between DBGOSLAR and DBGOSDLR. Also, you should update a comment listing unhandled registers (above DBGOSLAR). > + Please add: /* DBGDTR[TR]XINT share the same encoding */ > + case HSR_CPREG32(DBGDTRTXINT): > + { > /* > - * Implement debug status and control register as RAZ/WI. > - * The OS won't use Hardware debug if MDBGen not set. > + * As DBGDSCRINT is emulated which is architecturally mapped to AArch64 > + * register MDCCSR_EL0. MDSCR_EL1 is not emulated. So DBGDTR[TR]XINT can > + * only be accessed as EL0 level. > */ I struggle to understand this comment. The minimum EL for DBGDTR[TR]XINT is 0, which means it can be accessed at any level. I see no need for this comment (just like there is none for arm64). > - return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); > + return handle_raz_wi(regs, regidx, cp32.read, hsr, 0); > + } > > case HSR_CPREG32(DBGVCR): > case HSR_CPREG32(DBGBVR0): > @@ -590,7 +613,8 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) > * > * And all other unknown registers. > */ > - default: > + default: goto fail; > + fail: same as before > gdprintk(XENLOG_ERR, > "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n", > cp32.read ? "mrc" : "mcr", ~Michal
diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c index b5d54c569b..ebeb83dd65 100644 --- a/xen/arch/arm/arm64/vsysreg.c +++ b/xen/arch/arm/arm64/vsysreg.c @@ -159,9 +159,6 @@ void do_sysreg(struct cpu_user_regs *regs, * * Unhandled: * MDCCINT_EL1 - * DBGDTR_EL0 - * DBGDTRRX_EL0 - * DBGDTRTX_EL0 * OSDTRRX_EL1 * OSDTRTX_EL1 * OSECCR_EL1 @@ -172,11 +169,29 @@ void do_sysreg(struct cpu_user_regs *regs, case HSR_SYSREG_MDSCR_EL1: return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); case HSR_SYSREG_MDCCSR_EL0: + { /* + * 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. + * + * Bit 29: TX full + * * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that * register as RAZ/WI above. So RO at both EL0 and EL1. */ - return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0); + return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 0, + 1U << 29); + } + case HSR_SYSREG_DBGDTR_EL0: + /* DBGDTR[TR]X_EL0 share the same encoding */ + case HSR_SYSREG_DBGDTRTX_EL0: + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 0); HSR_SYSREG_DBG_CASES(DBGBVR): HSR_SYSREG_DBG_CASES(DBGBCR): HSR_SYSREG_DBG_CASES(DBGWVR): @@ -393,7 +408,8 @@ void do_sysreg(struct cpu_user_regs *regs, * * And all other unknown registers. */ - default: + default: goto fail; + fail: { const struct hsr_sysreg sysreg = hsr.sysreg; diff --git a/xen/arch/arm/include/asm/arm64/hsr.h b/xen/arch/arm/include/asm/arm64/hsr.h index e691d41c17..1495ccddea 100644 --- a/xen/arch/arm/include/asm/arm64/hsr.h +++ b/xen/arch/arm/include/asm/arm64/hsr.h @@ -47,6 +47,9 @@ #define HSR_SYSREG_OSDLR_EL1 HSR_SYSREG(2,0,c1,c3,4) #define HSR_SYSREG_DBGPRCR_EL1 HSR_SYSREG(2,0,c1,c4,4) #define HSR_SYSREG_MDCCSR_EL0 HSR_SYSREG(2,3,c0,c1,0) +#define HSR_SYSREG_DBGDTR_EL0 HSR_SYSREG(2,3,c0,c4,0) +#define HSR_SYSREG_DBGDTRTX_EL0 HSR_SYSREG(2,3,c0,c5,0) +#define HSR_SYSREG_DBGDTRRX_EL0 HSR_SYSREG(2,3,c0,c5,0) #define HSR_SYSREG_DBGBVRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,4) #define HSR_SYSREG_DBGBCRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,5) 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/include/asm/traps.h b/xen/arch/arm/include/asm/traps.h index 883dae368e..a2692722d5 100644 --- a/xen/arch/arm/include/asm/traps.h +++ b/xen/arch/arm/include/asm/traps.h @@ -56,6 +56,10 @@ void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read, void handle_ro_read_val(struct cpu_user_regs *regs, int regidx, bool read, const union hsr hsr, int min_el, register_t val); +/* Read only as value provided with 'val' argument, write ignore */ +void handle_read_val_wi(struct cpu_user_regs *regs, int regidx, + const union hsr hsr, int min_el, register_t val); + /* Co-processor registers emulation (see arch/arm/vcpreg.c). */ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr); void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr); diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 3784e8276e..f5ab555b19 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1676,6 +1676,24 @@ void handle_ro_read_val(struct cpu_user_regs *regs, advance_pc(regs, hsr); } +/* Read as value provided with 'val' argument of this function, write ignore */ +void handle_read_val_wi(struct cpu_user_regs *regs, + int regidx, + const union hsr hsr, + int min_el, + register_t val) +{ + ASSERT((min_el == 0) || (min_el == 1)); + + if ( min_el > 0 && regs_mode_is_user(regs) ) + return inject_undef_exception(regs, hsr); + + set_user_reg(regs, regidx, val); + + advance_pc(regs, hsr); +} + + /* Read only as read as zero */ void handle_ro_raz(struct cpu_user_regs *regs, int regidx, diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c index 39aeda9dab..5087125111 100644 --- a/xen/arch/arm/vcpreg.c +++ b/xen/arch/arm/vcpreg.c @@ -549,18 +549,41 @@ 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. + * + * Bit 29: TX full + * + * Accessible by EL0 if DBGDSCRext.UDCCdis is set to 0, which we emulate + * as RAZ/WI in the next case. */ - return handle_ro_raz(regs, regidx, cp32.read, hsr, 1); + + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 0, 1 << 29); + } case HSR_CPREG32(DBGDSCREXT): + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); + + case HSR_CPREG32(DBGOSLSR): + return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1, 1 << 3); + + case HSR_CPREG32(DBGDTRTXINT): + { /* - * Implement debug status and control register as RAZ/WI. - * The OS won't use Hardware debug if MDBGen not set. + * As DBGDSCRINT is emulated which is architecturally mapped to AArch64 + * register MDCCSR_EL0. MDSCR_EL1 is not emulated. So DBGDTR[TR]XINT can + * only be accessed as EL0 level. */ - return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 0); + } case HSR_CPREG32(DBGVCR): case HSR_CPREG32(DBGBVR0): @@ -590,7 +613,8 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * * And all other unknown registers. */ - default: + default: goto fail; + fail: gdprintk(XENLOG_ERR, "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n", cp32.read ? "mrc" : "mcr",