Hello, On 29/10/2020 11:30, yaoaili126@xxxxxxx wrote: > From: Aili Yao <yaoaili@xxxxxxxxxxxx> >> From: James Morse [mailto:james.morse >> 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. > yes, you are right, I think we can move the ACPI_ADR_SPACE_SYSTEM_MEMORY check to: > > 626 int apei_map_generic_address(struct acpi_generic_address *reg) > 627 { > 628 int rc; > 629 u32 access_bit_width; > 630 u64 address; > 631 > 632 rc = apei_check_gar(reg, &address, &access_bit_width); > 633 if (rc) > 634 return rc; > 635 > 636 if (!acpi_os_map_generic_address(reg) && > 637 reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > 638 return -ENXIO; > 639 > 640 return 0; > 641 } > 642 EXPORT_SYMBOL_GPL(apei_map_generic_address); This swallows the error code for other address spaces too, which would be nasty to debug in the future! Could you do something like: | rc = apei_check_gar(reg, &address, &access_bit_width); | if (rc) | return rc; | | /* IO space doesn't need mapping */ | if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) | return 0; | | return acpi_os_map_generic_address(reg); (and it continues to get-away with the unmap call as that doesn't return anything, which I think is fine) If you post this, please follow the most popular style of previous patches for the subject. See: git log --oneline drivers/acpi/apei/apei-base.c (This is to let the maintainer spot which patches they need to do something with) Thanks, James
