Hi Tony and Gong,
Should we remove the target memory address from the resource list of
ACPI5 trigger too? Otherwise we may also encounter resource conflict with ACPI5
trigger table.
@@ -365,7 +365,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
* This will cause resource conflict with regular memory. So
* remove it from trigger table resources.
*/
- if (param_extension && (type & 0x0038) && param2) {
+ if ((param_extension || acpi5) && (type & 0x0038) && param2) {
struct apei_resources addr_resources;
apei_resources_init(&addr_resources);
trigger_param_region = einj_get_trigger_parameter_region(
On 01/18/2012 04:10 AM, Tony Luck wrote:
> @@ -293,12 +386,56 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2)
> if (rc)
> return rc;
> apei_exec_ctx_set_input(&ctx, type);
> - rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
> - if (rc)
> - return rc;
> - if (einj_param) {
> - writeq(param1, &einj_param->param1);
> - writeq(param2, &einj_param->param2);
> + if (acpi5) {
> + struct set_error_type_with_address *v5param = einj_param;
> +
> + writel(type, &v5param->type);
> + if (type & 0x80000000) {
> + switch (vendor_flags) {
> + case SETWA_FLAGS_APICID:
> + writel(param1, &v5param->apicid);
> + break;
> + case SETWA_FLAGS_MEM:
> + writeq(param1, &v5param->memory_address);
> + writeq(param2, &v5param->memory_address_range);
> + break;
> + case SETWA_FLAGS_PCIE_SBDF:
> + writel(param1, &v5param->pcie_sbdf);
> + break;
> + }
> + writel(vendor_flags, &v5param->flags);
> + } else {
> + switch (type) {
> + case ACPI_EINJ_PROCESSOR_CORRECTABLE:
> + case ACPI_EINJ_PROCESSOR_UNCORRECTABLE:
> + case ACPI_EINJ_PROCESSOR_FATAL:
> + writel(param1, &v5param->apicid);
> + writel(SETWA_FLAGS_APICID, &v5param->flags);
> + break;
> + case ACPI_EINJ_MEMORY_CORRECTABLE:
> + case ACPI_EINJ_MEMORY_UNCORRECTABLE:
> + case ACPI_EINJ_MEMORY_FATAL:
> + writeq(param1, &v5param->memory_address);
> + writeq(param2, &v5param->memory_address_range);
> + writel(SETWA_FLAGS_MEM, &v5param->flags);
> + break;
> + case ACPI_EINJ_PCIX_CORRECTABLE:
> + case ACPI_EINJ_PCIX_UNCORRECTABLE:
> + case ACPI_EINJ_PCIX_FATAL:
> + writel(param1, &v5param->pcie_sbdf);
> + writel(SETWA_FLAGS_PCIE_SBDF, &v5param->flags);
> + break;
> + }
> + }
I'm a little confused about the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command in ACPI5.
The above code seems like a hard-coded version of
apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS).
Hi Gong, could you please share the ACPI5 version of EINJ table with me? So I could
get better understanding of the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command.
> + } else {
> + rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
> + if (rc)
> + return rc;
> + if (einj_param) {
> + struct einj_parameter *v4param = einj_param;
> + writeq(param1, &v4param->param1);
> + writeq(param2, &v4param->param2);
> + }
> }
> rc = apei_exec_run(&ctx, ACPI_EINJ_EXECUTE_OPERATION);
> if (rc)
> @@ -408,15 +545,25 @@ static int error_type_set(void *data, u64 val)
> {
> int rc;
> u32 available_error_type = 0;
> + u32 tval, vendor;
> +
> + /*
> + * Vendor defined types have 0x80000000 bit set, and
> + * are not enumerated by ACPI_EINJ_GET_ERROR_TYPE
> + */
> + vendor = val & 0x80000000;
> + tval = val & 0x7fffffff;
>
> /* Only one error type can be specified */
> - if (val & (val - 1))
> - return -EINVAL;
> - rc = einj_get_available_error_type(&available_error_type);
> - if (rc)
> - return rc;
> - if (!(val & available_error_type))
> + if (tval & (tval - 1))
The ACPI5 spec doesn't specify the format of vendor specific error type, so would it
be better to check above condition only if it's a standard error type?
> return -EINVAL;
> + if (!vendor) {
> + rc = einj_get_available_error_type(&available_error_type);
> + if (rc)
> + return rc;
> + if (!(val & available_error_type))
> + return -EINVAL;
> + }
> error_type = val;
>
> return 0;
于 2012/2/12 17:33, Jiang Liu 写道:
> Hi Tony and Gong,
> Should we remove the target memory address from the resource list of
> ACPI5 trigger too? Otherwise we may also encounter resource conflict with ACPI5
> trigger table.
>
> @@ -365,7 +365,7 @@ static int __einj_error_trigger(u64 trigger_paddr, u32 type,
> * This will cause resource conflict with regular memory. So
> * remove it from trigger table resources.
> */
> - if (param_extension&& (type& 0x0038)&& param2) {
> + if ((param_extension || acpi5)&& (type& 0x0038)&& param2) {
> struct apei_resources addr_resources;
> apei_resources_init(&addr_resources);
> trigger_param_region = einj_get_trigger_parameter_region(
>
> On 01/18/2012 04:10 AM, Tony Luck wrote:
>> @@ -293,12 +386,56 @@ static int __einj_error_inject(u32 type, u64 param1, u64 param2)
>> if (rc)
>> return rc;
>> apei_exec_ctx_set_input(&ctx, type);
>> - rc = apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE);
>> - if (rc)
>> - return rc;
>> - if (einj_param) {
>> - writeq(param1,&einj_param->param1);
>> - writeq(param2,&einj_param->param2);
>> + if (acpi5) {
>> + struct set_error_type_with_address *v5param = einj_param;
>> +
>> + writel(type,&v5param->type);
>> + if (type& 0x80000000) {
>> + switch (vendor_flags) {
>> + case SETWA_FLAGS_APICID:
>> + writel(param1,&v5param->apicid);
>> + break;
>> + case SETWA_FLAGS_MEM:
>> + writeq(param1,&v5param->memory_address);
>> + writeq(param2,&v5param->memory_address_range);
>> + break;
>> + case SETWA_FLAGS_PCIE_SBDF:
>> + writel(param1,&v5param->pcie_sbdf);
>> + break;
>> + }
>> + writel(vendor_flags,&v5param->flags);
>> + } else {
>> + switch (type) {
>> + case ACPI_EINJ_PROCESSOR_CORRECTABLE:
>> + case ACPI_EINJ_PROCESSOR_UNCORRECTABLE:
>> + case ACPI_EINJ_PROCESSOR_FATAL:
>> + writel(param1,&v5param->apicid);
>> + writel(SETWA_FLAGS_APICID,&v5param->flags);
>> + break;
>> + case ACPI_EINJ_MEMORY_CORRECTABLE:
>> + case ACPI_EINJ_MEMORY_UNCORRECTABLE:
>> + case ACPI_EINJ_MEMORY_FATAL:
>> + writeq(param1,&v5param->memory_address);
>> + writeq(param2,&v5param->memory_address_range);
>> + writel(SETWA_FLAGS_MEM,&v5param->flags);
>> + break;
>> + case ACPI_EINJ_PCIX_CORRECTABLE:
>> + case ACPI_EINJ_PCIX_UNCORRECTABLE:
>> + case ACPI_EINJ_PCIX_FATAL:
>> + writel(param1,&v5param->pcie_sbdf);
>> + writel(SETWA_FLAGS_PCIE_SBDF,&v5param->flags);
>> + break;
>> + }
>> + }
> I'm a little confused about the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command in ACPI5.
> The above code seems like a hard-coded version of
> apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS).
> Hi Gong, could you please share the ACPI5 version of EINJ table with me? So I could
> get better understanding of the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command.
>
You can check ACPI5.0 spec 18.6 for detail. We have internal BIOS to support
ACPI5 and I'm not sure if there is external product to support ACPI5 formally.
> Should we remove the target memory address from the resource list of
> ACPI5 trigger too? Otherwise we may also encounter resource conflict with ACPI5
> trigger table.
I haven't seen any problems while testing this - but perhaps the trigger sequence
generated by this BIOS in ACPI 5 mode doesn't access the target address (though
that would be strange).
-Tony
> I'm a little confused about the ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS command in ACPI5.
> The above code seems like a hard-coded version of
> apei_exec_run(&ctx, ACPI_EINJ_SET_ERROR_TYPE_WITH_ADDRESS).
If I mashed this code into the wrong place ... then help me understand how it
should fit in stylistically with the existing code.
Basic summary is that ACPI 5 added the _WITH_ADDRESS functionality. If it is present,
then there seems no point in using the old SET_ERROR_TYPE action from ACPI4.
-Tony
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?