diff mbox series

Fix incorrect return value of pre_map_gar_callback

Message ID 20201026061509.48212-1-yaoaili126@163.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Fix incorrect return value of pre_map_gar_callback | expand

Commit Message

yaoaili126@163.com Oct. 26, 2020, 6:15 a.m. UTC 
From: Aili Yao <yaoaili@kingsoft.com>

From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
acpi_os_map_generic_address() return value"),acpi_os_map_generic_address
will return logical address or NULL for error, but pre_map_gar_callback,for
ACPI_ADR_SPACE_SYSTEM_IO case, it should be also return 0,as it's a
normal case, but now it will return -ENXIO. so check it out for such case
to avoid einj module initialization fail.

Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
---
 drivers/acpi/apei/apei-base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Luck, Tony Oct. 26, 2020, 4:59 p.m. UTC | #1 
> -	if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
> +	if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER &&
> +	 entry->register_region.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)

I don't know (too long since I looked at APEI code) if this is the right fix, but I tried it out and it does allow einj.ko module to load.

Tested-by: Tony Luck <tony.luck@intel.com>

-Tony
James Morse Oct. 28, 2020, 7:19 p.m. UTC | #2 
Hi!

On 26/10/2020 06:15, yaoaili126@163.com wrote:
> From: Aili Yao <yaoaili@kingsoft.com>
> 
> From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
> acpi_os_map_generic_address() return value"),acpi_os_map_generic_address
> will return logical address or NULL for error, but pre_map_gar_callback,for
> ACPI_ADR_SPACE_SYSTEM_IO case, it should

'it should' refers to pre_map_gar_callback(), not acpi_os_map_generic_address()?


> be also return 0,as it's a
> normal case, but now it will return -ENXIO. so check it out for such case
> to avoid einj module initialization fail.

apei_map_generic_address() calls acpi_os_map_generic_address() which returns NULL for any
address space that isn't ACPI_ADR_SPACE_SYSTEM_MEMORY.
That commit now maps this to an error code, where-as before: this code was getting away
with it.

The bug is it tries to map a GAR that doesn't need mapping.


Could we avoid this problem more clearly by returning 0 from apei_map_generic_address()
for address spaces that don't need mapping? (e.g. IO)

This would also fix any other callers lurking in apei.


Thanks,

James


> diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
> index 552fd9ffaca4..042d2dbdb855 100644
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -230,7 +230,8 @@ static int pre_map_gar_callback(struct apei_exec_context *ctx,
>  {
>  	u8 ins = entry->instruction;
>  
> -	if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
> +	if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER &&
> +	 entry->register_region.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
>  		return apei_map_generic_address(&entry->register_region);
>  
>  	return 0;
diff mbox series

Patch

diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 552fd9ffaca4..042d2dbdb855 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -230,7 +230,8 @@  static int pre_map_gar_callback(struct apei_exec_context *ctx,
 {
 	u8 ins = entry->instruction;
 
-	if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER)
+	if (ctx->ins_table[ins].flags & APEI_EXEC_INS_ACCESS_REGISTER &&
+	 entry->register_region.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
 		return apei_map_generic_address(&entry->register_region);
 
 	return 0;