diff mbox series

[RFC,1/3] target/arm: Add dcc uart support

Message ID 20240614093026.328271-2-sai.pavan.boddu@amd.com
State New
Headers show
Series Add DCC uart console support | expand

Commit Message

Sai Pavan Boddu June 14, 2024, 9:30 a.m. UTC 
DCC is a debug port to transfer some data between debugger and
processor, we are using this feature to connect a chardev device.
Chardev frontends should be named as "dcc<cpu-index>" inorder to connect
to this interface.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@amd.com>
---
 target/arm/cpu.h       | 11 +++++
 target/arm/internals.h |  4 ++
 target/arm/debug-dcc.c | 99 ++++++++++++++++++++++++++++++++++++++++++
 target/arm/helper.c    |  3 ++
 target/arm/meson.build |  1 +
 5 files changed, 118 insertions(+)
 create mode 100644 target/arm/debug-dcc.c

Comments

Peter Maydell June 21, 2024, 3:42 p.m. UTC | #1 
On Fri, 14 Jun 2024 at 10:30, Sai Pavan Boddu <sai.pavan.boddu@amd.com> wrote:
>
> DCC is a debug port to transfer some data between debugger and
> processor, we are using this feature to connect a chardev device.
> Chardev frontends should be named as "dcc<cpu-index>" inorder to connect
> to this interface.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@amd.com>
> ---
>  target/arm/cpu.h       | 11 +++++
>  target/arm/internals.h |  4 ++
>  target/arm/debug-dcc.c | 99 ++++++++++++++++++++++++++++++++++++++++++
>  target/arm/helper.c    |  3 ++
>  target/arm/meson.build |  1 +
>  5 files changed, 118 insertions(+)
>  create mode 100644 target/arm/debug-dcc.c
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 3841359d0f..6b3cb8e70e 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -30,6 +30,8 @@
>  #include "qapi/qapi-types-common.h"
>  #include "target/arm/multiprocessing.h"
>  #include "target/arm/gtimer.h"
> +#include "chardev/char.h"
> +#include "chardev/char-fe.h"
>
>  #ifdef TARGET_AARCH64
>  #define KVM_HAVE_MCE_INJECTION 1
> @@ -523,6 +525,11 @@ typedef struct CPUArchState {
>
>          /* NV2 register */
>          uint64_t vncr_el2;
> +        /*
> +         * Debug Trace regsiters
> +         */
> +        uint32_t dbgdtr_tx;
> +        uint32_t dbgdtr_rx;
>      } cp15;
>
>      struct {
> @@ -1097,6 +1104,9 @@ struct ArchCPU {
>
>      /* Generic timer counter frequency, in Hz */
>      uint64_t gt_cntfrq_hz;
> +
> +    /* dcc chardev */
> +    CharBackend dcc;
>  };
>
>  typedef struct ARMCPUInfo {
> @@ -2388,6 +2398,7 @@ enum arm_features {
>       * CPU types added in future.
>       */
>      ARM_FEATURE_BACKCOMPAT_CNTFRQ, /* 62.5MHz timer default */
> +    ARM_FEATURE_DCC,

Generally speaking we want to avoid adding new ARM_FEATURE bits
if there's some way to identify the presence of the feature via
the ID registers. In this case, the debug registers always exist
as long as we have ARM_FEATURE_V7, I think. So I think we should
always provide this as part of the registers we define in
debug_cp_reginfo[] in debug_helper.c, rather than having a
separate feature bit.

>  };
>
>  static inline int arm_feature(CPUARMState *env, int feature)
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 11b5da2562..2fa797c5df 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1778,4 +1778,8 @@ uint64_t gt_get_countervalue(CPUARMState *env);
>   * and CNTVCT_EL0 (this will be either 0 or the value of CNTVOFF_EL2).
>   */
>  uint64_t gt_virt_cnt_offset(CPUARMState *env);
> +/*
> + * Initialise Coresight Debug interface
> + */
> +void arm_dcc_init(ARMCPU *cpu);
>  #endif
> diff --git a/target/arm/debug-dcc.c b/target/arm/debug-dcc.c
> new file mode 100644
> index 0000000000..9144b54994
> --- /dev/null
> +++ b/target/arm/debug-dcc.c
> @@ -0,0 +1,99 @@
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "internals.h"
> +#include "cpregs.h"
> +#include "chardev/char-fe.h"
> +
> +#define MDCCSR_EL0_RXFULL_MASK (1 << 30)
> +#define MDCCSR_EL0_TXFULL_MASK (1 << 29)
> +
> +static void debug_dcc_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                        uint64_t value)
> +{
> +    ARMCPU *cpu = ri->opaque;
> +    env->cp15.dbgdtr_tx = value;
> +
> +    if (qemu_chr_fe_get_driver(&cpu->dcc)) {

This is a weird function to have to call. Generally we set things
up so that you don't need to care if there's actually anything
connected to the chardev -- qemu_chr_fe_write() and friends will
do nothing if there's no chardev attached to this backend.

> +        /*
> +         * Usually dcc is used for putc/getc calls which expect only
> +         * 1 byte from external debugger.
> +         * TODO: This needs to be generalized for other use-cases.
> +         */

Is this referring to the way that the DCC is defined as a 64-bit
(or 32-bit for AArch32) channel but we're ignoring all but the bottom
8 bits of the values?

> +        qemu_chr_fe_write_all(&cpu->dcc, (uint8_t *)&env->cp15.dbgdtr_tx, 1);

The DCC has flow-control flags so we don't need to use _write_all, and
generally we should avoid that in new code because it might block.

> +    }
> +}
> +
> +static uint64_t debug_dcc_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    uint32_t ret = 0;
> +    ARMCPU *cpu = ri->opaque;
> +
> +    if (env->cp15.mdscr_el1 & MDCCSR_EL0_RXFULL_MASK) {
> +        ret = env->cp15.dbgdtr_rx;
> +        env->cp15.dbgdtr_rx = 0;
> +        env->cp15.mdscr_el1 &= ~MDCCSR_EL0_RXFULL_MASK;
> +        qemu_chr_fe_accept_input(&cpu->dcc);
> +    }
> +    return ret;
> +}
> +
> +static const ARMCPRegInfo dcc_cp_reginfo[] = {
> +    /* DBGDTRTX_EL0/DBGDTRRX_EL0 depend on direction */
> +    { .name = "DBGDTR_EL0", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 5, .opc2 = 0,
> +      .access = PL0_RW, .writefn = debug_dcc_write,
> +      .readfn = debug_dcc_read,
> +      .type = ARM_CP_OVERRIDE, .resetvalue = 0 },
> +    /* DBGDTRTXint/DBGDTRRXint depend on direction */
> +    { .name = "DBGDTRint", .state = ARM_CP_STATE_AA32, .cp = 14,
> +      .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0,
> +      .access = PL0_RW, .writefn = debug_dcc_write,
> +      .readfn = debug_dcc_read,
> +      .type = ARM_CP_OVERRIDE, .resetvalue = 0 },
> +};

Other registers that need updates for a real DCC:
 * MDCCSR_EL0
 * OSDTRRX_EL1
 * OSDTRTX_EL1

(all of which we currently have dummy versions of in
debug_helper.c).

> +
> +
> +static int dcc_chr_can_read(void *opaque)
> +{
> +    ARMCPU *cpu = opaque;
> +    CPUARMState *env = &cpu->env;
> +
> +    if (!(env->cp15.mdscr_el1 & MDCCSR_EL0_RXFULL_MASK)) {
> +        /*
> +         * Usually dcc is used for putc/getc calls which expect only
> +         * 1 byte from external debugger.
> +         * TODO: This needs to be generalized for other use-cases.
> +         */
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +static void dcc_chr_read(void *opaque, const uint8_t *buf, int size)
> +{
> +    ARMCPU *cpu = opaque;
> +    CPUARMState *env = &cpu->env;
> +
> +    env->cp15.dbgdtr_rx = *buf;
> +    env->cp15.mdscr_el1 |= MDCCSR_EL0_RXFULL_MASK;
> +}
> +
> +void arm_dcc_init(ARMCPU *cpu)
> +{
> +    Chardev *chr;
> +    char *dcc_name;
> +    CPUState *p = CPU(cpu);
> +
> +    dcc_name = g_strdup_printf("dcc%d", p->cpu_index);
> +    chr = qemu_chr_find(dcc_name);
> +    define_arm_cp_regs_with_opaque(cpu, dcc_cp_reginfo, cpu);
> +    if (chr) {
> +        qemu_chr_fe_init(&cpu->dcc, chr, NULL);
> +        qemu_chr_fe_set_handlers(&cpu->dcc,
> +                      dcc_chr_can_read,
> +                      dcc_chr_read,
> +                      NULL, NULL, cpu, NULL, true);
> +    }
> +    g_free(dcc_name);
> +}

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 3841359d0f..6b3cb8e70e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -30,6 +30,8 @@ 
 #include "qapi/qapi-types-common.h"
 #include "target/arm/multiprocessing.h"
 #include "target/arm/gtimer.h"
+#include "chardev/char.h"
+#include "chardev/char-fe.h"
 
 #ifdef TARGET_AARCH64
 #define KVM_HAVE_MCE_INJECTION 1
@@ -523,6 +525,11 @@  typedef struct CPUArchState {
 
         /* NV2 register */
         uint64_t vncr_el2;
+        /*
+         * Debug Trace regsiters
+         */
+        uint32_t dbgdtr_tx;
+        uint32_t dbgdtr_rx;
     } cp15;
 
     struct {
@@ -1097,6 +1104,9 @@  struct ArchCPU {
 
     /* Generic timer counter frequency, in Hz */
     uint64_t gt_cntfrq_hz;
+
+    /* dcc chardev */
+    CharBackend dcc;
 };
 
 typedef struct ARMCPUInfo {
@@ -2388,6 +2398,7 @@  enum arm_features {
      * CPU types added in future.
      */
     ARM_FEATURE_BACKCOMPAT_CNTFRQ, /* 62.5MHz timer default */
+    ARM_FEATURE_DCC,
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 11b5da2562..2fa797c5df 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1778,4 +1778,8 @@  uint64_t gt_get_countervalue(CPUARMState *env);
  * and CNTVCT_EL0 (this will be either 0 or the value of CNTVOFF_EL2).
  */
 uint64_t gt_virt_cnt_offset(CPUARMState *env);
+/*
+ * Initialise Coresight Debug interface
+ */
+void arm_dcc_init(ARMCPU *cpu);
 #endif
diff --git a/target/arm/debug-dcc.c b/target/arm/debug-dcc.c
new file mode 100644
index 0000000000..9144b54994
--- /dev/null
+++ b/target/arm/debug-dcc.c
@@ -0,0 +1,99 @@ 
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "internals.h"
+#include "cpregs.h"
+#include "chardev/char-fe.h"
+
+#define MDCCSR_EL0_RXFULL_MASK (1 << 30)
+#define MDCCSR_EL0_TXFULL_MASK (1 << 29)
+
+static void debug_dcc_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                        uint64_t value)
+{
+    ARMCPU *cpu = ri->opaque;
+    env->cp15.dbgdtr_tx = value;
+
+    if (qemu_chr_fe_get_driver(&cpu->dcc)) {
+        /*
+         * Usually dcc is used for putc/getc calls which expect only
+         * 1 byte from external debugger.
+         * TODO: This needs to be generalized for other use-cases.
+         */
+        qemu_chr_fe_write_all(&cpu->dcc, (uint8_t *)&env->cp15.dbgdtr_tx, 1);
+    }
+}
+
+static uint64_t debug_dcc_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    uint32_t ret = 0;
+    ARMCPU *cpu = ri->opaque;
+
+    if (env->cp15.mdscr_el1 & MDCCSR_EL0_RXFULL_MASK) {
+        ret = env->cp15.dbgdtr_rx;
+        env->cp15.dbgdtr_rx = 0;
+        env->cp15.mdscr_el1 &= ~MDCCSR_EL0_RXFULL_MASK;
+        qemu_chr_fe_accept_input(&cpu->dcc);
+    }
+    return ret;
+}
+
+static const ARMCPRegInfo dcc_cp_reginfo[] = {
+    /* DBGDTRTX_EL0/DBGDTRRX_EL0 depend on direction */
+    { .name = "DBGDTR_EL0", .state = ARM_CP_STATE_AA64,
+      .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 5, .opc2 = 0,
+      .access = PL0_RW, .writefn = debug_dcc_write,
+      .readfn = debug_dcc_read,
+      .type = ARM_CP_OVERRIDE, .resetvalue = 0 },
+    /* DBGDTRTXint/DBGDTRRXint depend on direction */
+    { .name = "DBGDTRint", .state = ARM_CP_STATE_AA32, .cp = 14,
+      .opc1 = 0, .crn = 0, .crm = 5, .opc2 = 0,
+      .access = PL0_RW, .writefn = debug_dcc_write,
+      .readfn = debug_dcc_read,
+      .type = ARM_CP_OVERRIDE, .resetvalue = 0 },
+};
+
+
+static int dcc_chr_can_read(void *opaque)
+{
+    ARMCPU *cpu = opaque;
+    CPUARMState *env = &cpu->env;
+
+    if (!(env->cp15.mdscr_el1 & MDCCSR_EL0_RXFULL_MASK)) {
+        /*
+         * Usually dcc is used for putc/getc calls which expect only
+         * 1 byte from external debugger.
+         * TODO: This needs to be generalized for other use-cases.
+         */
+        return 1;
+    }
+
+    return 0;
+}
+
+static void dcc_chr_read(void *opaque, const uint8_t *buf, int size)
+{
+    ARMCPU *cpu = opaque;
+    CPUARMState *env = &cpu->env;
+
+    env->cp15.dbgdtr_rx = *buf;
+    env->cp15.mdscr_el1 |= MDCCSR_EL0_RXFULL_MASK;
+}
+
+void arm_dcc_init(ARMCPU *cpu)
+{
+    Chardev *chr;
+    char *dcc_name;
+    CPUState *p = CPU(cpu);
+
+    dcc_name = g_strdup_printf("dcc%d", p->cpu_index);
+    chr = qemu_chr_find(dcc_name);
+    define_arm_cp_regs_with_opaque(cpu, dcc_cp_reginfo, cpu);
+    if (chr) {
+        qemu_chr_fe_init(&cpu->dcc, chr, NULL);
+        qemu_chr_fe_set_handlers(&cpu->dcc,
+                      dcc_chr_can_read,
+                      dcc_chr_read,
+                      NULL, NULL, cpu, NULL, true);
+    }
+    g_free(dcc_name);
+}
diff --git a/target/arm/helper.c b/target/arm/helper.c
index ce31957235..2b594f91cb 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9268,6 +9268,9 @@  void register_cp_regs_for_features(ARMCPU *cpu)
         }
     }
 
+    if (arm_feature(&cpu->env, ARM_FEATURE_DCC)) {
+        arm_dcc_init(cpu);
+    }
     /*
      * Register the base EL2 cpregs.
      * Pre v8, these registers are implemented only as part of the
diff --git a/target/arm/meson.build b/target/arm/meson.build
index 2e10464dbb..3ee38c6b45 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -5,6 +5,7 @@  arm_ss.add(files(
   'gdbstub.c',
   'helper.c',
   'vfp_helper.c',
+  'debug-dcc.c',
 ))
 arm_ss.add(zlib)