From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f70.google.com (mail-oi0-f70.google.com [209.85.218.70]) by kanga.kvack.org (Postfix) with ESMTP id 570594402ED for ; Thu, 16 Nov 2017 09:40:52 -0500 (EST) Received: by mail-oi0-f70.google.com with SMTP id a75so11527606oib.13 for ; Thu, 16 Nov 2017 06:40:52 -0800 (PST) Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com. [217.140.101.70]) by mx.google.com with ESMTP id x5si418671oig.131.2017.11.16.06.40.50 for ; Thu, 16 Nov 2017 06:40:50 -0800 (PST) From: Marc Zyngier Subject: Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory In-Reply-To: (liuwenliang@huawei.com's message of "Thu, 16 Nov 2017 14:24:31 +0000") References: <20171011082227.20546-1-liuwenliang@huawei.com> <20171011082227.20546-2-liuwenliang@huawei.com> <227e2c6e-f479-849d-8942-1d5ff4ccd440@arm.com> <8e959f69-a578-793b-6c32-18b5b0cd08c2@arm.com> <87a7znsubp.fsf@on-the-bus.cambridge.arm.com> <87po8ir1kg.fsf@on-the-bus.cambridge.arm.com> Date: Thu, 16 Nov 2017 14:40:40 +0000 Message-ID: <87375eqobb.fsf@on-the-bus.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Sender: owner-linux-mm@kvack.org List-ID: To: "Liuwenliang (Abbott Liu)" Cc: "linux@armlinux.org.uk" , "aryabinin@virtuozzo.com" , "afzal.mohd.ma@gmail.com" , "f.fainelli@gmail.com" , "labbott@redhat.com" , "kirill.shutemov@linux.intel.com" , "mhocko@suse.com" , "cdall@linaro.org" , "catalin.marinas@arm.com" , "akpm@linux-foundation.org" , "mawilcox@microsoft.com" , "tglx@linutronix.de" , "thgarnie@google.com" , "keescook@chromium.org" , "arnd@arndb.de" , "vladimir.murzin@arm.com" , "tixy@linaro.org" , "ard.biesheuvel@linaro.org" , "robin.murphy@arm.com" , "mingo@kernel.org" , "grygorii.strashko@linaro.org" , "glider@google.com" , "dvyukov@google.com" , "opendmb@gmail.com" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "kasan-dev@googlegroups.com" , "linux-mm@kvack.org" , Jiazhenghua , Dailei , Zengweilin , Heshaoliang On Thu, Nov 16 2017 at 2:24:31 pm GMT, "Liuwenliang (Abbott Liu)" wrote: > On 16/11/17 17:54 Marc Zyngier [mailto:marc.zyngier@arm.com] wrote: >>On Thu, Nov 16 2017 at 3:07:54 am GMT, "Liuwenliang (Abbott Liu)" >> wrote: >>>>On 15/11/17 13:16, Liuwenliang (Abbott Liu) wrote: >>>>> On 09/11/17 18:36 Marc Zyngier [mailto:marc.zyngier@arm.com] wrote: >>>>>> On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" >>>>>> 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 (=3DARM=C2=AE Architec= ture >>> Reference Manual ARMv7-A and ARMv7-R edition) which you can get the >>> document by google "ARM=C2=AE 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. --=20 Jazz is not dead, it just smell funny. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org