Hi Julien,
> On Aug 22, 2023, at 02:14, Julien Grall <jul...@xen.org> wrote:
>
> Hi Henry,
>
> On 14/08/2023 05:25, Henry Wang wrote:
>> From: Penny Zheng <penny.zh...@arm.com>
>>
>> diff --git a/xen/arch/arm/include/asm/fixmap.h
>> b/xen/arch/arm/include/asm/fixmap.h
>> index 734eb9b1d4..5d5de6995a 100644
>> --- a/xen/arch/arm/include/asm/fixmap.h
>> +++ b/xen/arch/arm/include/asm/fixmap.h
>> @@ -36,12 +36,7 @@ extern void clear_fixmap(unsigned int map);
>> #define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
>> -static inline unsigned int virt_to_fix(vaddr_t vaddr)
>> -{
>> - BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
>> -
>> - return ((vaddr - FIXADDR_START) >> PAGE_SHIFT);
>> -}
>> +extern unsigned int virt_to_fix(vaddr_t vaddr);
>
> AFAICT, virt_to_fix() is not going to be implemented for the MPU code. This
> implies that no-one should call it.
>
> Also, none of the definitions in fixmap.h actually makes sense for the MPU. I
> would prefer if we instead try to lmit the include of fixmap to when this is
> strictly necessary. Looking for the inclusion in staging I could find:
>
> 42sh> ack "\#include" | ack "fixmap" | ack -v x86
> arch/arm/acpi/lib.c:28:#include <asm/fixmap.h>
> arch/arm/kernel.c:19:#include <asm/fixmap.h>
> arch/arm/mm.c:27:#include <asm/fixmap.h>
> arch/arm/include/asm/fixmap.h:7:#include <xen/acpi.h>
> arch/arm/include/asm/fixmap.h:8:#include <xen/pmap.h>
> arch/arm/include/asm/pmap.h:6:#include <asm/fixmap.h>
> arch/arm/include/asm/early_printk.h:14:#include <asm/fixmap.h>
> common/efi/boot.c:30:#include <asm/fixmap.h>
> common/pmap.c:7:#include <asm/fixmap.h>
> drivers/acpi/apei/erst.c:36:#include <asm/fixmap.h>
> drivers/acpi/apei/apei-io.c:32:#include <asm/fixmap.h>
> drivers/char/xhci-dbc.c:30:#include <asm/fixmap.h>
> drivers/char/ehci-dbgp.c:16:#include <asm/fixmap.h>
> drivers/char/ns16550.c:40:#include <asm/fixmap.h>
> drivers/char/xen_pv_console.c:28:#include <asm/fixmap.h>
>
> Some of them are gone after your rework. The only remaining that we care are
> in kernel.h (but I think it can be removed after your series).
I think you are correct, so I reverted the virt_to_fix() change from this patch,
deleted the include of asm/fixmap.h in kernel.c and put this patch as the (last
- 1)th
patch of the series. The building of Xen works fine.
Does below updated patch look good to you?
```
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index eb0413336b..8a7b79b4b5 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 0d433a32e7..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>
```
I will update the commit message accordingly.
Kind regards,
Henry
>
> So I think it would be feasible to not touch fixmap.h at all.
>
> Cheers,
>
> --
> Julien Grall
>
