On Tue, Jun 17, 2014 at 4:51 AM, Will Deacon <[email protected]> wrote: > On Mon, Jun 16, 2014 at 11:29:38PM +0100, Abhimanyu Kapur wrote: >> Add support for debug communications channel based >> hvc console for arm64 cpus.
I know it's been a year since this patch was posted, but I'm working on re-submitting it. Unfortunately, I can't answer most of your questions, but I want to try to fix this patch up and get it accepted. > Should we be setting MDSCR_EL1.TDCC to prevent userspace access to the DCC? I personally don't know enough about ARM to answer that question, but it does makes sense that we don't want userspace to access the DCC registers. Where would MDSCR_EL1.TDCC be set? Should we add it to hvc_dcc_init()? >> Signed-off-by: Abhimanyu Kapur <[email protected]> >> --- >> arch/arm64/include/asm/dcc.h | 41 +++++++++++++++++++++++++++++++++++++++++ >> drivers/tty/hvc/Kconfig | 2 +- >> 2 files changed, 42 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..ef74324 >> --- /dev/null >> +++ b/arch/arm64/include/asm/dcc.h >> @@ -0,0 +1,41 @@ >> +/* 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. >> + */ >> + >> +#include <asm/barrier.h> >> + >> +static inline u32 __dcc_getstatus(void) >> +{ >> + u32 __ret; > > Can this result in the mrs receiving a W register? Another question I can't answer. I understand what you're saying, but I don't know what to do about it. >> + asm volatile("mrs %0, mdccsr_el0" : "=r" (__ret) >> + : : "cc"); > > Why the CC clobber? Why volatile? I don't think we need the CC clobber, but the volatiles do appear to be important. Without that, DCC does not work. >> + >> + return __ret; >> +} >> + >> +static inline char __dcc_getchar(void) >> +{ >> + char __c; >> + >> + asm volatile("mrs %0, dbgdtrrx_el0" : "=r" (__c)); >> + isb(); > > Why the isb and why volatile?? The driver does appear to work without the isb, but there's a reason it was added. On ARM32, reading/writing the DCC register is apparently a context change: http://www.serverphorums.com/read.php?12,395361 I don't know if the same thing is true on ARM64. However, I don't understand why reading/writing the DCC register is a context change on ARM32. >> + >> + return __c; >> +} >> + >> +static inline void __dcc_putchar(char c) >> +{ >> + asm volatile("msr dbgdtrtx_el0, %0" >> + : /* No output register */ >> + : "r" (c)); > > Can you guarantee that GCC hasn't put junk in the upper bits of c? Should I change the prototype to: static inline void __dcc_putchar(unsigned int c) > >> + isb(); > > Why the isb? > > Will > > _______________________________________________ > linux-arm-kernel mailing list > [email protected] > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
