Hi Ayan,
On 05/01/2024 12:21, Ayan Kumar Halder wrote:
> From: Michal Orzel <michal.or...@amd.com>
>
> 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 VM and Linux (running as VM) crashes.
>
> We wish to avoid this crash by adding a partial emulation of DBGDTRTX_EL0.
> MDCCSR_EL0 is emulated as TXfull.
>
> Refer ARM DDI 0487J.a ID042523, D19.3.8, DBGDTRTX_EL0
> "If TXfull is set to 1, set DTRRX and DTRTX to UNKNOWN".
>
> Thus, any OS is expected to read MDCCSR_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 VM from accessing
> DBGDTRTX_EL0 register.
AFAIK, we are preventing the VM from making use of HVC DCC as a console.
DBGDTRTX_EL0 and MDCCSR_EL0 will be accessed at least once as part of
hvc_dcc_check().
>
> We also emulate DBGDTR[TR]X_EL0 as RAZ/WI.
>
> Signed-off-by: Michal Orzel <michal.or...@amd.com>
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.hal...@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.
>
> v2 :- 1. Split the patch into two (separate patches for arm64 and arm32).
> 2. Removed the "fail" label.
> 3. Fixed the commit message.
>
> xen/arch/arm/arm64/vsysreg.c | 25 +++++++++++++++++++++----
> xen/arch/arm/include/asm/arm64/hsr.h | 3 +++
> 2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> index b5d54c569b..2f70eea2e5 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,31 @@ 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, 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);
I would assume that if partial emulation is not enabled, we should stick to
ro_raz
instead of emulating TX set.
> + }
> +#ifdef CONFIG_PARTIAL_EMULATION
> + case HSR_SYSREG_DBGDTR_EL0:
> + /* DBGDTR[TR]X_EL0 share the same encoding */
> + case HSR_SYSREG_DBGDTRTX_EL0:
> + if ( opt_partial_emulation )
> + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 0);
This is incorrect. What will happen if CONFIG_PARTIAL_EMULATION=y and
opt_partial_emulation=false ?
You would fall through to cases below and end up in RAZ/WI. Instead, you should
jump to default case,
so that we would inject undefined exception.
> +#endif
> HSR_SYSREG_DBG_CASES(DBGBVR):
> HSR_SYSREG_DBG_CASES(DBGBCR):
> HSR_SYSREG_DBG_CASES(DBGWVR):
> 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)
~Michal
