[v7,6/8] xen/arm: Fold pmap and fixmap into MMU system

Commit Message

Henry Wang Oct. 9, 2023, 1:03 a.m. UTC

fixmap and pmap are MMU-specific features, so fold them to the
MMU system. Do the folding for pmap by moving the HAS_PMAP Kconfig
selection under MMU. Since none of the definitions in asm/fixmap.h
actually makes sense for the MPU, so do the folding for fixmap by
limiting the inclusion of asm/fixmap.h for MPU code when necessary.
To guarantee that, moving the implementation of copy_from_paddr()
from kernel.c to mmu/setup.c, so that inclusion of asm/fixmap.h in
the kernel.c can be dropped.

Signed-off-by: Henry Wang <[e-mail]>
Signed-off-by: Penny Zheng <[e-mail]>
Signed-off-by: Wei Chen <[e-mail]>
---
v7:
- No change.
v6:
- Rework original patch:
  [v5,08/13] xen/arm: Fold pmap and fixmap into MMU system
  and fold in the original patch:
  [v5,12/13] xen/arm: mmu: relocate copy_from_paddr() to setup.c
---
 xen/arch/arm/Kconfig     |  2 +-
 xen/arch/arm/kernel.c    | 28 ----------------------------
 xen/arch/arm/mmu/setup.c | 27 +++++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 29 deletions(-)

Comments

Julien Grall Oct. 13, 2023, 5:44 p.m. UTC | #1

Hi Henry,

On 09/10/2023 02:03, Henry Wang wrote:
> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
> index c2df976ab2..dff81f6125 100644
> --- a/xen/arch/arm/mmu/setup.c
> +++ b/xen/arch/arm/mmu/setup.c
> @@ -339,6 +339,33 @@ void free_init_memory(void)
>       printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
>   }
>   
> +/**
> + * copy_from_paddr - copy data from a physical address
> + * @dst: destination virtual address
> + * @paddr: source physical address
> + * @len: length to copy
> + */
> +void __init copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
> +{
> +    void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
> +
> +    while (len) {
> +        unsigned long l, s;
> +
> +        s = paddr & (PAGE_SIZE-1);

NIT: I would take the opportunity to add a missing space before/after '-'.

Reviewed-by: Julien Grall <[e-mail]>

Cheers,

Henry Wang Oct. 14, 2023, 12:11 a.m. UTC | #2

Hi Julien,

> On Oct 14, 2023, at 01:44, Julien Grall <[e-mail]> wrote:
> 
> Hi Henry,
> 
> On 09/10/2023 02:03, Henry Wang wrote:
>> diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
>> index c2df976ab2..dff81f6125 100644
>> --- a/xen/arch/arm/mmu/setup.c
>> +++ b/xen/arch/arm/mmu/setup.c
>> @@ -339,6 +339,33 @@ void free_init_memory(void)
>>      printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
>>  }
>>  +/**
>> + * copy_from_paddr - copy data from a physical address
>> + * @dst: destination virtual address
>> + * @paddr: source physical address
>> + * @len: length to copy
>> + */
>> +void __init copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
>> +{
>> +    void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
>> +
>> +    while (len) {
>> +        unsigned long l, s;
>> +
>> +        s = paddr & (PAGE_SIZE-1);
> 
> NIT: I would take the opportunity to add a missing space before/after '-'.

Sounds good, I’ve done that in my local v8 and also mentioned this change
in commit message as:
```
Take the opportunity to add a missing space before and after '-' in
"s = paddr & (PAGE_SIZE-1);" of copy_from_paddr().
```

> 
> Reviewed-by: Julien Grall <[e-mail]>

Thanks!

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 632dd9792e..4b21a12b8b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -15,7 +15,6 @@  config ARM
 	select HAS_DEVICE_TREE
 	select HAS_PASSTHROUGH
 	select HAS_PDX
-	select HAS_PMAP
 	select HAS_UBSAN
 	select IOMMU_FORCE_PT_SHARE
 
@@ -61,6 +60,7 @@  config PADDR_BITS
 
 config MMU
 	def_bool y
+	select HAS_PMAP
 
 source "arch/Kconfig"
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 508c54824d..bc3e5bd6f9 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -16,7 +16,6 @@ 
 #include <xen/vmap.h>
 
 #include <asm/byteorder.h>
-#include <asm/fixmap.h>
 #include <asm/kernel.h>
 #include <asm/setup.h>
 
@@ -41,33 +40,6 @@  struct minimal_dtb_header {
 
 #define DTB_MAGIC 0xd00dfeedU
 
-/**
- * copy_from_paddr - copy data from a physical address
- * @dst: destination virtual address
- * @paddr: source physical address
- * @len: length to copy
- */
-void __init copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
-{
-    void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
-
-    while (len) {
-        unsigned long l, s;
-
-        s = paddr & (PAGE_SIZE-1);
-        l = min(PAGE_SIZE - s, len);
-
-        set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_WC);
-        memcpy(dst, src + s, l);
-        clean_dcache_va_range(dst, l);
-        clear_fixmap(FIXMAP_MISC);
-
-        paddr += l;
-        dst += l;
-        len -= l;
-    }
-}
-
 static void __init place_modules(struct kernel_info *info,
                                  paddr_t kernbase, paddr_t kernend)
 {
diff --git a/xen/arch/arm/mmu/setup.c b/xen/arch/arm/mmu/setup.c
index c2df976ab2..dff81f6125 100644
--- a/xen/arch/arm/mmu/setup.c
+++ b/xen/arch/arm/mmu/setup.c
@@ -339,6 +339,33 @@  void free_init_memory(void)
     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
 }
 
+/**
+ * copy_from_paddr - copy data from a physical address
+ * @dst: destination virtual address
+ * @paddr: source physical address
+ * @len: length to copy
+ */
+void __init copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
+{
+    void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
+
+    while (len) {
+        unsigned long l, s;
+
+        s = paddr & (PAGE_SIZE-1);
+        l = min(PAGE_SIZE - s, len);
+
+        set_fixmap(FIXMAP_MISC, maddr_to_mfn(paddr), PAGE_HYPERVISOR_WC);
+        memcpy(dst, src + s, l);
+        clean_dcache_va_range(dst, l);
+        clear_fixmap(FIXMAP_MISC);
+
+        paddr += l;
+        dst += l;
+        len -= l;
+    }
+}
+
 /*
  * Local variables:
  * mode: C