Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Oct 19, 2017 at 19:09, Russell King - ARM Linux 
[mailto:li...@armlinux.org.uk] wrote:
>On Wed, Oct 11, 2017 at 04:22:17PM +0800, Abbott Liu wrote:
>> +#else
>> +#define pud_populate(mm,pmd,pte)do { } while (0)
>> +#endif
>
>Please explain this change - we don't have a "pud" as far as the rest of
>the Linux MM layer is concerned, so why do we need it for kasan?
>
>I suspect it comes from the way we wrap up the page tables - where ARM
>does it one way (because it has to) vs the subsequently merged method
>which is completely upside down to what ARMs doing, and therefore is
>totally incompatible and impossible to fit in with our way.

We will use pud_polulate in kasan_populate_zero_shadow function.

>>  obj-$(CONFIG_CACHE_TAUROS2) += cache-tauros2.o
>> +
>> +KASAN_SANITIZE_kasan_init.o:= n
>> +obj-$(CONFIG_KASAN)+= kasan_init.o
>
>Why is this placed in the middle of the cache object listing?

Sorry, I will place this at the end of the arch/arm/mm/Makefile.
>> +
>> +
>>  obj-$(CONFIG_CACHE_UNIPHIER)+= cache-uniphier.o
...

>> +pgd_t * __meminit kasan_pgd_populate(unsigned long addr, int node)
>> +{
>> +pgd_t *pgd = pgd_offset_k(addr);
>> +if (pgd_none(*pgd)) {
>> +void *p = kasan_alloc_block(PAGE_SIZE, node);
>> +if (!p)
>> +return NULL;
>> +pgd_populate(_mm, pgd, p);
>> +}
>> +return pgd;
>> +}

>This all looks wrong - you are aware that on non-LPAE platforms, there
>is only a _two_ level page table - the top level page table is 16K in
>size, and each _individual_ lower level page table is actually 1024
>bytes, but we do some special handling in the kernel to combine two
>together.  It looks to me that you allocate memory for each Linux-
>abstracted page table level whether the hardware needs it or not.

You are right. If non-LPAE platform check if(pgd_none(*pgd)) true,
void *p = kasan_alloc_block(PAGE_SIZE, node) alloc space is not enough.
But the the function kasan_pgd_populate only used in :
Kasan_init-> create_mapping-> kasan_pgd_populate , so when non-LPAE platform
the if (pgd_none(*pgd)) always false.
But I also think change those code is much better :
if (IS_ENABLED(CONFIG_ARM_LPAE)) {
   p = kasan_alloc_block(PAGE_SIZE, node);
} else {
   /* non-LPAE need 16K for first level pagetabe*/
   p = kasan_alloc_block(PAGE_SIZE*4, node);
}

>Is there any reason why the pre-existing "create_mapping()" function
>can't be used, and you've had to rewrite that code here?

Two reason:
1) Here create_mapping can dynamic alloc phys memory space for mapping to 
virtual space 
Which from start to end, but the create_mapping in arch/arm/mm/mmu.c can't.
2) for LPAE, create_mapping need alloc pgd which we need use virtual space 
below 0xc000,
 here create_mapping can alloc pgd, but create_mapping in arch/arm/mm/mmu.c 
can't.

>> +
>> +static int __init create_mapping(unsigned long start, unsigned long end, 
>> int node)
>> +{
>> +unsigned long addr = start;
>> +pgd_t *pgd;
>> +pud_t *pud;
>> +pmd_t *pmd;
>> +pte_t *pte;
>
>A blank line would help between the auto variables and the code of the
>function.

Ok, I will add blank line in new version.
>> +pr_info("populating shadow for %lx, %lx\n", start, end);
>
>Blank line here too please.

Ok, I will add blank line in new version.

>> +for (; addr < end; addr += PAGE_SIZE) {
>> +pgd = kasan_pgd_populate(addr, node);
>> +if (!pgd)
>> +return -ENOMEM;
...
>> +void __init kasan_init(void)
>> +{
>> +struct memblock_region *reg;
>> +u64 orig_ttbr0;
>> +
>> +orig_ttbr0 = cpu_get_ttbr(0);
>> +
>> +#ifdef CONFIG_ARM_LPAE
>> +memcpy(tmp_pmd_table, 
>> pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_START)), sizeof(tmp_pmd_table));
>> +memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +set_pgd(_page_table[pgd_index(KASAN_SHADOW_START)], 
>> __pgd(__pa(tmp_pmd_table) | PMD_TYPE_TABLE | L_PGD_SWAPPER));
>> +cpu_set_ttbr0(__pa(tmp_page_table));
>> +#else
>> +memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +cpu_set_ttbr0(__pa(tmp_page_table));
>> +#endif
>> +flush_cache_all();
>> +local_flush_bp_all();
>> +local_flush_tlb_all();

>What are you trying to achieve with all this complexity?  Some comments
>might be useful, especially for those of us who don't know the internals
>of kasan.
OK, I will add some comments in kasan_init function in new version.
...
>> +for_each_memblock(memory, reg) {
>> +void *start = __va(reg->base);
>> +void *end = __va(reg->base + reg->size);
>
>Isn't this going to complain if the translation macro debugging is enabled?

Sorry, I don't what is the translation macro. Can you tell me.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Oct 19, 2017 at 19:09, Russell King - ARM Linux 
[mailto:li...@armlinux.org.uk] wrote:
>On Wed, Oct 11, 2017 at 04:22:17PM +0800, Abbott Liu wrote:
>> +#else
>> +#define pud_populate(mm,pmd,pte)do { } while (0)
>> +#endif
>
>Please explain this change - we don't have a "pud" as far as the rest of
>the Linux MM layer is concerned, so why do we need it for kasan?
>
>I suspect it comes from the way we wrap up the page tables - where ARM
>does it one way (because it has to) vs the subsequently merged method
>which is completely upside down to what ARMs doing, and therefore is
>totally incompatible and impossible to fit in with our way.

We will use pud_polulate in kasan_populate_zero_shadow function.

>>  obj-$(CONFIG_CACHE_TAUROS2) += cache-tauros2.o
>> +
>> +KASAN_SANITIZE_kasan_init.o:= n
>> +obj-$(CONFIG_KASAN)+= kasan_init.o
>
>Why is this placed in the middle of the cache object listing?

Sorry, I will place this at the end of the arch/arm/mm/Makefile.
>> +
>> +
>>  obj-$(CONFIG_CACHE_UNIPHIER)+= cache-uniphier.o
...

>> +pgd_t * __meminit kasan_pgd_populate(unsigned long addr, int node)
>> +{
>> +pgd_t *pgd = pgd_offset_k(addr);
>> +if (pgd_none(*pgd)) {
>> +void *p = kasan_alloc_block(PAGE_SIZE, node);
>> +if (!p)
>> +return NULL;
>> +pgd_populate(_mm, pgd, p);
>> +}
>> +return pgd;
>> +}

>This all looks wrong - you are aware that on non-LPAE platforms, there
>is only a _two_ level page table - the top level page table is 16K in
>size, and each _individual_ lower level page table is actually 1024
>bytes, but we do some special handling in the kernel to combine two
>together.  It looks to me that you allocate memory for each Linux-
>abstracted page table level whether the hardware needs it or not.

You are right. If non-LPAE platform check if(pgd_none(*pgd)) true,
void *p = kasan_alloc_block(PAGE_SIZE, node) alloc space is not enough.
But the the function kasan_pgd_populate only used in :
Kasan_init-> create_mapping-> kasan_pgd_populate , so when non-LPAE platform
the if (pgd_none(*pgd)) always false.
But I also think change those code is much better :
if (IS_ENABLED(CONFIG_ARM_LPAE)) {
   p = kasan_alloc_block(PAGE_SIZE, node);
} else {
   /* non-LPAE need 16K for first level pagetabe*/
   p = kasan_alloc_block(PAGE_SIZE*4, node);
}

>Is there any reason why the pre-existing "create_mapping()" function
>can't be used, and you've had to rewrite that code here?

Two reason:
1) Here create_mapping can dynamic alloc phys memory space for mapping to 
virtual space 
Which from start to end, but the create_mapping in arch/arm/mm/mmu.c can't.
2) for LPAE, create_mapping need alloc pgd which we need use virtual space 
below 0xc000,
 here create_mapping can alloc pgd, but create_mapping in arch/arm/mm/mmu.c 
can't.

>> +
>> +static int __init create_mapping(unsigned long start, unsigned long end, 
>> int node)
>> +{
>> +unsigned long addr = start;
>> +pgd_t *pgd;
>> +pud_t *pud;
>> +pmd_t *pmd;
>> +pte_t *pte;
>
>A blank line would help between the auto variables and the code of the
>function.

Ok, I will add blank line in new version.
>> +pr_info("populating shadow for %lx, %lx\n", start, end);
>
>Blank line here too please.

Ok, I will add blank line in new version.

>> +for (; addr < end; addr += PAGE_SIZE) {
>> +pgd = kasan_pgd_populate(addr, node);
>> +if (!pgd)
>> +return -ENOMEM;
...
>> +void __init kasan_init(void)
>> +{
>> +struct memblock_region *reg;
>> +u64 orig_ttbr0;
>> +
>> +orig_ttbr0 = cpu_get_ttbr(0);
>> +
>> +#ifdef CONFIG_ARM_LPAE
>> +memcpy(tmp_pmd_table, 
>> pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_START)), sizeof(tmp_pmd_table));
>> +memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +set_pgd(_page_table[pgd_index(KASAN_SHADOW_START)], 
>> __pgd(__pa(tmp_pmd_table) | PMD_TYPE_TABLE | L_PGD_SWAPPER));
>> +cpu_set_ttbr0(__pa(tmp_page_table));
>> +#else
>> +memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +cpu_set_ttbr0(__pa(tmp_page_table));
>> +#endif
>> +flush_cache_all();
>> +local_flush_bp_all();
>> +local_flush_tlb_all();

>What are you trying to achieve with all this complexity?  Some comments
>might be useful, especially for those of us who don't know the internals
>of kasan.
OK, I will add some comments in kasan_init function in new version.
...
>> +for_each_memblock(memory, reg) {
>> +void *start = __va(reg->base);
>> +void *end = __va(reg->base + reg->size);
>
>Isn't this going to complain if the translation macro debugging is enabled?

Sorry, I don't what is the translation macro. Can you tell me.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Nov 23, 2017  23:22  Russell King - ARM Linux [mailto:li...@armlinux.org.uk] 
 wrote:
>Please pay attention to the project coding style whenever creating code
>for a program.  It doesn't matter what the project coding style is, as
>long as you write your code to match the style that is already there.
>
>For the kernel, that is: tabs not spaces for indentation of code.
>You seem to be using a variable number of spaces for all the new code
>above.
>
>Some of it seems to be your email client thinking it knows better about
>white space - and such behaviours basically makes patches unapplyable.
>See Documentation/process/email-clients.rst for hints about email
>clients.
Thanks for your review.
I'm going to change it in the new version.  

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Nov 23, 2017  23:22  Russell King - ARM Linux [mailto:li...@armlinux.org.uk] 
 wrote:
>Please pay attention to the project coding style whenever creating code
>for a program.  It doesn't matter what the project coding style is, as
>long as you write your code to match the style that is already there.
>
>For the kernel, that is: tabs not spaces for indentation of code.
>You seem to be using a variable number of spaces for all the new code
>above.
>
>Some of it seems to be your email client thinking it knows better about
>white space - and such behaviours basically makes patches unapplyable.
>See Documentation/process/email-clients.rst for hints about email
>clients.
Thanks for your review.
I'm going to change it in the new version.  

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Mark Rutland]

On Wed, Nov 22, 2017 at 12:56:44PM +, Liuwenliang (Abbott Liu) wrote:
> +static inline u64 get_ttbr0(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR0_64);
> + else
> + return (u64)read_sysreg(TTBR0_32);
> +}

> +static inline u64 get_ttbr1(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR1_64);
> + else
> + return (u64)read_sysreg(TTBR1_32);
> +}

In addition to the whitespace damage that need to be fixed, there's no
need for the u64 casts here. The compiler will implicitly cast to the
return type, and as u32 and u64 are both arithmetic types, we don't need
an explicit cast here.

Thanks,
Mark.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Mark Rutland]

On Wed, Nov 22, 2017 at 12:56:44PM +, Liuwenliang (Abbott Liu) wrote:
> +static inline u64 get_ttbr0(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR0_64);
> + else
> + return (u64)read_sysreg(TTBR0_32);
> +}

> +static inline u64 get_ttbr1(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR1_64);
> + else
> + return (u64)read_sysreg(TTBR1_32);
> +}

In addition to the whitespace damage that need to be fixed, there's no
need for the u64 casts here. The compiler will implicitly cast to the
return type, and as u32 and u64 are both arithmetic types, we don't need
an explicit cast here.

Thanks,
Mark.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Thu, Nov 23, 2017 at 01:54:59AM +, Liuwenliang (Abbott Liu) wrote:
> On Nov 23, 2017  20:30  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
> >Please define both PAR accessors. Yes, I know the 32bit version is not
> >used yet, but it doesn't hurt to make it visible.
> 
> Thanks for your review.
> I'm going to change it in the new version.
> Here is the code I tested on vexpress_a9 and vexpress_a15:
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index dbdbce1..b8353b1 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -2,6 +2,7 @@
>  #define __ASM_ARM_CP15_H
> 
>  #include 
> +#include 
> 
>  /*
>   * CR1 bits (CP#15 CR1)
> @@ -64,8 +65,109 @@
>  #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_32 __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1_32 __ACCESS_CP15(c2, 0, c0, 1)
> +#define PAR_32   __ACCESS_CP15(c7, 0, c4, 0)
> +#define TTBR0_64 __ACCESS_CP15_64(0, c2)
> +#define TTBR1_64 __ACCESS_CP15_64(1, c2)
> +#define PAR_64   __ACCESS_CP15_64(0, c7)
> +#define VTTBR   __ACCESS_CP15_64(6, c2)
> +#define CNTV_CVAL   __ACCESS_CP15_64(3, c14)
> +#define CNTVOFF __ACCESS_CP15_64(4, c14)
> +
> +#define MIDR__ACCESS_CP15(c0, 0, c0, 0)
> +#define CSSELR  __ACCESS_CP15(c0, 2, c0, 0)
> +#define VPIDR   __ACCESS_CP15(c0, 4, c0, 0)
> +#define VMPIDR  __ACCESS_CP15(c0, 4, c0, 5)
> +#define SCTLR   __ACCESS_CP15(c1, 0, c0, 0)
> +#define CPACR   __ACCESS_CP15(c1, 0, c0, 2)
> +#define HCR __ACCESS_CP15(c1, 4, c1, 0)
> +#define HDCR__ACCESS_CP15(c1, 4, c1, 1)
> +#define HCPTR   __ACCESS_CP15(c1, 4, c1, 2)
> +#define HSTR__ACCESS_CP15(c1, 4, c1, 3)
> +#define TTBCR   __ACCESS_CP15(c2, 0, c0, 2)
> +#define HTCR__ACCESS_CP15(c2, 4, c0, 2)
> +#define VTCR__ACCESS_CP15(c2, 4, c1, 2)
> +#define DACR__ACCESS_CP15(c3, 0, c0, 0)
> +#define DFSR__ACCESS_CP15(c5, 0, c0, 0)
> +#define IFSR__ACCESS_CP15(c5, 0, c0, 1)
> +#define ADFSR   __ACCESS_CP15(c5, 0, c1, 0)
> +#define AIFSR   __ACCESS_CP15(c5, 0, c1, 1)
> +#define HSR __ACCESS_CP15(c5, 4, c2, 0)
> +#define DFAR__ACCESS_CP15(c6, 0, c0, 0)
> +#define IFAR__ACCESS_CP15(c6, 0, c0, 2)
> +#define HDFAR   __ACCESS_CP15(c6, 4, c0, 0)
> +#define HIFAR   __ACCESS_CP15(c6, 4, c0, 2)
> +#define HPFAR   __ACCESS_CP15(c6, 4, c0, 4)
> +#define ICIALLUIS   __ACCESS_CP15(c7, 0, c1, 0)
> +#define ATS1CPR __ACCESS_CP15(c7, 0, c8, 0)
> +#define TLBIALLIS   __ACCESS_CP15(c8, 0, c3, 0)
> +#define TLBIALL __ACCESS_CP15(c8, 0, c7, 0)
> +#define TLBIALLNSNHIS   __ACCESS_CP15(c8, 4, c3, 4)
> +#define PRRR__ACCESS_CP15(c10, 0, c2, 0)
> +#define NMRR__ACCESS_CP15(c10, 0, c2, 1)
> +#define AMAIR0  __ACCESS_CP15(c10, 0, c3, 0)
> +#define AMAIR1  __ACCESS_CP15(c10, 0, c3, 1)
> +#define VBAR__ACCESS_CP15(c12, 0, c0, 0)
> +#define CID __ACCESS_CP15(c13, 0, c0, 1)
> +#define TID_URW __ACCESS_CP15(c13, 0, c0, 2)
> +#define TID_URO __ACCESS_CP15(c13, 0, c0, 3)
> +#define TID_PRIV__ACCESS_CP15(c13, 0, c0, 4)
> +#define HTPIDR  __ACCESS_CP15(c13, 4, c0, 2)
> +#define CNTKCTL __ACCESS_CP15(c14, 0, c1, 0)
> +#define CNTV_CTL__ACCESS_CP15(c14, 0, c3, 1)
> +#define CNTHCTL __ACCESS_CP15(c14, 4, c1, 0)
> +
>  extern unsigned long cr_alignment; /* defined in entry-armv.S */
> 
> +static inline void set_par(u64 val)
> +{
> +if (IS_ENABLED(CONFIG_ARM_LPAE))
> +write_sysreg(val, PAR_64);
> +else
> +write_sysreg(val, PAR_32);
> +}
> +
> +static inline u64 get_par(void)
> +{
> +if (IS_ENABLED(CONFIG_ARM_LPAE))
> +return read_sysreg(PAR_64);
> +else
> +return (u64)read_sysreg(PAR_32);
> +}
> +
> +static inline void set_ttbr0(u64 val)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + write_sysreg(val, TTBR0_64);
> + else
> + write_sysreg(val, TTBR0_32);
> +}
> +
> +static inline u64 get_ttbr0(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR0_64);
> + else
> + return (u64)read_sysreg(TTBR0_32);
> +}
> +
> +static inline void set_ttbr1(u64 val)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + write_sysreg(val, TTBR1_64);
> + else
> + write_sysreg(val, TTBR1_32);
> +}
> +
> +static inline u64 get_ttbr1(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR1_64);
> + else
> + return (u64)read_sysreg(TTBR1_32);
> +}
> +

Please pay attention to the project coding style whenever creating code
for a program.  

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Thu, Nov 23, 2017 at 01:54:59AM +, Liuwenliang (Abbott Liu) wrote:
> On Nov 23, 2017  20:30  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
> >Please define both PAR accessors. Yes, I know the 32bit version is not
> >used yet, but it doesn't hurt to make it visible.
> 
> Thanks for your review.
> I'm going to change it in the new version.
> Here is the code I tested on vexpress_a9 and vexpress_a15:
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index dbdbce1..b8353b1 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -2,6 +2,7 @@
>  #define __ASM_ARM_CP15_H
> 
>  #include 
> +#include 
> 
>  /*
>   * CR1 bits (CP#15 CR1)
> @@ -64,8 +65,109 @@
>  #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_32 __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1_32 __ACCESS_CP15(c2, 0, c0, 1)
> +#define PAR_32   __ACCESS_CP15(c7, 0, c4, 0)
> +#define TTBR0_64 __ACCESS_CP15_64(0, c2)
> +#define TTBR1_64 __ACCESS_CP15_64(1, c2)
> +#define PAR_64   __ACCESS_CP15_64(0, c7)
> +#define VTTBR   __ACCESS_CP15_64(6, c2)
> +#define CNTV_CVAL   __ACCESS_CP15_64(3, c14)
> +#define CNTVOFF __ACCESS_CP15_64(4, c14)
> +
> +#define MIDR__ACCESS_CP15(c0, 0, c0, 0)
> +#define CSSELR  __ACCESS_CP15(c0, 2, c0, 0)
> +#define VPIDR   __ACCESS_CP15(c0, 4, c0, 0)
> +#define VMPIDR  __ACCESS_CP15(c0, 4, c0, 5)
> +#define SCTLR   __ACCESS_CP15(c1, 0, c0, 0)
> +#define CPACR   __ACCESS_CP15(c1, 0, c0, 2)
> +#define HCR __ACCESS_CP15(c1, 4, c1, 0)
> +#define HDCR__ACCESS_CP15(c1, 4, c1, 1)
> +#define HCPTR   __ACCESS_CP15(c1, 4, c1, 2)
> +#define HSTR__ACCESS_CP15(c1, 4, c1, 3)
> +#define TTBCR   __ACCESS_CP15(c2, 0, c0, 2)
> +#define HTCR__ACCESS_CP15(c2, 4, c0, 2)
> +#define VTCR__ACCESS_CP15(c2, 4, c1, 2)
> +#define DACR__ACCESS_CP15(c3, 0, c0, 0)
> +#define DFSR__ACCESS_CP15(c5, 0, c0, 0)
> +#define IFSR__ACCESS_CP15(c5, 0, c0, 1)
> +#define ADFSR   __ACCESS_CP15(c5, 0, c1, 0)
> +#define AIFSR   __ACCESS_CP15(c5, 0, c1, 1)
> +#define HSR __ACCESS_CP15(c5, 4, c2, 0)
> +#define DFAR__ACCESS_CP15(c6, 0, c0, 0)
> +#define IFAR__ACCESS_CP15(c6, 0, c0, 2)
> +#define HDFAR   __ACCESS_CP15(c6, 4, c0, 0)
> +#define HIFAR   __ACCESS_CP15(c6, 4, c0, 2)
> +#define HPFAR   __ACCESS_CP15(c6, 4, c0, 4)
> +#define ICIALLUIS   __ACCESS_CP15(c7, 0, c1, 0)
> +#define ATS1CPR __ACCESS_CP15(c7, 0, c8, 0)
> +#define TLBIALLIS   __ACCESS_CP15(c8, 0, c3, 0)
> +#define TLBIALL __ACCESS_CP15(c8, 0, c7, 0)
> +#define TLBIALLNSNHIS   __ACCESS_CP15(c8, 4, c3, 4)
> +#define PRRR__ACCESS_CP15(c10, 0, c2, 0)
> +#define NMRR__ACCESS_CP15(c10, 0, c2, 1)
> +#define AMAIR0  __ACCESS_CP15(c10, 0, c3, 0)
> +#define AMAIR1  __ACCESS_CP15(c10, 0, c3, 1)
> +#define VBAR__ACCESS_CP15(c12, 0, c0, 0)
> +#define CID __ACCESS_CP15(c13, 0, c0, 1)
> +#define TID_URW __ACCESS_CP15(c13, 0, c0, 2)
> +#define TID_URO __ACCESS_CP15(c13, 0, c0, 3)
> +#define TID_PRIV__ACCESS_CP15(c13, 0, c0, 4)
> +#define HTPIDR  __ACCESS_CP15(c13, 4, c0, 2)
> +#define CNTKCTL __ACCESS_CP15(c14, 0, c1, 0)
> +#define CNTV_CTL__ACCESS_CP15(c14, 0, c3, 1)
> +#define CNTHCTL __ACCESS_CP15(c14, 4, c1, 0)
> +
>  extern unsigned long cr_alignment; /* defined in entry-armv.S */
> 
> +static inline void set_par(u64 val)
> +{
> +if (IS_ENABLED(CONFIG_ARM_LPAE))
> +write_sysreg(val, PAR_64);
> +else
> +write_sysreg(val, PAR_32);
> +}
> +
> +static inline u64 get_par(void)
> +{
> +if (IS_ENABLED(CONFIG_ARM_LPAE))
> +return read_sysreg(PAR_64);
> +else
> +return (u64)read_sysreg(PAR_32);
> +}
> +
> +static inline void set_ttbr0(u64 val)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + write_sysreg(val, TTBR0_64);
> + else
> + write_sysreg(val, TTBR0_32);
> +}
> +
> +static inline u64 get_ttbr0(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR0_64);
> + else
> + return (u64)read_sysreg(TTBR0_32);
> +}
> +
> +static inline void set_ttbr1(u64 val)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + write_sysreg(val, TTBR1_64);
> + else
> + write_sysreg(val, TTBR1_32);
> +}
> +
> +static inline u64 get_ttbr1(void)
> +{
> + if (IS_ENABLED(CONFIG_ARM_LPAE))
> + return read_sysreg(TTBR1_64);
> + else
> + return (u64)read_sysreg(TTBR1_32);
> +}
> +

Please pay attention to the project coding style whenever creating code
for a program.  

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

t->cp15[c6_DFAR],   DFAR);
write_sysreg(ctxt->cp15[c6_IFAR],   IFAR);
-   write_sysreg(*cp15_64(ctxt, c7_PAR),PAR);
+ write_sysreg(*cp15_64(ctxt, c7_PAR),PAR_64);
write_sysreg(ctxt->cp15[c10_PRRR],  PRRR);
write_sysreg(ctxt->cp15[c10_NMRR],  NMRR);
write_sysreg(ctxt->cp15[c10_AMAIR0],AMAIR0);
diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
index ebd2dd4..4879588 100644
--- a/arch/arm/kvm/hyp/switch.c
+++ b/arch/arm/kvm/hyp/switch.c
@@ -133,12 +133,12 @@ static bool __hyp_text __populate_fault_info(struct 
kvm_vcpu *vcpu)
if (!(hsr & HSR_DABT_S1PTW) && (hsr & HSR_FSC_TYPE) == FSC_PERM) {
u64 par, tmp;

-   par = read_sysreg(PAR);
+ par = read_sysreg(PAR_64);
write_sysreg(far, ATS1CPR);
isb();

-   tmp = read_sysreg(PAR);
-   write_sysreg(par, PAR);
+ tmp = read_sysreg(PAR_64);
+ write_sysreg(par, PAR_64);

if (unlikely(tmp & 1))
return false; /* Translation failed, back to guest */
diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c
index 049ee0a..87c86c7 100644
--- a/arch/arm/mm/kasan_init.c
+++ b/arch/arm/mm/kasan_init.c
@@ -203,16 +203,16 @@ void __init kasan_init(void)
u64 orig_ttbr0;
int i;

-   orig_ttbr0 = cpu_get_ttbr(0);
+ orig_ttbr0 = get_ttbr0();

 #ifdef CONFIG_ARM_LPAE
memcpy(tmp_pmd_table, 
pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_START)), sizeof(tmp_pmd_table));
memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
set_pgd(_page_table[pgd_index(KASAN_SHADOW_START)], 
__pgd(__pa(tmp_pmd_table) | PMD_TYPE_TABLE | L_PGD_SWAPPER));
-   cpu_set_ttbr0(__pa(tmp_page_table));
+ set_ttbr0(__pa(tmp_page_table));
 #else
memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
-   cpu_set_ttbr0(__pa(tmp_page_table));
+ set_ttbr0((u64)__pa(tmp_page_table));
 #endif
flush_cache_all();
local_flush_bp_all();
@@ -257,7 +257,7 @@ void __init kasan_init(void)
 /*__pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | 
L_PTE_XN | L_PTE_RDONLY))*/
__pgprot(pgprot_val(PAGE_KERNEL) | 
L_PTE_RDONLY)));
memset(kasan_zero_page, 0, PAGE_SIZE);
-   cpu_set_ttbr0(orig_ttbr0);
+ set_ttbr0(orig_ttbr0);
flush_cache_all();
local_flush_bp_all();
local_flush_tlb_all();



-邮件原件-
发件人: Marc Zyngier [mailto:marc.zyng...@arm.com] 
发送时间: 2017年11月22日 21:06
收件人: Liuwenliang (Abbott Liu); Mark Rutland
抄送: t...@linaro.org; mho...@suse.com; grygorii.stras...@linaro.org; 
catalin.mari...@arm.com; linux...@kvack.org; gli...@google.com; 
afzal.mohd...@gmail.com; mi...@kernel.org; Christoffer Dall; 
f.faine...@gmail.com; mawil...@microsoft.com; li...@armlinux.org.uk; 
kasan-...@googlegroups.com; Dailei; linux-arm-ker...@lists.infradead.org; 
aryabi...@virtuozzo.com; labb...@redhat.com; vladimir.mur...@arm.com; 
keesc...@chromium.org; a...@arndb.de; Zengweilin; open...@gmail.com; 
Heshaoliang; t...@linutronix.de; dvyu...@google.com; ard.biesheu...@linaro.org; 
linux-kernel@vger.kernel.org; Jiazhenghua; a...@linux-foundation.org; 
robin.mur...@arm.com; thgar...@google.com; kirill.shute...@linux.intel.com
主题: Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

On 22/11/17 12:56, Liuwenliang (Abbott Liu) wrote:
> On Nov 22, 2017  20:30  Mark Rutland [mailto:mark.rutl...@arm.com] wrote:
>> On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
>>> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
>>>> On Sat, 18 Nov 2017 10:40:08 +
>>>> "Liuwenliang (Abbott Liu)" <liuwenli...@huawei.com> wrote:
>>>>> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> 
>>> Please don't ask people to limit to 4GB of physical space on CPU
>>> supporting LPAE, please don't ask people to guaranteed to have some
>>> memory below 4GB on CPU supporting LPAE.
> 
>> I don't think that Marc is suggesting that you'd always use the 32-bit
>> accessors on an LPAE system, just that all the definitions should exist
>> regardless of configuration.
> 
>> So rather than this:
> 
>>> +#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
> 
>> ... you'd have the following in cp15.h:
> 
>> #define TTBR0_64   __ACCESS_CP

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

t->cp15[c6_DFAR],   DFAR);
write_sysreg(ctxt->cp15[c6_IFAR],   IFAR);
-   write_sysreg(*cp15_64(ctxt, c7_PAR),PAR);
+ write_sysreg(*cp15_64(ctxt, c7_PAR),PAR_64);
write_sysreg(ctxt->cp15[c10_PRRR],  PRRR);
write_sysreg(ctxt->cp15[c10_NMRR],  NMRR);
write_sysreg(ctxt->cp15[c10_AMAIR0],AMAIR0);
diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
index ebd2dd4..4879588 100644
--- a/arch/arm/kvm/hyp/switch.c
+++ b/arch/arm/kvm/hyp/switch.c
@@ -133,12 +133,12 @@ static bool __hyp_text __populate_fault_info(struct 
kvm_vcpu *vcpu)
if (!(hsr & HSR_DABT_S1PTW) && (hsr & HSR_FSC_TYPE) == FSC_PERM) {
u64 par, tmp;

-   par = read_sysreg(PAR);
+ par = read_sysreg(PAR_64);
write_sysreg(far, ATS1CPR);
isb();

-   tmp = read_sysreg(PAR);
-   write_sysreg(par, PAR);
+ tmp = read_sysreg(PAR_64);
+ write_sysreg(par, PAR_64);

if (unlikely(tmp & 1))
return false; /* Translation failed, back to guest */
diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c
index 049ee0a..87c86c7 100644
--- a/arch/arm/mm/kasan_init.c
+++ b/arch/arm/mm/kasan_init.c
@@ -203,16 +203,16 @@ void __init kasan_init(void)
u64 orig_ttbr0;
int i;

-   orig_ttbr0 = cpu_get_ttbr(0);
+ orig_ttbr0 = get_ttbr0();

 #ifdef CONFIG_ARM_LPAE
memcpy(tmp_pmd_table, 
pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_START)), sizeof(tmp_pmd_table));
memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
set_pgd(_page_table[pgd_index(KASAN_SHADOW_START)], 
__pgd(__pa(tmp_pmd_table) | PMD_TYPE_TABLE | L_PGD_SWAPPER));
-   cpu_set_ttbr0(__pa(tmp_page_table));
+ set_ttbr0(__pa(tmp_page_table));
 #else
memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
-   cpu_set_ttbr0(__pa(tmp_page_table));
+ set_ttbr0((u64)__pa(tmp_page_table));
 #endif
flush_cache_all();
local_flush_bp_all();
@@ -257,7 +257,7 @@ void __init kasan_init(void)
 /*__pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | 
L_PTE_XN | L_PTE_RDONLY))*/
__pgprot(pgprot_val(PAGE_KERNEL) | 
L_PTE_RDONLY)));
memset(kasan_zero_page, 0, PAGE_SIZE);
-   cpu_set_ttbr0(orig_ttbr0);
+ set_ttbr0(orig_ttbr0);
flush_cache_all();
local_flush_bp_all();
local_flush_tlb_all();



-邮件原件-
发件人: Marc Zyngier [mailto:marc.zyng...@arm.com] 
发送时间: 2017年11月22日 21:06
收件人: Liuwenliang (Abbott Liu); Mark Rutland
抄送: t...@linaro.org; mho...@suse.com; grygorii.stras...@linaro.org; 
catalin.mari...@arm.com; linux...@kvack.org; gli...@google.com; 
afzal.mohd...@gmail.com; mi...@kernel.org; Christoffer Dall; 
f.faine...@gmail.com; mawil...@microsoft.com; li...@armlinux.org.uk; 
kasan-...@googlegroups.com; Dailei; linux-arm-ker...@lists.infradead.org; 
aryabi...@virtuozzo.com; labb...@redhat.com; vladimir.mur...@arm.com; 
keesc...@chromium.org; a...@arndb.de; Zengweilin; open...@gmail.com; 
Heshaoliang; t...@linutronix.de; dvyu...@google.com; ard.biesheu...@linaro.org; 
linux-kernel@vger.kernel.org; Jiazhenghua; a...@linux-foundation.org; 
robin.mur...@arm.com; thgar...@google.com; kirill.shute...@linux.intel.com
主题: Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

On 22/11/17 12:56, Liuwenliang (Abbott Liu) wrote:
> On Nov 22, 2017  20:30  Mark Rutland [mailto:mark.rutl...@arm.com] wrote:
>> On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
>>> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
>>>> On Sat, 18 Nov 2017 10:40:08 +
>>>> "Liuwenliang (Abbott Liu)"  wrote:
>>>>> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> 
>>> Please don't ask people to limit to 4GB of physical space on CPU
>>> supporting LPAE, please don't ask people to guaranteed to have some
>>> memory below 4GB on CPU supporting LPAE.
> 
>> I don't think that Marc is suggesting that you'd always use the 32-bit
>> accessors on an LPAE system, just that all the definitions should exist
>> regardless of configuration.
> 
>> So rather than this:
> 
>>> +#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
> 
>> ... you'd have the following in cp15.h:
> 
>> #define TTBR0_64   __ACCESS_CP15_64(0, c2)
>> #

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 22/11/17 12:56, Liuwenliang (Abbott Liu) wrote:
> On Nov 22, 2017  20:30  Mark Rutland [mailto:mark.rutl...@arm.com] wrote:
>> On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
>>> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
 On Sat, 18 Nov 2017 10:40:08 +
 "Liuwenliang (Abbott Liu)"  wrote:
> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> 
>>> Please don't ask people to limit to 4GB of physical space on CPU
>>> supporting LPAE, please don't ask people to guaranteed to have some
>>> memory below 4GB on CPU supporting LPAE.
> 
>> I don't think that Marc is suggesting that you'd always use the 32-bit
>> accessors on an LPAE system, just that all the definitions should exist
>> regardless of configuration.
> 
>> So rather than this:
> 
>>> +#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
> 
>> ... you'd have the following in cp15.h:
> 
>> #define TTBR0_64   __ACCESS_CP15_64(0, c2)
>> #define TTBR1_64   __ACCESS_CP15_64(1, c2)
>> #define PAR_64 __ACCESS_CP15_64(0, c7)
> 
>> #define TTBR0_32   __ACCESS_CP15(c2, 0, c0, 0)
>> #define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
>> #define PAR_32 __ACCESS_CP15(c7, 0, c4, 0)
> 
>> ... and elsewhere, where it matters, we choose which to use depending on
>> the kernel configuration, e.g.
> 
>> void set_ttbr0(u64 val)
>> {
>>   if (IS_ENABLED(CONFIG_ARM_LPAE))
>>   write_sysreg(val, TTBR0_64);
>>   else
>>   write_sysreg(val, TTBR0_32);
>> }
> 
>> Thanks,
>> Mark.
> 
> Thanks for your solution.
> I didn't know there was a IS_ENABLED macro that I can use, so I can't write a 
> function 
> like:
> void set_ttbr0(u64 val)
> {
>if (IS_ENABLED(CONFIG_ARM_LPAE))
>write_sysreg(val, TTBR0_64);
>else
>write_sysreg(val, TTBR0_32);
> }
> 
> 
> Here is the code I tested on vexpress_a9 and vexpress_a15:
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index dbdbce1..5eb0185 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -2,6 +2,7 @@
>  #define __ASM_ARM_CP15_H
> 
>  #include 
> +#include 
> 
>  /*
>   * CR1 bits (CP#15 CR1)
> @@ -64,8 +65,93 @@
>  #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_32   __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
> +#define TTBR0_64   __ACCESS_CP15_64(0, c2)
> +#define TTBR1_64   __ACCESS_CP15_64(1, c2)
> +#define PAR __ACCESS_CP15_64(0, c7)

Please define both PAR accessors. Yes, I know the 32bit version is not
used yet, but it doesn't hurt to make it visible.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 22/11/17 12:56, Liuwenliang (Abbott Liu) wrote:
> On Nov 22, 2017  20:30  Mark Rutland [mailto:mark.rutl...@arm.com] wrote:
>> On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
>>> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
 On Sat, 18 Nov 2017 10:40:08 +
 "Liuwenliang (Abbott Liu)"  wrote:
> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> 
>>> Please don't ask people to limit to 4GB of physical space on CPU
>>> supporting LPAE, please don't ask people to guaranteed to have some
>>> memory below 4GB on CPU supporting LPAE.
> 
>> I don't think that Marc is suggesting that you'd always use the 32-bit
>> accessors on an LPAE system, just that all the definitions should exist
>> regardless of configuration.
> 
>> So rather than this:
> 
>>> +#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
> 
>> ... you'd have the following in cp15.h:
> 
>> #define TTBR0_64   __ACCESS_CP15_64(0, c2)
>> #define TTBR1_64   __ACCESS_CP15_64(1, c2)
>> #define PAR_64 __ACCESS_CP15_64(0, c7)
> 
>> #define TTBR0_32   __ACCESS_CP15(c2, 0, c0, 0)
>> #define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
>> #define PAR_32 __ACCESS_CP15(c7, 0, c4, 0)
> 
>> ... and elsewhere, where it matters, we choose which to use depending on
>> the kernel configuration, e.g.
> 
>> void set_ttbr0(u64 val)
>> {
>>   if (IS_ENABLED(CONFIG_ARM_LPAE))
>>   write_sysreg(val, TTBR0_64);
>>   else
>>   write_sysreg(val, TTBR0_32);
>> }
> 
>> Thanks,
>> Mark.
> 
> Thanks for your solution.
> I didn't know there was a IS_ENABLED macro that I can use, so I can't write a 
> function 
> like:
> void set_ttbr0(u64 val)
> {
>if (IS_ENABLED(CONFIG_ARM_LPAE))
>write_sysreg(val, TTBR0_64);
>else
>write_sysreg(val, TTBR0_32);
> }
> 
> 
> Here is the code I tested on vexpress_a9 and vexpress_a15:
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index dbdbce1..5eb0185 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -2,6 +2,7 @@
>  #define __ASM_ARM_CP15_H
> 
>  #include 
> +#include 
> 
>  /*
>   * CR1 bits (CP#15 CR1)
> @@ -64,8 +65,93 @@
>  #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_32   __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
> +#define TTBR0_64   __ACCESS_CP15_64(0, c2)
> +#define TTBR1_64   __ACCESS_CP15_64(1, c2)
> +#define PAR __ACCESS_CP15_64(0, c7)

Please define both PAR accessors. Yes, I know the 32bit version is not
used yet, but it doesn't hurt to make it visible.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Nov 22, 2017  20:30  Mark Rutland [mailto:mark.rutl...@arm.com] wrote:
>On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
>> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
>> >On Sat, 18 Nov 2017 10:40:08 +
>> >"Liuwenliang (Abbott Liu)"  wrote:
>> >> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:

>> Please don't ask people to limit to 4GB of physical space on CPU
>> supporting LPAE, please don't ask people to guaranteed to have some
>> memory below 4GB on CPU supporting LPAE.

>I don't think that Marc is suggesting that you'd always use the 32-bit
>accessors on an LPAE system, just that all the definitions should exist
>regardless of configuration.

>So rather than this:

>> +#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

>... you'd have the following in cp15.h:

>#define TTBR0_64   __ACCESS_CP15_64(0, c2)
>#define TTBR1_64   __ACCESS_CP15_64(1, c2)
>#define PAR_64 __ACCESS_CP15_64(0, c7)

>#define TTBR0_32   __ACCESS_CP15(c2, 0, c0, 0)
>#define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
>#define PAR_32 __ACCESS_CP15(c7, 0, c4, 0)

>... and elsewhere, where it matters, we choose which to use depending on
>the kernel configuration, e.g.

>void set_ttbr0(u64 val)
>{
>   if (IS_ENABLED(CONFIG_ARM_LPAE))
>   write_sysreg(val, TTBR0_64);
>   else
>   write_sysreg(val, TTBR0_32);
>}

>Thanks,
>Mark.

Thanks for your solution.
I didn't know there was a IS_ENABLED macro that I can use, so I can't write a 
function 
like:
void set_ttbr0(u64 val)
{
   if (IS_ENABLED(CONFIG_ARM_LPAE))
   write_sysreg(val, TTBR0_64);
   else
   write_sysreg(val, TTBR0_32);
}


Here is the code I tested on vexpress_a9 and vexpress_a15:
diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
index dbdbce1..5eb0185 100644
--- a/arch/arm/include/asm/cp15.h
+++ b/arch/arm/include/asm/cp15.h
@@ -2,6 +2,7 @@
 #define __ASM_ARM_CP15_H

 #include 
+#include 

 /*
  * CR1 bits (CP#15 CR1)
@@ -64,8 +65,93 @@
 #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_32   __ACCESS_CP15(c2, 0, c0, 0)
+#define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
+#define TTBR0_64   __ACCESS_CP15_64(0, c2)
+#define TTBR1_64   __ACCESS_CP15_64(1, c2)
+#define PAR __ACCESS_CP15_64(0, c7)
+#define VTTBR   __ACCESS_CP15_64(6, c2)
+#define CNTV_CVAL   __ACCESS_CP15_64(3, c14)
+#define CNTVOFF __ACCESS_CP15_64(4, c14)
+
+#define MIDR__ACCESS_CP15(c0, 0, c0, 0)
+#define CSSELR  __ACCESS_CP15(c0, 2, c0, 0)
+#define VPIDR   __ACCESS_CP15(c0, 4, c0, 0)
+#define VMPIDR  __ACCESS_CP15(c0, 4, c0, 5)
+#define SCTLR   __ACCESS_CP15(c1, 0, c0, 0)
+#define CPACR   __ACCESS_CP15(c1, 0, c0, 2)
+#define HCR __ACCESS_CP15(c1, 4, c1, 0)
+#define HDCR__ACCESS_CP15(c1, 4, c1, 1)
+#define HCPTR   __ACCESS_CP15(c1, 4, c1, 2)
+#define HSTR__ACCESS_CP15(c1, 4, c1, 3)
+#define TTBCR   __ACCESS_CP15(c2, 0, c0, 2)
+#define HTCR__ACCESS_CP15(c2, 4, c0, 2)
+#define VTCR__ACCESS_CP15(c2, 4, c1, 2)
+#define DACR__ACCESS_CP15(c3, 0, c0, 0)
+#define DFSR__ACCESS_CP15(c5, 0, c0, 0)
+#define IFSR__ACCESS_CP15(c5, 0, c0, 1)
+#define ADFSR   __ACCESS_CP15(c5, 0, c1, 0)
+#define AIFSR   __ACCESS_CP15(c5, 0, c1, 1)
+#define HSR __ACCESS_CP15(c5, 4, c2, 0)
+#define DFAR__ACCESS_CP15(c6, 0, c0, 0)
+#define IFAR__ACCESS_CP15(c6, 0, c0, 2)
+#define HDFAR   __ACCESS_CP15(c6, 4, c0, 0)
+#define HIFAR   __ACCESS_CP15(c6, 4, c0, 2)
+#define HPFAR   __ACCESS_CP15(c6, 4, c0, 4)
+#define ICIALLUIS   __ACCESS_CP15(c7, 0, c1, 0)
+#define ATS1CPR __ACCESS_CP15(c7, 0, c8, 0)
+#define TLBIALLIS   __ACCESS_CP15(c8, 0, c3, 0)
+#define TLBIALL __ACCESS_CP15(c8, 0, c7, 0)
+#define TLBIALLNSNHIS   __ACCESS_CP15(c8, 4, c3, 4)
+#define PRRR__ACCESS_CP15(c10, 0, c2, 0)
+#define NMRR__ACCESS_CP15(c10, 0, c2, 1)
+#define AMAIR0  __ACCESS_CP15(c10, 0, c3, 0)
+#define AMAIR1  __ACCESS_CP15(c10, 0, c3, 1)
+#define VBAR__ACCESS_CP15(c12, 0, c0, 0)
+#define CID __ACCESS_CP15(c13, 0, c0, 1)
+#define TID_URW __ACCESS_CP15(c13, 0, c0, 2)
+#define TID_URO __ACCESS_CP15(c13, 0, c0, 3)
+#define TID_PRIV   

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Nov 22, 2017  20:30  Mark Rutland [mailto:mark.rutl...@arm.com] wrote:
>On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
>> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
>> >On Sat, 18 Nov 2017 10:40:08 +
>> >"Liuwenliang (Abbott Liu)"  wrote:
>> >> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:

>> Please don't ask people to limit to 4GB of physical space on CPU
>> supporting LPAE, please don't ask people to guaranteed to have some
>> memory below 4GB on CPU supporting LPAE.

>I don't think that Marc is suggesting that you'd always use the 32-bit
>accessors on an LPAE system, just that all the definitions should exist
>regardless of configuration.

>So rather than this:

>> +#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

>... you'd have the following in cp15.h:

>#define TTBR0_64   __ACCESS_CP15_64(0, c2)
>#define TTBR1_64   __ACCESS_CP15_64(1, c2)
>#define PAR_64 __ACCESS_CP15_64(0, c7)

>#define TTBR0_32   __ACCESS_CP15(c2, 0, c0, 0)
>#define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
>#define PAR_32 __ACCESS_CP15(c7, 0, c4, 0)

>... and elsewhere, where it matters, we choose which to use depending on
>the kernel configuration, e.g.

>void set_ttbr0(u64 val)
>{
>   if (IS_ENABLED(CONFIG_ARM_LPAE))
>   write_sysreg(val, TTBR0_64);
>   else
>   write_sysreg(val, TTBR0_32);
>}

>Thanks,
>Mark.

Thanks for your solution.
I didn't know there was a IS_ENABLED macro that I can use, so I can't write a 
function 
like:
void set_ttbr0(u64 val)
{
   if (IS_ENABLED(CONFIG_ARM_LPAE))
   write_sysreg(val, TTBR0_64);
   else
   write_sysreg(val, TTBR0_32);
}


Here is the code I tested on vexpress_a9 and vexpress_a15:
diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
index dbdbce1..5eb0185 100644
--- a/arch/arm/include/asm/cp15.h
+++ b/arch/arm/include/asm/cp15.h
@@ -2,6 +2,7 @@
 #define __ASM_ARM_CP15_H

 #include 
+#include 

 /*
  * CR1 bits (CP#15 CR1)
@@ -64,8 +65,93 @@
 #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_32   __ACCESS_CP15(c2, 0, c0, 0)
+#define TTBR1_32   __ACCESS_CP15(c2, 0, c0, 1)
+#define TTBR0_64   __ACCESS_CP15_64(0, c2)
+#define TTBR1_64   __ACCESS_CP15_64(1, c2)
+#define PAR __ACCESS_CP15_64(0, c7)
+#define VTTBR   __ACCESS_CP15_64(6, c2)
+#define CNTV_CVAL   __ACCESS_CP15_64(3, c14)
+#define CNTVOFF __ACCESS_CP15_64(4, c14)
+
+#define MIDR__ACCESS_CP15(c0, 0, c0, 0)
+#define CSSELR  __ACCESS_CP15(c0, 2, c0, 0)
+#define VPIDR   __ACCESS_CP15(c0, 4, c0, 0)
+#define VMPIDR  __ACCESS_CP15(c0, 4, c0, 5)
+#define SCTLR   __ACCESS_CP15(c1, 0, c0, 0)
+#define CPACR   __ACCESS_CP15(c1, 0, c0, 2)
+#define HCR __ACCESS_CP15(c1, 4, c1, 0)
+#define HDCR__ACCESS_CP15(c1, 4, c1, 1)
+#define HCPTR   __ACCESS_CP15(c1, 4, c1, 2)
+#define HSTR__ACCESS_CP15(c1, 4, c1, 3)
+#define TTBCR   __ACCESS_CP15(c2, 0, c0, 2)
+#define HTCR__ACCESS_CP15(c2, 4, c0, 2)
+#define VTCR__ACCESS_CP15(c2, 4, c1, 2)
+#define DACR__ACCESS_CP15(c3, 0, c0, 0)
+#define DFSR__ACCESS_CP15(c5, 0, c0, 0)
+#define IFSR__ACCESS_CP15(c5, 0, c0, 1)
+#define ADFSR   __ACCESS_CP15(c5, 0, c1, 0)
+#define AIFSR   __ACCESS_CP15(c5, 0, c1, 1)
+#define HSR __ACCESS_CP15(c5, 4, c2, 0)
+#define DFAR__ACCESS_CP15(c6, 0, c0, 0)
+#define IFAR__ACCESS_CP15(c6, 0, c0, 2)
+#define HDFAR   __ACCESS_CP15(c6, 4, c0, 0)
+#define HIFAR   __ACCESS_CP15(c6, 4, c0, 2)
+#define HPFAR   __ACCESS_CP15(c6, 4, c0, 4)
+#define ICIALLUIS   __ACCESS_CP15(c7, 0, c1, 0)
+#define ATS1CPR __ACCESS_CP15(c7, 0, c8, 0)
+#define TLBIALLIS   __ACCESS_CP15(c8, 0, c3, 0)
+#define TLBIALL __ACCESS_CP15(c8, 0, c7, 0)
+#define TLBIALLNSNHIS   __ACCESS_CP15(c8, 4, c3, 4)
+#define PRRR__ACCESS_CP15(c10, 0, c2, 0)
+#define NMRR__ACCESS_CP15(c10, 0, c2, 1)
+#define AMAIR0  __ACCESS_CP15(c10, 0, c3, 0)
+#define AMAIR1  __ACCESS_CP15(c10, 0, c3, 1)
+#define VBAR__ACCESS_CP15(c12, 0, c0, 0)
+#define CID __ACCESS_CP15(c13, 0, c0, 1)
+#define TID_URW __ACCESS_CP15(c13, 0, c0, 2)
+#define TID_URO __ACCESS_CP15(c13, 0, c0, 3)
+#define TID_PRIV__ACCESS_CP15(c13, 

Re: 答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Mark Rutland]

On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
> >On Sat, 18 Nov 2017 10:40:08 +
> >"Liuwenliang (Abbott Liu)"  wrote:
> >> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:

> Please don't ask people to limit to 4GB of physical space on CPU
> supporting LPAE, please don't ask people to guaranteed to have some
> memory below 4GB on CPU supporting LPAE.

I don't think that Marc is suggesting that you'd always use the 32-bit
accessors on an LPAE system, just that all the definitions should exist
regardless of configuration.

So rather than this:

> +#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

... you'd have the following in cp15.h:

#define TTBR0_64__ACCESS_CP15_64(0, c2)
#define TTBR1_64__ACCESS_CP15_64(1, c2)
#define PAR_64  __ACCESS_CP15_64(0, c7)

#define TTBR0_32__ACCESS_CP15(c2, 0, c0, 0)
#define TTBR1_32__ACCESS_CP15(c2, 0, c0, 1)
#define PAR_32  __ACCESS_CP15(c7, 0, c4, 0)

... and elsewhere, where it matters, we choose which to use depending on
the kernel configuration, e.g.

void set_ttbr0(u64 val)
{
if (IS_ENABLED(CONFIG_ARM_LPAE))
write_sysreg(val, TTBR0_64);
else
write_sysreg(val, TTBR0_32);
}

Thanks,
Mark.

Re: 答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Mark Rutland]

On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
> >On Sat, 18 Nov 2017 10:40:08 +
> >"Liuwenliang (Abbott Liu)"  wrote:
> >> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:

> Please don't ask people to limit to 4GB of physical space on CPU
> supporting LPAE, please don't ask people to guaranteed to have some
> memory below 4GB on CPU supporting LPAE.

I don't think that Marc is suggesting that you'd always use the 32-bit
accessors on an LPAE system, just that all the definitions should exist
regardless of configuration.

So rather than this:

> +#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

... you'd have the following in cp15.h:

#define TTBR0_64__ACCESS_CP15_64(0, c2)
#define TTBR1_64__ACCESS_CP15_64(1, c2)
#define PAR_64  __ACCESS_CP15_64(0, c7)

#define TTBR0_32__ACCESS_CP15(c2, 0, c0, 0)
#define TTBR1_32__ACCESS_CP15(c2, 0, c0, 1)
#define PAR_32  __ACCESS_CP15(c7, 0, c4, 0)

... and elsewhere, where it matters, we choose which to use depending on
the kernel configuration, e.g.

void set_ttbr0(u64 val)
{
if (IS_ENABLED(CONFIG_ARM_LPAE))
write_sysreg(val, TTBR0_64);
else
write_sysreg(val, TTBR0_32);
}

Thanks,
Mark.

Re: 答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 21/11/17 07:59, Liuwenliang (Abbott Liu) wrote:
> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
>> On Sat, 18 Nov 2017 10:40:08 +
>> "Liuwenliang (Abbott Liu)"  wrote:
> 
>>> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
 If your processor does support LPAE (like a Cortex-A15 for example),
 then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
 accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
 the lower 32-bits of the 64-bit register.

 Hope this helps,
 -Christoffer
>>>
>>> If you know the higher 32-bits of the 64-bits cp15's register is not useful 
>>> for your system,
>>> then you can use the 32-bit accessor to get or set the 64-bit cp15's 
>>> register.
>>> But if the higher 32-bits of the 64-bits cp15's register is useful for your 
>>> system,
>>> then you can't use the 32-bit accessor to get or set the 64-bit cp15's 
>>> register.
>>>
>>> TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
>>> The following description which comes from ARM(r) Architecture Reference
>>> Manual ARMv7-A and ARMv7-R edition tell us the reason:
>>>
>>> 64-bit TTBR0 and TTBR1 format:
>>> ...
>>> BADDR, bits[39:x] :
>>> Translation table base address, bits[39:x]. Defining the translation table 
>>> base address width on
>>> page B4-1698 describes how x is defined.
>>> The value of x determines the required alignment of the translation table, 
>>> which must be aligned to
>>> 2x bytes.
>>>
>>> Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max 
>>> value of 32-bit, so bits[39:32] may
>>> be valid value which is useful for the system.
>>>
>>> 64-bit PAR format
>>> ...
>>> PA[39:12]
>>> Physical Address. The physical address corresponding to the supplied 
>>> virtual address. This field
>>> returns address bits[39:12].
>>>
>>> Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger 
>>> than max value of 32-bit,
>>> so bits[39:32] may be valid value which is useful for the system.
>>>
>>> Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
>>> supporting LPAE,
>>> if you do that, your system may run error.
> 
>> That's not really true. You can run an non-LPAE kernel that uses the
>> 32bit accessors an a Cortex-A15 that supports LPAE. You're just limited
>> to 4GB of physical space. And you're pretty much guaranteed to have
>> some memory below 4GB (one way or another), or you'd have a slight
>> problem setting up your page tables.
> 
>>   M.
>> --
>> Without deviation from the norm, progress is not possible.
> 

> Thanks for your review.
> Please don't ask people to limit to 4GB of physical space on CPU
> supporting LPAE, please don't ask people to guaranteed to have some
> memory below 4GB on CPU supporting LPAE.

Please tell me how you enable LPAE if you don't. I've truly curious.
Because otherwise, you should really take a step back and seriously
reconsider what you're writing. Hint: where do you think the page tables
required to enable LPAE will be? How do you even *boot*?

> Why people select CPU supporting LPAE(just like cortex A15)? 
> Because some of people think 4GB physical space is not enough for their 
> system, maybe they want to use 8GB/16GB DDR space.
> Then you tell them that they must guaranteed to have some memory below 4GB,
> just only because you think the code as follow:
> +#define TTBR0   __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1   __ACCESS_CP15(c2, 0, c0, 1)
> +#define PAR __ACCESS_CP15(c7, 0, c4, 0)
> 
> is better than the code like this:
> 
> +#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
> 
> 
> So,I think the following code: 
> +#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
> 
> is better because it's not necessary to ask people to guaranteed to
> have some memory below 4GB on CPU supporting LPAE. 

NAK.

> If we want to ask people to guaranteed to have some memory below 4GB 
> on CPU supporting LPAE, there need to modify some other code.
> I think it makes the simple problem more complex to modify some other code 
> for this.

At this stage, you've proven that you don't understand the problem at hand.

M.
-- 
Jazz is not dead. It just smells funny...

Re: 答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 21/11/17 07:59, Liuwenliang (Abbott Liu) wrote:
> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
>> On Sat, 18 Nov 2017 10:40:08 +
>> "Liuwenliang (Abbott Liu)"  wrote:
> 
>>> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
 If your processor does support LPAE (like a Cortex-A15 for example),
 then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
 accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
 the lower 32-bits of the 64-bit register.

 Hope this helps,
 -Christoffer
>>>
>>> If you know the higher 32-bits of the 64-bits cp15's register is not useful 
>>> for your system,
>>> then you can use the 32-bit accessor to get or set the 64-bit cp15's 
>>> register.
>>> But if the higher 32-bits of the 64-bits cp15's register is useful for your 
>>> system,
>>> then you can't use the 32-bit accessor to get or set the 64-bit cp15's 
>>> register.
>>>
>>> TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
>>> The following description which comes from ARM(r) Architecture Reference
>>> Manual ARMv7-A and ARMv7-R edition tell us the reason:
>>>
>>> 64-bit TTBR0 and TTBR1 format:
>>> ...
>>> BADDR, bits[39:x] :
>>> Translation table base address, bits[39:x]. Defining the translation table 
>>> base address width on
>>> page B4-1698 describes how x is defined.
>>> The value of x determines the required alignment of the translation table, 
>>> which must be aligned to
>>> 2x bytes.
>>>
>>> Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max 
>>> value of 32-bit, so bits[39:32] may
>>> be valid value which is useful for the system.
>>>
>>> 64-bit PAR format
>>> ...
>>> PA[39:12]
>>> Physical Address. The physical address corresponding to the supplied 
>>> virtual address. This field
>>> returns address bits[39:12].
>>>
>>> Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger 
>>> than max value of 32-bit,
>>> so bits[39:32] may be valid value which is useful for the system.
>>>
>>> Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
>>> supporting LPAE,
>>> if you do that, your system may run error.
> 
>> That's not really true. You can run an non-LPAE kernel that uses the
>> 32bit accessors an a Cortex-A15 that supports LPAE. You're just limited
>> to 4GB of physical space. And you're pretty much guaranteed to have
>> some memory below 4GB (one way or another), or you'd have a slight
>> problem setting up your page tables.
> 
>>   M.
>> --
>> Without deviation from the norm, progress is not possible.
> 

> Thanks for your review.
> Please don't ask people to limit to 4GB of physical space on CPU
> supporting LPAE, please don't ask people to guaranteed to have some
> memory below 4GB on CPU supporting LPAE.

Please tell me how you enable LPAE if you don't. I've truly curious.
Because otherwise, you should really take a step back and seriously
reconsider what you're writing. Hint: where do you think the page tables
required to enable LPAE will be? How do you even *boot*?

> Why people select CPU supporting LPAE(just like cortex A15)? 
> Because some of people think 4GB physical space is not enough for their 
> system, maybe they want to use 8GB/16GB DDR space.
> Then you tell them that they must guaranteed to have some memory below 4GB,
> just only because you think the code as follow:
> +#define TTBR0   __ACCESS_CP15(c2, 0, c0, 0)
> +#define TTBR1   __ACCESS_CP15(c2, 0, c0, 1)
> +#define PAR __ACCESS_CP15(c7, 0, c4, 0)
> 
> is better than the code like this:
> 
> +#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
> 
> 
> So,I think the following code: 
> +#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
> 
> is better because it's not necessary to ask people to guaranteed to
> have some memory below 4GB on CPU supporting LPAE. 

NAK.

> If we want to ask people to guaranteed to have some memory below 4GB 
> on CPU supporting LPAE, there need to modify some other code.
> I think it makes the simple problem more complex to modify some other code 
> for this.

At this stage, you've proven that you don't understand the problem at hand.

M.
-- 
Jazz is not dead. It just smells funny...

Re: 答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
> >On Sat, 18 Nov 2017 10:40:08 +
> >"Liuwenliang (Abbott Liu)"  wrote:
> 
> >> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> >> >If your processor does support LPAE (like a Cortex-A15 for example),
> >> >then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
> >> >accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
> >> >the lower 32-bits of the 64-bit register.
> >> >
> >> >Hope this helps,
> >> >-Christoffer
> >>
> >> If you know the higher 32-bits of the 64-bits cp15's register is not 
> >> useful for your system,
> >> then you can use the 32-bit accessor to get or set the 64-bit cp15's 
> >> register.
> >> But if the higher 32-bits of the 64-bits cp15's register is useful for 
> >> your system,
> >> then you can't use the 32-bit accessor to get or set the 64-bit cp15's 
> >> register.
> >>
> >> TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
> >> The following description which comes from ARM(r) Architecture Reference
> >> Manual ARMv7-A and ARMv7-R edition tell us the reason:
> >>
> >> 64-bit TTBR0 and TTBR1 format:
> >> ...
> >> BADDR, bits[39:x] :
> >> Translation table base address, bits[39:x]. Defining the translation table 
> >> base address width on
> >> page B4-1698 describes how x is defined.
> >> The value of x determines the required alignment of the translation table, 
> >> which must be aligned to
> >> 2x bytes.
> >>
> >> Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max 
> >> value of 32-bit, so bits[39:32] may
> >> be valid value which is useful for the system.
> >>
> >> 64-bit PAR format
> >> ...
> >> PA[39:12]
> >> Physical Address. The physical address corresponding to the supplied 
> >> virtual address. This field
> >> returns address bits[39:12].
> >>
> >> Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger 
> >> than max value of 32-bit,
> >> so bits[39:32] may be valid value which is useful for the system.
> >>
> >> Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
> >> supporting LPAE,
> >> if you do that, your system may run error.
> 
> >That's not really true. You can run an non-LPAE kernel that uses the
> >32bit accessors an a Cortex-A15 that supports LPAE. You're just limited
> >to 4GB of physical space. And you're pretty much guaranteed to have
> >some memory below 4GB (one way or another), or you'd have a slight
> >problem setting up your page tables.
> 
> >   M.
> >--
> >Without deviation from the norm, progress is not possible.
> 
> Thanks for your review.
> Please don't ask people to limit to 4GB of physical space on CPU
> supporting LPAE, please don't ask people to guaranteed to have some
> memory below 4GB on CPU supporting LPAE.

A LPAE-capable CPU must always have memory below 4GB physical, no ifs
no buts.

If there's no memory below 4GB physical, then the CPU has no accessible
memory before the MMU is enabled.  That means operating systems such as
Linux are completely unbootable.

There must _always_ be accessible memory below 4GB physical.  This is
not negotiable, it's a fundamental requirement.

The location of physical memory has nothing to do with the accessors.
This point I'm making also has nothing to do with the accessors.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: 答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Tue, Nov 21, 2017 at 07:59:01AM +, Liuwenliang (Abbott Liu) wrote:
> On Nov 17, 2017  21:49  Marc Zyngier [mailto:marc.zyng...@arm.com]  wrote:
> >On Sat, 18 Nov 2017 10:40:08 +
> >"Liuwenliang (Abbott Liu)"  wrote:
> 
> >> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> >> >If your processor does support LPAE (like a Cortex-A15 for example),
> >> >then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
> >> >accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
> >> >the lower 32-bits of the 64-bit register.
> >> >
> >> >Hope this helps,
> >> >-Christoffer
> >>
> >> If you know the higher 32-bits of the 64-bits cp15's register is not 
> >> useful for your system,
> >> then you can use the 32-bit accessor to get or set the 64-bit cp15's 
> >> register.
> >> But if the higher 32-bits of the 64-bits cp15's register is useful for 
> >> your system,
> >> then you can't use the 32-bit accessor to get or set the 64-bit cp15's 
> >> register.
> >>
> >> TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
> >> The following description which comes from ARM(r) Architecture Reference
> >> Manual ARMv7-A and ARMv7-R edition tell us the reason:
> >>
> >> 64-bit TTBR0 and TTBR1 format:
> >> ...
> >> BADDR, bits[39:x] :
> >> Translation table base address, bits[39:x]. Defining the translation table 
> >> base address width on
> >> page B4-1698 describes how x is defined.
> >> The value of x determines the required alignment of the translation table, 
> >> which must be aligned to
> >> 2x bytes.
> >>
> >> Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max 
> >> value of 32-bit, so bits[39:32] may
> >> be valid value which is useful for the system.
> >>
> >> 64-bit PAR format
> >> ...
> >> PA[39:12]
> >> Physical Address. The physical address corresponding to the supplied 
> >> virtual address. This field
> >> returns address bits[39:12].
> >>
> >> Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger 
> >> than max value of 32-bit,
> >> so bits[39:32] may be valid value which is useful for the system.
> >>
> >> Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
> >> supporting LPAE,
> >> if you do that, your system may run error.
> 
> >That's not really true. You can run an non-LPAE kernel that uses the
> >32bit accessors an a Cortex-A15 that supports LPAE. You're just limited
> >to 4GB of physical space. And you're pretty much guaranteed to have
> >some memory below 4GB (one way or another), or you'd have a slight
> >problem setting up your page tables.
> 
> >   M.
> >--
> >Without deviation from the norm, progress is not possible.
> 
> Thanks for your review.
> Please don't ask people to limit to 4GB of physical space on CPU
> supporting LPAE, please don't ask people to guaranteed to have some
> memory below 4GB on CPU supporting LPAE.

A LPAE-capable CPU must always have memory below 4GB physical, no ifs
no buts.

If there's no memory below 4GB physical, then the CPU has no accessible
memory before the MMU is enabled.  That means operating systems such as
Linux are completely unbootable.

There must _always_ be accessible memory below 4GB physical.  This is
not negotiable, it's a fundamental requirement.

The location of physical memory has nothing to do with the accessors.
This point I'm making also has nothing to do with the accessors.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On Sat, 18 Nov 2017 10:40:08 +
"Liuwenliang (Abbott Liu)"  wrote:

> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> >If your processor does support LPAE (like a Cortex-A15 for example),
> >then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
> >accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
> >the lower 32-bits of the 64-bit register.
> >
> >Hope this helps,
> >-Christoffer  
> 
> If you know the higher 32-bits of the 64-bits cp15's register is not useful 
> for your system,
> then you can use the 32-bit accessor to get or set the 64-bit cp15's register.
> But if the higher 32-bits of the 64-bits cp15's register is useful for your 
> system,
> then you can't use the 32-bit accessor to get or set the 64-bit cp15's 
> register.
> 
> TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
> The following description which comes from ARM(r) Architecture Reference
> Manual ARMv7-A and ARMv7-R edition tell us the reason:
> 
> 64-bit TTBR0 and TTBR1 format:
> ...
> BADDR, bits[39:x] : 
> Translation table base address, bits[39:x]. Defining the translation table 
> base address width on
> page B4-1698 describes how x is defined.
> The value of x determines the required alignment of the translation table, 
> which must be aligned to
> 2x bytes.
> 
> Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max value 
> of 32-bit, so bits[39:32] may 
> be valid value which is useful for the system.
> 
> 64-bit PAR format
> ...
> PA[39:12]
> Physical Address. The physical address corresponding to the supplied virtual 
> address. This field
> returns address bits[39:12].
> 
> Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger 
> than max value of 32-bit, 
> so bits[39:32] may be valid value which is useful for the system.
> 
> Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
> supporting LPAE,
> if you do that, your system may run error.

That's not really true. You can run an non-LPAE kernel that uses the
32bit accessors an a Cortex-A15 that supports LPAE. You're just limited
to 4GB of physical space. And you're pretty much guaranteed to have
some memory below 4GB (one way or another), or you'd have a slight
problem setting up your page tables.

M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On Sat, 18 Nov 2017 10:40:08 +
"Liuwenliang (Abbott Liu)"  wrote:

> On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
> >If your processor does support LPAE (like a Cortex-A15 for example),
> >then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
> >accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
> >the lower 32-bits of the 64-bit register.
> >
> >Hope this helps,
> >-Christoffer  
> 
> If you know the higher 32-bits of the 64-bits cp15's register is not useful 
> for your system,
> then you can use the 32-bit accessor to get or set the 64-bit cp15's register.
> But if the higher 32-bits of the 64-bits cp15's register is useful for your 
> system,
> then you can't use the 32-bit accessor to get or set the 64-bit cp15's 
> register.
> 
> TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
> The following description which comes from ARM(r) Architecture Reference
> Manual ARMv7-A and ARMv7-R edition tell us the reason:
> 
> 64-bit TTBR0 and TTBR1 format:
> ...
> BADDR, bits[39:x] : 
> Translation table base address, bits[39:x]. Defining the translation table 
> base address width on
> page B4-1698 describes how x is defined.
> The value of x determines the required alignment of the translation table, 
> which must be aligned to
> 2x bytes.
> 
> Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max value 
> of 32-bit, so bits[39:32] may 
> be valid value which is useful for the system.
> 
> 64-bit PAR format
> ...
> PA[39:12]
> Physical Address. The physical address corresponding to the supplied virtual 
> address. This field
> returns address bits[39:12].
> 
> Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger 
> than max value of 32-bit, 
> so bits[39:32] may be valid value which is useful for the system.
> 
> Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
> supporting LPAE,
> if you do that, your system may run error.

That's not really true. You can run an non-LPAE kernel that uses the
32bit accessors an a Cortex-A15 that supports LPAE. You're just limited
to 4GB of physical space. And you're pretty much guaranteed to have
some memory below 4GB (one way or another), or you'd have a slight
problem setting up your page tables.

M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
>If your processor does support LPAE (like a Cortex-A15 for example),
>then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
>accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
>the lower 32-bits of the 64-bit register.
>
>Hope this helps,
>-Christoffer

If you know the higher 32-bits of the 64-bits cp15's register is not useful for 
your system,
then you can use the 32-bit accessor to get or set the 64-bit cp15's register.
But if the higher 32-bits of the 64-bits cp15's register is useful for your 
system,
then you can't use the 32-bit accessor to get or set the 64-bit cp15's register.

TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
The following description which comes from ARM(r) Architecture Reference
Manual ARMv7-A and ARMv7-R edition tell us the reason:

64-bit TTBR0 and TTBR1 format:
...
BADDR, bits[39:x] : 
Translation table base address, bits[39:x]. Defining the translation table base 
address width on
page B4-1698 describes how x is defined.
The value of x determines the required alignment of the translation table, 
which must be aligned to
2x bytes.

Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max value 
of 32-bit, so bits[39:32] may 
be valid value which is useful for the system.

64-bit PAR format
...
PA[39:12]
Physical Address. The physical address corresponding to the supplied virtual 
address. This field
returns address bits[39:12].

Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger than 
max value of 32-bit, 
so bits[39:32] may be valid value which is useful for the system.

Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
supporting LPAE,
if you do that, your system may run error.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On Nov 17, 2017  15:36 Christoffer Dall [mailto:cd...@linaro.org]  wrote:
>If your processor does support LPAE (like a Cortex-A15 for example),
>then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
>accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
>the lower 32-bits of the 64-bit register.
>
>Hope this helps,
>-Christoffer

If you know the higher 32-bits of the 64-bits cp15's register is not useful for 
your system,
then you can use the 32-bit accessor to get or set the 64-bit cp15's register.
But if the higher 32-bits of the 64-bits cp15's register is useful for your 
system,
then you can't use the 32-bit accessor to get or set the 64-bit cp15's register.

TTBR0/TTBR1/PAR's higher 32-bits is useful for CPU supporting LPAE.
The following description which comes from ARM(r) Architecture Reference
Manual ARMv7-A and ARMv7-R edition tell us the reason:

64-bit TTBR0 and TTBR1 format:
...
BADDR, bits[39:x] : 
Translation table base address, bits[39:x]. Defining the translation table base 
address width on
page B4-1698 describes how x is defined.
The value of x determines the required alignment of the translation table, 
which must be aligned to
2x bytes.

Abbott Liu: Because BADDR on CPU supporting LPAE may be bigger than max value 
of 32-bit, so bits[39:32] may 
be valid value which is useful for the system.

64-bit PAR format
...
PA[39:12]
Physical Address. The physical address corresponding to the supplied virtual 
address. This field
returns address bits[39:12].

Abbott Liu: Because Physical Address on CPU supporting LPAE may be bigger than 
max value of 32-bit, 
so bits[39:32] may be valid value which is useful for the system.

Conclusion: Don't use 32-bit accessor to get or set TTBR0/TTBR1/PAR on CPU 
supporting LPAE,
if you do that, your system may run error.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Christoffer Dall]

On Fri, Nov 17, 2017 at 07:18:45AM +, Liuwenliang (Abbott Liu) wrote:
> On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
> >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.
> 
> I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
> but I don't use the definition TTBR0 as __ACCESS_CP15_64. 
> 
> Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
> LPAE(vexpress_a9)

What does a "CPU supporting LPAE(vexpress_a9) mean?  As Marc pointed
out, a Cortex-A9 doesn't support LPAE.  If you configure your kernel
with LPAE it's not going to work on a Cortex-A9.

> I find it doesn't work and report undefined instruction error
> when execute "mrrc" instruction.
> 
> So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.
> 

It's the other way around.  It doesn't work WITHOUT LPAE, it only works
WITH LPAE.

The ARM ARM explains this quite clearly:

"Accessing TTBR0

To access TTBR0 in an implementation that does not include the Large
Physical Address Extension, or bits[31:0] of TTBR0 in an implementation
that includes the Large Physical Address Extension, software reads or
writes the CP15 registers with  set to 0,  set to c2, 
set to c0, and  set to 0. For example:

MRC p15, 0, , c2, c0, 0
  ; Read 32-bit TTBR0 into Rt
MCR p15, 0, , c2, c0, 0
  ; Write Rt to 32-bit TTBR0

In an implementation that includes the Large Physical Address Extension,
to access all 64 bits of TTBR0, software performs a 64-bit read or write
of the CP15 registers with  set to c2 and  set to 0. For
example:

MRRC p15, 0, , , c2
  ; Read 64-bit TTBR0 into Rt (low word) and Rt2 (high word)
MCRR p15, 0, , , c2
  ; Write Rt (low word) and Rt2 (high word) to 64-bit TTBR0

In these MRRC and MCRR instructions, Rt holds the least-significant word
of TTBR0, and Rt2 holds the most-significant word."

That is, if your processor (like the Cortex-A9) does NOT support LPAE,
all you have is the 32-bit accessors (MRC and MCR).

If your processor does support LPAE (like a Cortex-A15 for example),
then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
the lower 32-bits of the 64-bit register.

Hope this helps,
-Christoffer

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Christoffer Dall]

On Fri, Nov 17, 2017 at 07:18:45AM +, Liuwenliang (Abbott Liu) wrote:
> On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
> >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.
> 
> I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
> but I don't use the definition TTBR0 as __ACCESS_CP15_64. 
> 
> Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
> LPAE(vexpress_a9)

What does a "CPU supporting LPAE(vexpress_a9) mean?  As Marc pointed
out, a Cortex-A9 doesn't support LPAE.  If you configure your kernel
with LPAE it's not going to work on a Cortex-A9.

> I find it doesn't work and report undefined instruction error
> when execute "mrrc" instruction.
> 
> So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.
> 

It's the other way around.  It doesn't work WITHOUT LPAE, it only works
WITH LPAE.

The ARM ARM explains this quite clearly:

"Accessing TTBR0

To access TTBR0 in an implementation that does not include the Large
Physical Address Extension, or bits[31:0] of TTBR0 in an implementation
that includes the Large Physical Address Extension, software reads or
writes the CP15 registers with  set to 0,  set to c2, 
set to c0, and  set to 0. For example:

MRC p15, 0, , c2, c0, 0
  ; Read 32-bit TTBR0 into Rt
MCR p15, 0, , c2, c0, 0
  ; Write Rt to 32-bit TTBR0

In an implementation that includes the Large Physical Address Extension,
to access all 64 bits of TTBR0, software performs a 64-bit read or write
of the CP15 registers with  set to c2 and  set to 0. For
example:

MRRC p15, 0, , , c2
  ; Read 64-bit TTBR0 into Rt (low word) and Rt2 (high word)
MCRR p15, 0, , , c2
  ; Write Rt (low word) and Rt2 (high word) to 64-bit TTBR0

In these MRRC and MCRR instructions, Rt holds the least-significant word
of TTBR0, and Rt2 holds the most-significant word."

That is, if your processor (like the Cortex-A9) does NOT support LPAE,
all you have is the 32-bit accessors (MRC and MCR).

If your processor does support LPAE (like a Cortex-A15 for example),
then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
the lower 32-bits of the 64-bit register.

Hope this helps,
-Christoffer

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>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.

I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
but I don't use the definition TTBR0 as __ACCESS_CP15_64. 

Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
LPAE(vexpress_a9), I find it doesn't work and report undefined instruction error
when execute "mrrc" instruction.

So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>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.

I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
but I don't use the definition TTBR0 as __ACCESS_CP15_64. 

Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
LPAE(vexpress_a9), I find it doesn't work and report undefined instruction error
when execute "mrrc" instruction.

So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.

答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>- If the CPU supports LPAE, then both 32 and 64bit accessors work


I don't how 32bit accessor can work on CPU supporting LPAE, give me your 
solution.

Thanks.

答复: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>- If the CPU supports LPAE, then both 32 and 64bit accessors work


I don't how 32bit accessor can work on CPU supporting LPAE, give me your 
solution.

Thanks.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

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.zyng...@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.zyng...@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 (=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 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

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.zyng...@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.zyng...@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 (=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 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  17:54 Marc Zyngier [mailto:marc.zyng...@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.zyng...@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 (=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.

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)
+#define VTTBR   __ACCESS_CP15_64(6, c2)
+#define CNTV_CVAL   __ACCESS_CP15_64(3, c14)
+#define CNTVOFF __ACCESS_CP15_64(4, c14)
+

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  17:54 Marc Zyngier [mailto:marc.zyng...@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.zyng...@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 (=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.

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)
+#define VTTBR   __ACCESS_CP15_64(6, c2)
+#define CNTV_CVAL   __ACCESS_CP15_64(3, c14)
+#define CNTVOFF __ACCESS_CP15_64(4, c14)
+
+#define MIDR__ACCESS_CP15(c0, 0, 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

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.zyng...@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 (=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,

M.
-- 
Jazz is not dead, it just smell funny.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

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.zyng...@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 (=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,

M.
-- 
Jazz is not dead, it just smell funny.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

>On 15/11/17 13:16, Liuwenliang (Abbott Liu) wrote:
>> On 09/11/17  18:36 Marc Zyngier [mailto:marc.zyng...@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
(=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". 

64-bit TTBR0 and TTBR1 format
The bit assignments for the 64-bit implementations of TTBR0 and TTBR1 are 
identical, and are:
Bits[63:56] UNK/SBZP.
ASID, bits[55:48]:
  An ASID for the translation table base address. The TTBCR.A1 field selects 
either TTBR0.ASID
or TTBR1.ASID.
Bits[47:40] UNK/SBZP.
BADDR, bits[39:x]:
   Translation table base address, bits[39:x]. Defining the translation table 
base address width on
page B4-1698 describes how x is defined.
The value of x determines the required alignment of the translation table, 
which must be aligned to
2x bytes.

Bits[x-1:0] UNK/SBZP.
...
To access a 64-bit TTBR0, software performs a 64-bit read or write of the CP15 
registers with  set to c2 and
 set to 0. For example:
MRRC p15,0,,, c2 ; Read 64-bit TTBR0 into Rt (low word) and Rt2 (high 
word)
MCRR p15,0,,, c2 ; Write Rt (low word) and Rt2 (high word) to 64-bit 
TTBR0

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.

>Also, if I follow your reasoning, why are you bothering defining PAR in
>the non-LPAE case? It is not used by anything, as far as I can see...

I don't use the PAR, I change the defining PAR just because I think it will be 
wrong in
a non LPAE CPU.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

>On 15/11/17 13:16, Liuwenliang (Abbott Liu) wrote:
>> On 09/11/17  18:36 Marc Zyngier [mailto:marc.zyng...@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
(=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". 

64-bit TTBR0 and TTBR1 format
The bit assignments for the 64-bit implementations of TTBR0 and TTBR1 are 
identical, and are:
Bits[63:56] UNK/SBZP.
ASID, bits[55:48]:
  An ASID for the translation table base address. The TTBCR.A1 field selects 
either TTBR0.ASID
or TTBR1.ASID.
Bits[47:40] UNK/SBZP.
BADDR, bits[39:x]:
   Translation table base address, bits[39:x]. Defining the translation table 
base address width on
page B4-1698 describes how x is defined.
The value of x determines the required alignment of the translation table, 
which must be aligned to
2x bytes.

Bits[x-1:0] UNK/SBZP.
...
To access a 64-bit TTBR0, software performs a 64-bit read or write of the CP15 
registers with  set to c2 and
 set to 0. For example:
MRRC p15,0,,, c2 ; Read 64-bit TTBR0 into Rt (low word) and Rt2 (high 
word)
MCRR p15,0,,, c2 ; Write Rt (low word) and Rt2 (high word) to 64-bit 
TTBR0

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.

>Also, if I follow your reasoning, why are you bothering defining PAR in
>the non-LPAE case? It is not used by anything, as far as I can see...

I don't use the PAR, I change the defining PAR just because I think it will be 
wrong in
a non LPAE CPU.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 15/11/17 13:16, Liuwenliang (Abbott Liu) wrote:
> On 09/11/17  18:36 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>> On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" 
>>  wrote:
>>> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
>  #include 
>  #include 
> +#include 

 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:
[...]

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.

Also, if I follow your reasoning, why are you bothering defining PAR in
the non-LPAE case? It is not used by anything, as far as I can see...

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 15/11/17 13:16, Liuwenliang (Abbott Liu) wrote:
> On 09/11/17  18:36 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>> On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" 
>>  wrote:
>>> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
>  #include 
>  #include 
> +#include 

 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:
[...]

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.

Also, if I follow your reasoning, why are you bothering defining PAR in
the non-LPAE case? It is not used by anything, as far as I can see...

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 09/11/17  18:36 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" 
> wrote:
>> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
  #include 
  #include 
 +#include 
>>>
>>>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 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 09/11/17  18:36 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" 
> wrote:
>> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
  #include 
  #include 
 +#include 
>>>
>>>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 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" 
 wrote:
> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
>>>  #include 
>>>  #include 
>>> +#include 
>>
>>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.

>
> 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.

Thanks,

M.
-- 
Jazz is not dead, it just smell funny.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On Wed, Nov 15 2017 at 10:20:02 am GMT, "Liuwenliang (Abbott Liu)" 
 wrote:
> On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
>>>  #include 
>>>  #include 
>>> +#include 
>>
>>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.

>
> 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.

Thanks,

M.
-- 
Jazz is not dead, it just smell funny.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
>>  #include 
>>  #include 
>> +#include 
>
>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.

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
+#define MIDR__ACCESS_CP15(c0, 0, c0, 0)
+#define CSSELR  __ACCESS_CP15(c0, 2, c0, 0)
+#define SCTLR   __ACCESS_CP15(c1, 0, c0, 0)
+#define CPACR   __ACCESS_CP15(c1, 0, c0, 2)
+#define TTBCR   __ACCESS_CP15(c2, 0, c0, 2)
+#define DACR__ACCESS_CP15(c3, 0, c0, 0)
+#define DFSR__ACCESS_CP15(c5, 0, c0, 0)
+#define IFSR__ACCESS_CP15(c5, 0, c0, 1)
+#define ADFSR   __ACCESS_CP15(c5, 0, c1, 0)
+#define AIFSR   __ACCESS_CP15(c5, 0, c1, 1)
+#define DFAR__ACCESS_CP15(c6, 0, c0, 0)
+#define IFAR__ACCESS_CP15(c6, 0, c0, 2)
+#define ICIALLUIS   __ACCESS_CP15(c7, 0, c1, 0)
+#define ATS1CPR __ACCESS_CP15(c7, 0, c8, 0)
+#define TLBIALLIS   __ACCESS_CP15(c8, 0, c3, 0)
+#define TLBIALL __ACCESS_CP15(c8, 0, c7, 0)
+#define TLBIALLNSNHIS   __ACCESS_CP15(c8, 4, c3, 4)
+#define PRRR__ACCESS_CP15(c10, 0, c2, 0)
+#define NMRR__ACCESS_CP15(c10, 0, c2, 1)
+#define AMAIR0  __ACCESS_CP15(c10, 0, c3, 0)
+#define AMAIR1  __ACCESS_CP15(c10, 0, c3, 1)
+#define CID __ACCESS_CP15(c13, 0, c0, 1)
+#define TID_URW __ACCESS_CP15(c13, 0, c0, 2)
+#define TID_URO __ACCESS_CP15(c13, 0, c0, 3)
+#define TID_PRIV__ACCESS_CP15(c13, 0, c0, 4)
+#define CNTKCTL __ACCESS_CP15(c14, 0, c1, 0)
+#define CNTHCTL __ACCESS_CP15(c14, 4, c1, 0)
+
 extern unsigned long cr_alignment; /* defined in entry-armv.S */

 static inline unsigned long get_cr(void)
diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
index 14b5903..db8d8db 100644
--- a/arch/arm/include/asm/kvm_hyp.h
+++ b/arch/arm/include/asm/kvm_hyp.h
@@ -37,55 +37,25 @@
__val;  \
 })

-#define TTBR0  __ACCESS_CP15_64(0, c2)
-#define TTBR1  __ACCESS_CP15_64(1, c2)
 #define VTTBR  __ACCESS_CP15_64(6, c2)
-#define PAR__ACCESS_CP15_64(0, c7)
 #define CNTV_CVAL  __ACCESS_CP15_64(3, c14)
 #define CNTVOFF__ACCESS_CP15_64(4, c14)

-#define MIDR   __ACCESS_CP15(c0, 0, c0, 0)
-#define CSSELR __ACCESS_CP15(c0, 2, c0, 0)
 #define VPIDR  __ACCESS_CP15(c0, 4, c0, 0)
 #define VMPIDR __ACCESS_CP15(c0, 4, c0, 5)
-#define SCTLR  __ACCESS_CP15(c1, 0, c0, 0)
-#define CPACR  __ACCESS_CP15(c1, 0, c0, 2)
 #define HCR__ACCESS_CP15(c1, 4, c1, 0)
 #define HDCR   __ACCESS_CP15(c1, 4, c1, 1)
 #define HCPTR  __ACCESS_CP15(c1, 4, c1, 2)
 #define HSTR   __ACCESS_CP15(c1, 4, c1, 3)
-#define TTBCR  __ACCESS_CP15(c2, 0, c0, 2)
 #define HTCR   __ACCESS_CP15(c2, 4, c0, 2)
 #define VTCR   __ACCESS_CP15(c2, 4, c1, 2)
-#define DACR   __ACCESS_CP15(c3, 0, c0, 0)
-#define DFSR   __ACCESS_CP15(c5, 0, c0, 0)
-#define IFSR   __ACCESS_CP15(c5, 0, c0, 1)
-#define ADFSR  __ACCESS_CP15(c5, 0, c1, 0)
-#define AIFSR  __ACCESS_CP15(c5, 0, c1, 1)
 #define HSR__ACCESS_CP15(c5, 4, c2, 0)
-#define DFAR   __ACCESS_CP15(c6, 0, c0, 0)
-#define IFAR   __ACCESS_CP15(c6, 0, c0, 2)
 #define HDFAR  __ACCESS_CP15(c6, 4, c0, 0)
 #define HIFAR 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 09/11/17 18:11, Marc Zyngier [mailto:marc.zyng...@arm.com] 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 
>>  #include 
>>  #include 
>> +#include 
>
>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.

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
+#define MIDR__ACCESS_CP15(c0, 0, c0, 0)
+#define CSSELR  __ACCESS_CP15(c0, 2, c0, 0)
+#define SCTLR   __ACCESS_CP15(c1, 0, c0, 0)
+#define CPACR   __ACCESS_CP15(c1, 0, c0, 2)
+#define TTBCR   __ACCESS_CP15(c2, 0, c0, 2)
+#define DACR__ACCESS_CP15(c3, 0, c0, 0)
+#define DFSR__ACCESS_CP15(c5, 0, c0, 0)
+#define IFSR__ACCESS_CP15(c5, 0, c0, 1)
+#define ADFSR   __ACCESS_CP15(c5, 0, c1, 0)
+#define AIFSR   __ACCESS_CP15(c5, 0, c1, 1)
+#define DFAR__ACCESS_CP15(c6, 0, c0, 0)
+#define IFAR__ACCESS_CP15(c6, 0, c0, 2)
+#define ICIALLUIS   __ACCESS_CP15(c7, 0, c1, 0)
+#define ATS1CPR __ACCESS_CP15(c7, 0, c8, 0)
+#define TLBIALLIS   __ACCESS_CP15(c8, 0, c3, 0)
+#define TLBIALL __ACCESS_CP15(c8, 0, c7, 0)
+#define TLBIALLNSNHIS   __ACCESS_CP15(c8, 4, c3, 4)
+#define PRRR__ACCESS_CP15(c10, 0, c2, 0)
+#define NMRR__ACCESS_CP15(c10, 0, c2, 1)
+#define AMAIR0  __ACCESS_CP15(c10, 0, c3, 0)
+#define AMAIR1  __ACCESS_CP15(c10, 0, c3, 1)
+#define CID __ACCESS_CP15(c13, 0, c0, 1)
+#define TID_URW __ACCESS_CP15(c13, 0, c0, 2)
+#define TID_URO __ACCESS_CP15(c13, 0, c0, 3)
+#define TID_PRIV__ACCESS_CP15(c13, 0, c0, 4)
+#define CNTKCTL __ACCESS_CP15(c14, 0, c1, 0)
+#define CNTHCTL __ACCESS_CP15(c14, 4, c1, 0)
+
 extern unsigned long cr_alignment; /* defined in entry-armv.S */

 static inline unsigned long get_cr(void)
diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
index 14b5903..db8d8db 100644
--- a/arch/arm/include/asm/kvm_hyp.h
+++ b/arch/arm/include/asm/kvm_hyp.h
@@ -37,55 +37,25 @@
__val;  \
 })

-#define TTBR0  __ACCESS_CP15_64(0, c2)
-#define TTBR1  __ACCESS_CP15_64(1, c2)
 #define VTTBR  __ACCESS_CP15_64(6, c2)
-#define PAR__ACCESS_CP15_64(0, c7)
 #define CNTV_CVAL  __ACCESS_CP15_64(3, c14)
 #define CNTVOFF__ACCESS_CP15_64(4, c14)

-#define MIDR   __ACCESS_CP15(c0, 0, c0, 0)
-#define CSSELR __ACCESS_CP15(c0, 2, c0, 0)
 #define VPIDR  __ACCESS_CP15(c0, 4, c0, 0)
 #define VMPIDR __ACCESS_CP15(c0, 4, c0, 5)
-#define SCTLR  __ACCESS_CP15(c1, 0, c0, 0)
-#define CPACR  __ACCESS_CP15(c1, 0, c0, 2)
 #define HCR__ACCESS_CP15(c1, 4, c1, 0)
 #define HDCR   __ACCESS_CP15(c1, 4, c1, 1)
 #define HCPTR  __ACCESS_CP15(c1, 4, c1, 2)
 #define HSTR   __ACCESS_CP15(c1, 4, c1, 3)
-#define TTBCR  __ACCESS_CP15(c2, 0, c0, 2)
 #define HTCR   __ACCESS_CP15(c2, 4, c0, 2)
 #define VTCR   __ACCESS_CP15(c2, 4, c1, 2)
-#define DACR   __ACCESS_CP15(c3, 0, c0, 0)
-#define DFSR   __ACCESS_CP15(c5, 0, c0, 0)
-#define IFSR   __ACCESS_CP15(c5, 0, c0, 1)
-#define ADFSR  __ACCESS_CP15(c5, 0, c1, 0)
-#define AIFSR  __ACCESS_CP15(c5, 0, c1, 1)
 #define HSR__ACCESS_CP15(c5, 4, c2, 0)
-#define DFAR   __ACCESS_CP15(c6, 0, c0, 0)
-#define IFAR   __ACCESS_CP15(c6, 0, c0, 2)
 #define HDFAR  __ACCESS_CP15(c6, 4, c0, 0)
 #define HIFAR 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 09/11/17 07:46, Liuwenliang (Abbott Liu) wrote:
> On 12/10/17 15:59, Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>> On 11/10/17 09:22, Abbott Liu wrote:
>>> diff --git a/arch/arm/include/asm/proc-fns.h 
>>> b/arch/arm/include/asm/proc-fns.h
>>> index f2e1af4..6e26714 100644
>>> --- a/arch/arm/include/asm/proc-fns.h
>>> +++ b/arch/arm/include/asm/proc-fns.h
>>> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>>> pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>>> (pgd_t *)phys_to_virt(pg);  \
>>> })
>>> +
>>> +#define cpu_set_ttbr0(val) \
>>> +   do {\
>>> +   u64 ttbr = val; \
>>> +   __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
>>> +   : : "r" (ttbr));\
>>> +   } while (0)
>>> +
>>> +
>>>  #else
>>>  #define cpu_get_pgd()  \
>>> ({  \
>>> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>>> pg &= ~0x3fff;  \
>>> (pgd_t *)phys_to_virt(pg);  \
>>> })
>>> +
>>> +#define cpu_set_ttbr(nr, val)  \
>>> +   do {\
>>> +   u64 ttbr = val; \
>>> +   __asm__("mcrp15, 0, %0, c2, c0, 0"  \
>>> +   : : "r" (ttbr));\
>>> +   } while (0)
>>> +
>>> +#define cpu_get_ttbr(nr)   \
>>> +   ({  \
>>> +   unsigned long ttbr; \
>>> +   __asm__("mrcp15, 0, %0, c2, c0, 0"  \
>>> +   : "=r" (ttbr)); \
>>> +   ttbr;   \
>>> +   })
>>> +
>>> +#define cpu_set_ttbr0(val) \
>>> +   do {\
>>> +   u64 ttbr = val; \
>>> +   __asm__("mcrp15, 0, %0, c2, c0, 0"  \
>>> +   : : "r" (ttbr));\
>>> +   } while (0)
>>> +
>>> +
>>
>> You could instead lift and extend the definitions provided in kvm_hyp.h,
>> and use the read_sysreg/write_sysreg helpers defined in cp15.h.
> 
> Thanks for your review. 
> I extend definitions of TTBR0/TTBR1/PAR in kvm_hyp.h when the CONFIG_ARM_LPAE 
> is 
> not defined. 
> Because cortex A9 don't support virtualization, so use CONFIG_ARM_LPAE to 
> exclude
> some functions and macros which are only used in virtualization.
> 
> Here is the code which I tested on vexpress_a15 and vexpress_a9:
> 
> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
> index 14b5903..2592608 100644
> --- a/arch/arm/include/asm/kvm_hyp.h
> +++ b/arch/arm/include/asm/kvm_hyp.h
> @@ -19,12 +19,14 @@
>  #define __ARM_KVM_HYP_H__
> 
>  #include 
> -#include 
>  #include 
> +
> +#ifdef CONFIG_ARM_LPAE
> +#include 
>  #include 
>  #include 
> -
>  #define __hyp_text __section(.hyp.text) notrace
> +#endif
> 
>  #define __ACCESS_VFP(CRn)  \
> "mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
> @@ -37,12 +39,18 @@
> __val;  \
>  })
> 
> +#ifdef CONFIG_ARM_LPAE
>  #define TTBR0  __ACCESS_CP15_64(0, c2)
>  #define TTBR1  __ACCESS_CP15_64(1, c2)
>  #define VTTBR  __ACCESS_CP15_64(6, c2)
>  #define PAR__ACCESS_CP15_64(0, c7)
>  #define CNTV_CVAL  __ACCESS_CP15_64(3, c14)
>  #define CNTVOFF__ACCESS_CP15_64(4, c14)
> +#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

There is no reason for this LPAE vs non LPAE dichotomy. Both registers
do exist if your system supports LPAE. So you can either suffix the
64bit version with an _64 (and change the KVM code), or suffix the bit
version with _32.

> 
>  #define MIDR   __ACCESS_CP15(c0, 0, c0, 0)
>  #define CSSELR __ACCESS_CP15(c0, 2, c0, 0)
> @@ -98,6 +106,7 @@
>  #define cntvoff_el2CNTVOFF
>  #define cnthctl_el2CNTHCTL
> 
> +#ifdef CONFIG_ARM_LPAE
>  void __timer_save_state(struct kvm_vcpu *vcpu);
>  void __timer_restore_state(struct kvm_vcpu *vcpu);
> 
> @@ -123,5 +132,6 @@ void __hyp_text __banked_restore_state(struct 
> kvm_cpu_context *ctxt);
>  asmlinkage int __guest_enter(struct kvm_vcpu *vcpu,
>  struct kvm_cpu_context *host);
>  asmlinkage int __hyp_do_panic(const char *, int, u32);
> +#endif
> 
>  #endif /* __ARM_KVM_HYP_H__ */
> diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c
> index 049ee0a..359a782 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 09/11/17 07:46, Liuwenliang (Abbott Liu) wrote:
> On 12/10/17 15:59, Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>> On 11/10/17 09:22, Abbott Liu wrote:
>>> diff --git a/arch/arm/include/asm/proc-fns.h 
>>> b/arch/arm/include/asm/proc-fns.h
>>> index f2e1af4..6e26714 100644
>>> --- a/arch/arm/include/asm/proc-fns.h
>>> +++ b/arch/arm/include/asm/proc-fns.h
>>> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>>> pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>>> (pgd_t *)phys_to_virt(pg);  \
>>> })
>>> +
>>> +#define cpu_set_ttbr0(val) \
>>> +   do {\
>>> +   u64 ttbr = val; \
>>> +   __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
>>> +   : : "r" (ttbr));\
>>> +   } while (0)
>>> +
>>> +
>>>  #else
>>>  #define cpu_get_pgd()  \
>>> ({  \
>>> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>>> pg &= ~0x3fff;  \
>>> (pgd_t *)phys_to_virt(pg);  \
>>> })
>>> +
>>> +#define cpu_set_ttbr(nr, val)  \
>>> +   do {\
>>> +   u64 ttbr = val; \
>>> +   __asm__("mcrp15, 0, %0, c2, c0, 0"  \
>>> +   : : "r" (ttbr));\
>>> +   } while (0)
>>> +
>>> +#define cpu_get_ttbr(nr)   \
>>> +   ({  \
>>> +   unsigned long ttbr; \
>>> +   __asm__("mrcp15, 0, %0, c2, c0, 0"  \
>>> +   : "=r" (ttbr)); \
>>> +   ttbr;   \
>>> +   })
>>> +
>>> +#define cpu_set_ttbr0(val) \
>>> +   do {\
>>> +   u64 ttbr = val; \
>>> +   __asm__("mcrp15, 0, %0, c2, c0, 0"  \
>>> +   : : "r" (ttbr));\
>>> +   } while (0)
>>> +
>>> +
>>
>> You could instead lift and extend the definitions provided in kvm_hyp.h,
>> and use the read_sysreg/write_sysreg helpers defined in cp15.h.
> 
> Thanks for your review. 
> I extend definitions of TTBR0/TTBR1/PAR in kvm_hyp.h when the CONFIG_ARM_LPAE 
> is 
> not defined. 
> Because cortex A9 don't support virtualization, so use CONFIG_ARM_LPAE to 
> exclude
> some functions and macros which are only used in virtualization.
> 
> Here is the code which I tested on vexpress_a15 and vexpress_a9:
> 
> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
> index 14b5903..2592608 100644
> --- a/arch/arm/include/asm/kvm_hyp.h
> +++ b/arch/arm/include/asm/kvm_hyp.h
> @@ -19,12 +19,14 @@
>  #define __ARM_KVM_HYP_H__
> 
>  #include 
> -#include 
>  #include 
> +
> +#ifdef CONFIG_ARM_LPAE
> +#include 
>  #include 
>  #include 
> -
>  #define __hyp_text __section(.hyp.text) notrace
> +#endif
> 
>  #define __ACCESS_VFP(CRn)  \
> "mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
> @@ -37,12 +39,18 @@
> __val;  \
>  })
> 
> +#ifdef CONFIG_ARM_LPAE
>  #define TTBR0  __ACCESS_CP15_64(0, c2)
>  #define TTBR1  __ACCESS_CP15_64(1, c2)
>  #define VTTBR  __ACCESS_CP15_64(6, c2)
>  #define PAR__ACCESS_CP15_64(0, c7)
>  #define CNTV_CVAL  __ACCESS_CP15_64(3, c14)
>  #define CNTVOFF__ACCESS_CP15_64(4, c14)
> +#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

There is no reason for this LPAE vs non LPAE dichotomy. Both registers
do exist if your system supports LPAE. So you can either suffix the
64bit version with an _64 (and change the KVM code), or suffix the bit
version with _32.

> 
>  #define MIDR   __ACCESS_CP15(c0, 0, c0, 0)
>  #define CSSELR __ACCESS_CP15(c0, 2, c0, 0)
> @@ -98,6 +106,7 @@
>  #define cntvoff_el2CNTVOFF
>  #define cnthctl_el2CNTHCTL
> 
> +#ifdef CONFIG_ARM_LPAE
>  void __timer_save_state(struct kvm_vcpu *vcpu);
>  void __timer_restore_state(struct kvm_vcpu *vcpu);
> 
> @@ -123,5 +132,6 @@ void __hyp_text __banked_restore_state(struct 
> kvm_cpu_context *ctxt);
>  asmlinkage int __guest_enter(struct kvm_vcpu *vcpu,
>  struct kvm_cpu_context *host);
>  asmlinkage int __hyp_do_panic(const char *, int, u32);
> +#endif
> 
>  #endif /* __ARM_KVM_HYP_H__ */
> diff --git a/arch/arm/mm/kasan_init.c b/arch/arm/mm/kasan_init.c
> index 049ee0a..359a782 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 12/10/17 15:59, Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
> On 11/10/17 09:22, Abbott Liu wrote:
>> diff --git a/arch/arm/include/asm/proc-fns.h 
>> b/arch/arm/include/asm/proc-fns.h
>> index f2e1af4..6e26714 100644
>> --- a/arch/arm/include/asm/proc-fns.h
>> +++ b/arch/arm/include/asm/proc-fns.h
>> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>>  pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>>  (pgd_t *)phys_to_virt(pg);  \
>>  })
>> +
>> +#define cpu_set_ttbr0(val)  \
>> +do {\
>> +u64 ttbr = val; \
>> +__asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
>> +: : "r" (ttbr));\
>> +} while (0)
>> +
>> +
>>  #else
>>  #define cpu_get_pgd()   \
>>  ({  \
>> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>>  pg &= ~0x3fff;  \
>>  (pgd_t *)phys_to_virt(pg);  \
>>  })
>> +
>> +#define cpu_set_ttbr(nr, val)   \
>> +do {\
>> +u64 ttbr = val; \
>> +__asm__("mcrp15, 0, %0, c2, c0, 0"  \
>> +: : "r" (ttbr));\
>> +} while (0)
>> +
>> +#define cpu_get_ttbr(nr)\
>> +({  \
>> +unsigned long ttbr; \
>> +__asm__("mrcp15, 0, %0, c2, c0, 0"  \
>> +: "=r" (ttbr)); \
>> +ttbr;   \
>> +})
>> +
>> +#define cpu_set_ttbr0(val)  \
>> +do {\
>> +u64 ttbr = val; \
>> +__asm__("mcrp15, 0, %0, c2, c0, 0"  \
>> +: : "r" (ttbr));\
>> +} while (0)
>> +
>> +
>
>You could instead lift and extend the definitions provided in kvm_hyp.h,
>and use the read_sysreg/write_sysreg helpers defined in cp15.h.

Thanks for your review. 
I extend definitions of TTBR0/TTBR1/PAR in kvm_hyp.h when the CONFIG_ARM_LPAE 
is 
not defined. 
Because cortex A9 don't support virtualization, so use CONFIG_ARM_LPAE to 
exclude
some functions and macros which are only used in virtualization.

Here is the code which I tested on vexpress_a15 and vexpress_a9:

diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
index 14b5903..2592608 100644
--- a/arch/arm/include/asm/kvm_hyp.h
+++ b/arch/arm/include/asm/kvm_hyp.h
@@ -19,12 +19,14 @@
 #define __ARM_KVM_HYP_H__

 #include 
-#include 
 #include 
+
+#ifdef CONFIG_ARM_LPAE
+#include 
 #include 
 #include 
-
 #define __hyp_text __section(.hyp.text) notrace
+#endif

 #define __ACCESS_VFP(CRn)  \
"mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
@@ -37,12 +39,18 @@
__val;  \
 })

+#ifdef CONFIG_ARM_LPAE
 #define TTBR0  __ACCESS_CP15_64(0, c2)
 #define TTBR1  __ACCESS_CP15_64(1, c2)
 #define VTTBR  __ACCESS_CP15_64(6, c2)
 #define PAR__ACCESS_CP15_64(0, c7)
 #define CNTV_CVAL  __ACCESS_CP15_64(3, c14)
 #define CNTVOFF__ACCESS_CP15_64(4, c14)
+#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

 #define MIDR   __ACCESS_CP15(c0, 0, c0, 0)
 #define CSSELR __ACCESS_CP15(c0, 2, c0, 0)
@@ -98,6 +106,7 @@
 #define cntvoff_el2CNTVOFF
 #define cnthctl_el2CNTHCTL

+#ifdef CONFIG_ARM_LPAE
 void __timer_save_state(struct kvm_vcpu *vcpu);
 void __timer_restore_state(struct kvm_vcpu *vcpu);

@@ -123,5 +132,6 @@ void __hyp_text __banked_restore_state(struct 
kvm_cpu_context *ctxt);
 asmlinkage int __guest_enter(struct kvm_vcpu *vcpu,
 struct kvm_cpu_context *host);
 asmlinkage int __hyp_do_panic(const char *, int, u32);
+#endif

 #endif /* __ARM_KVM_HYP_H__ */
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 
 #include 
 #include 
+#include 
 #include 

 #include "mm.h"
@@ -203,16 +204,16 @@ void __init kasan_init(void)
u64 orig_ttbr0;
int i;

-   orig_ttbr0 = cpu_get_ttbr(0);
+ orig_ttbr0 = read_sysreg(TTBR0);

 #ifdef CONFIG_ARM_LPAE
memcpy(tmp_pmd_table, 
pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_START)), 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 12/10/17 15:59, Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
> On 11/10/17 09:22, Abbott Liu wrote:
>> diff --git a/arch/arm/include/asm/proc-fns.h 
>> b/arch/arm/include/asm/proc-fns.h
>> index f2e1af4..6e26714 100644
>> --- a/arch/arm/include/asm/proc-fns.h
>> +++ b/arch/arm/include/asm/proc-fns.h
>> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>>  pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>>  (pgd_t *)phys_to_virt(pg);  \
>>  })
>> +
>> +#define cpu_set_ttbr0(val)  \
>> +do {\
>> +u64 ttbr = val; \
>> +__asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
>> +: : "r" (ttbr));\
>> +} while (0)
>> +
>> +
>>  #else
>>  #define cpu_get_pgd()   \
>>  ({  \
>> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>>  pg &= ~0x3fff;  \
>>  (pgd_t *)phys_to_virt(pg);  \
>>  })
>> +
>> +#define cpu_set_ttbr(nr, val)   \
>> +do {\
>> +u64 ttbr = val; \
>> +__asm__("mcrp15, 0, %0, c2, c0, 0"  \
>> +: : "r" (ttbr));\
>> +} while (0)
>> +
>> +#define cpu_get_ttbr(nr)\
>> +({  \
>> +unsigned long ttbr; \
>> +__asm__("mrcp15, 0, %0, c2, c0, 0"  \
>> +: "=r" (ttbr)); \
>> +ttbr;   \
>> +})
>> +
>> +#define cpu_set_ttbr0(val)  \
>> +do {\
>> +u64 ttbr = val; \
>> +__asm__("mcrp15, 0, %0, c2, c0, 0"  \
>> +: : "r" (ttbr));\
>> +} while (0)
>> +
>> +
>
>You could instead lift and extend the definitions provided in kvm_hyp.h,
>and use the read_sysreg/write_sysreg helpers defined in cp15.h.

Thanks for your review. 
I extend definitions of TTBR0/TTBR1/PAR in kvm_hyp.h when the CONFIG_ARM_LPAE 
is 
not defined. 
Because cortex A9 don't support virtualization, so use CONFIG_ARM_LPAE to 
exclude
some functions and macros which are only used in virtualization.

Here is the code which I tested on vexpress_a15 and vexpress_a9:

diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
index 14b5903..2592608 100644
--- a/arch/arm/include/asm/kvm_hyp.h
+++ b/arch/arm/include/asm/kvm_hyp.h
@@ -19,12 +19,14 @@
 #define __ARM_KVM_HYP_H__

 #include 
-#include 
 #include 
+
+#ifdef CONFIG_ARM_LPAE
+#include 
 #include 
 #include 
-
 #define __hyp_text __section(.hyp.text) notrace
+#endif

 #define __ACCESS_VFP(CRn)  \
"mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
@@ -37,12 +39,18 @@
__val;  \
 })

+#ifdef CONFIG_ARM_LPAE
 #define TTBR0  __ACCESS_CP15_64(0, c2)
 #define TTBR1  __ACCESS_CP15_64(1, c2)
 #define VTTBR  __ACCESS_CP15_64(6, c2)
 #define PAR__ACCESS_CP15_64(0, c7)
 #define CNTV_CVAL  __ACCESS_CP15_64(3, c14)
 #define CNTVOFF__ACCESS_CP15_64(4, c14)
+#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

 #define MIDR   __ACCESS_CP15(c0, 0, c0, 0)
 #define CSSELR __ACCESS_CP15(c0, 2, c0, 0)
@@ -98,6 +106,7 @@
 #define cntvoff_el2CNTVOFF
 #define cnthctl_el2CNTHCTL

+#ifdef CONFIG_ARM_LPAE
 void __timer_save_state(struct kvm_vcpu *vcpu);
 void __timer_restore_state(struct kvm_vcpu *vcpu);

@@ -123,5 +132,6 @@ void __hyp_text __banked_restore_state(struct 
kvm_cpu_context *ctxt);
 asmlinkage int __guest_enter(struct kvm_vcpu *vcpu,
 struct kvm_cpu_context *host);
 asmlinkage int __hyp_do_panic(const char *, int, u32);
+#endif

 #endif /* __ARM_KVM_HYP_H__ */
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 
 #include 
 #include 
+#include 
 #include 

 #include "mm.h"
@@ -203,16 +204,16 @@ void __init kasan_init(void)
u64 orig_ttbr0;
int i;

-   orig_ttbr0 = cpu_get_ttbr(0);
+ orig_ttbr0 = read_sysreg(TTBR0);

 #ifdef CONFIG_ARM_LPAE
memcpy(tmp_pmd_table, 
pgd_page_vaddr(*pgd_offset_k(KASAN_SHADOW_START)), 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Thu, Oct 12, 2017 at 02:42:49AM +0300, Dmitry Osipenko wrote:
> On 11.10.2017 11:22, Abbott Liu wrote:
> > +void __init kasan_map_early_shadow(pgd_t *pgdp)
> > +{
> > +   int i;
> > +   unsigned long start = KASAN_SHADOW_START;
> > +   unsigned long end = KASAN_SHADOW_END;
> > +   unsigned long addr;
> > +   unsigned long next;
> > +   pgd_t *pgd;
> > +
> > +   for (i = 0; i < PTRS_PER_PTE; i++)
> > +   set_pte_at(_mm, KASAN_SHADOW_START + i*PAGE_SIZE,
> > +   _zero_pte[i], pfn_pte(
> > +   virt_to_pfn(kasan_zero_page),
> > +   __pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | 
> > L_PTE_XN)));
> 
> Shouldn't all __pgprot's contain L_PTE_MT_WRITETHROUGH ?

One of the architecture restrictions is that the cache attributes of
all aliases should match (but there is a specific workaround that
permits this, provided that the dis-similar mappings aren't accessed
without certain intervening instructions.)

Why should it be L_PTE_MT_WRITETHROUGH, and not the same cache
attributes as the lowmem mapping?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Thu, Oct 12, 2017 at 02:42:49AM +0300, Dmitry Osipenko wrote:
> On 11.10.2017 11:22, Abbott Liu wrote:
> > +void __init kasan_map_early_shadow(pgd_t *pgdp)
> > +{
> > +   int i;
> > +   unsigned long start = KASAN_SHADOW_START;
> > +   unsigned long end = KASAN_SHADOW_END;
> > +   unsigned long addr;
> > +   unsigned long next;
> > +   pgd_t *pgd;
> > +
> > +   for (i = 0; i < PTRS_PER_PTE; i++)
> > +   set_pte_at(_mm, KASAN_SHADOW_START + i*PAGE_SIZE,
> > +   _zero_pte[i], pfn_pte(
> > +   virt_to_pfn(kasan_zero_page),
> > +   __pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | 
> > L_PTE_XN)));
> 
> Shouldn't all __pgprot's contain L_PTE_MT_WRITETHROUGH ?

One of the architecture restrictions is that the cache attributes of
all aliases should match (but there is a specific workaround that
permits this, provided that the dis-similar mappings aren't accessed
without certain intervening instructions.)

Why should it be L_PTE_MT_WRITETHROUGH, and not the same cache
attributes as the lowmem mapping?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Wed, Oct 11, 2017 at 04:22:17PM +0800, Abbott Liu wrote:
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index b2902a5..10cee6a 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, 
> pud_t *pud, pmd_t *pmd)
>   */
>  #define pmd_alloc_one(mm,addr)   ({ BUG(); ((pmd_t *)2); })
>  #define pmd_free(mm, pmd)do { } while (0)
> +#ifndef CONFIG_KASAN
>  #define pud_populate(mm,pmd,pte) BUG()
> -
> +#else
> +#define pud_populate(mm,pmd,pte) do { } while (0)
> +#endif

Please explain this change - we don't have a "pud" as far as the rest of
the Linux MM layer is concerned, so why do we need it for kasan?

I suspect it comes from the way we wrap up the page tables - where ARM
does it one way (because it has to) vs the subsequently merged method
which is completely upside down to what ARMs doing, and therefore is
totally incompatible and impossible to fit in with our way.

> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index f2e1af4..6e26714 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>   pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #else
>  #define cpu_get_pgd()\
>   ({  \
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +#define cpu_get_ttbr(nr) \
> + ({  \
> + unsigned long ttbr; \
> + __asm__("mrcp15, 0, %0, c2, c0, 0"  \
> + : "=r" (ttbr)); \
> + ttbr;   \
> + })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #endif
>  
>  #else/*!CONFIG_MMU */
> diff --git a/arch/arm/include/asm/thread_info.h 
> b/arch/arm/include/asm/thread_info.h
> index 1d468b5..52c4858 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -16,7 +16,11 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_KASAN
> +#define THREAD_SIZE_ORDER   2
> +#else
>  #define THREAD_SIZE_ORDER1
> +#endif
>  #define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP  (THREAD_SIZE - 8)
>  
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index 8733012..c17f4a2 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -101,7 +101,11 @@ __mmap_switched:
>   str r2, [r6]@ Save atags pointer
>   cmp r7, #0
>   strne   r0, [r7]@ Save control register values
> +#ifdef CONFIG_KASAN
> + b   kasan_early_init
> +#else
>   b   start_kernel
> +#endif
>  ENDPROC(__mmap_switched)
>  
>   .align  2
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 8e9a3e4..985d9a3 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -62,6 +62,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "atags.h"
>  
> @@ -1108,6 +1109,7 @@ void __init setup_arch(char **cmdline_p)
>   early_ioremap_reset();
>  
>   paging_init(mdesc);
> + kasan_init();
>   request_standard_resources(mdesc);
>  
>   if (mdesc->restart)
> diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
> index 950d19b..498c316 100644
> --- a/arch/arm/mm/Makefile
> +++ b/arch/arm/mm/Makefile
> @@ -106,4 +106,9 @@ obj-$(CONFIG_CACHE_L2X0)  += cache-l2x0.o 
> 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Wed, Oct 11, 2017 at 04:22:17PM +0800, Abbott Liu wrote:
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index b2902a5..10cee6a 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, 
> pud_t *pud, pmd_t *pmd)
>   */
>  #define pmd_alloc_one(mm,addr)   ({ BUG(); ((pmd_t *)2); })
>  #define pmd_free(mm, pmd)do { } while (0)
> +#ifndef CONFIG_KASAN
>  #define pud_populate(mm,pmd,pte) BUG()
> -
> +#else
> +#define pud_populate(mm,pmd,pte) do { } while (0)
> +#endif

Please explain this change - we don't have a "pud" as far as the rest of
the Linux MM layer is concerned, so why do we need it for kasan?

I suspect it comes from the way we wrap up the page tables - where ARM
does it one way (because it has to) vs the subsequently merged method
which is completely upside down to what ARMs doing, and therefore is
totally incompatible and impossible to fit in with our way.

> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index f2e1af4..6e26714 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>   pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #else
>  #define cpu_get_pgd()\
>   ({  \
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +#define cpu_get_ttbr(nr) \
> + ({  \
> + unsigned long ttbr; \
> + __asm__("mrcp15, 0, %0, c2, c0, 0"  \
> + : "=r" (ttbr)); \
> + ttbr;   \
> + })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #endif
>  
>  #else/*!CONFIG_MMU */
> diff --git a/arch/arm/include/asm/thread_info.h 
> b/arch/arm/include/asm/thread_info.h
> index 1d468b5..52c4858 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -16,7 +16,11 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_KASAN
> +#define THREAD_SIZE_ORDER   2
> +#else
>  #define THREAD_SIZE_ORDER1
> +#endif
>  #define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP  (THREAD_SIZE - 8)
>  
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index 8733012..c17f4a2 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -101,7 +101,11 @@ __mmap_switched:
>   str r2, [r6]@ Save atags pointer
>   cmp r7, #0
>   strne   r0, [r7]@ Save control register values
> +#ifdef CONFIG_KASAN
> + b   kasan_early_init
> +#else
>   b   start_kernel
> +#endif
>  ENDPROC(__mmap_switched)
>  
>   .align  2
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 8e9a3e4..985d9a3 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -62,6 +62,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "atags.h"
>  
> @@ -1108,6 +1109,7 @@ void __init setup_arch(char **cmdline_p)
>   early_ioremap_reset();
>  
>   paging_init(mdesc);
> + kasan_init();
>   request_standard_resources(mdesc);
>  
>   if (mdesc->restart)
> diff --git a/arch/arm/mm/Makefile b/arch/arm/mm/Makefile
> index 950d19b..498c316 100644
> --- a/arch/arm/mm/Makefile
> +++ b/arch/arm/mm/Makefile
> @@ -106,4 +106,9 @@ obj-$(CONFIG_CACHE_L2X0)  += cache-l2x0.o 
> 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Lamb)]

On 2017.10.12 7:43AM  Dmitry Osipenko [mailto:dig...@gmail.com] wrote:
>Shouldn't all __pgprot's contain L_PTE_MT_WRITETHROUGH ?
>
>[...]
>
>--
>Dmitry

Thanks for your review. I'm sorry that my replay is so late.

I don't think L_PTE_MT_WRITETHROUGH is need for all arm soc. So I think kasan's
mapping can use PAGE_KERNEL which can be initialized for different arm soc and 
__pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)).

I don't think the mapping table flags in kasan_early_init need be changed 
because of the follow reason:
1) PAGE_KERNEL can't be used in early_kasan_init because the pgprot_kernel 
which is used to define 
  PAGE_KERNEL doesn't be initialized. 

2) all of the kasan shadow's mapping table is going to be created again in 
kasan_init function.


All what I say is: I think only the mapping table flags in kasan_init function 
need to be changed into PAGE_KERNEL 
or  __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)). 

Here is the code, I has already tested:
--- a/arch/arm/mm/kasan_init.c
+++ b/arch/arm/mm/kasan_init.c
@@ -124,7 +124,7 @@ pte_t * __meminit kasan_pte_populate(pmd_t *pmd, unsigned 
long addr, int node)
void *p = kasan_alloc_block(PAGE_SIZE, node);
if (!p)
return NULL;
-   entry = pfn_pte(virt_to_pfn(p), __pgprot(_L_PTE_DEFAULT | 
L_PTE_DIRTY | L_PTE_XN));
+ entry = pfn_pte(virt_to_pfn(p), __pgprot(pgprot_val(PAGE_KERNEL)));
set_pte_at(_mm, addr, pte, entry);
}
return pte;
@@ -253,7 +254,7 @@ void __init kasan_init(void)
 set_pte_at(_mm, KASAN_SHADOW_START + i*PAGE_SIZE,
 _zero_pte[i], pfn_pte(
 virt_to_pfn(kasan_zero_page),
-__pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | 
L_PTE_XN | L_PTE_RDONLY)));
+ __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)));
memset(kasan_zero_page, 0, PAGE_SIZE);
cpu_set_ttbr0(orig_ttbr0);
flush_cache_all();

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Lamb)]

On 2017.10.12 7:43AM  Dmitry Osipenko [mailto:dig...@gmail.com] wrote:
>Shouldn't all __pgprot's contain L_PTE_MT_WRITETHROUGH ?
>
>[...]
>
>--
>Dmitry

Thanks for your review. I'm sorry that my replay is so late.

I don't think L_PTE_MT_WRITETHROUGH is need for all arm soc. So I think kasan's
mapping can use PAGE_KERNEL which can be initialized for different arm soc and 
__pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)).

I don't think the mapping table flags in kasan_early_init need be changed 
because of the follow reason:
1) PAGE_KERNEL can't be used in early_kasan_init because the pgprot_kernel 
which is used to define 
  PAGE_KERNEL doesn't be initialized. 

2) all of the kasan shadow's mapping table is going to be created again in 
kasan_init function.


All what I say is: I think only the mapping table flags in kasan_init function 
need to be changed into PAGE_KERNEL 
or  __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)). 

Here is the code, I has already tested:
--- a/arch/arm/mm/kasan_init.c
+++ b/arch/arm/mm/kasan_init.c
@@ -124,7 +124,7 @@ pte_t * __meminit kasan_pte_populate(pmd_t *pmd, unsigned 
long addr, int node)
void *p = kasan_alloc_block(PAGE_SIZE, node);
if (!p)
return NULL;
-   entry = pfn_pte(virt_to_pfn(p), __pgprot(_L_PTE_DEFAULT | 
L_PTE_DIRTY | L_PTE_XN));
+ entry = pfn_pte(virt_to_pfn(p), __pgprot(pgprot_val(PAGE_KERNEL)));
set_pte_at(_mm, addr, pte, entry);
}
return pte;
@@ -253,7 +254,7 @@ void __init kasan_init(void)
 set_pte_at(_mm, KASAN_SHADOW_START + i*PAGE_SIZE,
 _zero_pte[i], pfn_pte(
 virt_to_pfn(kasan_zero_page),
-__pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | 
L_PTE_XN | L_PTE_RDONLY)));
+ __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)));
memset(kasan_zero_page, 0, PAGE_SIZE);
cpu_set_ttbr0(orig_ttbr0);
flush_cache_all();

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Lamb)]

2017.10.12  05:42 AM  Russell King - ARM Linux [mailto:li...@armlinux.org.uk] 
wrote:

>> Please don't make this "exclusive" just conditionally call 
>> kasan_early_init(), remove the call to start_kernel from 
>> kasan_early_init and keep the call to start_kernel here.
>iow:
>
>#ifdef CONFIG_KASAN
>   bl  kasan_early_init
>#endif
>   b   start_kernel
>
>This has the advantage that we don't leave any stack frame from
>kasan_early_init() on the init task stack.

Thanks for your review.  I tested your opinion and it work well.
I agree with you that it is better to use follow code
#ifdef CONFIG_KASAN
bl  kasan_early_init
#endif
b   start_kernel

than :
#ifdef CONFIG_KASAN
bl  kasan_early_init
#else
b   start_kernel
#endif

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Lamb)]

2017.10.12  05:42 AM  Russell King - ARM Linux [mailto:li...@armlinux.org.uk] 
wrote:

>> Please don't make this "exclusive" just conditionally call 
>> kasan_early_init(), remove the call to start_kernel from 
>> kasan_early_init and keep the call to start_kernel here.
>iow:
>
>#ifdef CONFIG_KASAN
>   bl  kasan_early_init
>#endif
>   b   start_kernel
>
>This has the advantage that we don't leave any stack frame from
>kasan_early_init() on the init task stack.

Thanks for your review.  I tested your opinion and it work well.
I agree with you that it is better to use follow code
#ifdef CONFIG_KASAN
bl  kasan_early_init
#endif
b   start_kernel

than :
#ifdef CONFIG_KASAN
bl  kasan_early_init
#else
b   start_kernel
#endif

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 11/10/17 09:22, Abbott Liu wrote:
> From: Andrey Ryabinin 
> 
> This patch initializes KASan shadow region's page table and memory.
> There are two stage for KASan initializing:
> 1. At early boot stage the whole shadow region is mapped to just
>one physical page (kasan_zero_page). It's finished by the function
>kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
>head-common.S)
> 
> 2. After the calling of paging_init, we use kasan_zero_page as zero
>shadow for some memory that KASan don't need to track, and we alloc
>new shadow space for the other memory that KASan need to track. These
>issues are finished by the function kasan_init which is call by setup_arch.
> 
> Cc: Andrey Ryabinin 
> Signed-off-by: Abbott Liu 
> ---
>  arch/arm/include/asm/kasan.h   |  20 +++
>  arch/arm/include/asm/pgalloc.h |   5 +-
>  arch/arm/include/asm/pgtable.h |   1 +
>  arch/arm/include/asm/proc-fns.h|  33 +
>  arch/arm/include/asm/thread_info.h |   4 +
>  arch/arm/kernel/head-common.S  |   4 +
>  arch/arm/kernel/setup.c|   2 +
>  arch/arm/mm/Makefile   |   5 +
>  arch/arm/mm/kasan_init.c   | 257 
> +
>  mm/kasan/kasan.c   |   2 +-
>  10 files changed, 331 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/include/asm/kasan.h
>  create mode 100644 arch/arm/mm/kasan_init.c
> 
> diff --git a/arch/arm/include/asm/kasan.h b/arch/arm/include/asm/kasan.h
> new file mode 100644
> index 000..90ee60c
> --- /dev/null
> +++ b/arch/arm/include/asm/kasan.h
> @@ -0,0 +1,20 @@
> +#ifndef __ASM_KASAN_H
> +#define __ASM_KASAN_H
> +
> +#ifdef CONFIG_KASAN
> +
> +#include 
> +/*
> + * Compiler uses shadow offset assuming that addresses start
> + * from 0. Kernel addresses don't start from 0, so shadow
> + * for kernel really starts from 'compiler's shadow offset' +
> + * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
> + */
> +
> +extern void kasan_init(void);
> +
> +#else
> +static inline void kasan_init(void) { }
> +#endif
> +
> +#endif
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index b2902a5..10cee6a 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, 
> pud_t *pud, pmd_t *pmd)
>   */
>  #define pmd_alloc_one(mm,addr)   ({ BUG(); ((pmd_t *)2); })
>  #define pmd_free(mm, pmd)do { } while (0)
> +#ifndef CONFIG_KASAN
>  #define pud_populate(mm,pmd,pte) BUG()
> -
> +#else
> +#define pud_populate(mm,pmd,pte) do { } while (0)
> +#endif
>  #endif   /* CONFIG_ARM_LPAE */
>  
>  extern pgd_t *pgd_alloc(struct mm_struct *mm);
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 1c46238..fdf343f 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -97,6 +97,7 @@ extern pgprot_t pgprot_s2_device;
>  #define PAGE_READONLY_MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY | L_PTE_XN)
>  #define PAGE_READONLY_EXEC   _MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL  _MOD_PROT(pgprot_kernel, L_PTE_XN)
> +#define PAGE_KERNEL_RO   _MOD_PROT(pgprot_kernel, L_PTE_XN | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL_EXEC pgprot_kernel
>  #define PAGE_HYP _MOD_PROT(pgprot_kernel, L_PTE_HYP | L_PTE_XN)
>  #define PAGE_HYP_EXEC_MOD_PROT(pgprot_kernel, L_PTE_HYP | 
> L_PTE_RDONLY)
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index f2e1af4..6e26714 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>   pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #else
>  #define cpu_get_pgd()\
>   ({  \
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));  

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Marc Zyngier]

On 11/10/17 09:22, Abbott Liu wrote:
> From: Andrey Ryabinin 
> 
> This patch initializes KASan shadow region's page table and memory.
> There are two stage for KASan initializing:
> 1. At early boot stage the whole shadow region is mapped to just
>one physical page (kasan_zero_page). It's finished by the function
>kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
>head-common.S)
> 
> 2. After the calling of paging_init, we use kasan_zero_page as zero
>shadow for some memory that KASan don't need to track, and we alloc
>new shadow space for the other memory that KASan need to track. These
>issues are finished by the function kasan_init which is call by setup_arch.
> 
> Cc: Andrey Ryabinin 
> Signed-off-by: Abbott Liu 
> ---
>  arch/arm/include/asm/kasan.h   |  20 +++
>  arch/arm/include/asm/pgalloc.h |   5 +-
>  arch/arm/include/asm/pgtable.h |   1 +
>  arch/arm/include/asm/proc-fns.h|  33 +
>  arch/arm/include/asm/thread_info.h |   4 +
>  arch/arm/kernel/head-common.S  |   4 +
>  arch/arm/kernel/setup.c|   2 +
>  arch/arm/mm/Makefile   |   5 +
>  arch/arm/mm/kasan_init.c   | 257 
> +
>  mm/kasan/kasan.c   |   2 +-
>  10 files changed, 331 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/include/asm/kasan.h
>  create mode 100644 arch/arm/mm/kasan_init.c
> 
> diff --git a/arch/arm/include/asm/kasan.h b/arch/arm/include/asm/kasan.h
> new file mode 100644
> index 000..90ee60c
> --- /dev/null
> +++ b/arch/arm/include/asm/kasan.h
> @@ -0,0 +1,20 @@
> +#ifndef __ASM_KASAN_H
> +#define __ASM_KASAN_H
> +
> +#ifdef CONFIG_KASAN
> +
> +#include 
> +/*
> + * Compiler uses shadow offset assuming that addresses start
> + * from 0. Kernel addresses don't start from 0, so shadow
> + * for kernel really starts from 'compiler's shadow offset' +
> + * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
> + */
> +
> +extern void kasan_init(void);
> +
> +#else
> +static inline void kasan_init(void) { }
> +#endif
> +
> +#endif
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index b2902a5..10cee6a 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, 
> pud_t *pud, pmd_t *pmd)
>   */
>  #define pmd_alloc_one(mm,addr)   ({ BUG(); ((pmd_t *)2); })
>  #define pmd_free(mm, pmd)do { } while (0)
> +#ifndef CONFIG_KASAN
>  #define pud_populate(mm,pmd,pte) BUG()
> -
> +#else
> +#define pud_populate(mm,pmd,pte) do { } while (0)
> +#endif
>  #endif   /* CONFIG_ARM_LPAE */
>  
>  extern pgd_t *pgd_alloc(struct mm_struct *mm);
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 1c46238..fdf343f 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -97,6 +97,7 @@ extern pgprot_t pgprot_s2_device;
>  #define PAGE_READONLY_MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY | L_PTE_XN)
>  #define PAGE_READONLY_EXEC   _MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL  _MOD_PROT(pgprot_kernel, L_PTE_XN)
> +#define PAGE_KERNEL_RO   _MOD_PROT(pgprot_kernel, L_PTE_XN | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL_EXEC pgprot_kernel
>  #define PAGE_HYP _MOD_PROT(pgprot_kernel, L_PTE_HYP | L_PTE_XN)
>  #define PAGE_HYP_EXEC_MOD_PROT(pgprot_kernel, L_PTE_HYP | 
> L_PTE_RDONLY)
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index f2e1af4..6e26714 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>   pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #else
>  #define cpu_get_pgd()\
>   ({  \
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +#define 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Dmitry Osipenko]

On 11.10.2017 11:22, Abbott Liu wrote:
> From: Andrey Ryabinin 
> 
> This patch initializes KASan shadow region's page table and memory.
> There are two stage for KASan initializing:
> 1. At early boot stage the whole shadow region is mapped to just
>one physical page (kasan_zero_page). It's finished by the function
>kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
>head-common.S)
> 
> 2. After the calling of paging_init, we use kasan_zero_page as zero
>shadow for some memory that KASan don't need to track, and we alloc
>new shadow space for the other memory that KASan need to track. These
>issues are finished by the function kasan_init which is call by setup_arch.
> 
> Cc: Andrey Ryabinin 
> Signed-off-by: Abbott Liu 
> ---
>  arch/arm/include/asm/kasan.h   |  20 +++
>  arch/arm/include/asm/pgalloc.h |   5 +-
>  arch/arm/include/asm/pgtable.h |   1 +
>  arch/arm/include/asm/proc-fns.h|  33 +
>  arch/arm/include/asm/thread_info.h |   4 +
>  arch/arm/kernel/head-common.S  |   4 +
>  arch/arm/kernel/setup.c|   2 +
>  arch/arm/mm/Makefile   |   5 +
>  arch/arm/mm/kasan_init.c   | 257 
> +
>  mm/kasan/kasan.c   |   2 +-
>  10 files changed, 331 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/include/asm/kasan.h
>  create mode 100644 arch/arm/mm/kasan_init.c
> 
> diff --git a/arch/arm/include/asm/kasan.h b/arch/arm/include/asm/kasan.h
> new file mode 100644
> index 000..90ee60c
> --- /dev/null
> +++ b/arch/arm/include/asm/kasan.h
> @@ -0,0 +1,20 @@
> +#ifndef __ASM_KASAN_H
> +#define __ASM_KASAN_H
> +
> +#ifdef CONFIG_KASAN
> +
> +#include 
> +/*
> + * Compiler uses shadow offset assuming that addresses start
> + * from 0. Kernel addresses don't start from 0, so shadow
> + * for kernel really starts from 'compiler's shadow offset' +
> + * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
> + */
> +
> +extern void kasan_init(void);
> +
> +#else
> +static inline void kasan_init(void) { }
> +#endif
> +
> +#endif
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index b2902a5..10cee6a 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, 
> pud_t *pud, pmd_t *pmd)
>   */
>  #define pmd_alloc_one(mm,addr)   ({ BUG(); ((pmd_t *)2); })
>  #define pmd_free(mm, pmd)do { } while (0)
> +#ifndef CONFIG_KASAN
>  #define pud_populate(mm,pmd,pte) BUG()
> -
> +#else
> +#define pud_populate(mm,pmd,pte) do { } while (0)
> +#endif
>  #endif   /* CONFIG_ARM_LPAE */
>  
>  extern pgd_t *pgd_alloc(struct mm_struct *mm);
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 1c46238..fdf343f 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -97,6 +97,7 @@ extern pgprot_t pgprot_s2_device;
>  #define PAGE_READONLY_MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY | L_PTE_XN)
>  #define PAGE_READONLY_EXEC   _MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL  _MOD_PROT(pgprot_kernel, L_PTE_XN)
> +#define PAGE_KERNEL_RO   _MOD_PROT(pgprot_kernel, L_PTE_XN | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL_EXEC pgprot_kernel
>  #define PAGE_HYP _MOD_PROT(pgprot_kernel, L_PTE_HYP | L_PTE_XN)
>  #define PAGE_HYP_EXEC_MOD_PROT(pgprot_kernel, L_PTE_HYP | 
> L_PTE_RDONLY)
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index f2e1af4..6e26714 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>   pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #else
>  #define cpu_get_pgd()\
>   ({  \
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Dmitry Osipenko]

On 11.10.2017 11:22, Abbott Liu wrote:
> From: Andrey Ryabinin 
> 
> This patch initializes KASan shadow region's page table and memory.
> There are two stage for KASan initializing:
> 1. At early boot stage the whole shadow region is mapped to just
>one physical page (kasan_zero_page). It's finished by the function
>kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
>head-common.S)
> 
> 2. After the calling of paging_init, we use kasan_zero_page as zero
>shadow for some memory that KASan don't need to track, and we alloc
>new shadow space for the other memory that KASan need to track. These
>issues are finished by the function kasan_init which is call by setup_arch.
> 
> Cc: Andrey Ryabinin 
> Signed-off-by: Abbott Liu 
> ---
>  arch/arm/include/asm/kasan.h   |  20 +++
>  arch/arm/include/asm/pgalloc.h |   5 +-
>  arch/arm/include/asm/pgtable.h |   1 +
>  arch/arm/include/asm/proc-fns.h|  33 +
>  arch/arm/include/asm/thread_info.h |   4 +
>  arch/arm/kernel/head-common.S  |   4 +
>  arch/arm/kernel/setup.c|   2 +
>  arch/arm/mm/Makefile   |   5 +
>  arch/arm/mm/kasan_init.c   | 257 
> +
>  mm/kasan/kasan.c   |   2 +-
>  10 files changed, 331 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/include/asm/kasan.h
>  create mode 100644 arch/arm/mm/kasan_init.c
> 
> diff --git a/arch/arm/include/asm/kasan.h b/arch/arm/include/asm/kasan.h
> new file mode 100644
> index 000..90ee60c
> --- /dev/null
> +++ b/arch/arm/include/asm/kasan.h
> @@ -0,0 +1,20 @@
> +#ifndef __ASM_KASAN_H
> +#define __ASM_KASAN_H
> +
> +#ifdef CONFIG_KASAN
> +
> +#include 
> +/*
> + * Compiler uses shadow offset assuming that addresses start
> + * from 0. Kernel addresses don't start from 0, so shadow
> + * for kernel really starts from 'compiler's shadow offset' +
> + * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
> + */
> +
> +extern void kasan_init(void);
> +
> +#else
> +static inline void kasan_init(void) { }
> +#endif
> +
> +#endif
> diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
> index b2902a5..10cee6a 100644
> --- a/arch/arm/include/asm/pgalloc.h
> +++ b/arch/arm/include/asm/pgalloc.h
> @@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, 
> pud_t *pud, pmd_t *pmd)
>   */
>  #define pmd_alloc_one(mm,addr)   ({ BUG(); ((pmd_t *)2); })
>  #define pmd_free(mm, pmd)do { } while (0)
> +#ifndef CONFIG_KASAN
>  #define pud_populate(mm,pmd,pte) BUG()
> -
> +#else
> +#define pud_populate(mm,pmd,pte) do { } while (0)
> +#endif
>  #endif   /* CONFIG_ARM_LPAE */
>  
>  extern pgd_t *pgd_alloc(struct mm_struct *mm);
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 1c46238..fdf343f 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -97,6 +97,7 @@ extern pgprot_t pgprot_s2_device;
>  #define PAGE_READONLY_MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY | L_PTE_XN)
>  #define PAGE_READONLY_EXEC   _MOD_PROT(pgprot_user, L_PTE_USER | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL  _MOD_PROT(pgprot_kernel, L_PTE_XN)
> +#define PAGE_KERNEL_RO   _MOD_PROT(pgprot_kernel, L_PTE_XN | 
> L_PTE_RDONLY)
>  #define PAGE_KERNEL_EXEC pgprot_kernel
>  #define PAGE_HYP _MOD_PROT(pgprot_kernel, L_PTE_HYP | L_PTE_XN)
>  #define PAGE_HYP_EXEC_MOD_PROT(pgprot_kernel, L_PTE_HYP | 
> L_PTE_RDONLY)
> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
> index f2e1af4..6e26714 100644
> --- a/arch/arm/include/asm/proc-fns.h
> +++ b/arch/arm/include/asm/proc-fns.h
> @@ -131,6 +131,15 @@ extern void cpu_resume(void);
>   pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +
>  #else
>  #define cpu_get_pgd()\
>   ({  \
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +
> +#define 

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Wed, Oct 11, 2017 at 12:39:39PM -0700, Florian Fainelli wrote:
> On 10/11/2017 01:22 AM, Abbott Liu wrote:
> > diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> > index 8733012..c17f4a2 100644
> > --- a/arch/arm/kernel/head-common.S
> > +++ b/arch/arm/kernel/head-common.S
> > @@ -101,7 +101,11 @@ __mmap_switched:
> > str r2, [r6]@ Save atags pointer
> > cmp r7, #0
> > strne   r0, [r7]@ Save control register values
> > +#ifdef CONFIG_KASAN
> > +   b   kasan_early_init
> > +#else
> > b   start_kernel
> > +#endif
> 
> Please don't make this "exclusive" just conditionally call
> kasan_early_init(), remove the call to start_kernel from
> kasan_early_init and keep the call to start_kernel here.

iow:

#ifdef CONFIG_KASAN
bl  kasan_early_init
#endif
b   start_kernel

This has the advantage that we don't leave any stack frame from
kasan_early_init() on the init task stack.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Russell King - ARM Linux]

On Wed, Oct 11, 2017 at 12:39:39PM -0700, Florian Fainelli wrote:
> On 10/11/2017 01:22 AM, Abbott Liu wrote:
> > diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> > index 8733012..c17f4a2 100644
> > --- a/arch/arm/kernel/head-common.S
> > +++ b/arch/arm/kernel/head-common.S
> > @@ -101,7 +101,11 @@ __mmap_switched:
> > str r2, [r6]@ Save atags pointer
> > cmp r7, #0
> > strne   r0, [r7]@ Save control register values
> > +#ifdef CONFIG_KASAN
> > +   b   kasan_early_init
> > +#else
> > b   start_kernel
> > +#endif
> 
> Please don't make this "exclusive" just conditionally call
> kasan_early_init(), remove the call to start_kernel from
> kasan_early_init and keep the call to start_kernel here.

iow:

#ifdef CONFIG_KASAN
bl  kasan_early_init
#endif
b   start_kernel

This has the advantage that we don't leave any stack frame from
kasan_early_init() on the init task stack.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Florian Fainelli]

On 10/11/2017 01:22 AM, Abbott Liu wrote:
> From: Andrey Ryabinin 
> 
> This patch initializes KASan shadow region's page table and memory.
> There are two stage for KASan initializing:
> 1. At early boot stage the whole shadow region is mapped to just
>one physical page (kasan_zero_page). It's finished by the function
>kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
>head-common.S)
> 
> 2. After the calling of paging_init, we use kasan_zero_page as zero
>shadow for some memory that KASan don't need to track, and we alloc
>new shadow space for the other memory that KASan need to track. These
>issues are finished by the function kasan_init which is call by setup_arch.
> 
> Cc: Andrey Ryabinin 
> Signed-off-by: Abbott Liu 
> ---

[snip]

\
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)

nr seems to be unused here?

> +
> +#define cpu_get_ttbr(nr) \
> + ({  \
> + unsigned long ttbr; \
> + __asm__("mrcp15, 0, %0, c2, c0, 0"  \
> + : "=r" (ttbr)); \
> + ttbr;   \
> + })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +

Why is not cpu_set_ttbr0() not using cpu_set_ttbr()?

> +
>  #endif
>  
>  #else/*!CONFIG_MMU */
> diff --git a/arch/arm/include/asm/thread_info.h 
> b/arch/arm/include/asm/thread_info.h
> index 1d468b5..52c4858 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -16,7 +16,11 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_KASAN
> +#define THREAD_SIZE_ORDER   2
> +#else
>  #define THREAD_SIZE_ORDER1
> +#endif
>  #define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP  (THREAD_SIZE - 8)
>  
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index 8733012..c17f4a2 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -101,7 +101,11 @@ __mmap_switched:
>   str r2, [r6]@ Save atags pointer
>   cmp r7, #0
>   strne   r0, [r7]@ Save control register values
> +#ifdef CONFIG_KASAN
> + b   kasan_early_init
> +#else
>   b   start_kernel
> +#endif

Please don't make this "exclusive" just conditionally call
kasan_early_init(), remove the call to start_kernel from
kasan_early_init and keep the call to start_kernel here.

>  ENDPROC(__mmap_switched)
>  
>   .align  2

[snip]

> +void __init kasan_early_init(void)
> +{
> + struct proc_info_list *list;
> +
> + /*
> +  * locate processor in the list of supported processor
> +  * types.  The linker builds this table for us from the
> +  * entries in arch/arm/mm/proc-*.S
> +  */
> + list = lookup_processor_type(read_cpuid_id());
> + if (list) {
> +#ifdef MULTI_CPU
> + processor = *list->proc;
> +#endif
> + }

I could not quite spot in your patch series when do you need this
information?
-- 
Florian

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Florian Fainelli]

On 10/11/2017 01:22 AM, Abbott Liu wrote:
> From: Andrey Ryabinin 
> 
> This patch initializes KASan shadow region's page table and memory.
> There are two stage for KASan initializing:
> 1. At early boot stage the whole shadow region is mapped to just
>one physical page (kasan_zero_page). It's finished by the function
>kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
>head-common.S)
> 
> 2. After the calling of paging_init, we use kasan_zero_page as zero
>shadow for some memory that KASan don't need to track, and we alloc
>new shadow space for the other memory that KASan need to track. These
>issues are finished by the function kasan_init which is call by setup_arch.
> 
> Cc: Andrey Ryabinin 
> Signed-off-by: Abbott Liu 
> ---

[snip]

\
> @@ -140,6 +149,30 @@ extern void cpu_resume(void);
>   pg &= ~0x3fff;  \
>   (pgd_t *)phys_to_virt(pg);  \
>   })
> +
> +#define cpu_set_ttbr(nr, val)\
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)

nr seems to be unused here?

> +
> +#define cpu_get_ttbr(nr) \
> + ({  \
> + unsigned long ttbr; \
> + __asm__("mrcp15, 0, %0, c2, c0, 0"  \
> + : "=r" (ttbr)); \
> + ttbr;   \
> + })
> +
> +#define cpu_set_ttbr0(val)   \
> + do {\
> + u64 ttbr = val; \
> + __asm__("mcrp15, 0, %0, c2, c0, 0"  \
> + : : "r" (ttbr));\
> + } while (0)
> +

Why is not cpu_set_ttbr0() not using cpu_set_ttbr()?

> +
>  #endif
>  
>  #else/*!CONFIG_MMU */
> diff --git a/arch/arm/include/asm/thread_info.h 
> b/arch/arm/include/asm/thread_info.h
> index 1d468b5..52c4858 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -16,7 +16,11 @@
>  #include 
>  #include 
>  
> +#ifdef CONFIG_KASAN
> +#define THREAD_SIZE_ORDER   2
> +#else
>  #define THREAD_SIZE_ORDER1
> +#endif
>  #define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)
>  #define THREAD_START_SP  (THREAD_SIZE - 8)
>  
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index 8733012..c17f4a2 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -101,7 +101,11 @@ __mmap_switched:
>   str r2, [r6]@ Save atags pointer
>   cmp r7, #0
>   strne   r0, [r7]@ Save control register values
> +#ifdef CONFIG_KASAN
> + b   kasan_early_init
> +#else
>   b   start_kernel
> +#endif

Please don't make this "exclusive" just conditionally call
kasan_early_init(), remove the call to start_kernel from
kasan_early_init and keep the call to start_kernel here.

>  ENDPROC(__mmap_switched)
>  
>   .align  2

[snip]

> +void __init kasan_early_init(void)
> +{
> + struct proc_info_list *list;
> +
> + /*
> +  * locate processor in the list of supported processor
> +  * types.  The linker builds this table for us from the
> +  * entries in arch/arm/mm/proc-*.S
> +  */
> + list = lookup_processor_type(read_cpuid_id());
> + if (list) {
> +#ifdef MULTI_CPU
> + processor = *list->proc;
> +#endif
> + }

I could not quite spot in your patch series when do you need this
information?
-- 
Florian

[PATCH 01/11] Initialize the mapping of KASan shadow memory

[Abbott Liu]

From: Andrey Ryabinin 

This patch initializes KASan shadow region's page table and memory.
There are two stage for KASan initializing:
1. At early boot stage the whole shadow region is mapped to just
   one physical page (kasan_zero_page). It's finished by the function
   kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
   head-common.S)

2. After the calling of paging_init, we use kasan_zero_page as zero
   shadow for some memory that KASan don't need to track, and we alloc
   new shadow space for the other memory that KASan need to track. These
   issues are finished by the function kasan_init which is call by setup_arch.

Cc: Andrey Ryabinin 
Signed-off-by: Abbott Liu 
---
 arch/arm/include/asm/kasan.h   |  20 +++
 arch/arm/include/asm/pgalloc.h |   5 +-
 arch/arm/include/asm/pgtable.h |   1 +
 arch/arm/include/asm/proc-fns.h|  33 +
 arch/arm/include/asm/thread_info.h |   4 +
 arch/arm/kernel/head-common.S  |   4 +
 arch/arm/kernel/setup.c|   2 +
 arch/arm/mm/Makefile   |   5 +
 arch/arm/mm/kasan_init.c   | 257 +
 mm/kasan/kasan.c   |   2 +-
 10 files changed, 331 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/kasan.h
 create mode 100644 arch/arm/mm/kasan_init.c

diff --git a/arch/arm/include/asm/kasan.h b/arch/arm/include/asm/kasan.h
new file mode 100644
index 000..90ee60c
--- /dev/null
+++ b/arch/arm/include/asm/kasan.h
@@ -0,0 +1,20 @@
+#ifndef __ASM_KASAN_H
+#define __ASM_KASAN_H
+
+#ifdef CONFIG_KASAN
+
+#include 
+/*
+ * Compiler uses shadow offset assuming that addresses start
+ * from 0. Kernel addresses don't start from 0, so shadow
+ * for kernel really starts from 'compiler's shadow offset' +
+ * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
+ */
+
+extern void kasan_init(void);
+
+#else
+static inline void kasan_init(void) { }
+#endif
+
+#endif
diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index b2902a5..10cee6a 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, pud_t 
*pud, pmd_t *pmd)
  */
 #define pmd_alloc_one(mm,addr) ({ BUG(); ((pmd_t *)2); })
 #define pmd_free(mm, pmd)  do { } while (0)
+#ifndef CONFIG_KASAN
 #define pud_populate(mm,pmd,pte)   BUG()
-
+#else
+#define pud_populate(mm,pmd,pte)   do { } while (0)
+#endif
 #endif /* CONFIG_ARM_LPAE */
 
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 1c46238..fdf343f 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -97,6 +97,7 @@ extern pgprot_t   pgprot_s2_device;
 #define PAGE_READONLY  _MOD_PROT(pgprot_user, L_PTE_USER | 
L_PTE_RDONLY | L_PTE_XN)
 #define PAGE_READONLY_EXEC _MOD_PROT(pgprot_user, L_PTE_USER | 
L_PTE_RDONLY)
 #define PAGE_KERNEL_MOD_PROT(pgprot_kernel, L_PTE_XN)
+#define PAGE_KERNEL_RO _MOD_PROT(pgprot_kernel, L_PTE_XN | 
L_PTE_RDONLY)
 #define PAGE_KERNEL_EXEC   pgprot_kernel
 #define PAGE_HYP   _MOD_PROT(pgprot_kernel, L_PTE_HYP | L_PTE_XN)
 #define PAGE_HYP_EXEC  _MOD_PROT(pgprot_kernel, L_PTE_HYP | 
L_PTE_RDONLY)
diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index f2e1af4..6e26714 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -131,6 +131,15 @@ extern void cpu_resume(void);
pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
(pgd_t *)phys_to_virt(pg);  \
})
+
+#define cpu_set_ttbr0(val) \
+   do {\
+   u64 ttbr = val; \
+   __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
+   : : "r" (ttbr));\
+   } while (0)
+
+
 #else
 #define cpu_get_pgd()  \
({  \
@@ -140,6 +149,30 @@ extern void cpu_resume(void);
pg &= ~0x3fff;  \
(pgd_t *)phys_to_virt(pg);  \
})
+
+#define cpu_set_ttbr(nr, val)  \
+   do {\
+   u64 ttbr = val; \
+   __asm__("mcrp15, 0, %0, c2, c0, 0"  \
+   : : "r" (ttbr));\
+   } while (0)
+
+#define cpu_get_ttbr(nr)   \
+   ({  \
+   unsigned long ttbr; \
+   __asm__("mrc  

[PATCH 01/11] Initialize the mapping of KASan shadow memory

[Abbott Liu]

From: Andrey Ryabinin 

This patch initializes KASan shadow region's page table and memory.
There are two stage for KASan initializing:
1. At early boot stage the whole shadow region is mapped to just
   one physical page (kasan_zero_page). It's finished by the function
   kasan_early_init which is called by __mmap_switched(arch/arm/kernel/
   head-common.S)

2. After the calling of paging_init, we use kasan_zero_page as zero
   shadow for some memory that KASan don't need to track, and we alloc
   new shadow space for the other memory that KASan need to track. These
   issues are finished by the function kasan_init which is call by setup_arch.

Cc: Andrey Ryabinin 
Signed-off-by: Abbott Liu 
---
 arch/arm/include/asm/kasan.h   |  20 +++
 arch/arm/include/asm/pgalloc.h |   5 +-
 arch/arm/include/asm/pgtable.h |   1 +
 arch/arm/include/asm/proc-fns.h|  33 +
 arch/arm/include/asm/thread_info.h |   4 +
 arch/arm/kernel/head-common.S  |   4 +
 arch/arm/kernel/setup.c|   2 +
 arch/arm/mm/Makefile   |   5 +
 arch/arm/mm/kasan_init.c   | 257 +
 mm/kasan/kasan.c   |   2 +-
 10 files changed, 331 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/kasan.h
 create mode 100644 arch/arm/mm/kasan_init.c

diff --git a/arch/arm/include/asm/kasan.h b/arch/arm/include/asm/kasan.h
new file mode 100644
index 000..90ee60c
--- /dev/null
+++ b/arch/arm/include/asm/kasan.h
@@ -0,0 +1,20 @@
+#ifndef __ASM_KASAN_H
+#define __ASM_KASAN_H
+
+#ifdef CONFIG_KASAN
+
+#include 
+/*
+ * Compiler uses shadow offset assuming that addresses start
+ * from 0. Kernel addresses don't start from 0, so shadow
+ * for kernel really starts from 'compiler's shadow offset' +
+ * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
+ */
+
+extern void kasan_init(void);
+
+#else
+static inline void kasan_init(void) { }
+#endif
+
+#endif
diff --git a/arch/arm/include/asm/pgalloc.h b/arch/arm/include/asm/pgalloc.h
index b2902a5..10cee6a 100644
--- a/arch/arm/include/asm/pgalloc.h
+++ b/arch/arm/include/asm/pgalloc.h
@@ -50,8 +50,11 @@ static inline void pud_populate(struct mm_struct *mm, pud_t 
*pud, pmd_t *pmd)
  */
 #define pmd_alloc_one(mm,addr) ({ BUG(); ((pmd_t *)2); })
 #define pmd_free(mm, pmd)  do { } while (0)
+#ifndef CONFIG_KASAN
 #define pud_populate(mm,pmd,pte)   BUG()
-
+#else
+#define pud_populate(mm,pmd,pte)   do { } while (0)
+#endif
 #endif /* CONFIG_ARM_LPAE */
 
 extern pgd_t *pgd_alloc(struct mm_struct *mm);
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 1c46238..fdf343f 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -97,6 +97,7 @@ extern pgprot_t   pgprot_s2_device;
 #define PAGE_READONLY  _MOD_PROT(pgprot_user, L_PTE_USER | 
L_PTE_RDONLY | L_PTE_XN)
 #define PAGE_READONLY_EXEC _MOD_PROT(pgprot_user, L_PTE_USER | 
L_PTE_RDONLY)
 #define PAGE_KERNEL_MOD_PROT(pgprot_kernel, L_PTE_XN)
+#define PAGE_KERNEL_RO _MOD_PROT(pgprot_kernel, L_PTE_XN | 
L_PTE_RDONLY)
 #define PAGE_KERNEL_EXEC   pgprot_kernel
 #define PAGE_HYP   _MOD_PROT(pgprot_kernel, L_PTE_HYP | L_PTE_XN)
 #define PAGE_HYP_EXEC  _MOD_PROT(pgprot_kernel, L_PTE_HYP | 
L_PTE_RDONLY)
diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
index f2e1af4..6e26714 100644
--- a/arch/arm/include/asm/proc-fns.h
+++ b/arch/arm/include/asm/proc-fns.h
@@ -131,6 +131,15 @@ extern void cpu_resume(void);
pg &= ~(PTRS_PER_PGD*sizeof(pgd_t)-1);  \
(pgd_t *)phys_to_virt(pg);  \
})
+
+#define cpu_set_ttbr0(val) \
+   do {\
+   u64 ttbr = val; \
+   __asm__("mcrr   p15, 0, %Q0, %R0, c2"   \
+   : : "r" (ttbr));\
+   } while (0)
+
+
 #else
 #define cpu_get_pgd()  \
({  \
@@ -140,6 +149,30 @@ extern void cpu_resume(void);
pg &= ~0x3fff;  \
(pgd_t *)phys_to_virt(pg);  \
})
+
+#define cpu_set_ttbr(nr, val)  \
+   do {\
+   u64 ttbr = val; \
+   __asm__("mcrp15, 0, %0, c2, c0, 0"  \
+   : : "r" (ttbr));\
+   } while (0)
+
+#define cpu_get_ttbr(nr)   \
+   ({  \
+   unsigned long ttbr; \
+   __asm__("mrcp15, 0, %0, c2, c0, 0"  \
+   : "=r"