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

From: Myron Stowe <[email protected]>

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 <[email protected]>

---

 drivers/acpi/osl.c |   36 ++++++++++++++++++++++++++++++++++--

 1 files changed, 34 insertions(+), 2 deletions(-)

drivers/acpi/osl.c +9 -0

index f363a55..8ee64ea 100644
@@ -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)

drivers/acpi/osl.c +2 -0

+}
+

 void __iomem *__init_refok acpi_os_map_memory(acpi_physical_address phys, acpi_size size) {

drivers/acpi/osl.c +1 -1

@@ -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);

Quoted 39 lines · Myron Stowe

From: Myron Stowe <[email protected]>

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 <[email protected]>

---

 drivers/acpi/osl.c |   36 ++++++++++++++++++++++++++++++++++--

 1 files changed, 34 insertions(+), 2 deletions(-)

drivers/acpi/osl.c +9 -0

index f363a55..8ee64ea 100644
@@ -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)

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 342

Quoted 10 lines

drivers/acpi/osl.c +10 -0

+{
+       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?

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 353

Quoted 11 lines

drivers/acpi/osl.c +11 -0

+}
+
+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().

Quoted 24 lines

drivers/acpi/osl.c +2 -0

+}
+

 void __iomem *__init_refok acpi_os_map_memory(acpi_physical_address phys, acpi_size size) {

drivers/acpi/osl.c +2 -2

@@ -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);
        }
  }

Quoted 39 lines · Myron Stowe

From: Myron Stowe <[email protected]>

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 <[email protected]>

---

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

drivers/acpi/osl.c +9 -0

index f363a55..8ee64ea 100644
@@ -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)

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 342

Quoted 10 lines

drivers/acpi/osl.c +10 -0

+{
+       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?

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 353

Quoted 11 lines

drivers/acpi/osl.c +11 -0

+}
+
+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().

Quoted 25 lines

drivers/acpi/osl.c +4 -2

+}
+
 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);
       }
 }

Quoted 9 lines

drivers/acpi/osl.c +9 -0

+
+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;

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 342

Quoted 3 lines

drivers/acpi/osl.c +3 -0

+               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?

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 353

Quoted 11 lines

drivers/acpi/osl.c +11 -0

+}
+
+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().

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]

Quoted 9 lines

Quoted 9 lines

drivers/acpi/osl.c +9 -0

+
+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;

Quoted 8 lines

Quoted 3 lines

+               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.

Quoted 2 lines

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?

Quoted 24 lines · 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]

Quoted 9 lines

Quoted 9 lines

drivers/acpi/osl.c +9 -0

+
+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;

Quoted 8 lines

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 342

Quoted 3 lines

drivers/acpi/osl.c +3 -0

+               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?

Quoted 4 lines

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.

Quoted 2 lines

Quoted 2 lines

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?

Quoted 24 lines · 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]

Quoted 9 lines

Quoted 9 lines

drivers/acpi/osl.c +9 -0

+
+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;

Quoted 8 lines

drivers/acpi/osl.c acpi_map_lookup_virt(void __iomem *virt, acpi_size size) · line 342

Quoted 3 lines

drivers/acpi/osl.c +3 -0

+               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?

Quoted 4 lines

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.

Quoted 2 lines

Quoted 2 lines

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?

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