On Saturday 05 November 2011 00:54:39 Myron Stowe wrote:
...
> > Still, if Windows has duplicated code for APEI GAR handling (with
> > additional mask value, for example ignoring bit width) and does it
> > slightly different than they do it in other parts,
> > we also might not come around APEI specific GAR checking/workarounds.
> >
>
> I've hit a snag - hopefully I'm just not seeing something obvious so I thought
> I would relay what I'm encountering and see if you or Bjorn have any ideas.
I see that this is yet another nasty problem of trying to let APEI use generic
acpi_read/write funcs.
...
> Thomas - this issue exists with either conversion approach (my approach of
> getting acpi_read()/acpi_write() augmented to handle 'bit_offset' values and
> using those or your idea of copying acpi_atomicio_read()/acpi_atomicio_write(),
> changing their names to apei_read()/apei_write(), augmenting those to
> handle 'bit_offset' values and using them).
I can't see how it affects the two steps approach (if generic read/write could
ever get used... The main and really nice win of your patches is getting
rid of the duplicated and complicated/complex pre_map stuff).
What about below patch which even already compiles, it should work with your
atomicio.c removal one(s) (but not without!)?
Why would bit_offset handling need to be inside read/write funcs?
It could be done as before.
Thomas
---
drivers/acpi/apei/apei-base.c | 91 +++++++++++++++++++++++++++++++++++-
drivers/acpi/apei/apei-internal.h | 3 +
drivers/acpi/apei/ghes.c | 2 +-
drivers/acpi/atomicio.c | 42 -----------------
include/acpi/atomicio.h | 3 -
5 files changed, 92 insertions(+), 49 deletions(-)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 6154036..9be1a58 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -70,7 +70,7 @@ int __apei_exec_read_register(struct acpi_whea_header *entry, u64 *val)
{
int rc;
- rc = acpi_atomic_read(val, &entry->register_region);
+ rc = apei_gar_read(val, &entry->register_region);
if (rc)
return rc;
*val >>= entry->register_region.bit_offset;
@@ -116,13 +116,13 @@ int __apei_exec_write_register(struct acpi_whea_header *entry, u64 val)
val <<= entry->register_region.bit_offset;
if (entry->flags & APEI_EXEC_PRESERVE_REGISTER) {
u64 valr = 0;
- rc = acpi_atomic_read(&valr, &entry->register_region);
+ rc = apei_gar_read(&valr, &entry->register_region);
if (rc)
return rc;
valr &= ~(entry->mask << entry->register_region.bit_offset);
val |= valr;
}
- rc = acpi_atomic_write(val, &entry->register_region);
+ rc = apei_gar_write(val, &entry->register_region);
return rc;
}
@@ -553,6 +553,91 @@ static int apei_check_gar(struct acpi_generic_address *reg, u64 *paddr)
return 0;
}
+int apei_gar_read(u64 *val, struct acpi_generic_address *reg)
+{
+ u64 paddr;
+ int rc;
+ u32 tmp_val;
+ acpi_status status;
+ u32 width = reg->bit_width;
+
+ if (width == 64)
+ width = 32; /* Break into two 32-bit transfers */
+
+ rc = apei_check_gar(reg, &paddr);
+ if (rc)
+ return rc;
+
+ *val = 0;
+ switch (reg->space_id) {
+ case ACPI_ADR_SPACE_SYSTEM_MEMORY:
+ /* This part is copy and pasted from acpi_read */
+ status = acpi_os_read_memory((acpi_physical_address)
+ paddr, &tmp_val, width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ *val = tmp_val;
+
+ if (reg->bit_width == 64) {
+
+ /* Read the top 32 bits */
+
+ status = acpi_os_read_memory((acpi_physical_address)
+ (paddr + 4), &tmp_val, 32);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ *val |= ((u64)tmp_val << 32);
+ }
+ case ACPI_ADR_SPACE_SYSTEM_IO:
+ status = acpi_os_read_port(paddr, (u32 *)val, reg->bit_width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ default:
+ return -EINVAL;
+ }
+}
+EXPORT_SYMBOL_GPL(apei_gar_read);
+
+int apei_gar_write(u64 val, struct acpi_generic_address *reg)
+{
+ u64 paddr;
+ int rc;
+ acpi_status status;
+ u32 width = reg->bit_width;
+
+ if (width == 64)
+ width = 32; /* Break into two 32-bit transfers */
+
+ rc = apei_check_gar(reg, &paddr);
+ if (rc)
+ return rc;
+
+ switch (reg->space_id) {
+ case ACPI_ADR_SPACE_SYSTEM_MEMORY:
+ /* This part is copy and pasted from acpi_write */
+ status = acpi_os_write_memory((acpi_physical_address)
+ paddr, ACPI_LODWORD(val), width);
+
+ if (ACPI_FAILURE(status))
+ return -EIO;
+
+ if (reg->bit_width == 64) {
+ status = acpi_os_write_memory((acpi_physical_address)
+ (paddr + 4), ACPI_HIDWORD(val), 32);
+
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ }
+ case ACPI_ADR_SPACE_SYSTEM_IO:
+ status = acpi_os_write_port(paddr, val, reg->bit_width);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ default:
+ return -EINVAL;
+ }
+}
+EXPORT_SYMBOL_GPL(apei_gar_write);
+
static int collect_res_callback(struct apei_exec_context *ctx,
struct acpi_whea_header *entry,
void *data)
diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h
index f57050e..0380cf7 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -68,6 +68,9 @@ static inline int apei_exec_run_optional(struct apei_exec_context *ctx, u8 actio
/* IP has been set in instruction function */
#define APEI_EXEC_SET_IP 1
+int apei_gar_read(u64 *val, struct acpi_generic_address *reg);
+int apei_gar_write(u64 val, struct acpi_generic_address *reg);
+
int __apei_exec_read_register(struct acpi_whea_header *entry, u64 *val);
int __apei_exec_write_register(struct acpi_whea_header *entry, u64 val);
int apei_exec_read_register(struct apei_exec_context *ctx,
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b8e08cb..4273b2f 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -399,7 +399,7 @@ static int ghes_read_estatus(struct ghes *ghes, int silent)
u32 len;
int rc;
- rc = acpi_atomic_read(&buf_paddr, &g->error_status_address);
+ rc = apei_gar_read(&buf_paddr, &g->error_status_address);
if (rc) {
if (!silent && printk_ratelimit())
pr_warning(FW_WARN GHES_PFX
diff --git a/drivers/acpi/atomicio.c b/drivers/acpi/atomicio.c
index 7489b89..2fb3fc0 100644
--- a/drivers/acpi/atomicio.c
+++ b/drivers/acpi/atomicio.c
@@ -321,45 +321,3 @@ static int acpi_atomic_write_mem(u64 paddr, u64 val, u32 width)
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
index 8b9fb4b..e38090c 100644
--- a/include/acpi/atomicio.h
+++ b/include/acpi/atomicio.h
@@ -4,7 +4,4 @@
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-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
