| Message ID | 1431893048-5214-6-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 18 May 2015 at 18:56, Julien Grall <julien.grall@citrix.com> wrote: > Hi Parth, > > 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); > > +} > > I would have prefer two distinct patch: one for the refactoring of > 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... > > I did not found any document which says it will always reside in RAM or otherwise.. > > /* > > * 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 >
+shannon On 24 May 2015 at 13:01, Julien Grall <julien.grall@citrix.com> wrote: > Hi Parth, > > On 24/05/2015 07:40, Parth Dixit wrote: >> >> 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); >> > +} >> >> I would have prefer two distinct patch: one for the refactoring of >> 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... >> >> I did not found any document which says it will always reside in RAM or >> otherwise.. > > > If so, you have use vmap or ioremap_cache as suggested by Jan. > > Regards, > > -- > Julien Grall
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); +} diff --git a/xen/arch/x86/acpi/lib.c b/xen/arch/x86/acpi/lib.c index 1f98c31..cb1b98e 100644 --- a/xen/arch/x86/acpi/lib.c +++ b/xen/arch/x86/acpi/lib.c @@ -34,6 +34,21 @@ u8 __read_mostly acpi_disable_value; u32 __read_mostly x86_acpiid_to_apicid[MAX_MADT_ENTRIES] = {[0 ... MAX_MADT_ENTRIES - 1] = BAD_APICID }; +void __iomem * +acpi_os_map_iomem(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); +} + /* * 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); } void acpi_os_unmap_memory(void __iomem * virt, acpi_size size) diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h index 3aeba4a..afe2dca 100644 --- a/xen/include/xen/acpi.h +++ b/xen/include/xen/acpi.h @@ -55,6 +55,8 @@ typedef int (*acpi_table_handler) (struct acpi_table_header *table); typedef int (*acpi_table_entry_handler) (struct acpi_subtable_header *header, const unsigned long end); +void __iomem * +acpi_os_map_iomem(acpi_physical_address phys, acpi_size size); unsigned int acpi_get_processor_id (unsigned int cpu); char * __acpi_map_table (paddr_t phys_addr, unsigned long size); int acpi_boot_init (void);
common acpi code for memory mapping is specific to x86. Add a new helper function for mapping memory based on architecture. Signed-off-by: Parth Dixit <parth.dixit@linaro.org> --- xen/arch/arm/Makefile | 1 + xen/arch/arm/acpi/Makefile | 1 + xen/arch/arm/acpi/lib.c | 8 ++++++++ xen/arch/x86/acpi/lib.c | 15 +++++++++++++++ xen/drivers/acpi/osl.c | 11 +---------- xen/include/xen/acpi.h | 2 ++ 6 files changed, 28 insertions(+), 10 deletions(-) create mode 100644 xen/arch/arm/acpi/Makefile create mode 100644 xen/arch/arm/acpi/lib.c