On 09/11/17 18:36 Marc Zyngier [mailto:marc.zyngier@xxxxxxx] wrote: >On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" <liuwenliang@xxxxxxxxxx> wrote: >> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyngier@xxxxxxx] wrote: >>>On 09/11/17 07:46, Liuwenliang (Abbott Liu) wrote: >>>> diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c >>>> index 049ee0a..359a782 100644 >>>> --- a/arch/arm/mm/kasan_init.c >>>> +++ b/arch/arm/mm/kasan_init.c >>>> @@ -15,6 +15,7 @@ >>>> #include <asm/proc-fns.h> >>>> #include <asm/tlbflush.h> >>>> #include <asm/cp15.h> >>>> +#include <asm/kvm_hyp.h> >>> >>>No, please don't do that. You shouldn't have to include KVM stuff in >>>unrelated code. Instead of adding stuff to kvm_hyp.h, move all the >>>__ACCESS_CP15* to cp15.h, and it will be obvious to everyone that this >>>is where new definition should be added. >> >> Thanks for your review. You are right. It is better to move >> __ACCESS_CP15* to cp15.h than to include kvm_hyp.h. But I don't think >> it is a good idea to move registers definition which is used in >> virtualization to cp15.h, Because there is no virtualization stuff in >> cp15.h. > >It is not about virtualization at all. > >It is about what is a CP15 register and what is not. This file is called >"cp15.h", not "cp15-except-virtualization-and-maybe-some-others.h". But >at the end of the day, that's for Russell to decide. Thanks for your review. You are right, all __ACCESS_CP15* are cp15 registers. I splited normal cp15 register (such as TTBR0/TTBR1/SCTLR and so on) and virtualizaton cp15 registers(such as VTTBR/ CNTV_CVAL/HCR) because I didn't think we need use those virtualization cp15 registers in non virtualization system. But now I think all __ACCESS_CP15* move to cp15.h may be a better choise. >> >> Here is the code which I tested on vexpress_a15 and vexpress_a9: >> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h >> index dbdbce1..6db1f51 100644 >> --- a/arch/arm/include/asm/cp15.h >> +++ b/arch/arm/include/asm/cp15.h >> @@ -64,6 +64,43 @@ >> #define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v))) >> #define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__) >> >> +#ifdef CONFIG_ARM_LPAE >> +#define TTBR0 __ACCESS_CP15_64(0, c2) >> +#define TTBR1 __ACCESS_CP15_64(1, c2) >> +#define PAR __ACCESS_CP15_64(0, c7) >> +#else >> +#define TTBR0 __ACCESS_CP15(c2, 0, c0, 0) >> +#define TTBR1 __ACCESS_CP15(c2, 0, c0, 1) >> +#define PAR __ACCESS_CP15(c7, 0, c4, 0) >> +#endif > >Again: there is no point in not having these register encodings >cohabiting. They are both perfectly defined in the architecture. Just >suffix one (or even both) with their respective size, making it obvious >which one you're talking about. I am sorry that I didn't point why I need to define TTBR0/ TTBR1/PAR in to different way between CONFIG_ARM_LPAE and non CONFIG_ARM_LPAE. The following description is the reason: Here is the description come from DDI0406C2c_arm_architecture_reference_manual.pdf: B4.1.155 TTBR0, Translation Table Base Register 0, VMSA ... The Multiprocessing Extensions change the TTBR0 32-bit register format. The Large Physical Address Extension extends TTBR0 to a 64-bit register. In an implementation that includes the Large Physical Address Extension, TTBCR.EAE determines which TTBR0 format is used: EAE==0 32-bit format is used. TTBR0[63:32] are ignored. EAE==1 64-bit format is used. B4.1.156 TTBR1, Translation Table Base Register 1, VMSA ... The Multiprocessing Extensions change the TTBR0 32-bit register format. The Large Physical Address Extension extends TTBR1 to a 64-bit register. In an implementation that includes the Large Physical Address Extension, TTBCR.EAE determines which TTBR1 format is used: EAE==0 32-bit format is used. TTBR1[63:32] are ignored. EAE==1 64-bit format is used. B4.1.154 TTBCR, Translation Table Base Control Register, VMSA ... EAE, bit[31], if implementation includes the Large Physical Address Extension Extended Address Enable. The meanings of the possible values of this bit are: 0 Use the 32-bit translation system, with the Short-descriptor translation table format. In this case, the format of the TTBCR is as described in this section. 1 Use the 40-bit translation system, with the Long-descriptor translation table format. In this case, the format of the TTBCR is as described in TTBCR format when using the Long-descriptor translation table format on page B4-1692. B4.1.112 PAR, Physical Address Register, VMSA If the implementation includes the Large Physical Address Extension, the PAR is extended to be a 64-bit register and: * The 64-bit PAR is used: - when using the Long-descriptor translation table format - in an implementation that includes the Virtualization Extensions, for the result of an ATS1Cxx operation performed from Hyp mode. * The 32-bit PAR is used when using the Short-descriptor translation table format. In this case, PAR[63:32] is UNK/SBZP. Otherwise, the PAR is a 32-bit register. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href
