[Xen-devel,04/24] xen/arm: mm: Introduce clear_table and use it

Message ID 20170613161323.25196-5-julien.grall@arm.com
State Accepted
Commit 615656a91050898f49b00e6933ab195ada08ec49
Headers show
Series
  • xen/arm: Extend the usage of typesafe MFN
Related show

Commit Message

Julien Grall June 13, 2017, 4:13 p.m.
Add a new helper clear_table to clear a page table entry and invalidate
the cache.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/mm.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Stefano Stabellini June 15, 2017, 10:31 p.m. | #1
On Tue, 13 Jun 2017, Julien Grall wrote:
> Add a new helper clear_table to clear a page table entry and invalidate
> the cache.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/mm.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 082c872c72..b4ff777b55 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -529,6 +529,13 @@ void __init remove_early_mappings(void)
>  
>  extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
>  
> +/* Clear a translation table and clean & invalidate the cache */
> +static void clear_table(void *table)

This could be a static inline. In any case:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> +{
> +    clear_page(table);
> +    clean_and_invalidate_dcache_va_range(table, PAGE_SIZE);
> +}
> +
>  /* Boot-time pagetable setup.
>   * Changes here may need matching changes in head.S */
>  void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> @@ -604,18 +611,13 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
>  
>      /* Clear the copy of the boot pagetables. Each secondary CPU
>       * rebuilds these itself (see head.S) */
> -    memset(boot_pgtable, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_pgtable);
> +    clear_table(boot_pgtable);
>  #ifdef CONFIG_ARM_64
> -    memset(boot_first, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_first);
> -    memset(boot_first_id, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_first_id);
> +    clear_table(boot_first);
> +    clear_table(boot_first_id);
>  #endif
> -    memset(boot_second, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_second);
> -    memset(boot_third, 0x0, PAGE_SIZE);
> -    clean_and_invalidate_dcache(boot_third);
> +    clear_table(boot_second);
> +    clear_table(boot_third);
>  
>      /* Break up the Xen mapping into 4k pages and protect them separately. */
>      for ( i = 0; i < LPAE_ENTRIES; i++ )
> -- 
> 2.11.0
>
Julien Grall June 16, 2017, 6:55 a.m. | #2
Hi Stefano,

On 15/06/2017 23:31, Stefano Stabellini wrote:
> On Tue, 13 Jun 2017, Julien Grall wrote:
>> Add a new helper clear_table to clear a page table entry and invalidate
>> the cache.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>  xen/arch/arm/mm.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 082c872c72..b4ff777b55 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -529,6 +529,13 @@ void __init remove_early_mappings(void)
>>
>>  extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
>>
>> +/* Clear a translation table and clean & invalidate the cache */
>> +static void clear_table(void *table)
>
> This could be a static inline. In any case:

I see you already committed. I see very limited use here for static 
inline as this function might quite big (clear_page and 
clean_and_invalidate_dcache_va_range). The compiler is in better 
position to decide what to do than us.

IHMO, static inline should only be used in header and very small 
function (basically just checking a condition).

Cheers,

Patch hide | download patch | download mbox

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 082c872c72..b4ff777b55 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -529,6 +529,13 @@  void __init remove_early_mappings(void)
 
 extern void relocate_xen(uint64_t ttbr, void *src, void *dst, size_t len);
 
+/* Clear a translation table and clean & invalidate the cache */
+static void clear_table(void *table)
+{
+    clear_page(table);
+    clean_and_invalidate_dcache_va_range(table, PAGE_SIZE);
+}
+
 /* Boot-time pagetable setup.
  * Changes here may need matching changes in head.S */
 void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
@@ -604,18 +611,13 @@  void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
 
     /* Clear the copy of the boot pagetables. Each secondary CPU
      * rebuilds these itself (see head.S) */
-    memset(boot_pgtable, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_pgtable);
+    clear_table(boot_pgtable);
 #ifdef CONFIG_ARM_64
-    memset(boot_first, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_first);
-    memset(boot_first_id, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_first_id);
+    clear_table(boot_first);
+    clear_table(boot_first_id);
 #endif
-    memset(boot_second, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_second);
-    memset(boot_third, 0x0, PAGE_SIZE);
-    clean_and_invalidate_dcache(boot_third);
+    clear_table(boot_second);
+    clear_table(boot_third);
 
     /* Break up the Xen mapping into 4k pages and protect them separately. */
     for ( i = 0; i < LPAE_ENTRIES; i++ )