Hi Parth,
I would have prefer two distinct patch: one for the refactoring of
On 17/05/15 21:03, Parth Dixit wrote:
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 935999e..096e9ef 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -2,6 +2,7 @@ subdir-$(arm32) += arm32
> subdir-$(arm64) += arm64
> subdir-y += platforms
> subdir-$(arm64) += efi
> +subdir-$(HAS_ACPI) += acpi
>
> obj-$(EARLY_PRINTK) += early_printk.o
> obj-y += cpu.o
> diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> new file mode 100644
> index 0000000..b5be22d
> --- /dev/null
> +++ b/xen/arch/arm/acpi/Makefile
> @@ -0,0 +1 @@
> +obj-y += lib.o
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> new file mode 100644
> index 0000000..650beed
> --- /dev/null
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -0,0 +1,8 @@
> +#include <xen/acpi.h>
> +#include <asm/mm.h>
> +
> +void __iomem *
> +acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
> +{
> + return __va(phys);
> +}
acpi_os_map_memory and the other for implementing the ARM part
explaining why only using __va.
__va should only be used when the memory is direct-mapped to Xen (i.e
accessible directly). On ARM64, this only the case for the RAM. Can you
confirm that ACPI will always reside to the RAM?
I already asked the same question on the previous version but got no
answer from you...
> /*
> * Important Safety Note: The fixed ACPI page numbers are *subtracted*
> * from the fixed base. That's why we start at FIX_ACPI_END and
> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
> index 93c983c..958caae 100644
> --- a/xen/drivers/acpi/osl.c
> +++ b/xen/drivers/acpi/osl.c
> @@ -87,16 +87,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
> void __iomem *
> acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
> {
> - if (system_state >= SYS_STATE_active) {
> - unsigned long pfn = PFN_DOWN(phys);
> - unsigned int offs = phys & (PAGE_SIZE - 1);
> -
> - /* The low first Mb is always mapped. */
> - if ( !((phys + size - 1) >> 20) )
> - return __va(phys);
> - return __vmap(&pfn, PFN_UP(offs + size), 1, 1, PAGE_HYPERVISOR_NOCACHE) + offs;
> - }
> - return __acpi_map_table(phys, size);
> + return acpi_os_map_iomem(phys, size);
The naming is wrong. It's really hard to differentiate
acpi_os_map_memory from acpi_os_map_iomem. I would rename to something
more meaningful such as arch_acpi_os_map_memory.
Although, given that acpi_os_map_memory only call acpi_os_map_iomem. I
would move acpi_os_map_memory per-architecture. FWIW, it's what Linux does.
--
Julien Grall