Re: [RFC 1/2] ACPI, APEI, EINJ, Fix resource conflict on some machine

Chen Gong
2011-09-08T02:18:07Z

于 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