2022-04-02 12:32:49

by James Liu

[permalink] [raw]
Subject: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

From: James Liu <[email protected]>

Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
to address a data structure; in this case, the GAS has the field of
"Register Bit Width" equal to 0.

For example, "Injection Instruction Entry" (Table 18.25 in ACPI 6.4)
has a RegisterRegion field that is a GAS that points to a data
structure SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required
when using EINJ (Error Injection module).

This fix preserves a fairly sufficient memory space (i.e., page size)
to store the data structure so as to prevent EINJ module from loading
failure if platform firmware can support Injection Instruction Entry in
an EINJ table.

Signed-off-by: James Liu <[email protected]>
---
drivers/acpi/osl.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 45c5c0e45..ab2f584b1 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
return NULL;

+ /* Handle a case that GAS is used to address an ACPI data structure */
+ if (!gas->bit_width) {
+ pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
+ return acpi_os_map_iomem(addr, PAGE_SIZE);
+ }
+
/* Handle possible alignment issues */
memcpy(&addr, &gas->address, sizeof(addr));
- if (!addr || !gas->bit_width)
+ if (!addr)
return NULL;

return acpi_os_map_iomem(addr, gas->bit_width / 8);
@@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
return;

+ /* Handle a case that GAS is used to address an ACPI data structure */
+ if (!gas->bit_width) {
+ pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
+ mutex_lock(&acpi_ioremap_lock);
+ map = acpi_map_lookup(addr, PAGE_SIZE);
+ if (!map) {
+ mutex_unlock(&acpi_ioremap_lock);
+ return;
+ }
+ acpi_os_drop_map_ref(map);
+ mutex_unlock(&acpi_ioremap_lock);
+ }
+
/* Handle possible alignment issues */
memcpy(&addr, &gas->address, sizeof(addr));
- if (!addr || !gas->bit_width)
+ if (!addr)
return;

mutex_lock(&acpi_ioremap_lock);
--
2.25.1


2022-04-05 02:18:43

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on v5.17 next-20220401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/james-liu-hpe-com/ACPI-OSL-Fix-the-memory-mapping-of-an-ACPI-GAS-that-addresses-a-data-structure/20220402-013755
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220402/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c4a1b07d0979e7ff20d7d541af666d822d66b566)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/594496ff7d62c6d42b3c8a436fca46cc289aea67
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review james-liu-hpe-com/ACPI-OSL-Fix-the-memory-mapping-of-an-ACPI-GAS-that-addresses-a-data-structure/20220402-013755
git checkout 594496ff7d62c6d42b3c8a436fca46cc289aea67
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/acpi/osl.c:463:29: warning: variable 'addr' is uninitialized when used here [-Wuninitialized]
return acpi_os_map_iomem(addr, PAGE_SIZE);
^~~~
drivers/acpi/osl.c:455:10: note: initialize the variable 'addr' to silence this warning
u64 addr;
^
= 0
drivers/acpi/osl.c:487:25: warning: variable 'addr' is uninitialized when used here [-Wuninitialized]
map = acpi_map_lookup(addr, PAGE_SIZE);
^~~~
drivers/acpi/osl.c:477:10: note: initialize the variable 'addr' to silence this warning
u64 addr;
^
= 0
2 warnings generated.


vim +/addr +463 drivers/acpi/osl.c

452
453 void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
454 {
455 u64 addr;
456
457 if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
458 return NULL;
459
460 /* Handle a case that GAS is used to address an ACPI data structure */
461 if (!gas->bit_width) {
462 pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
> 463 return acpi_os_map_iomem(addr, PAGE_SIZE);
464 }
465
466 /* Handle possible alignment issues */
467 memcpy(&addr, &gas->address, sizeof(addr));
468 if (!addr)
469 return NULL;
470
471 return acpi_os_map_iomem(addr, gas->bit_width / 8);
472 }
473 EXPORT_SYMBOL(acpi_os_map_generic_address);
474

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-04-05 20:23:29

by kernel test robot

[permalink] [raw]
Subject: [ACPI] 594496ff7d: WARNING:at_arch/x86/mm/ioremap.c:#__ioremap_caller.cold



Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: 594496ff7d62c6d42b3c8a436fca46cc289aea67 ("[PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure")
url: https://github.com/intel-lab-lkp/linux/commits/james-liu-hpe-com/ACPI-OSL-Fix-the-memory-mapping-of-an-ACPI-GAS-that-addresses-a-data-structure/20220402-013755
base: https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git linux-next
patch link: https://lore.kernel.org/linux-acpi/[email protected]

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu Icelake-Server -smp 4 -m 16G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):



If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>


[ 1.769252][ T0] WARNING: CPU: 0 PID: 0 at arch/x86/mm/ioremap.c:200 __ioremap_caller.cold+0x62/0x91
[ 1.770857][ T0] Modules linked in:
[ 1.771541][ T0] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.17.0-00676-g594496ff7d62 #1
[ 1.772881][ T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 1.774319][ T0] RIP: __ioremap_caller.cold+0x62/0x91
[ 1.775363][ T0] Code: 00 e9 fa aa c2 fd 48 8b 34 24 48 c7 c7 a0 ab 64 87 e8 ff dd 00 00 e9 e5 aa c2 fd 4c 89 f6 48 c7 c7 40 aa 64 87 e8 eb dd 00 00 <0f> 0b 45 31 f6 e9 cc aa c2 fd 48 8b b4 24 80 00 00 00 44 8b 44 24
All code
========
0: 00 e9 add %ch,%cl
2: fa cli
3: aa stos %al,%es:(%rdi)
4: c2 fd 48 retq $0x48fd
7: 8b 34 24 mov (%rsp),%esi
a: 48 c7 c7 a0 ab 64 87 mov $0xffffffff8764aba0,%rdi
11: e8 ff dd 00 00 callq 0xde15
16: e9 e5 aa c2 fd jmpq 0xfffffffffdc2ab00
1b: 4c 89 f6 mov %r14,%rsi
1e: 48 c7 c7 40 aa 64 87 mov $0xffffffff8764aa40,%rdi
25: e8 eb dd 00 00 callq 0xde15
2a:* 0f 0b ud2 <-- trapping instruction
2c: 45 31 f6 xor %r14d,%r14d
2f: e9 cc aa c2 fd jmpq 0xfffffffffdc2ab00
34: 48 8b b4 24 80 00 00 mov 0x80(%rsp),%rsi
3b: 00
3c: 44 rex.R
3d: 8b .byte 0x8b
3e: 44 rex.R
3f: 24 .byte 0x24

Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 45 31 f6 xor %r14d,%r14d
5: e9 cc aa c2 fd jmpq 0xfffffffffdc2aad6
a: 48 8b b4 24 80 00 00 mov 0x80(%rsp),%rsi
11: 00
12: 44 rex.R
13: 8b .byte 0x8b
14: 44 rex.R
15: 24 .byte 0x24
[ 1.778141][ T0] RSP: 0000:ffffffff88007d40 EFLAGS: 00010286
[ 1.779092][ T0] RAX: 0000000000000032 RBX: 1ffffffff1000fac RCX: 0000000000000000
[ 1.780361][ T0] RDX: 0000000000000000 RSI: 0000000000000008 RDI: fffffbfff1000f9b
[ 1.781584][ T0] RBP: ffffffff895c0e20 R08: 0000000000000032 R09: ffffffff880079d7
[ 1.782834][ T0] R10: fffffbfff1000f3a R11: 0000000000000001 R12: 0000000000001000
[ 1.784135][ T0] R13: ffffffff895c0e20 R14: ffffffff895c0e20 R15: 0000000000000000
[ 1.785384][ T0] FS: 0000000000000000(0000) GS:ffff8883dd600000(0000) knlGS:0000000000000000
[ 1.786775][ T0] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1.787823][ T0] CR2: ffff88843ffff000 CR3: 000000003da0e001 CR4: 00000000000606b0
[ 1.789091][ T0] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1.790395][ T0] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1.791731][ T0] Call Trace:
[ 1.792329][ T0] <TASK>
[ 1.792870][ T0] ? acpi_os_map_iomem (include/acpi/acpi_io.h:13 drivers/acpi/osl.c:297 drivers/acpi/osl.c:356)
[ 1.793691][ T0] ? iounmap (arch/x86/mm/ioremap.c:178)
[ 1.794417][ T0] ? page_is_ram+0x7c/0x100
[ 1.795167][ T0] ? devm_request_resource (kernel/resource.c:512)
[ 1.796042][ T0] ? acpi_table_print_srat_entry (drivers/acpi/numa/srat.c:101 (discriminator 20))
[ 1.796974][ T0] ? acpi_tb_parse_root_table (drivers/acpi/acpica/tbxface.c:134)
[ 1.797868][ T0] ? acpi_tb_parse_root_table (drivers/acpi/acpica/tbxface.c:134)
[ 1.798770][ T0] ? acpi_parse_prmt (drivers/acpi/prmt.c:115)
[ 1.799580][ T0] acpi_os_map_iomem (include/acpi/acpi_io.h:13 drivers/acpi/osl.c:297 drivers/acpi/osl.c:356)
[ 1.800387][ T0] ? acpi_tb_parse_root_table (drivers/acpi/acpica/tbxface.c:134)
[ 1.801291][ T0] acpi_os_map_generic_address.cold (drivers/acpi/osl.c:463)
[ 1.802240][ T0] ? acpi_os_get_iomem (drivers/acpi/osl.c:454)
[ 1.803069][ T0] ? acpi_tb_parse_root_table (drivers/acpi/acpica/tbxface.c:134)
[ 1.803985][ T0] ? dmi_matches (drivers/firmware/dmi_scan.c:850)
[ 1.804730][ T0] acpi_os_initialize (drivers/acpi/osl.c:1755)
[ 1.805536][ T0] acpi_initialize_subsystem (drivers/acpi/acpica/utxfinit.c:49)
[ 1.806389][ T0] acpi_early_init (drivers/acpi/bus.c:1163)
[ 1.807143][ T0] start_kernel (init/main.c:1101)
[ 1.807884][ T0] secondary_startup_64_no_verify (arch/x86/kernel/head_64.S:300)
[ 1.808804][ T0] </TASK>
[ 1.809338][ T0] ---[ end trace 0000000000000000 ]---
[ 1.810204][ T0] ACPI: OSL: An ACPI data structure at 0xffffffff895c0e20 is mapped
[ 1.811548][ T0] ioremap: invalid physical address ffffffff895c0e20
[ 1.813065][ T0] clocksource: hpet: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604467 ns
[ 1.814806][ T0] APIC: Switch to symmetric I/O mode setup
[ 1.815946][ T0] x2apic enabled
[ 1.816807][ T0] Switched APIC routing to physical x2apic.
[ 1.817737][ T0] masked ExtINT on CPU#0
[ 1.819214][ T0] ENABLING IO-APIC IRQs
[ 1.819954][ T0] init IO_APIC IRQs
[ 1.820594][ T0] apic 0 pin 0 not connected
[ 1.821411][ T0] IOAPIC[0]: Preconfigured routing entry (0-1 -> IRQ 1 Level:0 ActiveLow:0)
[ 1.822885][ T0] IOAPIC[0]: Preconfigured routing entry (0-2 -> IRQ 0 Level:0 ActiveLow:0)
[ 1.824338][ T0] IOAPIC[0]: Preconfigured routing entry (0-3 -> IRQ 3 Level:0 ActiveLow:0)
[ 1.825775][ T0] IOAPIC[0]: Preconfigured routing entry (0-4 -> IRQ 4 Level:0 ActiveLow:0)
[ 1.827221][ T0] IOAPIC[0]: Preconfigured routing entry (0-5 -> IRQ 5 Level:1 ActiveLow:0)
[ 1.828668][ T0] IOAPIC[0]: Preconfigured routing entry (0-6 -> IRQ 6 Level:0 ActiveLow:0)
[ 1.830106][ T0] IOAPIC[0]: Preconfigured routing entry (0-7 -> IRQ 7 Level:0 ActiveLow:0)
[ 1.831571][ T0] IOAPIC[0]: Preconfigured routing entry (0-8 -> IRQ 8 Level:0 ActiveLow:0)
[ 1.833035][ T0] IOAPIC[0]: Preconfigured routing entry (0-9 -> IRQ 9 Level:1 ActiveLow:0)
[ 1.834503][ T0] IOAPIC[0]: Preconfigured routing entry (0-10 -> IRQ 10 Level:1 ActiveLow:0)
[ 1.835967][ T0] IOAPIC[0]: Preconfigured routing entry (0-11 -> IRQ 11 Level:1 ActiveLow:0)
[ 1.837428][ T0] IOAPIC[0]: Preconfigured routing entry (0-12 -> IRQ 12 Level:0 ActiveLow:0)
[ 1.838888][ T0] IOAPIC[0]: Preconfigured routing entry (0-13 -> IRQ 13 Level:0 ActiveLow:0)
[ 1.840391][ T0] IOAPIC[0]: Preconfigured routing entry (0-14 -> IRQ 14 Level:0 ActiveLow:0)
[ 1.841878][ T0] IOAPIC[0]: Preconfigured routing entry (0-15 -> IRQ 15 Level:0 ActiveLow:0)
[ 1.843325][ T0] apic 0 pin 16 not connected
[ 1.844133][ T0] apic 0 pin 17 not connected
[ 1.844891][ T0] apic 0 pin 18 not connected
[ 1.845665][ T0] apic 0 pin 19 not connected
[ 1.846487][ T0] apic 0 pin 20 not connected
[ 1.847314][ T0] apic 0 pin 21 not connected
[ 1.848102][ T0] apic 0 pin 22 not connected
[ 1.848889][ T0] apic 0 pin 23 not connected
[ 1.853588][ T0] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[ 1.854706][ T0] clocksource: tsc-early: mask: 0xffffffffffffffff max_cycles: 0x1fa3704c1a9, max_idle_ns: 440795296692 ns
[ 1.856587][ T0] Calibrating delay loop (skipped) preset value.. 4389.83 BogoMIPS (lpj=2194916)
[ 1.857570][ T0] pid_max: default: 32768 minimum: 301
[ 1.858839][ T0] LSM: Security Framework initializing
[ 1.859590][ T0] Yama: becoming mindful.
[ 1.860790][ T0] Mount-cache hash table entries: 32768 (order: 6, 262144 bytes, linear)
[ 1.862573][ T0] Mountpoint-cache hash table entries: 32768 (order: 6, 262144 bytes, linear)
Poking KASLR using RDRAND RDTSC...
[ 1.865787][ T0] x86/cpu: User Mode Instruction Prevention (UMIP) activated
[ 1.866673][ T0] Last level iTLB entries: 4KB 0, 2MB 0, 4MB 0
[ 1.867568][ T0] Last level dTLB entries: 4KB 0, 2MB 0, 4MB 0, 1GB 0
[ 1.868581][ T0] Spectre V1 : Mitigation: usercopy/swapgs barriers and __user pointer sanitization
[ 1.869578][ T0] Spectre V2 : Mitigation: Retpolines
[ 1.870568][ T0] Spectre V2 : Spectre v2 / SpectreRSB mitigation: Filling RSB on context switch
[ 1.871569][ T0] Speculative Store Bypass: Vulnerable
[ 1.872581][ T0] TAA: Vulnerable: Clear CPU buffers attempted, no microcode
[ 1.873568][ T0] MDS: Vulnerable: Clear CPU buffers attempted, no microcode
[ 1.887061][ T0] Freeing SMP alternatives memory: 40K
[ 1.888693][ T1] smpboot: CPU0: Intel Xeon Processor (Icelake) (family: 0x6, model: 0x86, stepping: 0x0)
[ 1.890326][ T1] cblist_init_generic: Setting adjustable number of callback queues.
[ 1.890569][ T1] cblist_init_generic: Setting shift to 2 and lim to 1.
[ 1.891709][ T1] cblist_init_generic: Setting shift to 2 and lim to 1.
[ 1.892695][ T1] cblist_init_generic: Setting shift to 2 and lim to 1.
[ 1.893689][ T1] Performance Events: unsupported p6 CPU model 134 no PMU driver, software events only.
[ 1.894744][ T1] rcu: Hierarchical SRCU implementation.
[ 1.897502][ T1] NMI watchdog: Perf NMI watchdog permanently disabled
[ 1.898709][ T1] smp: Bringing up secondary CPUs ...
[ 1.900191][ T1] x86: Booting SMP configuration:
[ 1.900576][ T1] .... node #0, CPUs: #1
[ 0.163365][ T0] masked ExtINT on CPU#1
[ 0.163365][ T0] smpboot: CPU 1 Converting physical 0 to logical die 1
[ 1.904631][ T1] #2
[ 0.163365][ T0] masked ExtINT on CPU#2
[ 0.163365][ T0] smpboot: CPU 2 Converting physical 0 to logical die 2
[ 1.907580][ T1] #3
[ 0.163365][ T0] masked ExtINT on CPU#3
[ 0.163365][ T0] smpboot: CPU 3 Converting physical 0 to logical die 3
[ 1.909723][ T1] smp: Brought up 1 node, 4 CPUs
[ 1.910571][ T1] smpboot: Max logical packages: 4
[ 1.911423][ T1] smpboot: Total of 4 processors activated (17559.32 BogoMIPS)
[ 2.015595][ T32] node 0 deferred pages initialised in 95ms
[ 2.133491][ T1] allocated 201326592 bytes of page_ext
[ 2.133932][ T1] Node 0, zone DMA: page owner found early allocated 0 pages
[ 2.136540][ T1] Node 0, zone DMA32: page owner found early allocated 10 pages


To reproduce:

# build kernel
cd linux
cp config-5.17.0-00676-g594496ff7d62 .config
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



--
0-DAY CI Kernel Test Service
https://01.org/lkp



Attachments:
(No filename) (11.72 kB)
config-5.17.0-00676-g594496ff7d62 (168.48 kB)
job-script (5.03 kB)
dmesg.xz (14.25 kB)
Download all attachments

2022-05-17 19:29:59

by James Liu

[permalink] [raw]
Subject: Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

Hi Rafael and all,
Could you take a look into this patches? The mentioned bug blocks EINJ
testing whenever a system firmware can correctly support GAS according
to ACPI 6.4.

On Fri, Apr 01, 2022 at 05:28:40PM +0000, [email protected] wrote:
> From: James Liu <[email protected]>
>
> Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
> to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
> to address a data structure; in this case, the GAS has the field of
> "Register Bit Width" equal to 0.
>
> For example, "Injection Instruction Entry" (Table 18.25 in ACPI 6.4)
> has a RegisterRegion field that is a GAS that points to a data
> structure SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required
> when using EINJ (Error Injection module).
>
> This fix preserves a fairly sufficient memory space (i.e., page size)
> to store the data structure so as to prevent EINJ module from loading
> failure if platform firmware can support Injection Instruction Entry in
> an EINJ table.
>
> Signed-off-by: James Liu <[email protected]>
> ---
> drivers/acpi/osl.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 45c5c0e45..ab2f584b1 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
> if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> return NULL;
>
> + /* Handle a case that GAS is used to address an ACPI data structure */
> + if (!gas->bit_width) {
> + pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
> + return acpi_os_map_iomem(addr, PAGE_SIZE);
> + }
> +
> /* Handle possible alignment issues */
> memcpy(&addr, &gas->address, sizeof(addr));
> - if (!addr || !gas->bit_width)
> + if (!addr)
> return NULL;
>
> return acpi_os_map_iomem(addr, gas->bit_width / 8);
> @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
> if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> return;
>
> + /* Handle a case that GAS is used to address an ACPI data structure */
> + if (!gas->bit_width) {
> + pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
> + mutex_lock(&acpi_ioremap_lock);
> + map = acpi_map_lookup(addr, PAGE_SIZE);
> + if (!map) {
> + mutex_unlock(&acpi_ioremap_lock);
> + return;
> + }
> + acpi_os_drop_map_ref(map);
> + mutex_unlock(&acpi_ioremap_lock);
> + }
> +
> /* Handle possible alignment issues */
> memcpy(&addr, &gas->address, sizeof(addr));
> - if (!addr || !gas->bit_width)
> + if (!addr)
> return;
>
> mutex_lock(&acpi_ioremap_lock);
> --
> 2.25.1
>

2022-05-18 04:16:48

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

On Tue, May 17, 2022 at 7:56 PM James Liu <[email protected]> wrote:
>
> Hi Rafael and all,
> Could you take a look into this patches? The mentioned bug blocks EINJ
> testing whenever a system firmware can correctly support GAS according
> to ACPI 6.4.

The kernel test robot reported an issue with it. Have you seen that report?

> On Fri, Apr 01, 2022 at 05:28:40PM +0000, [email protected] wrote:
> > From: James Liu <[email protected]>
> >
> > Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
> > to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
> > to address a data structure; in this case, the GAS has the field of
> > "Register Bit Width" equal to 0.
> >
> > For example, "Injection Instruction Entry" (Table 18.25 in ACPI 6.4)
> > has a RegisterRegion field that is a GAS that points to a data
> > structure SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required
> > when using EINJ (Error Injection module).
> >
> > This fix preserves a fairly sufficient memory space (i.e., page size)
> > to store the data structure so as to prevent EINJ module from loading
> > failure if platform firmware can support Injection Instruction Entry in
> > an EINJ table.
> >
> > Signed-off-by: James Liu <[email protected]>
> > ---
> > drivers/acpi/osl.c | 23 +++++++++++++++++++++--
> > 1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 45c5c0e45..ab2f584b1 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
> > if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> > return NULL;
> >
> > + /* Handle a case that GAS is used to address an ACPI data structure */
> > + if (!gas->bit_width) {
> > + pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
> > + return acpi_os_map_iomem(addr, PAGE_SIZE);
> > + }
> > +
> > /* Handle possible alignment issues */
> > memcpy(&addr, &gas->address, sizeof(addr));
> > - if (!addr || !gas->bit_width)
> > + if (!addr)
> > return NULL;
> >
> > return acpi_os_map_iomem(addr, gas->bit_width / 8);
> > @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
> > if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> > return;
> >
> > + /* Handle a case that GAS is used to address an ACPI data structure */
> > + if (!gas->bit_width) {
> > + pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
> > + mutex_lock(&acpi_ioremap_lock);
> > + map = acpi_map_lookup(addr, PAGE_SIZE);
> > + if (!map) {
> > + mutex_unlock(&acpi_ioremap_lock);
> > + return;
> > + }
> > + acpi_os_drop_map_ref(map);
> > + mutex_unlock(&acpi_ioremap_lock);
> > + }
> > +
> > /* Handle possible alignment issues */
> > memcpy(&addr, &gas->address, sizeof(addr));
> > - if (!addr || !gas->bit_width)
> > + if (!addr)
> > return;
> >
> > mutex_lock(&acpi_ioremap_lock);
> > --
> > 2.25.1
> >

2022-05-18 04:58:41

by James Liu

[permalink] [raw]
Subject: Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

O Tue, May 17, 2022 at 08:25:55PM +0200, Rafael J. Wysocki wrote:
> On Tue, May 17, 2022 at 7:56 PM James Liu <[email protected]> wrote:
> >
> > Hi Rafael and all,
> > Could you take a look into this patches? The mentioned bug blocks EINJ
> > testing whenever a system firmware can correctly support GAS according
> > to ACPI 6.4.
>
> The kernel test robot reported an issue with it. Have you seen that report?
>
Yes, I saw that the issue is warnings about the initialization of "u64 addr".
Will you suggest fixing it in another patch or just merge the change
(i.e., u64 addr = 0) into this one? Thanks for reviewing this.

> > On Fri, Apr 01, 2022 at 05:28:40PM +0000, [email protected] wrote:
> > > From: James Liu <[email protected]>
> > >
> > > Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
> > > to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
> > > to address a data structure; in this case, the GAS has the field of
> > > "Register Bit Width" equal to 0.
> > >
> > > For example, "Injection Instruction Entry" (Table 18.25 in ACPI 6.4)
> > > has a RegisterRegion field that is a GAS that points to a data
> > > structure SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required
> > > when using EINJ (Error Injection module).
> > >
> > > This fix preserves a fairly sufficient memory space (i.e., page size)
> > > to store the data structure so as to prevent EINJ module from loading
> > > failure if platform firmware can support Injection Instruction Entry in
> > > an EINJ table.
> > >
> > > Signed-off-by: James Liu <[email protected]>
> > > ---
> > > drivers/acpi/osl.c | 23 +++++++++++++++++++++--
> > > 1 file changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > > index 45c5c0e45..ab2f584b1 100644
> > > --- a/drivers/acpi/osl.c
> > > +++ b/drivers/acpi/osl.c
> > > @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
> > > if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> > > return NULL;
> > >
> > > + /* Handle a case that GAS is used to address an ACPI data structure */
> > > + if (!gas->bit_width) {
> > > + pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
> > > + return acpi_os_map_iomem(addr, PAGE_SIZE);
> > > + }
> > > +
> > > /* Handle possible alignment issues */
> > > memcpy(&addr, &gas->address, sizeof(addr));
> > > - if (!addr || !gas->bit_width)
> > > + if (!addr)
> > > return NULL;
> > >
> > > return acpi_os_map_iomem(addr, gas->bit_width / 8);
> > > @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
> > > if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> > > return;
> > >
> > > + /* Handle a case that GAS is used to address an ACPI data structure */
> > > + if (!gas->bit_width) {
> > > + pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
> > > + mutex_lock(&acpi_ioremap_lock);
> > > + map = acpi_map_lookup(addr, PAGE_SIZE);
> > > + if (!map) {
> > > + mutex_unlock(&acpi_ioremap_lock);
> > > + return;
> > > + }
> > > + acpi_os_drop_map_ref(map);
> > > + mutex_unlock(&acpi_ioremap_lock);
> > > + }
> > > +
> > > /* Handle possible alignment issues */
> > > memcpy(&addr, &gas->address, sizeof(addr));
> > > - if (!addr || !gas->bit_width)
> > > + if (!addr)
> > > return;
> > >
> > > mutex_lock(&acpi_ioremap_lock);
> > > --
> > > 2.25.1
> > >