linux-kernel - Re: [PATCH] acpi/apei/einj: Add extensions to EINJ from rev 5.0 of acpi spec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 12 Feb 2012 17:33:03 +0800
From:	Jiang Liu <liuj97@...il.com>
To:	Tony Luck <tony.luck@...el.com>,
	Chen Gong <gong.chen@...ux.intel.com>
CC:	Len Brown <len.brown@...el.com>, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org, ying.huang@...el.com
Subject: Re: [PATCH] acpi/apei/einj: Add extensions to EINJ from rev 5.0 of
 acpi spec

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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/