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

November 07th, 2011 - 06:30 pm ET by Myron Stowe | Report spam
Late last year I 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:
Huang Ying's commit 15651291a2f8c11e7e6a42d8bfde7a213ff13262,
My series: http://marc.info/?l=linux-acpi&m8769263327206&w=2.

While the impetus of the series was to resolve
https://bugzilla.kernel.org/show_bug.cgi?id012, an underlying goal
of the effort was to re-factor ./drivers/acpi/atomicio.c into
./drivers/acpi/osl.c, providing equivalent functinality but in a more
generalized manner, to allow usage in non-specific (i.e. APEI) contexts
and remove atomicio.c.

At that point in time, there was controversy as to the approach that
was being taken concerning APEI. Due the controversy, all of the
series' re-factoring efforts were taken in except for patch 7/7 which
took the last step of converting over to the new ACPI generic register
capabilities and removing ./drivers/acpi/atomicio.[ch]. As a result,
./drivers/acpi/atomicio.c still exists leaving us with two sets of code
providing similar functionality.

Shortly after the new ACPI generic register capabilities were accepted
upstream Rafael Wysocki submitted a series of subsequent patches which
resolved a couple of bugs and provided a number of optimizations,
especially with respect locking, to the new capabilities. As mentioned
in the original re-factoring series' preamble ([PATCH 0/7]), some of its
implementation was based upon Huang Ying's original work - specifically
patches 5/7 and 6/7 - and so at least some of the issues that Rafael
addressed in the new capabilities remain unresolved within atomicio.c [*].

This patch series addresses both of these issues - the duplication of
code and likely latent issues that remain within atomicio.c - by
reintroducing patch 7/7 from last year: "Re-factor and remove
./drivers/acpi/atomicio.[ch]".

-v2:
- Prepended "ACPI: Fix possible alignment issues with GAS 'address'
references" patch to the series to account for possible alignment
issues.
- Split atomicio.[ch] removal patch into two separate patches
"ACPI APEI: Convert atomicio routines", and
"ACPI: Remove ./drivers/acpi/atomicio.[ch]"
to distinguish the more interesting conversion aspects from the
removal.
- As part of the conversion patch, I introduced apei_read() and
apei_write() which currently are functional equivalents of
acpi_read() and acpi_write(). This is mainly proactive, to
prevent APEI breakages if acpi_read() and acpi_write() are ever
augmented to support the 'bit_offset' field of GAS, as APEI's
__apei_exec_write_register() precludes splitting up functionality
related to 'bit_offset' and APEI's 'mask' (see its
APEI_EXEC_PRESERVE_REGISTER block).


[*] Bug fixes and optimizations that Len and Rafael subsequently provided
to the new ACPI generic register capabilities:
6d5bbf00.. ACPI: Use ioremap_cache()
2d6d9fd3.. ACPI: Introduce acpi_os_ioremap()
884b821f.. ACPI: Fix acpi_os_read_memory() and acpi_os_write_memory()
7bbb8903.. ACPI: Change acpi_ioremap_lock into a mutex
7fe135dc.. ACPI: Avoid walking the list of memory mappings in osl.c twice
7ffd0443.. ACPI: Make acpi_os_map_memory() avoid unnecessary mappings
b7c1fadd.. ACPI: Do not use krefs under a mutex in osl.c


Myron Stowe (4):
ACPI: Remove ./drivers/acpi/atomicio.[ch]
ACPI APEI: Convert atomicio routines
ACPI: Export interfaces for ioremapping/iounmapping ACPI registers
ACPI: Fix possible alignment issues with GAS 'address' references


drivers/acpi/Makefile | 1
drivers/acpi/apei/apei-base.c | 102 ++++++++++
drivers/acpi/apei/apei-internal.h | 3
drivers/acpi/apei/ghes.c | 10 +
drivers/acpi/atomicio.c | 365 -
drivers/acpi/osl.c | 40 +++-
include/acpi/atomicio.h | 10 -
include/linux/acpi_io.h | 3
8 files changed, 133 insertions(+), 401 deletions(-)
delete mode 100644 drivers/acpi/atomicio.c
delete mode 100644 include/acpi/atomicio.h

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

Replies

#1 Myron Stowe
November 07th, 2011 - 06:30 pm ET | Report spam
From: Myron Stowe

With the conversion of atomicio routines in place, atomicio.[ch] can be
removed, replacing the APEI specific pre-mapping capabilities with the
more generalized versions that commits 620242ae8c3 and 29718521237
provide.

Signed-off-by: Myron Stowe


drivers/acpi/Makefile | 1
drivers/acpi/atomicio.c | 365 --
include/acpi/atomicio.h | 10 -
3 files changed, 0 insertions(+), 376 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 ecb26b4..e7c1d0f 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

# sleep related files
acpi-y += wakeup.o
diff --git a/drivers/acpi/atomicio.c b/drivers/acpi/atomicio.c
deleted file mode 100644
index 7489b89..0000000
a/drivers/acpi/atomicio.c
+++ /dev/null
@@ -1,365 +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
- *
- * 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/module.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 <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);
- 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;
-}
-
-/*
- * 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 = ioremap(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);
- iounmap(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:
- iounmap(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();
- iounmap(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]",
- *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]",
- *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]",
- *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);
-
-/*
- * 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;
-#ifdef readq
- case 64:
- *val = readq(addr);
- break;
-#endif
- default:
- return -EINVAL;
- }
- rcu_read_unlock();
-
- return 0;
-}
-
-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;
-#ifdef writeq
- case 64:
- writeq(val, addr);
- break;
-#endif
- 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

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

Similar topics