[PATCH 0/3] ACPI: Re-factor and remove ./drivers/acpi/atomicio.[ch]

[PATCH 0/3] ACPI: Re-factor and remove ./drivers/acpi/atomicio.[ch]

[Myron Stowe]

I recently submitted a patch series that re-factored some existing
work that Huang Ying introduced adding support for accessing ACPI
generic registers, backed by Memory Mapped I/O (MMIO), while within
interrupt context:
  Original series: http://marc.info/?l=linux-acpi&m=128769263327206&w=2
  Follow on series: http://marc.info/?l=linux-kernel&m=132070825520957&w=2

An underlying goal has been to re-factor ./drivers/acpi/atomicio.c into
./drivers/acpi/osl.c, providing equivalent functionality but in a more
generalized manner, to allow usage in non-specific (i.e. APEI) contexts
and remove atomicio.c.

In the most recent follow on series, there ended up being additions to
atomicio.c that coincided with the attempt to remove it and as such, the
series final removal patch could not be taken without first pulling the
additions into osl.c.  The fact that the follow on series conversion
patch - commit 700130b41f4 - was taken has inadvertently created
regressions with respect to the recent atomicio.c additions due to the
conversion patch effectively making atomicio.c obsolete.

The additions were commit 04c25997c97 "ACPI, Add 64bit read/write
support to atomicio on i386" and commit 76da3fb3575 "ACPI, Add RAM
mapping support to ACPI atomic IO support".

This patch series addresses both of these issues, bringing in the
additional functionality into the re-factored files (apei-base.c and
osl.c) and then again, reintroduces the patch to remove
./drivers/acpi/atomicio.[ch].
---

Myron Stowe (3):
      ACPI: Remove ./drivers/acpi/atomicio.[ch]
      ACPI, APEI: Add RAM mapping support to ACPI
      ACPI, APEI: Add 64-bit read/write support for APEI on i386


 drivers/acpi/Makefile         |    1 
 drivers/acpi/apei/apei-base.c |   35 ---
 drivers/acpi/atomicio.c       |  422 -----------------------------------------
 drivers/acpi/osl.c            |  152 +++++++++++++++
 include/acpi/acpiosxf.h       |    4 
 include/acpi/atomicio.h       |   10 -
 6 files changed, 158 insertions(+), 466 deletions(-)
 delete mode 100644 drivers/acpi/atomicio.c
 delete mode 100644 include/acpi/atomicio.h

-- 

[PATCH 1/3] ACPI, APEI: Add 64-bit read/write support for APEI on i386

[Myron Stowe]

From: Myron Stowe <mstowe@redhat.com>

Base ACPI (CA) currently does not support atomic 64-bit reads and writes
(acpi_read() and acpi_write() split 64-bit loads/stores into two
32-bit transfers) yet APEI expects 64-bit transfer capability, even
when running on 32-bit systems.

This patch implements 64-bit read and write routines for APEI usage.

This patch re-factors similar functionality introduced in commit
04c25997c97, bringing it into the ACPI subsystem in preparation for
removing ./drivers/acpi/atomicio.[ch].  In the implementation I have
replicated acpi_os_read_memory() and acpi_os_write_memory(), creating
64-bit versions for APEI to utilize, as opposed to something more
elegant.  My thinking is that we should attempt to see if we can get
ACPI's CA/OSL changed so that the existing acpi_read() and acpi_write()
interfaces are natively 64-bit capable and then subsequently remove the
replication.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/apei/apei-base.c |   35 +-----------
 drivers/acpi/osl.c            |  116 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpiosxf.h       |    4 +
 3 files changed, 124 insertions(+), 31 deletions(-)

diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index e45350c..e5d53b7 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -596,33 +596,19 @@ int apei_read(u64 *val, struct acpi_generic_address *reg)
 {
 	int rc;
 	u64 address;
-	u32 tmp, width = reg->bit_width;
 	acpi_status status;
 
 	rc = apei_check_gar(reg, &address);
 	if (rc)
 		return rc;
 
-	if (width == 64)
-		width = 32;	/* Break into two 32-bit transfers */
-
 	*val = 0;
 	switch(reg->space_id) {
 	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
-		status = acpi_os_read_memory((acpi_physical_address)
-					     address, &tmp, width);
+		status = acpi_os_read_memory64((acpi_physical_address)
+					     address, val, reg->bit_width);
 		if (ACPI_FAILURE(status))
 			return -EIO;
-		*val = tmp;
-
-		if (reg->bit_width == 64) {
-			/* Read the top 32 bits */
-			status = acpi_os_read_memory((acpi_physical_address)
-						     (address + 4), &tmp, 32);
-			if (ACPI_FAILURE(status))
-				return -EIO;
-			*val |= ((u64)tmp << 32);
-		}
 		break;
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		status = acpi_os_read_port(address, (u32 *)val, reg->bit_width);
@@ -642,31 +628,18 @@ int apei_write(u64 val, struct acpi_generic_address *reg)
 {
 	int rc;
 	u64 address;
-	u32 width = reg->bit_width;
 	acpi_status status;
 
 	rc = apei_check_gar(reg, &address);
 	if (rc)
 		return rc;
 
-	if (width == 64)
-		width = 32;	/* Break into two 32-bit transfers */
-
 	switch (reg->space_id) {
 	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
-		status = acpi_os_write_memory((acpi_physical_address)
-					      address, ACPI_LODWORD(val),
-					      width);
+		status = acpi_os_write_memory64((acpi_physical_address)
+					      address, val, reg->bit_width);
 		if (ACPI_FAILURE(status))
 			return -EIO;
-
-		if (reg->bit_width == 64) {
-			status = acpi_os_write_memory((acpi_physical_address)
-						      (address + 4),
-						      ACPI_HIDWORD(val), 32);
-			if (ACPI_FAILURE(status))
-				return -EIO;
-		}
 		break;
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		status = acpi_os_write_port(address, val, reg->bit_width);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fcc12d8..5498a6d 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -710,6 +710,67 @@ acpi_os_read_memory(acpi_physical_address phys_addr, u32 * value, u32 width)
 	return AE_OK;
 }
 
+#ifdef readq
+static inline u64 read64(const volatile void __iomem *addr)
+{
+	return readq(addr);
+}
+#else
+static inline u64 read64(const volatile void __iomem *addr)
+{
+	u64 l, h;
+	l = readl(addr);
+	h = readl(addr+4);
+	return l | (h << 32);
+}
+#endif
+
+acpi_status
+acpi_os_read_memory64(acpi_physical_address phys_addr, u64 *value, u32 width)
+{
+	void __iomem *virt_addr;
+	unsigned int size = width / 8;
+	bool unmap = false;
+	u64 dummy;
+
+	rcu_read_lock();
+	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
+	if (!virt_addr) {
+		rcu_read_unlock();
+		virt_addr = acpi_os_ioremap(phys_addr, size);
+		if (!virt_addr)
+			return AE_BAD_ADDRESS;
+		unmap = true;
+	}
+
+	if (!value)
+		value = &dummy;
+
+	switch (width) {
+	case 8:
+		*(u8 *) value = readb(virt_addr);
+		break;
+	case 16:
+		*(u16 *) value = readw(virt_addr);
+		break;
+	case 32:
+		*(u32 *) value = readl(virt_addr);
+		break;
+	case 64:
+		*(u64 *) value = read64(virt_addr);
+		break;
+	default:
+		BUG();
+	}
+
+	if (unmap)
+		iounmap(virt_addr);
+	else
+		rcu_read_unlock();
+
+	return AE_OK;
+}
+
 acpi_status
 acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
 {
@@ -749,6 +810,61 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
 	return AE_OK;
 }
 
+#ifdef writeq
+static inline void write64(u64 val, volatile void __iomem *addr)
+{
+	writeq(val, addr);
+}
+#else
+static inline void write64(u64 val, volatile void __iomem *addr)
+{
+	writel(val, addr);
+	writel(val>>32, addr+4);
+}
+#endif
+
+acpi_status
+acpi_os_write_memory64(acpi_physical_address phys_addr, u64 value, u32 width)
+{
+	void __iomem *virt_addr;
+	unsigned int size = width / 8;
+	bool unmap = false;
+
+	rcu_read_lock();
+	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
+	if (!virt_addr) {
+		rcu_read_unlock();
+		virt_addr = acpi_os_ioremap(phys_addr, size);
+		if (!virt_addr)
+			return AE_BAD_ADDRESS;
+		unmap = true;
+	}
+
+	switch (width) {
+	case 8:
+		writeb(value, virt_addr);
+		break;
+	case 16:
+		writew(value, virt_addr);
+		break;
+	case 32:
+		writel(value, virt_addr);
+		break;
+	case 64:
+		write64(value, virt_addr);
+		break;
+	default:
+		BUG();
+	}
+
+	if (unmap)
+		iounmap(virt_addr);
+	else
+		rcu_read_unlock();
+
+	return AE_OK;
+}
+
 acpi_status
 acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 			       u64 *value, u32 width)
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 2fe8639..7c9aebe 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -218,9 +218,13 @@ acpi_status acpi_os_write_port(acpi_io_address address, u32 value, u32 width);
  */
 acpi_status
 acpi_os_read_memory(acpi_physical_address address, u32 * value, u32 width);
+acpi_status
+acpi_os_read_memory64(acpi_physical_address address, u64 *value, u32 width);
 
 acpi_status
 acpi_os_write_memory(acpi_physical_address address, u32 value, u32 width);
+acpi_status
+acpi_os_write_memory64(acpi_physical_address address, u64 value, u32 width);
 
 /*
  * Platform and hardware-independent PCI configuration space access

[PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Myron Stowe]

From: Myron Stowe <mstowe@redhat.com>

This patch adds support for RAM to ACPI's mapping capabilities in order
to support APEI error injection (EINJ) actions.

This patch re-factors similar functionality introduced in commit
76da3fb3575, bringing it into osl.c in preparation for removing
./drivers/acpi/atomicio.[ch].

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/osl.c |   36 ++++++++++++++++++++++++++++++++++--
 1 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f363a55..8ee64ea 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -31,6 +31,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/mm.h>
+#include <linux/highmem.h>
 #include <linux/pci.h>
 #include <linux/interrupt.h>
 #include <linux/kmod.h>
@@ -321,6 +322,37 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
 	return NULL;
 }
 
+#ifndef CONFIG_IA64
+#define should_use_kmap(pfn)   page_is_ram(pfn)
+#else
+/* ioremap will take care of cache attributes */
+#define should_use_kmap(pfn)   0
+#endif
+
+static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
+{
+	unsigned long pfn;
+
+	pfn = pg_off >> PAGE_SHIFT;
+	if (should_use_kmap(pfn)) {
+		if (pg_sz > PAGE_SIZE)
+			return NULL;
+		return (void __iomem __force *)kmap(pfn_to_page(pfn));
+	} else
+		return acpi_os_ioremap(pg_off, pg_sz);
+}
+
+static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
+{
+	unsigned long pfn;
+
+	pfn = pg_off >> PAGE_SHIFT;
+	if (page_is_ram(pfn))
+		kunmap(pfn_to_page(pfn));
+	else
+		iounmap(vaddr);
+}
+
 void __iomem *__init_refok
 acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
 {
@@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
+	virt = acpi_map(pg_off, pg_sz);
 	if (!virt) {
 		mutex_unlock(&acpi_ioremap_lock);
 		kfree(map);
@@ -384,7 +416,7 @@ static void acpi_os_map_cleanup(struct acpi_ioremap *map)
 {
 	if (!map->refcount) {
 		synchronize_rcu();
-		iounmap(map->virt);
+		acpi_unmap(map->phys, map->virt);
 		kfree(map);
 	}
 }

[PATCH 3/3] ACPI: Remove ./drivers/acpi/atomicio.[ch]

[Myron Stowe]

From: Myron Stowe <mstowe@redhat.com>

With the conversion of atomicio's routines in place (see commits
6f68c91c55e and 700130b41f4), atomicio.[ch] can be removed, replacing
the APEI specific pre-mapping capabilities with the more generalized
versions that drivers/acpi/osl.c provides.

Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---

 drivers/acpi/Makefile   |    1 
 drivers/acpi/atomicio.c |  422 -----------------------------------------------
 include/acpi/atomicio.h |   10 -
 3 files changed, 0 insertions(+), 433 deletions(-)
 delete mode 100644 drivers/acpi/atomicio.c
 delete mode 100644 include/acpi/atomicio.h

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c07f44f..1567028 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -19,7 +19,6 @@ obj-y				+= acpi.o \
 
 # All the builtin files are in the "acpi." module_param namespace.
 acpi-y				+= osl.o utils.o reboot.o
-acpi-y				+= atomicio.o
 acpi-y				+= nvs.o
 
 # sleep related files
diff --git a/drivers/acpi/atomicio.c b/drivers/acpi/atomicio.c
deleted file mode 100644
index d4a5b3d..0000000
--- a/drivers/acpi/atomicio.c
+++ /dev/null
@@ -1,422 +0,0 @@
-/*
- * atomicio.c - ACPI IO memory pre-mapping/post-unmapping, then
- * accessing in atomic context.
- *
- * This is used for NMI handler to access IO memory area, because
- * ioremap/iounmap can not be used in NMI handler. The IO memory area
- * is pre-mapped in process context and accessed in NMI handler.
- *
- * Copyright (C) 2009-2010, Intel Corp.
- *	Author: Huang Ying <ying.huang@intel.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License version
- * 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- */
-
-#include <linux/kernel.h>
-#include <linux/export.h>
-#include <linux/init.h>
-#include <linux/acpi.h>
-#include <linux/io.h>
-#include <linux/kref.h>
-#include <linux/rculist.h>
-#include <linux/interrupt.h>
-#include <linux/slab.h>
-#include <linux/mm.h>
-#include <linux/highmem.h>
-#include <acpi/atomicio.h>
-
-#define ACPI_PFX "ACPI: "
-
-static LIST_HEAD(acpi_iomaps);
-/*
- * Used for mutual exclusion between writers of acpi_iomaps list, for
- * synchronization between readers and writer, RCU is used.
- */
-static DEFINE_SPINLOCK(acpi_iomaps_lock);
-
-struct acpi_iomap {
-	struct list_head list;
-	void __iomem *vaddr;
-	unsigned long size;
-	phys_addr_t paddr;
-	struct kref ref;
-};
-
-/* acpi_iomaps_lock or RCU read lock must be held before calling */
-static struct acpi_iomap *__acpi_find_iomap(phys_addr_t paddr,
-					    unsigned long size)
-{
-	struct acpi_iomap *map;
-
-	list_for_each_entry_rcu(map, &acpi_iomaps, list) {
-		if (map->paddr + map->size >= paddr + size &&
-		    map->paddr <= paddr)
-			return map;
-	}
-	return NULL;
-}
-
-/*
- * Atomic "ioremap" used by NMI handler, if the specified IO memory
- * area is not pre-mapped, NULL will be returned.
- *
- * acpi_iomaps_lock or RCU read lock must be held before calling
- */
-static void __iomem *__acpi_ioremap_fast(phys_addr_t paddr,
-					 unsigned long size)
-{
-	struct acpi_iomap *map;
-
-	map = __acpi_find_iomap(paddr, size/8);
-	if (map)
-		return map->vaddr + (paddr - map->paddr);
-	else
-		return NULL;
-}
-
-/* acpi_iomaps_lock must be held before calling */
-static void __iomem *__acpi_try_ioremap(phys_addr_t paddr,
-					unsigned long size)
-{
-	struct acpi_iomap *map;
-
-	map = __acpi_find_iomap(paddr, size);
-	if (map) {
-		kref_get(&map->ref);
-		return map->vaddr + (paddr - map->paddr);
-	} else
-		return NULL;
-}
-
-#ifndef CONFIG_IA64
-#define should_use_kmap(pfn)	page_is_ram(pfn)
-#else
-/* ioremap will take care of cache attributes */
-#define should_use_kmap(pfn)	0
-#endif
-
-static void __iomem *acpi_map(phys_addr_t pg_off, unsigned long pg_sz)
-{
-	unsigned long pfn;
-
-	pfn = pg_off >> PAGE_SHIFT;
-	if (should_use_kmap(pfn)) {
-		if (pg_sz > PAGE_SIZE)
-			return NULL;
-		return (void __iomem __force *)kmap(pfn_to_page(pfn));
-	} else
-		return ioremap(pg_off, pg_sz);
-}
-
-static void acpi_unmap(phys_addr_t pg_off, void __iomem *vaddr)
-{
-	unsigned long pfn;
-
-	pfn = pg_off >> PAGE_SHIFT;
-	if (page_is_ram(pfn))
-		kunmap(pfn_to_page(pfn));
-	else
-		iounmap(vaddr);
-}
-
-/*
- * Used to pre-map the specified IO memory area. First try to find
- * whether the area is already pre-mapped, if it is, increase the
- * reference count (in __acpi_try_ioremap) and return; otherwise, do
- * the real ioremap, and add the mapping into acpi_iomaps list.
- */
-static void __iomem *acpi_pre_map(phys_addr_t paddr,
-				  unsigned long size)
-{
-	void __iomem *vaddr;
-	struct acpi_iomap *map;
-	unsigned long pg_sz, flags;
-	phys_addr_t pg_off;
-
-	spin_lock_irqsave(&acpi_iomaps_lock, flags);
-	vaddr = __acpi_try_ioremap(paddr, size);
-	spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-	if (vaddr)
-		return vaddr;
-
-	pg_off = paddr & PAGE_MASK;
-	pg_sz = ((paddr + size + PAGE_SIZE - 1) & PAGE_MASK) - pg_off;
-	vaddr = acpi_map(pg_off, pg_sz);
-	if (!vaddr)
-		return NULL;
-	map = kmalloc(sizeof(*map), GFP_KERNEL);
-	if (!map)
-		goto err_unmap;
-	INIT_LIST_HEAD(&map->list);
-	map->paddr = pg_off;
-	map->size = pg_sz;
-	map->vaddr = vaddr;
-	kref_init(&map->ref);
-
-	spin_lock_irqsave(&acpi_iomaps_lock, flags);
-	vaddr = __acpi_try_ioremap(paddr, size);
-	if (vaddr) {
-		spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-		acpi_unmap(pg_off, map->vaddr);
-		kfree(map);
-		return vaddr;
-	}
-	list_add_tail_rcu(&map->list, &acpi_iomaps);
-	spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-
-	return map->vaddr + (paddr - map->paddr);
-err_unmap:
-	acpi_unmap(pg_off, vaddr);
-	return NULL;
-}
-
-/* acpi_iomaps_lock must be held before calling */
-static void __acpi_kref_del_iomap(struct kref *ref)
-{
-	struct acpi_iomap *map;
-
-	map = container_of(ref, struct acpi_iomap, ref);
-	list_del_rcu(&map->list);
-}
-
-/*
- * Used to post-unmap the specified IO memory area. The iounmap is
- * done only if the reference count goes zero.
- */
-static void acpi_post_unmap(phys_addr_t paddr, unsigned long size)
-{
-	struct acpi_iomap *map;
-	unsigned long flags;
-	int del;
-
-	spin_lock_irqsave(&acpi_iomaps_lock, flags);
-	map = __acpi_find_iomap(paddr, size);
-	BUG_ON(!map);
-	del = kref_put(&map->ref, __acpi_kref_del_iomap);
-	spin_unlock_irqrestore(&acpi_iomaps_lock, flags);
-
-	if (!del)
-		return;
-
-	synchronize_rcu();
-	acpi_unmap(map->paddr, map->vaddr);
-	kfree(map);
-}
-
-/* In NMI handler, should set silent = 1 */
-static int acpi_check_gar(struct acpi_generic_address *reg,
-			  u64 *paddr, int silent)
-{
-	u32 width, space_id;
-
-	width = reg->bit_width;
-	space_id = reg->space_id;
-	/* Handle possible alignment issues */
-	memcpy(paddr, &reg->address, sizeof(*paddr));
-	if (!*paddr) {
-		if (!silent)
-			pr_warning(FW_BUG ACPI_PFX
-			"Invalid physical address in GAR [0x%llx/%u/%u]\n",
-				   *paddr, width, space_id);
-		return -EINVAL;
-	}
-
-	if ((width != 8) && (width != 16) && (width != 32) && (width != 64)) {
-		if (!silent)
-			pr_warning(FW_BUG ACPI_PFX
-				   "Invalid bit width in GAR [0x%llx/%u/%u]\n",
-				   *paddr, width, space_id);
-		return -EINVAL;
-	}
-
-	if (space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
-	    space_id != ACPI_ADR_SPACE_SYSTEM_IO) {
-		if (!silent)
-			pr_warning(FW_BUG ACPI_PFX
-			"Invalid address space type in GAR [0x%llx/%u/%u]\n",
-				   *paddr, width, space_id);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-/* Pre-map, working on GAR */
-int acpi_pre_map_gar(struct acpi_generic_address *reg)
-{
-	u64 paddr;
-	void __iomem *vaddr;
-	int rc;
-
-	if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		return 0;
-
-	rc = acpi_check_gar(reg, &paddr, 0);
-	if (rc)
-		return rc;
-
-	vaddr = acpi_pre_map(paddr, reg->bit_width / 8);
-	if (!vaddr)
-		return -EIO;
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(acpi_pre_map_gar);
-
-/* Post-unmap, working on GAR */
-int acpi_post_unmap_gar(struct acpi_generic_address *reg)
-{
-	u64 paddr;
-	int rc;
-
-	if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		return 0;
-
-	rc = acpi_check_gar(reg, &paddr, 0);
-	if (rc)
-		return rc;
-
-	acpi_post_unmap(paddr, reg->bit_width / 8);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(acpi_post_unmap_gar);
-
-#ifdef readq
-static inline u64 read64(const volatile void __iomem *addr)
-{
-	return readq(addr);
-}
-#else
-static inline u64 read64(const volatile void __iomem *addr)
-{
-	u64 l, h;
-	l = readl(addr);
-	h = readl(addr+4);
-	return l | (h << 32);
-}
-#endif
-
-/*
- * Can be used in atomic (including NMI) or process context. RCU read
- * lock can only be released after the IO memory area accessing.
- */
-static int acpi_atomic_read_mem(u64 paddr, u64 *val, u32 width)
-{
-	void __iomem *addr;
-
-	rcu_read_lock();
-	addr = __acpi_ioremap_fast(paddr, width);
-	switch (width) {
-	case 8:
-		*val = readb(addr);
-		break;
-	case 16:
-		*val = readw(addr);
-		break;
-	case 32:
-		*val = readl(addr);
-		break;
-	case 64:
-		*val = read64(addr);
-		break;
-	default:
-		return -EINVAL;
-	}
-	rcu_read_unlock();
-
-	return 0;
-}
-
-#ifdef writeq
-static inline void write64(u64 val, volatile void __iomem *addr)
-{
-	writeq(val, addr);
-}
-#else
-static inline void write64(u64 val, volatile void __iomem *addr)
-{
-	writel(val, addr);
-	writel(val>>32, addr+4);
-}
-#endif
-
-static int acpi_atomic_write_mem(u64 paddr, u64 val, u32 width)
-{
-	void __iomem *addr;
-
-	rcu_read_lock();
-	addr = __acpi_ioremap_fast(paddr, width);
-	switch (width) {
-	case 8:
-		writeb(val, addr);
-		break;
-	case 16:
-		writew(val, addr);
-		break;
-	case 32:
-		writel(val, addr);
-		break;
-	case 64:
-		write64(val, addr);
-		break;
-	default:
-		return -EINVAL;
-	}
-	rcu_read_unlock();
-
-	return 0;
-}
-
-/* GAR accessing in atomic (including NMI) or process context */
-int acpi_atomic_read(u64 *val, struct acpi_generic_address *reg)
-{
-	u64 paddr;
-	int rc;
-
-	rc = acpi_check_gar(reg, &paddr, 1);
-	if (rc)
-		return rc;
-
-	*val = 0;
-	switch (reg->space_id) {
-	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
-		return acpi_atomic_read_mem(paddr, val, reg->bit_width);
-	case ACPI_ADR_SPACE_SYSTEM_IO:
-		return acpi_os_read_port(paddr, (u32 *)val, reg->bit_width);
-	default:
-		return -EINVAL;
-	}
-}
-EXPORT_SYMBOL_GPL(acpi_atomic_read);
-
-int acpi_atomic_write(u64 val, struct acpi_generic_address *reg)
-{
-	u64 paddr;
-	int rc;
-
-	rc = acpi_check_gar(reg, &paddr, 1);
-	if (rc)
-		return rc;
-
-	switch (reg->space_id) {
-	case ACPI_ADR_SPACE_SYSTEM_MEMORY:
-		return acpi_atomic_write_mem(paddr, val, reg->bit_width);
-	case ACPI_ADR_SPACE_SYSTEM_IO:
-		return acpi_os_write_port(paddr, val, reg->bit_width);
-	default:
-		return -EINVAL;
-	}
-}
-EXPORT_SYMBOL_GPL(acpi_atomic_write);
diff --git a/include/acpi/atomicio.h b/include/acpi/atomicio.h
deleted file mode 100644
index 8b9fb4b..0000000
--- a/include/acpi/atomicio.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef ACPI_ATOMIC_IO_H
-#define ACPI_ATOMIC_IO_H
-
-int acpi_pre_map_gar(struct acpi_generic_address *reg);
-int acpi_post_unmap_gar(struct acpi_generic_address *reg);
-
-int acpi_atomic_read(u64 *val, struct acpi_generic_address *reg);
-int acpi_atomic_write(u64 val, struct acpi_generic_address *reg);
-
-#endif

Re: [PATCH 0/3] ACPI: Re-factor and remove ./drivers/acpi/atomicio.[ch]

[Huang Ying]

On Fri, 2012-01-20 at 19:13 -0700, Myron Stowe wrote:
> I recently submitted a patch series that re-factored some existing
> work that Huang Ying introduced adding support for accessing ACPI
> generic registers, backed by Memory Mapped I/O (MMIO), while within
> interrupt context:
>   Original series: http://marc.info/?l=linux-acpi&m=128769263327206&w=2
>   Follow on series: http://marc.info/?l=linux-kernel&m=132070825520957&w=2
> 
> An underlying goal has been to re-factor ./drivers/acpi/atomicio.c into
> ./drivers/acpi/osl.c, providing equivalent functionality but in a more
> generalized manner, to allow usage in non-specific (i.e. APEI) contexts
> and remove atomicio.c.
> 
> In the most recent follow on series, there ended up being additions to
> atomicio.c that coincided with the attempt to remove it and as such, the
> series final removal patch could not be taken without first pulling the
> additions into osl.c.  The fact that the follow on series conversion
> patch - commit 700130b41f4 - was taken has inadvertently created
> regressions with respect to the recent atomicio.c additions due to the
> conversion patch effectively making atomicio.c obsolete.
> 
> The additions were commit 04c25997c97 "ACPI, Add 64bit read/write
> support to atomicio on i386" and commit 76da3fb3575 "ACPI, Add RAM
> mapping support to ACPI atomic IO support".
> 
> This patch series addresses both of these issues, bringing in the
> additional functionality into the re-factored files (apei-base.c and
> osl.c) and then again, reintroduces the patch to remove
> ./drivers/acpi/atomicio.[ch].
> ---
> 
> Myron Stowe (3):
>       ACPI: Remove ./drivers/acpi/atomicio.[ch]

It appears there is no one uses atomicio.[ch] now.  So I think it is
safe to push this into 3.3.

>       ACPI, APEI: Add RAM mapping support to ACPI
>       ACPI, APEI: Add 64-bit read/write support for APEI on i386

These two patches are just fixes.  So I think we should push these two
patches into 3.3.

Len, Do you agree?

Best Regards,
Huang Ying

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Bjorn Helgaas]

On Fri, Jan 20, 2012 at 7:13 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
> From: Myron Stowe <mstowe@redhat.com>
>
> This patch adds support for RAM to ACPI's mapping capabilities in order
> to support APEI error injection (EINJ) actions.
>
> This patch re-factors similar functionality introduced in commit
> 76da3fb3575, bringing it into osl.c in preparation for removing
> ./drivers/acpi/atomicio.[ch].
>
> Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
> ---
>
>  drivers/acpi/osl.c |   36 ++++++++++++++++++++++++++++++++++--
>  1 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index f363a55..8ee64ea 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -31,6 +31,7 @@
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
>  #include <linux/mm.h>
> +#include <linux/highmem.h>
>  #include <linux/pci.h>
>  #include <linux/interrupt.h>
>  #include <linux/kmod.h>
> @@ -321,6 +322,37 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
>        return NULL;
>  }
>
> +#ifndef CONFIG_IA64
> +#define should_use_kmap(pfn)   page_is_ram(pfn)
> +#else
> +/* ioremap will take care of cache attributes */
> +#define should_use_kmap(pfn)   0
> +#endif
> +
> +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
> +{
> +       unsigned long pfn;
> +
> +       pfn = pg_off >> PAGE_SHIFT;
> +       if (should_use_kmap(pfn)) {
> +               if (pg_sz > PAGE_SIZE)
> +                       return NULL;
> +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
> +       } else
> +               return acpi_os_ioremap(pg_off, pg_sz);

This implies that ioremap() works differently on ia64 than on x86.
Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
different?  Shouldn't we instead fix ioremap() on ia64 so it works the
same as on x86?

I looked at the ia64 ioremap(), and I can't see the reason it fails
for RAM.  Huang, do you remember the details from 76da3fb3575?

> +}
> +
> +static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
> +{
> +       unsigned long pfn;
> +
> +       pfn = pg_off >> PAGE_SHIFT;
> +       if (page_is_ram(pfn))
> +               kunmap(pfn_to_page(pfn));
> +       else
> +               iounmap(vaddr);

I hope we can resolve the ioremap() question so we don't need this
patch at all.  But if we do need this, I don't like the asymmetry here
-- on x86 RAM, I think we use ioremap() and kunmap(), which seems
wrong.  We should be able to use ioremap() and iounmap().

> +}
> +
>  void __iomem *__init_refok
>  acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
>  {
> @@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
> +       virt = acpi_map(pg_off, pg_sz);
>        if (!virt) {
>                mutex_unlock(&acpi_ioremap_lock);
>                kfree(map);
> @@ -384,7 +416,7 @@ static void acpi_os_map_cleanup(struct acpi_ioremap *map)
>  {
>        if (!map->refcount) {
>                synchronize_rcu();
> -               iounmap(map->virt);
> +               acpi_unmap(map->phys, map->virt);
>                kfree(map);
>        }
>  }
>

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Bjorn Helgaas]

+cc Tony, linux-ia64

On Sat, Jan 21, 2012 at 8:04 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Fri, Jan 20, 2012 at 7:13 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
>> From: Myron Stowe <mstowe@redhat.com>
>>
>> This patch adds support for RAM to ACPI's mapping capabilities in order
>> to support APEI error injection (EINJ) actions.
>>
>> This patch re-factors similar functionality introduced in commit
>> 76da3fb3575, bringing it into osl.c in preparation for removing
>> ./drivers/acpi/atomicio.[ch].
>>
>> Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
>> ---
>>
>>  drivers/acpi/osl.c |   36 ++++++++++++++++++++++++++++++++++--
>>  1 files changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index f363a55..8ee64ea 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/kernel.h>
>>  #include <linux/slab.h>
>>  #include <linux/mm.h>
>> +#include <linux/highmem.h>
>>  #include <linux/pci.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/kmod.h>
>> @@ -321,6 +322,37 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
>>        return NULL;
>>  }
>>
>> +#ifndef CONFIG_IA64
>> +#define should_use_kmap(pfn)   page_is_ram(pfn)
>> +#else
>> +/* ioremap will take care of cache attributes */
>> +#define should_use_kmap(pfn)   0
>> +#endif
>> +
>> +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
>> +{
>> +       unsigned long pfn;
>> +
>> +       pfn = pg_off >> PAGE_SHIFT;
>> +       if (should_use_kmap(pfn)) {
>> +               if (pg_sz > PAGE_SIZE)
>> +                       return NULL;
>> +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
>> +       } else
>> +               return acpi_os_ioremap(pg_off, pg_sz);
>
> This implies that ioremap() works differently on ia64 than on x86.
> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
> same as on x86?
>
> I looked at the ia64 ioremap(), and I can't see the reason it fails
> for RAM.  Huang, do you remember the details from 76da3fb3575?
>
>> +}
>> +
>> +static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
>> +{
>> +       unsigned long pfn;
>> +
>> +       pfn = pg_off >> PAGE_SHIFT;
>> +       if (page_is_ram(pfn))
>> +               kunmap(pfn_to_page(pfn));
>> +       else
>> +               iounmap(vaddr);
>
> I hope we can resolve the ioremap() question so we don't need this
> patch at all.  But if we do need this, I don't like the asymmetry here
> -- on x86 RAM, I think we use ioremap() and kunmap(), which seems
> wrong.  We should be able to use ioremap() and iounmap().
>
>> +}
>> +
>>  void __iomem *__init_refok
>>  acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
>>  {
>> @@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
>> +       virt = acpi_map(pg_off, pg_sz);
>>        if (!virt) {
>>                mutex_unlock(&acpi_ioremap_lock);
>>                kfree(map);
>> @@ -384,7 +416,7 @@ static void acpi_os_map_cleanup(struct acpi_ioremap *map)
>>  {
>>        if (!map->refcount) {
>>                synchronize_rcu();
>> -               iounmap(map->virt);
>> +               acpi_unmap(map->phys, map->virt);
>>                kfree(map);
>>        }
>>  }
>>

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Myron Stowe]

On Sat, 2012-01-21 at 08:04 -0700, Bjorn Helgaas wrote:
> On Fri, Jan 20, 2012 at 7:13 PM, Myron Stowe <myron.stowe@redhat.com> wrote:
> > From: Myron Stowe <mstowe@redhat.com>
> >
> > This patch adds support for RAM to ACPI's mapping capabilities in order
> > to support APEI error injection (EINJ) actions.
> >
> > This patch re-factors similar functionality introduced in commit
> > 76da3fb3575, bringing it into osl.c in preparation for removing
> > ./drivers/acpi/atomicio.[ch].
> >
> > Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
> > ---
> >
> >  drivers/acpi/osl.c |   36 ++++++++++++++++++++++++++++++++++--
> >  1 files changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index f363a55..8ee64ea 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -31,6 +31,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> >  #include <linux/mm.h>
> > +#include <linux/highmem.h>
> >  #include <linux/pci.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/kmod.h>
> > @@ -321,6 +322,37 @@ acpi_map_lookup_virt(void __iomem *virt, acpi_size size)
> >        return NULL;
> >  }
> >
> > +#ifndef CONFIG_IA64
> > +#define should_use_kmap(pfn)   page_is_ram(pfn)
> > +#else
> > +/* ioremap will take care of cache attributes */
> > +#define should_use_kmap(pfn)   0
> > +#endif
> > +
> > +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
> > +{
> > +       unsigned long pfn;
> > +
> > +       pfn = pg_off >> PAGE_SHIFT;
> > +       if (should_use_kmap(pfn)) {
> > +               if (pg_sz > PAGE_SIZE)
> > +                       return NULL;
> > +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
> > +       } else
> > +               return acpi_os_ioremap(pg_off, pg_sz);
> 
> This implies that ioremap() works differently on ia64 than on x86.
> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
> same as on x86?
> 
> I looked at the ia64 ioremap(), and I can't see the reason it fails
> for RAM.  Huang, do you remember the details from 76da3fb3575?
> 
> > +}
> > +
> > +static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
> > +{
> > +       unsigned long pfn;
> > +
> > +       pfn = pg_off >> PAGE_SHIFT;
> > +       if (page_is_ram(pfn))
> > +               kunmap(pfn_to_page(pfn));
> > +       else
> > +               iounmap(vaddr);
> 
> I hope we can resolve the ioremap() question so we don't need this
> patch at all.  But if we do need this, I don't like the asymmetry here
> -- on x86 RAM, I think we use ioremap() and kunmap(), which seems
> wrong.  We should be able to use ioremap() and iounmap().

Huang:

There seems to be a lot of people wondering about this - could you
respond please.

Myron
> 
> > +}
> > +
> >  void __iomem *__init_refok
> >  acpi_os_map_memory(acpi_physical_address phys, acpi_size size)
> >  {
> > @@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
> > +       virt = acpi_map(pg_off, pg_sz);
> >        if (!virt) {
> >                mutex_unlock(&acpi_ioremap_lock);
> >                kfree(map);
> > @@ -384,7 +416,7 @@ static void acpi_os_map_cleanup(struct acpi_ioremap *map)
> >  {
> >        if (!map->refcount) {
> >                synchronize_rcu();
> > -               iounmap(map->virt);
> > +               acpi_unmap(map->phys, map->virt);
> >                kfree(map);
> >        }
> >  }
> >

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Huang Ying]

Hi, Bjorn,

Sorry for late.  Just return from Chinese new year holiday.

On Sat, 2012-01-21 at 08:04 -0700, Bjorn Helgaas wrote:
[snip]
> > +
> > +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
> > +{
> > +       unsigned long pfn;
> > +
> > +       pfn = pg_off >> PAGE_SHIFT;
> > +       if (should_use_kmap(pfn)) {
> > +               if (pg_sz > PAGE_SIZE)
> > +                       return NULL;
> > +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
> > +       } else
> > +               return acpi_os_ioremap(pg_off, pg_sz);
> 
> This implies that ioremap() works differently on ia64 than on x86.
> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
> same as on x86?

If my understanding were correct, ioremap can not work for RAM on x86.
So we need to use kmap for RAM.  And on IA64, ioremap works for RAM and
will take care of cache attributes while kmap will not.  So ioremap is
used on IA64, while kmap is used on x86.

> I looked at the ia64 ioremap(), and I can't see the reason it fails
> for RAM.  Huang, do you remember the details from 76da3fb3575?
> 
> > +}
> > +
> > +static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
> > +{
> > +       unsigned long pfn;
> > +
> > +       pfn = pg_off >> PAGE_SHIFT;
> > +       if (page_is_ram(pfn))
> > +               kunmap(pfn_to_page(pfn));
> > +       else
> > +               iounmap(vaddr);
> 
> I hope we can resolve the ioremap() question so we don't need this
> patch at all.  But if we do need this, I don't like the asymmetry here
> -- on x86 RAM, I think we use ioremap() and kunmap(), which seems
> wrong.  We should be able to use ioremap() and iounmap().

Yes.   The asymmetry here is not good.  We should use should_use_kmap()
instead of page_is_ram directly here.

Best Regards,
Huang Ying

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Bjorn Helgaas]

On Sat, Jan 28, 2012 at 5:58 PM, Huang Ying <ying.huang@intel.com> wrote:
> Hi, Bjorn,
>
> Sorry for late.  Just return from Chinese new year holiday.
>
> On Sat, 2012-01-21 at 08:04 -0700, Bjorn Helgaas wrote:
> [snip]
>> > +
>> > +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
>> > +{
>> > +       unsigned long pfn;
>> > +
>> > +       pfn = pg_off >> PAGE_SHIFT;
>> > +       if (should_use_kmap(pfn)) {
>> > +               if (pg_sz > PAGE_SIZE)
>> > +                       return NULL;
>> > +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
>> > +       } else
>> > +               return acpi_os_ioremap(pg_off, pg_sz);
>>
>> This implies that ioremap() works differently on ia64 than on x86.
>> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
>> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
>> same as on x86?
>
> If my understanding were correct, ioremap can not work for RAM on x86.
> So we need to use kmap for RAM.  And on IA64, ioremap works for RAM and
> will take care of cache attributes while kmap will not.  So ioremap is
> used on IA64, while kmap is used on x86.

My point is that the *user* of ioremap() shouldn't need to care what
architecture we're on.  For example, maybe the ioremap()
implementation could be changed so that it uses kmap() internally when
necessary.

>> I looked at the ia64 ioremap(), and I can't see the reason it fails
>> for RAM.  Huang, do you remember the details from 76da3fb3575?

This question is still open.  Do you remember anything about it?

Bjorn

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Huang Ying]

On Sun, 2012-01-29 at 13:41 -0700, Bjorn Helgaas wrote:
> On Sat, Jan 28, 2012 at 5:58 PM, Huang Ying <ying.huang@intel.com> wrote:
> > Hi, Bjorn,
> >
> > Sorry for late.  Just return from Chinese new year holiday.
> >
> > On Sat, 2012-01-21 at 08:04 -0700, Bjorn Helgaas wrote:
> > [snip]
> >> > +
> >> > +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
> >> > +{
> >> > +       unsigned long pfn;
> >> > +
> >> > +       pfn = pg_off >> PAGE_SHIFT;
> >> > +       if (should_use_kmap(pfn)) {
> >> > +               if (pg_sz > PAGE_SIZE)
> >> > +                       return NULL;
> >> > +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
> >> > +       } else
> >> > +               return acpi_os_ioremap(pg_off, pg_sz);
> >>
> >> This implies that ioremap() works differently on ia64 than on x86.
> >> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
> >> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
> >> same as on x86?
> >
> > If my understanding were correct, ioremap can not work for RAM on x86.
> > So we need to use kmap for RAM.  And on IA64, ioremap works for RAM and
> > will take care of cache attributes while kmap will not.  So ioremap is
> > used on IA64, while kmap is used on x86.
> 
> My point is that the *user* of ioremap() shouldn't need to care what
> architecture we're on.  For example, maybe the ioremap()
> implementation could be changed so that it uses kmap() internally when
> necessary.

I think that is about the semantics of ioremap().

Hi, Ingo,

Can you describe why normal RAM is not allowed to be mapped by
ioremap()?

> >> I looked at the ia64 ioremap(), and I can't see the reason it fails
> >> for RAM.  Huang, do you remember the details from 76da3fb3575?
> 
> This question is still open.  Do you remember anything about it?

Copy from Tony's mail,

This (Ying: use kmap insted of ioremap to map RAM) might be a problem on
ia64 - it is s/w responsibility to make sure that we don't map the same
underlying physical address using different cache attributes - e.g. we
must not map memory both cacheable and uncacheable at the same time.
Accessing such a mis-attributed page will result in a machine check.

So I'd worry that if the memory in question was being used as
uncacheable, this code might result in a cached access, which would
crash the machine.

Best Regards,
Huang Ying

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Huang Ying]

Hi,

On Sun, 2012-01-29 at 13:41 -0700, Bjorn Helgaas wrote:
> On Sat, Jan 28, 2012 at 5:58 PM, Huang Ying <ying.huang@intel.com> wrote:
> > Hi, Bjorn,
> >
> > Sorry for late.  Just return from Chinese new year holiday.
> >
> > On Sat, 2012-01-21 at 08:04 -0700, Bjorn Helgaas wrote:
> > [snip]
> >> > +
> >> > +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
> >> > +{
> >> > +       unsigned long pfn;
> >> > +
> >> > +       pfn = pg_off >> PAGE_SHIFT;
> >> > +       if (should_use_kmap(pfn)) {
> >> > +               if (pg_sz > PAGE_SIZE)
> >> > +                       return NULL;
> >> > +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
> >> > +       } else
> >> > +               return acpi_os_ioremap(pg_off, pg_sz);
> >>
> >> This implies that ioremap() works differently on ia64 than on x86.
> >> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
> >> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
> >> same as on x86?
> >
> > If my understanding were correct, ioremap can not work for RAM on x86.
> > So we need to use kmap for RAM.  And on IA64, ioremap works for RAM and
> > will take care of cache attributes while kmap will not.  So ioremap is
> > used on IA64, while kmap is used on x86.
> 
> My point is that the *user* of ioremap() shouldn't need to care what
> architecture we're on.  For example, maybe the ioremap()
> implementation could be changed so that it uses kmap() internally when
> necessary.
> 
> >> I looked at the ia64 ioremap(), and I can't see the reason it fails
> >> for RAM.  Huang, do you remember the details from 76da3fb3575?
> 
> This question is still open.  Do you remember anything about it?

Another question about kmap on IA64.  If my understanding were correct,
kmap() is just page_address() on IA64.  So it is just uses the identity
map instead of creating a new map.  Will there be any problem with it
compared with ioremap()?

Best Regards,
Huang Ying

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Huang Ying]

Hi, Thomas,

Sorry for late.  Just return from Chinese new year holiday.

On Mon, 2012-01-23 at 16:51 +0100, Thomas Renninger wrote:
> Hi,
> 
> Please ignore my previous mail, my mailer wrongly formatted
> it with html tags and it got filtered out from the lists.
> 
> Firstly: This has to get in quickly if it shouldn't miss
> 3.3 (again). Tiny adjustings (I don't see any, beside the
> issue Bjorn brought up) can still be done later, but I
> guess this rather big one gets rejected by Linus after
> the merge window closed.
> 
> Here again:
> 
> On Saturday, January 21, 2012 09:38:27 PM Bjorn Helgaas wrote:
> > +cc Tony, linux-ia64
> > 
> ...
> > >>  {
> > >> @@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
> > >> +       virt = acpi_map(pg_off, pg_sz);
> Ok, that worked before, also on IA64.
> That means that typically/normally/always io mem is tried to be mapped.
> 
> It's due to the new APEI infrastructure/features that ram could
> get mapped via acpi_os_map_memory.
> 
> Is there any Itanium out there implementing any APEI table?
> Will there ever be one?
> Even if, isn't it a BIOS bug if such stuff is declared in ram and not
> in reserved memory (also on X86)?
> Especially on an Enterprise Itanium platform,
> I guess the vendor should or better has to fix it up.

The reason for this patch can be found in description of my original
patch: 

    ACPI, Add RAM mapping support to ACPI atomic IO support
    
    On one of our testing machine, the following EINJ command lines:
    
      # echo 0x10000000 > param1
      # echo 0xfffffffffffff000 > param2
      # echo 0x8 > error_type
      # echo 1 > error_inject
    
    Will get:
    
      echo: write error: Input/output error
    
    The EIO comes from:
    
        rc = apei_exec_pre_map_gars(&trigger_ctx);
    
    The root cause is as follow.  Normally, ACPI atomic IO support is used
    to access IO memory.  But in EINJ of that machine, it is used to
    access RAM to trigger the injected error.  And the ioremap() called by
    apei_exec_pre_map_gars() can not map the RAM.
    
    This patch add RAM mapping support to ACPI atomic IO support to
    satisfy EINJ requirement.


I think that is reasonable to access the injected address to trigger
some memory error.  So it is hard to say this is a BIOS bug.

Best Regards,
Huang Ying

RE: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Luck, Tony]

> Is there any Itanium out there implementing any APEI table?
> Will there ever be one?

With respect to the EINJ table - the answer is definitely no.
Itanium has a PAL mechanism for injecting errors, so there is
no need to implement EINJ.

-Tony

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Myron Stowe]

On Mon, 2012-01-23 at 16:51 +0100, Thomas Renninger wrote:
> Hi,
> 
> Please ignore my previous mail, my mailer wrongly formatted
> it with html tags and it got filtered out from the lists.
> 
> Firstly: This has to get in quickly if it shouldn't miss
> 3.3 (again). Tiny adjustings (I don't see any, beside the
> issue Bjorn brought up) can still be done later, but I
> guess this rather big one gets rejected by Linus after
> the merge window closed.

Yes, I believe the obsolescence of atomicio.c from having taken the
conversion patch (patch 3/4 of the series) was inadvertent.  As such I
was trying to help by producing a series that addresses the two recent
additions to atomicio.c that needed ported over to osl.c so that we
would not loose such functionality with atomicio's obsolence as fast as
possible.  Linus may not allow such now that the 3.3 merge window is
closed but I think we all are thinking we should at least attempt to get
the regressions resolved immediately if possible.
> 
> Here again:
> 
> On Saturday, January 21, 2012 09:38:27 PM Bjorn Helgaas wrote:
> > +cc Tony, linux-ia64
> > 
> ...
> > >>  {
> > >> @@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
> > >> +       virt = acpi_map(pg_off, pg_sz);
> Ok, that worked before, also on IA64.
> That means that typically/normally/always io mem is tried to be mapped.
> 
> It's due to the new APEI infrastructure/features that ram could
> get mapped via acpi_os_map_memory.
> 
> Is there any Itanium out there implementing any APEI table?
> Will there ever be one?
> Even if, isn't it a BIOS bug if such stuff is declared in ram and not
> in reserved memory (also on X86)?
> Especially on an Enterprise Itanium platform,
> I guess the vendor should or better has to fix it up.
> 
> I'd do:
> can_use_ioremap(pfn) instead of should_use_kmap(pfn)
> and let it return false in ram + ia64 case, something like:
> #ifdef IA64
> #define can_use_ioremap(pfn)   !page_is_ram(pfn)
> #endif
> 
> Pass the error upwards and APEI should get disabled on IA64,
> if any ACPI code tries to ioremap real memory early (when
> the APEI table parsing happens).
> A nice FW_BUG message could be added as well (also on X86?).
> 
> Don't forget to use:
> iounmap(vaddr);
> only in acpi_unmap() then.

I must admit, while I understand the reason and concept related to
handling RAM for APEI, I do not understand memory management well enough
understand the details of the original implementation.  I was just
trying to cross-port the functionality over.  In doing such, I too, was
concerned as to the asymmetric nature of the paths (mapping vs.
unmapping) with respect to x86 as it just *felt* wrong.

Len - please let me know if you want me to adjust the patch with Thomas'
suggestions.

> 
> FWIW I even grepped for APEI tables on the most recent IA
> machine we have -> no APEI tables.
> 
> If the rest is functionally the same as the patch series
> you've send some months ago, feel free to add:
> 
> Reviewed-by: Thomas Renninger <trenn@suse.de>
> 
> I had a rather close look at those patches.

Thanks Thomas - yes, the removal patch - 3/3 - is the same.

Myron
> 
>    Thomas

Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Thomas Renninger]

Hi,

Please ignore my previous mail, my mailer wrongly formatted
it with html tags and it got filtered out from the lists.

Firstly: This has to get in quickly if it shouldn't miss
3.3 (again). Tiny adjustings (I don't see any, beside the
issue Bjorn brought up) can still be done later, but I
guess this rather big one gets rejected by Linus after
the merge window closed.

Here again:

On Saturday, January 21, 2012 09:38:27 PM Bjorn Helgaas wrote:
> +cc Tony, linux-ia64
> 
...
> >>  {
> >> @@ -353,7 +385,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 = acpi_os_ioremap(pg_off, pg_sz);
> >> +       virt = acpi_map(pg_off, pg_sz);
Ok, that worked before, also on IA64.
That means that typically/normally/always io mem is tried to be mapped.

It's due to the new APEI infrastructure/features that ram could
get mapped via acpi_os_map_memory.

Is there any Itanium out there implementing any APEI table?
Will there ever be one?
Even if, isn't it a BIOS bug if such stuff is declared in ram and not
in reserved memory (also on X86)?
Especially on an Enterprise Itanium platform,
I guess the vendor should or better has to fix it up.

I'd do:
can_use_ioremap(pfn) instead of should_use_kmap(pfn)
and let it return false in ram + ia64 case, something like:
#ifdef IA64
#define can_use_ioremap(pfn)   !page_is_ram(pfn)
#endif

Pass the error upwards and APEI should get disabled on IA64,
if any ACPI code tries to ioremap real memory early (when
the APEI table parsing happens).
A nice FW_BUG message could be added as well (also on X86?).

Don't forget to use:
iounmap(vaddr);
only in acpi_unmap() then.

FWIW I even grepped for APEI tables on the most recent IA
machine we have -> no APEI tables.

If the rest is functionally the same as the patch series
you've send some months ago, feel free to add:

Reviewed-by: Thomas Renninger <trenn@suse.de>

I had a rather close look at those patches.

   Thomas