Re: [PATCH] ACPI: use ioremap_cache()

[Shaohua Li]

On Fri, 2010-12-17 at 16:08 +0800, Len Brown wrote:
> From: Len Brown <len.brown@xxxxxxxxx>
> 
> Although the temporary boot-time ACPI table mappings
> were set up with CPU caching enabled, the permanent table
> mappings and AML run-time region memory accesses were
> set up with ioremap(), which on x86 is a synonym for
> ioremap_nocache().
> 
> Changing this to ioremap_cache() improves performance as
> seen when accessing the tables via acpidump,
> or /sys/firmware/acpi/tables.  It should also improve
> AML run-time performance.
> 
> No change on ia64.
> 
> Reported-by: Jack Steiner <steiner@xxxxxxx>
> Signed-off-by: Len Brown <len.brown@xxxxxxxxx>
> ---
>  arch/ia64/include/asm/io.h |    5 +++++
>  drivers/acpi/osl.c         |    6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
> index cc8335e..009a7e0 100644
> --- a/arch/ia64/include/asm/io.h
> +++ b/arch/ia64/include/asm/io.h
> @@ -426,6 +426,11 @@ extern void __iomem * ioremap_nocache (unsigned long offset, unsigned long size)
>  extern void iounmap (volatile void __iomem *addr);
>  extern void __iomem * early_ioremap (unsigned long phys_addr, unsigned long size);
>  extern void early_iounmap (volatile void __iomem *addr, unsigned long size);
> +static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned long size)
> +{
> +	return ioremap(unsigned long phys_addr, unsigned long size);
> +}
> +
>  
>  /*
>   * String version of IO memory access ops:
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 966fedd..85eba53 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -324,7 +324,7 @@ acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
>  
>  	pg_off = round_down(phys, PAGE_SIZE);
>  	pg_sz = round_up(phys + size, PAGE_SIZE) - pg_off;
> -	virt = ioremap(pg_off, pg_sz);
> +	virt = ioremap_cache(pg_off, pg_sz);
>  	if (!virt) {
>  		kfree(map);
>  		return NULL;
> @@ -646,7 +646,7 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u32 * value, u32 width)
>  	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
>  	rcu_read_unlock();
>  	if (!virt_addr) {
> -		virt_addr = ioremap(phys_addr, size);
> +		virt_addr = ioremap_cache(phys_addr, size);
>  		unmap = 1;
>  	}
>  	if (!value)
> @@ -682,7 +682,7 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
>  	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
>  	rcu_read_unlock();
>  	if (!virt_addr) {
> -		virt_addr = ioremap(phys_addr, size);
> +		virt_addr = ioremap_cache(phys_addr, size);
>  		unmap = 1;
>  	}
blindly map the range as cache is not ok. for example:
    OperationRegion (RCRB, SystemMemory, 0xFED1C000, 0x4000)
    Field (RCRB, DWordAcc, Lock, Preserve)
    {
        Offset (0x1000),
        Offset (0x3000),
        Offset (0x3404),
        HPAS,   2,
            ,   5,
        HPAE,   1,
        Offset (0x3418),
            ,   1,
        PATD,   1,
        SATD,   1,
        SMBD,   1,
        HDAD,   1,
        A97D,   1,
        Offset (0x341A),
        RP1D,   1,
        RP2D,   1,
        RP3D,   1,
        RP4D,   1,
        RP5D,   1,
        RP6D,   1
    }
RCRB is a memory mapped io. In ICH, it's chipset configuration
registers. this range can't be cached.
I thought we should add a check like
if page is E820_RAM or E820_ACPI then
	cached_map
else
	uncached_map
we have page_is_ram() API which just checks E820_RAM, I thought we can
add a new API to check E820_ACPI.

Thanks,
Shaohua

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/