* [RFC 0/2] ACPI, APEI, EINJ, Fix RAM accessing @ 2011-08-30 6:28 Huang Ying 2011-08-30 6:28 ` [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine Huang Ying 2011-08-30 6:28 ` [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support Huang Ying 0 siblings, 2 replies; 7+ messages in thread From: Huang Ying @ 2011-08-30 6:28 UTC (permalink / raw) To: Len Brown; +Cc: linux-kernel, Andi Kleen, Tony Luck, ying.huang, linux-acpi Some APEI firmware implementaiton will access injected address specified in param1 to trigger the error when injecting memory error. This may cause resource conflict with RAM and RAM accessing issue in ACPI atomic IO support. This patchset fixes this. [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support Best Regards, Huang Ying ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine 2011-08-30 6:28 [RFC 0/2] ACPI, APEI, EINJ, Fix RAM accessing Huang Ying @ 2011-08-30 6:28 ` Huang Ying 2011-08-30 16:40 ` Luck, Tony 2011-09-08 2:18 ` Chen Gong 2011-08-30 6:28 ` [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support Huang Ying 1 sibling, 2 replies; 7+ messages in thread From: Huang Ying @ 2011-08-30 6:28 UTC (permalink / raw) To: Len Brown; +Cc: linux-kernel, Andi Kleen, Tony Luck, ying.huang, linux-acpi Some APEI firmware implementaiton will access injected address specified in param1 to trigger the error when injecting memory error. This will cause resource conflict with RAM. So remove it from trigger table resources to avoid conflict. Signed-off-by: Huang Ying <ying.huang@intel.com> Cc: Tony Luck <tony.luck@intel.com> --- drivers/acpi/apei/apei-base.c | 11 +++++++++++ drivers/acpi/apei/apei-internal.h | 3 +++ drivers/acpi/apei/einj.c | 24 ++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c index ac11aff..35a7a3a 100644 --- a/drivers/acpi/apei/apei-base.c +++ b/drivers/acpi/apei/apei-base.c @@ -421,6 +421,17 @@ static int apei_resources_merge(struct apei_resources *resources1, return 0; } +int apei_resources_add(struct apei_resources *resources, + unsigned long start, unsigned long size, + bool iomem) +{ + if (iomem) + return apei_res_add(&resources->iomem, start, size); + else + return apei_res_add(&resources->ioport, start, size); +} +EXPORT_SYMBOL_GPL(apei_resources_add); + /* * EINJ has two groups of GARs (EINJ table entry and trigger table * entry), so common resources are subtracted from the trigger table diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h index f57050e..d778edd 100644 --- a/drivers/acpi/apei/apei-internal.h +++ b/drivers/acpi/apei/apei-internal.h @@ -95,6 +95,9 @@ static inline void apei_resources_init(struct apei_resources *resources) } void apei_resources_fini(struct apei_resources *resources); +int apei_resources_add(struct apei_resources *resources, + unsigned long start, unsigned long size, + bool iomem); int apei_resources_sub(struct apei_resources *resources1, struct apei_resources *resources2); int apei_resources_request(struct apei_resources *resources, diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c index 3178521..a126c67 100644 --- a/drivers/acpi/apei/einj.c +++ b/drivers/acpi/apei/einj.c @@ -195,7 +195,8 @@ static int einj_check_trigger_header(struct acpi_einj_trigger *trigger_tab) } /* Execute instructions in trigger error action table */ -static int __einj_error_trigger(u64 trigger_paddr) +static int __einj_error_trigger(u64 trigger_paddr, u32 type, + u64 param1, u64 param2) { struct acpi_einj_trigger *trigger_tab = NULL; struct apei_exec_context trigger_ctx; @@ -255,6 +256,25 @@ static int __einj_error_trigger(u64 trigger_paddr) rc = apei_resources_sub(&trigger_resources, &einj_resources); if (rc) goto out_fini; + /* + * Some firmware will access target address specified in + * param1 to trigger the error when injecting memory error. + * This will cause resource conflict with regular memory. So + * remove it from trigger table resources. + */ + if (param_extension && (type & 0x0038) && param2) { + struct apei_resources addr_resources; + apei_resources_init(&addr_resources); + rc = apei_resources_add(&addr_resources, + param1 & param2, + ~param2 + 1, true); + if (rc) + goto out_fini; + rc = apei_resources_sub(&trigger_resources, &addr_resources); + apei_resources_fini(&addr_resources); + if (rc) + goto out_fini; + } rc = apei_resources_request(&trigger_resources, "APEI EINJ Trigger"); if (rc) goto out_fini; @@ -324,7 +344,7 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2) if (rc) return rc; trigger_paddr = apei_exec_ctx_get_output(&ctx); - rc = __einj_error_trigger(trigger_paddr); + rc = __einj_error_trigger(trigger_paddr, type, param1, param2); if (rc) return rc; rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine 2011-08-30 6:28 ` [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine Huang Ying @ 2011-08-30 16:40 ` Luck, Tony 2011-09-08 2:18 ` Chen Gong 1 sibling, 0 replies; 7+ messages in thread From: Luck, Tony @ 2011-08-30 16:40 UTC (permalink / raw) To: Huang, Ying, Len Brown; +Cc: linux-kernel, Andi Kleen, linux-acpi >Some APEI firmware implementaiton will access injected address >specified in param1 to trigger the error when injecting memory error. >This will cause resource conflict with RAM. So remove it from trigger >table resources to avoid conflict. Tested-by: Tony Luck <tony.luck@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine 2011-08-30 6:28 ` [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine Huang Ying @ 2011-09-08 2:18 ` Chen Gong 2011-09-08 2:18 ` Chen Gong 1 sibling, 0 replies; 7+ messages in thread From: Chen Gong @ 2011-09-08 2:18 UTC (permalink / raw) To: Huang Ying; +Cc: Len Brown, linux-kernel, Andi Kleen, Tony Luck, linux-acpi 于 2011/8/30 14:28, Huang Ying 写道: > Some APEI firmware implementaiton will access injected address > specified in param1 to trigger the error when injecting memory error. > This will cause resource conflict with RAM. So remove it from trigger > table resources to avoid conflict. > > Signed-off-by: Huang Ying<ying.huang@intel.com> > Cc: Tony Luck<tony.luck@intel.com> > --- > drivers/acpi/apei/apei-base.c | 11 +++++++++++ > drivers/acpi/apei/apei-internal.h | 3 +++ > drivers/acpi/apei/einj.c | 24 ++++++++++++++++++++++-- > 3 files changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c > index ac11aff..35a7a3a 100644 > --- a/drivers/acpi/apei/apei-base.c > +++ b/drivers/acpi/apei/apei-base.c > @@ -421,6 +421,17 @@ static int apei_resources_merge(struct apei_resources *resources1, > return 0; > } > > +int apei_resources_add(struct apei_resources *resources, > + unsigned long start, unsigned long size, > + bool iomem) > +{ > + if (iomem) > + return apei_res_add(&resources->iomem, start, size); > + else > + return apei_res_add(&resources->ioport, start, size); > +} > +EXPORT_SYMBOL_GPL(apei_resources_add); > + > /* > * EINJ has two groups of GARs (EINJ table entry and trigger table > * entry), so common resources are subtracted from the trigger table > diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h > index f57050e..d778edd 100644 > --- a/drivers/acpi/apei/apei-internal.h > +++ b/drivers/acpi/apei/apei-internal.h > @@ -95,6 +95,9 @@ static inline void apei_resources_init(struct apei_resources *resources) > } > > void apei_resources_fini(struct apei_resources *resources); > +int apei_resources_add(struct apei_resources *resources, > + unsigned long start, unsigned long size, > + bool iomem); > int apei_resources_sub(struct apei_resources *resources1, > struct apei_resources *resources2); > int apei_resources_request(struct apei_resources *resources, > diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c > index 3178521..a126c67 100644 > --- a/drivers/acpi/apei/einj.c > +++ b/drivers/acpi/apei/einj.c > @@ -195,7 +195,8 @@ static int einj_check_trigger_header(struct acpi_einj_trigger *trigger_tab) > } > > /* Execute instructions in trigger error action table */ > -static int __einj_error_trigger(u64 trigger_paddr) > +static int __einj_error_trigger(u64 trigger_paddr, u32 type, > + u64 param1, u64 param2) > { > struct acpi_einj_trigger *trigger_tab = NULL; > struct apei_exec_context trigger_ctx; > @@ -255,6 +256,25 @@ static int __einj_error_trigger(u64 trigger_paddr) > rc = apei_resources_sub(&trigger_resources,&einj_resources); > if (rc) > goto out_fini; > + /* > + * Some firmware will access target address specified in > + * param1 to trigger the error when injecting memory error. > + * This will cause resource conflict with regular memory. So > + * remove it from trigger table resources. > + */ > + if (param_extension&& (type& 0x0038)&& param2) { > + struct apei_resources addr_resources; > + apei_resources_init(&addr_resources); > + rc = apei_resources_add(&addr_resources, > + param1& param2, > + ~param2 + 1, true); assuming following scenario: param1: 0x827809000 param2: 0xffffffffffffffff after above operation, only 1 byte will be added and be subtracted by apei_resources_sub below, which means if 8 bytes are necessary to be excluded from ioremap, finally only 1 byte is excluded, left 7 bytes still be mapped via ioremap. The same thing will happen: APEI: Can not request iomem region <00000000bf7b522a-00000000bf7b522c> for GARs We can't control the value of param2 here because 1) it is read from the user; 2) the param1 is already aligned, the value of param2 is not important under this kind of situation. We have hit above error in our tests. > + if (rc) > + goto out_fini; > + rc = apei_resources_sub(&trigger_resources,&addr_resources); > + apei_resources_fini(&addr_resources); > + if (rc) > + goto out_fini; > + } > rc = apei_resources_request(&trigger_resources, "APEI EINJ Trigger"); > if (rc) > goto out_fini; > @@ -324,7 +344,7 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2) > if (rc) > return rc; > trigger_paddr = apei_exec_ctx_get_output(&ctx); > - rc = __einj_error_trigger(trigger_paddr); > + rc = __einj_error_trigger(trigger_paddr, type, param1, param2); > if (rc) > return rc; > rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION); -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine @ 2011-09-08 2:18 ` Chen Gong 0 siblings, 0 replies; 7+ messages in thread From: Chen Gong @ 2011-09-08 2:18 UTC (permalink / raw) To: Huang Ying; +Cc: Len Brown, linux-kernel, Andi Kleen, Tony Luck, linux-acpi 于 2011/8/30 14:28, Huang Ying 写道: > Some APEI firmware implementaiton will access injected address > specified in param1 to trigger the error when injecting memory error. > This will cause resource conflict with RAM. So remove it from trigger > table resources to avoid conflict. > > Signed-off-by: Huang Ying<ying.huang@intel.com> > Cc: Tony Luck<tony.luck@intel.com> > --- > drivers/acpi/apei/apei-base.c | 11 +++++++++++ > drivers/acpi/apei/apei-internal.h | 3 +++ > drivers/acpi/apei/einj.c | 24 ++++++++++++++++++++++-- > 3 files changed, 36 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c > index ac11aff..35a7a3a 100644 > --- a/drivers/acpi/apei/apei-base.c > +++ b/drivers/acpi/apei/apei-base.c > @@ -421,6 +421,17 @@ static int apei_resources_merge(struct apei_resources *resources1, > return 0; > } > > +int apei_resources_add(struct apei_resources *resources, > + unsigned long start, unsigned long size, > + bool iomem) > +{ > + if (iomem) > + return apei_res_add(&resources->iomem, start, size); > + else > + return apei_res_add(&resources->ioport, start, size); > +} > +EXPORT_SYMBOL_GPL(apei_resources_add); > + > /* > * EINJ has two groups of GARs (EINJ table entry and trigger table > * entry), so common resources are subtracted from the trigger table > diff --git a/drivers/acpi/apei/apei-internal.h b/drivers/acpi/apei/apei-internal.h > index f57050e..d778edd 100644 > --- a/drivers/acpi/apei/apei-internal.h > +++ b/drivers/acpi/apei/apei-internal.h > @@ -95,6 +95,9 @@ static inline void apei_resources_init(struct apei_resources *resources) > } > > void apei_resources_fini(struct apei_resources *resources); > +int apei_resources_add(struct apei_resources *resources, > + unsigned long start, unsigned long size, > + bool iomem); > int apei_resources_sub(struct apei_resources *resources1, > struct apei_resources *resources2); > int apei_resources_request(struct apei_resources *resources, > diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c > index 3178521..a126c67 100644 > --- a/drivers/acpi/apei/einj.c > +++ b/drivers/acpi/apei/einj.c > @@ -195,7 +195,8 @@ static int einj_check_trigger_header(struct acpi_einj_trigger *trigger_tab) > } > > /* Execute instructions in trigger error action table */ > -static int __einj_error_trigger(u64 trigger_paddr) > +static int __einj_error_trigger(u64 trigger_paddr, u32 type, > + u64 param1, u64 param2) > { > struct acpi_einj_trigger *trigger_tab = NULL; > struct apei_exec_context trigger_ctx; > @@ -255,6 +256,25 @@ static int __einj_error_trigger(u64 trigger_paddr) > rc = apei_resources_sub(&trigger_resources,&einj_resources); > if (rc) > goto out_fini; > + /* > + * Some firmware will access target address specified in > + * param1 to trigger the error when injecting memory error. > + * This will cause resource conflict with regular memory. So > + * remove it from trigger table resources. > + */ > + if (param_extension&& (type& 0x0038)&& param2) { > + struct apei_resources addr_resources; > + apei_resources_init(&addr_resources); > + rc = apei_resources_add(&addr_resources, > + param1& param2, > + ~param2 + 1, true); assuming following scenario: param1: 0x827809000 param2: 0xffffffffffffffff after above operation, only 1 byte will be added and be subtracted by apei_resources_sub below, which means if 8 bytes are necessary to be excluded from ioremap, finally only 1 byte is excluded, left 7 bytes still be mapped via ioremap. The same thing will happen: APEI: Can not request iomem region <00000000bf7b522a-00000000bf7b522c> for GARs We can't control the value of param2 here because 1) it is read from the user; 2) the param1 is already aligned, the value of param2 is not important under this kind of situation. We have hit above error in our tests. > + if (rc) > + goto out_fini; > + rc = apei_resources_sub(&trigger_resources,&addr_resources); > + apei_resources_fini(&addr_resources); > + if (rc) > + goto out_fini; > + } > rc = apei_resources_request(&trigger_resources, "APEI EINJ Trigger"); > if (rc) > goto out_fini; > @@ -324,7 +344,7 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2) > if (rc) > return rc; > trigger_paddr = apei_exec_ctx_get_output(&ctx); > - rc = __einj_error_trigger(trigger_paddr); > + rc = __einj_error_trigger(trigger_paddr, type, param1, param2); > if (rc) > return rc; > rc = apei_exec_run_optional(&ctx, ACPI_EINJ_END_OPERATION); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support 2011-08-30 6:28 [RFC 0/2] ACPI, APEI, EINJ, Fix RAM accessing Huang Ying 2011-08-30 6:28 ` [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine Huang Ying @ 2011-08-30 6:28 ` Huang Ying 2011-08-30 16:41 ` Luck, Tony 1 sibling, 1 reply; 7+ messages in thread From: Huang Ying @ 2011-08-30 6:28 UTC (permalink / raw) To: Len Brown; +Cc: linux-kernel, Andi Kleen, Tony Luck, ying.huang, linux-acpi Normally, ACPI atomic IO support is used to access IO memory. But in EINJ, it may be used to access RAM to trigger the injected error. This patch add RAM mapping support to satisfy EINJ requirement. Signed-off-by: Huang Ying <ying.huang@intel.com> Cc: Tony Luck <tony.luck@intel.com> --- drivers/acpi/atomicio.c | 34 ++++++++++++++++++++++++++++++---- 1 files changed, 30 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/atomicio.c b/drivers/acpi/atomicio.c index 7489b89..edf2f11 100644 --- a/drivers/acpi/atomicio.c +++ b/drivers/acpi/atomicio.c @@ -32,6 +32,8 @@ #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: " @@ -97,6 +99,30 @@ static void __iomem *__acpi_try_ioremap(phys_addr_t paddr, return NULL; } +static void __iomem *acpi_map(phys_addr_t pg_off, unsigned long pg_sz) +{ + unsigned long pfn; + + pfn = pg_off >> PAGE_SHIFT; + if (page_is_ram(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 @@ -119,7 +145,7 @@ static void __iomem *acpi_pre_map(phys_addr_t paddr, pg_off = paddr & PAGE_MASK; pg_sz = ((paddr + size + PAGE_SIZE - 1) & PAGE_MASK) - pg_off; - vaddr = ioremap(pg_off, pg_sz); + vaddr = acpi_map(pg_off, pg_sz); if (!vaddr) return NULL; map = kmalloc(sizeof(*map), GFP_KERNEL); @@ -135,7 +161,7 @@ static void __iomem *acpi_pre_map(phys_addr_t paddr, vaddr = __acpi_try_ioremap(paddr, size); if (vaddr) { spin_unlock_irqrestore(&acpi_iomaps_lock, flags); - iounmap(map->vaddr); + acpi_unmap(pg_off, map->vaddr); kfree(map); return vaddr; } @@ -144,7 +170,7 @@ static void __iomem *acpi_pre_map(phys_addr_t paddr, return map->vaddr + (paddr - map->paddr); err_unmap: - iounmap(vaddr); + acpi_unmap(pg_off, vaddr); return NULL; } @@ -177,7 +203,7 @@ static void acpi_post_unmap(phys_addr_t paddr, unsigned long size) return; synchronize_rcu(); - iounmap(map->vaddr); + acpi_unmap(map->paddr, map->vaddr); kfree(map); } -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support 2011-08-30 6:28 ` [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support Huang Ying @ 2011-08-30 16:41 ` Luck, Tony 0 siblings, 0 replies; 7+ messages in thread From: Luck, Tony @ 2011-08-30 16:41 UTC (permalink / raw) To: Huang, Ying, Len Brown; +Cc: linux-kernel, Andi Kleen, linux-acpi > Normally, ACPI atomic IO support is used to access IO memory. But in > EINJ, it may be used to access RAM to trigger the injected error. > This patch add RAM mapping support to satisfy EINJ requirement. Tested-by: Tony Luck <tony.luck@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-09-08 2:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-08-30 6:28 [RFC 0/2] ACPI, APEI, EINJ, Fix RAM accessing Huang Ying 2011-08-30 6:28 ` [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine Huang Ying 2011-08-30 16:40 ` Luck, Tony 2011-09-08 2:18 ` Chen Gong 2011-09-08 2:18 ` Chen Gong 2011-08-30 6:28 ` [RFC 2/2] ACPI, Add RAM mapping support to ACPI atomic IO support Huang Ying 2011-08-30 16:41 ` Luck, Tony
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.