On Thu, Nov 16 2017 at 2:24:31 pm GMT, "Liuwenliang (Abbott Liu)" <liuwenliang@xxxxxxxxxx> wrote: > On 16/11/17 17:54 Marc Zyngier [mailto:marc.zyngier@xxxxxxx] wrote: >>On Thu, Nov 16 2017 at 3:07:54 am GMT, "Liuwenliang (Abbott Liu)" >> <liuwenliang@xxxxxxxxxx> wrote: >>>>On 15/11/17 13:16, Liuwenliang (Abbott Liu) wrote: >>>>> 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: >>>>>>> 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: >>>>[...] >>>> >>>>You're missing the point. TTBR0 existence as a 64bit CP15 register has >>>>nothing to do the kernel being compiled with LPAE or not. It has >>>>everything to do with the HW supporting LPAE, and it is the kernel's job >>>>to use the right accessor depending on how it is compiled. On a CPU >>>>supporting LPAE, both TTBR0 accessors are valid. It is the kernel that >>>>chooses to use one rather than the other. >>> >>> Thanks for your review. I don't think both TTBR0 accessors(64bit >>> accessor and 32bit accessor) are valid on a CPU supporting LPAE which >>> the LPAE is enabled. Here is the description come form >>> DDI0406C2c_arm_architecture_reference_manual.pdf (=ARM® Architecture >>> Reference Manual ARMv7-A and ARMv7-R edition) which you can get the >>> document by google "ARM® Architecture Reference Manual ARMv7-A and >>> ARMv7-R edition". > >>Trust me, from where I seat, I have a much better source than Google for >>that document. Who would have thought? > >>Nothing in what you randomly quote invalids what I've been saying. And >>to show you what's wrong with your reasoning, let me describe a >>scenario, > >>I have a non-LPAE kernel that runs on my system. It uses the 32bit >>version of the TTBRs. It turns out that this kernel runs under a >>hypervisor (KVM, Xen, or your toy of the day). The hypervisor >>context-switches vcpus without even looking at whether the configuration >>of that guest. It doesn't have to care. It just blindly uses the 64bit >>version of the TTBRs. > >>The architecture *guarantees* that it works (it even works with a 32bit >>guest under a 64bit hypervisor). In your world, this doesn't work. I >>guess the architecture wins. > >>> So, I think if you access TTBR0/TTBR1 on CPU supporting LPAE, you must >>> use "mcrr/mrrc" instruction (__ACCESS_CP15_64). If you access >>> TTBR0/TTBR1 on CPU supporting LPAE by "mcr/mrc" instruction which is >>> 32bit version (__ACCESS_CP15), even if the CPU doesn't report error, >>> you also lose the high or low 32bit of the TTBR0/TTBR1. > >>It is not about "supporting LPAE". It is about using the accessor that >>makes sense in a particular context. Yes, the architecture allows you to >>do something stupid. Don't do it. It doesn't mean the accessors cannot >>be used, and I hope that my example above demonstrates it. > >>Conclusion: I still stand by my request that both versions of TTBRs/PAR >>are described without depending on the kernel configuration, because >>this has nothing to do with the kernel configuration. > > Thanks for your reviews. > Yes, you are right. I have tested that "mcrr/mrrc" instruction > (__ACCESS_CP15_64) can work on non LPAE on vexpress_a9. No, it doesn't. It cannot work, because Cortex-A9 predates the invention of the 64bit accessor. I suspect that you are testing stuff in QEMU, which is giving you a SW model that always supports LPAE. I suggest you test this code on *real* HW, and not only on QEMU. What I have said is: - If the CPU supports LPAE, then both 32 and 64bit accessors work - If the CPU doesn't support LPAE, then only the 32bit accssor work - In both cases, that's a function of the CPU, and not of the kernel configuration. - Both accessors can be safely defined as long as we ensure that they are used in the right context. > Here is the code I tested on vexpress_a9 and vexpress_a15: > --- a/arch/arm/include/asm/cp15.h > +++ b/arch/arm/include/asm/cp15.h > @@ -64,6 +64,56 @@ > #define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v))) > #define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__) > > +#define TTBR0 __ACCESS_CP15_64(0, c2) > +#define TTBR1 __ACCESS_CP15_64(1, c2) > +#define PAR __ACCESS_CP15_64(0, c7) You still need to add the 32bit accessors. M. -- Jazz is not dead, it just smell funny. -- 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
