From: Abhimanyu Kapur <[email protected]>
Add support for debug communications channel based
hvc console for arm64 cpus.
Signed-off-by: Abhimanyu Kapur <[email protected]>
Signed-off-by: Timur Tabi <[email protected]>
---
arch/arm64/include/asm/dcc.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
drivers/tty/hvc/Kconfig | 2 +-
2 files changed, 50 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/dcc.h
diff --git a/arch/arm64/include/asm/dcc.h b/arch/arm64/include/asm/dcc.h
new file mode 100644
index 0000000..f038d61
--- /dev/null
+++ b/arch/arm64/include/asm/dcc.h
@@ -0,0 +1,49 @@
+/* Copyright (c) 2014 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * A call to __dcc_getchar() or __dcc_putchar() is typically followed by
+ * a call to __dcc_getstatus(). We want to make sure that the CPU does
+ * not speculative read the DCC status before executing the read or write
+ * instruction. That's what the ISBs are for.
+ *
+ * The 'volatile' ensures that the compiler does not cache the status bits,
+ * and instead reads the DCC register every time.
+ */
+
+#include <asm/barrier.h>
+
+static inline u32 __dcc_getstatus(void)
+{
+ u32 __ret;
+
+ asm volatile("mrs %0, mdccsr_el0" : "=r" (__ret)
+ : : "cc");
+
+ return __ret;
+}
+
+static inline char __dcc_getchar(void)
+{
+ char __c;
+
+ asm volatile("mrs %0, dbgdtrrx_el0" : "=r" (__c));
+ isb();
+
+ return __c;
+}
+
+static inline void __dcc_putchar(char c)
+{
+ asm volatile("msr dbgdtrtx_el0, %0"
+ : /* No output register */
+ : "r" (c));
+ isb();
+}
diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 2c6883c..9a60d18 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -88,7 +88,7 @@ config HVC_UDBG
config HVC_DCC
bool "ARM JTAG DCC console"
- depends on ARM
+ depends on ARM || ARM64
select HVC_DRIVER
help
This console uses the JTAG DCC on ARM to create a console under the HVC
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
On Fri, Jun 19, 2015 at 11:08:54PM +0100, Timur Tabi wrote:
> From: Abhimanyu Kapur <[email protected]>
>
> Add support for debug communications channel based
> hvc console for arm64 cpus.
I still think we should be disabling userspace access to the DCC if the
kernel is using it as its console.
> Signed-off-by: Abhimanyu Kapur <[email protected]>
> Signed-off-by: Timur Tabi <[email protected]>
> ---
> arch/arm64/include/asm/dcc.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/tty/hvc/Kconfig | 2 +-
> 2 files changed, 50 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/dcc.h
>
> diff --git a/arch/arm64/include/asm/dcc.h b/arch/arm64/include/asm/dcc.h
> new file mode 100644
> index 0000000..f038d61
> --- /dev/null
> +++ b/arch/arm64/include/asm/dcc.h
> @@ -0,0 +1,49 @@
> +/* Copyright (c) 2014 The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * A call to __dcc_getchar() or __dcc_putchar() is typically followed by
> + * a call to __dcc_getstatus(). We want to make sure that the CPU does
> + * not speculative read the DCC status before executing the read or write
> + * instruction. That's what the ISBs are for.
> + *
> + * The 'volatile' ensures that the compiler does not cache the status bits,
> + * and instead reads the DCC register every time.
> + */
Missing header guards.
> +#include <asm/barrier.h>
> +
> +static inline u32 __dcc_getstatus(void)
> +{
> + u32 __ret;
> +
> + asm volatile("mrs %0, mdccsr_el0" : "=r" (__ret)
> + : : "cc");
You don't need the "cc" clobber.
Will
Will Deacon wrote:
> On Fri, Jun 19, 2015 at 11:08:54PM +0100, Timur Tabi wrote:
>> From: Abhimanyu Kapur <[email protected]>
>>
>> Add support for debug communications channel based
>> hvc console for arm64 cpus.
>
> I still think we should be disabling userspace access to the DCC if the
> kernel is using it as its console.
I don't disagree, I just don't know how to do that.
>> + * A call to __dcc_getchar() or __dcc_putchar() is typically followed by
>> + * a call to __dcc_getstatus(). We want to make sure that the CPU does
>> + * not speculative read the DCC status before executing the read or write
>> + * instruction. That's what the ISBs are for.
>> + *
>> + * The 'volatile' ensures that the compiler does not cache the status bits,
>> + * and instead reads the DCC register every time.
>> + */
>
> Missing header guards.
Will fix.
>> +#include <asm/barrier.h>
>> +
>> +static inline u32 __dcc_getstatus(void)
>> +{
>> + u32 __ret;
>> +
>> + asm volatile("mrs %0, mdccsr_el0" : "=r" (__ret)
>> + : : "cc");
>
> You don't need the "cc" clobber.
Will fix.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
On 06/22/2015 08:12 AM, Will Deacon wrote:
> I still think we should be disabling userspace access to the DCC if the
> kernel is using it as its console.
I still need help with this. I know you said a year ago that
MDSCR_EL1.TDCC needs to be set to disable userspace access. Where and
how should I do this? I can do this:
static int __init hvc_dcc_console_init(void)
{
#ifdef CONFIG_ARM64
u32 val;
asm("msr mdscr_el1, %0 "
"orr %0, %0, #4096 " /* TDCC */
"msr %0, mdscr_el1 "
: "=r" (val));
#endif
But this seems clunky.
I am concerned about KVM, though. There appears to be code in KVM in
hyp.s and sys_regs.c that touches and/or emulates MDSCR_EL1.
On a side note, it does not appear that ARM32 blocks userspace DCC. I
don't see where DBGDSCR.UDCCdis is set.
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
On Wed, Jun 24, 2015 at 09:11:24PM +0100, Timur Tabi wrote:
> On 06/22/2015 08:12 AM, Will Deacon wrote:
> > I still think we should be disabling userspace access to the DCC if the
> > kernel is using it as its console.
>
> I still need help with this. I know you said a year ago that
> MDSCR_EL1.TDCC needs to be set to disable userspace access. Where and
> how should I do this? I can do this:
Well, it's up to you to figure out the details, but I'd start by adding
some static inlines to the arch-specific header files for enabling/disabling
userspace access.
>From there, I think I'd get the architecture init code to reset the thing
to "disabled" (so it's disabled regardless of whether we build the hvc_dcc
driver) and then if you wanted to go all-out, we could have a sysfs entry
provided by the driver to toggle it on and off.
> static int __init hvc_dcc_console_init(void)
> {
> #ifdef CONFIG_ARM64
> u32 val;
>
> asm("msr mdscr_el1, %0 "
> "orr %0, %0, #4096 " /* TDCC */
> "msr %0, mdscr_el1 "
> : "=r" (val));
> #endif
>
> But this seems clunky.
Yeah, that's super ugly.
> I am concerned about KVM, though. There appears to be code in KVM in
> hyp.s and sys_regs.c that touches and/or emulates MDSCR_EL1.
>
> On a side note, it does not appear that ARM32 blocks userspace DCC. I
> don't see where DBGDSCR.UDCCdis is set.
That's a bug imo.
Will
Will Deacon wrote:
> Well, it's up to you to figure out the details, but I'd start by adding
> some static inlines to the arch-specific header files for enabling/disabling
> userspace access.
>
> From there, I think I'd get the architecture init code to reset the thing
> to "disabled" (so it's disabled regardless of whether we build the hvc_dcc
> driver) and then if you wanted to go all-out, we could have a sysfs entry
> provided by the driver to toggle it on and off.
>
>> >static int __init hvc_dcc_console_init(void)
>> >{
>> >#ifdef CONFIG_ARM64
>> > u32 val;
>> >
>> > asm("msr mdscr_el1, %0 "
>> > "orr %0, %0, #4096 " /* TDCC */
>> > "msr %0, mdscr_el1 "
>> > : "=r" (val));
>> >#endif
>> >
>> >But this seems clunky.
> Yeah, that's super ugly.
>
>> >I am concerned about KVM, though. There appears to be code in KVM in
>> >hyp.s and sys_regs.c that touches and/or emulates MDSCR_EL1.
>> >
>> >On a side note, it does not appear that ARM32 blocks userspace DCC. I
>> >don't see where DBGDSCR.UDCCdis is set.
> That's a bug imo.
So wouldn't it be more appropriate to have a separate patch that handles
disabling of user-space DCC for ARM32 and ARM64? All I really want to
do at this point is provide basic DCC support for ARM64, just like we
have for ARM32.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.