2013-05-22 09:43:14

by Matt Fleming

[permalink] [raw]
Subject: [PATCH] x86/PCI: setup data may be in highmem

From: Matt Fleming <[email protected]>

pcibios_add_device() assumes that the physical addresses stored in
setup_data are accessible via the direct kernel mapping, and that
calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
where the direct mapping range is much smaller than on x86-64.

Calling phys_to_virt() on a highmem address results in the following,

BUG: unable to handle kernel paging request at 39a3c198
IP: [<c262be0f>] pcibios_add_device+0x2f/0x90
*pde = 00000000
Oops: 0000 [#1] SMP
Modules linked in:
Pid: 1, comm: swapper/0 Tainted: G W I 3.9.0-rc2+ #280
EIP: 0060:[<c262be0f>] EFLAGS: 00010206 CPU: 1
EIP is at pcibios_add_device+0x2f/0x90
EAX: f6258800 EBX: f6258800 ECX: 79a3c190 EDX: 39a3c190
ESI: f62d9814 EDI: f6258864 EBP: f60add38 ESP: f60add2c
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 8005003b CR2: 39a3c198 CR3: 02b91000 CR4: 001007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process swapper/0 (pid: 1, ti=f60ac000 task=f60b0000 task.ti=f60ac000)
Stack:
f6258800 f62d9814 f6258864 f60add4c c2370c73 00000000 f62d9800 00000000
f60add6c c274640b 0000ea60 f6258800 0f008086 f62d9800 f62d9800 00000000
f60add84 c2370d08 00000000 00000008 f62d9800 00000000 f60adda4 c2371904
Call Trace:
[<c2370c73>] pci_device_add+0xe3/0x130
[<c274640b>] pci_scan_single_device+0x8b/0xb0
[<c2370d08>] pci_scan_slot+0x48/0x100
[<c2371904>] pci_scan_child_bus+0x24/0xc0
[<c262a7b0>] pci_acpi_scan_root+0x2c0/0x490
[<c23b7203>] acpi_pci_root_add+0x312/0x42f
[<c23b29d7>] ? acpi_device_notify_fixed+0x1d/0x1d
[<c23b36a8>] acpi_bus_device_attach+0x77/0xdd
[<c23cb6be>] acpi_ns_walk_namespace+0xb1/0x163
[<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
[<c23cbd4e>] acpi_walk_namespace+0x7e/0xa8
[<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
[<c23b46e0>] acpi_bus_scan+0x9a/0xa6
[<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
[<c2b17ec9>] acpi_scan_init+0x51/0x144
[<c2b252a2>] ? pci_mmcfg_late_init+0x49/0x4b
[<c2b17cdc>] acpi_init+0x224/0x28c
[<c2001144>] do_one_initcall+0x34/0x170
[<c2b17ab8>] ? acpi_sleep_proc_init+0x2e/0x2e
[<c2aeeb83>] kernel_init_freeable+0x119/0x1b6
[<c2aee4da>] ? do_early_param+0x74/0x74
[<c2743f10>] kernel_init+0x10/0xd0
[<c2765697>] ret_from_kernel_thread+0x1b/0x28
[<c2743f00>] ? rest_init+0x60/0x60

The most reliable way to trigger this crash seems to be booting a 32-bit
kernel via the EFI boot stub.

The solution is to use early_ioremap() instead of phys_to_virt() to map
the setup data into the kernel address space.

Tested-by: Jani Nikula <[email protected]>
Cc: Matthew Garrett <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Seth Forshee <[email protected]>
Cc: Jesse Barnes <[email protected]>
Cc: <[email protected]>
Signed-off-by: Matt Fleming <[email protected]>
---
arch/x86/pci/common.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 305c68b..7ae6671 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -628,7 +628,7 @@ int pcibios_add_device(struct pci_dev *dev)

pa_data = boot_params.hdr.setup_data;
while (pa_data) {
- data = phys_to_virt(pa_data);
+ data = early_ioremap(pa_data, sizeof(*rom));

if (data->type == SETUP_PCI) {
rom = (struct pci_setup_rom *)data;
@@ -645,6 +645,7 @@ int pcibios_add_device(struct pci_dev *dev)
}
}
pa_data = data->next;
+ early_iounmap(data, sizeof(*rom));
}
return 0;
}
--
1.8.1.4


2013-05-23 22:47:01

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On 05/22/2013 02:43 AM, Matt Fleming wrote:
> From: Matt Fleming <[email protected]>
>
> pcibios_add_device() assumes that the physical addresses stored in
> setup_data are accessible via the direct kernel mapping, and that
> calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
> where the direct mapping range is much smaller than on x86-64.
>
> Calling phys_to_virt() on a highmem address results in the following,
>

Bjorn: yours or mine?

-hpa

2013-05-24 14:38:33

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Wed, May 22, 2013 at 3:43 AM, Matt Fleming <[email protected]> wrote:
> From: Matt Fleming <[email protected]>
>
> pcibios_add_device() assumes that the physical addresses stored in
> setup_data are accessible via the direct kernel mapping, and that
> calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
> where the direct mapping range is much smaller than on x86-64.
>
> Calling phys_to_virt() on a highmem address results in the following,
>
> BUG: unable to handle kernel paging request at 39a3c198
> IP: [<c262be0f>] pcibios_add_device+0x2f/0x90
> *pde = 00000000
> Oops: 0000 [#1] SMP
> Modules linked in:
> Pid: 1, comm: swapper/0 Tainted: G W I 3.9.0-rc2+ #280
> EIP: 0060:[<c262be0f>] EFLAGS: 00010206 CPU: 1
> EIP is at pcibios_add_device+0x2f/0x90
> EAX: f6258800 EBX: f6258800 ECX: 79a3c190 EDX: 39a3c190
> ESI: f62d9814 EDI: f6258864 EBP: f60add38 ESP: f60add2c
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> CR0: 8005003b CR2: 39a3c198 CR3: 02b91000 CR4: 001007d0
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Process swapper/0 (pid: 1, ti=f60ac000 task=f60b0000 task.ti=f60ac000)
> Stack:
> f6258800 f62d9814 f6258864 f60add4c c2370c73 00000000 f62d9800 00000000
> f60add6c c274640b 0000ea60 f6258800 0f008086 f62d9800 f62d9800 00000000
> f60add84 c2370d08 00000000 00000008 f62d9800 00000000 f60adda4 c2371904
> Call Trace:
> [<c2370c73>] pci_device_add+0xe3/0x130
> [<c274640b>] pci_scan_single_device+0x8b/0xb0
> [<c2370d08>] pci_scan_slot+0x48/0x100
> [<c2371904>] pci_scan_child_bus+0x24/0xc0
> [<c262a7b0>] pci_acpi_scan_root+0x2c0/0x490
> [<c23b7203>] acpi_pci_root_add+0x312/0x42f
> [<c23b29d7>] ? acpi_device_notify_fixed+0x1d/0x1d
> [<c23b36a8>] acpi_bus_device_attach+0x77/0xdd
> [<c23cb6be>] acpi_ns_walk_namespace+0xb1/0x163
> [<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
> [<c23cbd4e>] acpi_walk_namespace+0x7e/0xa8
> [<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
> [<c23b46e0>] acpi_bus_scan+0x9a/0xa6
> [<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
> [<c2b17ec9>] acpi_scan_init+0x51/0x144
> [<c2b252a2>] ? pci_mmcfg_late_init+0x49/0x4b
> [<c2b17cdc>] acpi_init+0x224/0x28c
> [<c2001144>] do_one_initcall+0x34/0x170
> [<c2b17ab8>] ? acpi_sleep_proc_init+0x2e/0x2e
> [<c2aeeb83>] kernel_init_freeable+0x119/0x1b6
> [<c2aee4da>] ? do_early_param+0x74/0x74
> [<c2743f10>] kernel_init+0x10/0xd0
> [<c2765697>] ret_from_kernel_thread+0x1b/0x28
> [<c2743f00>] ? rest_init+0x60/0x60
>
> The most reliable way to trigger this crash seems to be booting a 32-bit
> kernel via the EFI boot stub.
>
> The solution is to use early_ioremap() instead of phys_to_virt() to map
> the setup data into the kernel address space.
>
> Tested-by: Jani Nikula <[email protected]>
> Cc: Matthew Garrett <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Seth Forshee <[email protected]>
> Cc: Jesse Barnes <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>
> ---
> arch/x86/pci/common.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 305c68b..7ae6671 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -628,7 +628,7 @@ int pcibios_add_device(struct pci_dev *dev)
>
> pa_data = boot_params.hdr.setup_data;
> while (pa_data) {
> - data = phys_to_virt(pa_data);
> + data = early_ioremap(pa_data, sizeof(*rom));

pcibios_add_device() is mostly called at boot-time, when
early_ioremap() probably works well. But it's also called when we
hot-add devices later, and it looks like early_ioremap() will then
generate warnings because "system_state != SYSTEM_BOOTING".

> if (data->type == SETUP_PCI) {
> rom = (struct pci_setup_rom *)data;
> @@ -645,6 +645,7 @@ int pcibios_add_device(struct pci_dev *dev)
> }
> }
> pa_data = data->next;
> + early_iounmap(data, sizeof(*rom));
> }
> return 0;
> }
> --
> 1.8.1.4
>

2013-05-28 11:36:22

by Matt Fleming

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Fri, 24 May, at 08:38:10AM, Bjorn Helgaas wrote:
> pcibios_add_device() is mostly called at boot-time, when
> early_ioremap() probably works well. But it's also called when we
> hot-add devices later, and it looks like early_ioremap() will then
> generate warnings because "system_state != SYSTEM_BOOTING".

Oops. Good point. Is there any reason we can't use ioremap()?

--
Matt Fleming, Intel Open Source Technology Center

2013-05-28 16:32:14

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Tue, May 28, 2013 at 5:36 AM, Matt Fleming <[email protected]> wrote:
> On Fri, 24 May, at 08:38:10AM, Bjorn Helgaas wrote:
>> pcibios_add_device() is mostly called at boot-time, when
>> early_ioremap() probably works well. But it's also called when we
>> hot-add devices later, and it looks like early_ioremap() will then
>> generate warnings because "system_state != SYSTEM_BOOTING".
>
> Oops. Good point. Is there any reason we can't use ioremap()?

I assume Matthew had some reason for avoiding that in the first place,
so I hope he'll chime in.

2013-05-28 16:48:17

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Tue, May 28, 2013 at 10:31:51AM -0600, Bjorn Helgaas wrote:
> On Tue, May 28, 2013 at 5:36 AM, Matt Fleming <[email protected]> wrote:
> > On Fri, 24 May, at 08:38:10AM, Bjorn Helgaas wrote:
> >> pcibios_add_device() is mostly called at boot-time, when
> >> early_ioremap() probably works well. But it's also called when we
> >> hot-add devices later, and it looks like early_ioremap() will then
> >> generate warnings because "system_state != SYSTEM_BOOTING".
> >
> > Oops. Good point. Is there any reason we can't use ioremap()?
>
> I assume Matthew had some reason for avoiding that in the first place,
> so I hope he'll chime in.

I have no recollection of why I did it that way. If this is all
happening late enough for ioremap() to work then that sounds fine.

--
Matthew Garrett | [email protected]

2013-05-28 17:29:17

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Tue, May 28, 2013 at 10:48 AM, Matthew Garrett <[email protected]> wrote:
> On Tue, May 28, 2013 at 10:31:51AM -0600, Bjorn Helgaas wrote:
>> On Tue, May 28, 2013 at 5:36 AM, Matt Fleming <[email protected]> wrote:
>> > On Fri, 24 May, at 08:38:10AM, Bjorn Helgaas wrote:
>> >> pcibios_add_device() is mostly called at boot-time, when
>> >> early_ioremap() probably works well. But it's also called when we
>> >> hot-add devices later, and it looks like early_ioremap() will then
>> >> generate warnings because "system_state != SYSTEM_BOOTING".
>> >
>> > Oops. Good point. Is there any reason we can't use ioremap()?
>>
>> I assume Matthew had some reason for avoiding that in the first place,
>> so I hope he'll chime in.
>
> I have no recollection of why I did it that way. If this is all
> happening late enough for ioremap() to work then that sounds fine.

OK, hopefully somebody will do the analysis to verify that ioremap()
is appropriate in both cases. It seems likely, but I haven't looked
in detail.

Bjorn

2013-06-05 14:15:52

by Matt Fleming

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Tue, 28 May, at 11:28:54AM, Bjorn Helgaas wrote:
> OK, hopefully somebody will do the analysis to verify that ioremap()
> is appropriate in both cases. It seems likely, but I haven't looked
> in detail.

Switching from early_ioremap() to ioremap() seems to work fine from the
testing I've seen. I've included the new patch below. Would you prefer a
proper submission?

---

>From 7a82fbe1d0c74533162487fa1e4bc23877a8a502 Mon Sep 17 00:00:00 2001
From: Matt Fleming <[email protected]>
Date: Wed, 22 May 2013 09:56:23 +0100
Subject: [PATCH] x86/PCI: setup data may be in highmem

pcibios_add_device() assumes that the physical addresses stored in
setup_data are accessible via the direct kernel mapping, and that
calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
where the direct mapping range is much smaller than on x86-64.

Calling phys_to_virt() on a highmem address results in the following,

BUG: unable to handle kernel paging request at 39a3c198
IP: [<c262be0f>] pcibios_add_device+0x2f/0x90
*pde = 00000000
Oops: 0000 [#1] SMP
Modules linked in:
Pid: 1, comm: swapper/0 Tainted: G W I 3.9.0-rc2+ #280
EIP: 0060:[<c262be0f>] EFLAGS: 00010206 CPU: 1
EIP is at pcibios_add_device+0x2f/0x90
EAX: f6258800 EBX: f6258800 ECX: 79a3c190 EDX: 39a3c190
ESI: f62d9814 EDI: f6258864 EBP: f60add38 ESP: f60add2c
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 8005003b CR2: 39a3c198 CR3: 02b91000 CR4: 001007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process swapper/0 (pid: 1, ti=f60ac000 task=f60b0000 task.ti=f60ac000)
Stack:
f6258800 f62d9814 f6258864 f60add4c c2370c73 00000000 f62d9800 00000000
f60add6c c274640b 0000ea60 f6258800 0f008086 f62d9800 f62d9800 00000000
f60add84 c2370d08 00000000 00000008 f62d9800 00000000 f60adda4 c2371904
Call Trace:
[<c2370c73>] pci_device_add+0xe3/0x130
[<c274640b>] pci_scan_single_device+0x8b/0xb0
[<c2370d08>] pci_scan_slot+0x48/0x100
[<c2371904>] pci_scan_child_bus+0x24/0xc0
[<c262a7b0>] pci_acpi_scan_root+0x2c0/0x490
[<c23b7203>] acpi_pci_root_add+0x312/0x42f
[<c23b29d7>] ? acpi_device_notify_fixed+0x1d/0x1d
[<c23b36a8>] acpi_bus_device_attach+0x77/0xdd
[<c23cb6be>] acpi_ns_walk_namespace+0xb1/0x163
[<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
[<c23cbd4e>] acpi_walk_namespace+0x7e/0xa8
[<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
[<c23b46e0>] acpi_bus_scan+0x9a/0xa6
[<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
[<c2b17ec9>] acpi_scan_init+0x51/0x144
[<c2b252a2>] ? pci_mmcfg_late_init+0x49/0x4b
[<c2b17cdc>] acpi_init+0x224/0x28c
[<c2001144>] do_one_initcall+0x34/0x170
[<c2b17ab8>] ? acpi_sleep_proc_init+0x2e/0x2e
[<c2aeeb83>] kernel_init_freeable+0x119/0x1b6
[<c2aee4da>] ? do_early_param+0x74/0x74
[<c2743f10>] kernel_init+0x10/0xd0
[<c2765697>] ret_from_kernel_thread+0x1b/0x28
[<c2743f00>] ? rest_init+0x60/0x60

The most reliable way to trigger this crash seems to be booting a 32-bit
kernel via the EFI boot stub.

The solution is to use ioremap() instead of phys_to_virt() to map the
setup data into the kernel address space.

Tested-by: Jani Nikula <[email protected]>
Cc: Matthew Garrett <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Seth Forshee <[email protected]>
Cc: Jesse Barnes <[email protected]>
Cc: <[email protected]>
Signed-off-by: Matt Fleming <[email protected]>
---
arch/x86/pci/common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 305c68b..981c2db 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -628,7 +628,9 @@ int pcibios_add_device(struct pci_dev *dev)

pa_data = boot_params.hdr.setup_data;
while (pa_data) {
- data = phys_to_virt(pa_data);
+ data = ioremap(pa_data, sizeof(*rom));
+ if (!data)
+ return -ENOMEM;

if (data->type == SETUP_PCI) {
rom = (struct pci_setup_rom *)data;
@@ -645,6 +647,7 @@ int pcibios_add_device(struct pci_dev *dev)
}
}
pa_data = data->next;
+ iounmap(data);
}
return 0;
}
--
1.8.1.4

--
Matt Fleming, Intel Open Source Technology Center

2013-06-05 17:10:50

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Wed, Jun 5, 2013 at 8:15 AM, Matt Fleming <[email protected]> wrote:
> On Tue, 28 May, at 11:28:54AM, Bjorn Helgaas wrote:
>> OK, hopefully somebody will do the analysis to verify that ioremap()
>> is appropriate in both cases. It seems likely, but I haven't looked
>> in detail.
>
> Switching from early_ioremap() to ioremap() seems to work fine from the
> testing I've seen. I've included the new patch below. Would you prefer a
> proper submission?

This is fine, thanks. I applied this to my for-linus branch and will
ask Linus to pull it for v3.10.

> From 7a82fbe1d0c74533162487fa1e4bc23877a8a502 Mon Sep 17 00:00:00 2001
> From: Matt Fleming <[email protected]>
> Date: Wed, 22 May 2013 09:56:23 +0100
> Subject: [PATCH] x86/PCI: setup data may be in highmem
>
> pcibios_add_device() assumes that the physical addresses stored in
> setup_data are accessible via the direct kernel mapping, and that
> calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
> where the direct mapping range is much smaller than on x86-64.
>
> Calling phys_to_virt() on a highmem address results in the following,
>
> BUG: unable to handle kernel paging request at 39a3c198
> IP: [<c262be0f>] pcibios_add_device+0x2f/0x90
> *pde = 00000000
> Oops: 0000 [#1] SMP
> Modules linked in:
> Pid: 1, comm: swapper/0 Tainted: G W I 3.9.0-rc2+ #280
> EIP: 0060:[<c262be0f>] EFLAGS: 00010206 CPU: 1
> EIP is at pcibios_add_device+0x2f/0x90
> EAX: f6258800 EBX: f6258800 ECX: 79a3c190 EDX: 39a3c190
> ESI: f62d9814 EDI: f6258864 EBP: f60add38 ESP: f60add2c
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> CR0: 8005003b CR2: 39a3c198 CR3: 02b91000 CR4: 001007d0
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Process swapper/0 (pid: 1, ti=f60ac000 task=f60b0000 task.ti=f60ac000)
> Stack:
> f6258800 f62d9814 f6258864 f60add4c c2370c73 00000000 f62d9800 00000000
> f60add6c c274640b 0000ea60 f6258800 0f008086 f62d9800 f62d9800 00000000
> f60add84 c2370d08 00000000 00000008 f62d9800 00000000 f60adda4 c2371904
> Call Trace:
> [<c2370c73>] pci_device_add+0xe3/0x130
> [<c274640b>] pci_scan_single_device+0x8b/0xb0
> [<c2370d08>] pci_scan_slot+0x48/0x100
> [<c2371904>] pci_scan_child_bus+0x24/0xc0
> [<c262a7b0>] pci_acpi_scan_root+0x2c0/0x490
> [<c23b7203>] acpi_pci_root_add+0x312/0x42f
> [<c23b29d7>] ? acpi_device_notify_fixed+0x1d/0x1d
> [<c23b36a8>] acpi_bus_device_attach+0x77/0xdd
> [<c23cb6be>] acpi_ns_walk_namespace+0xb1/0x163
> [<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
> [<c23cbd4e>] acpi_walk_namespace+0x7e/0xa8
> [<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
> [<c23b46e0>] acpi_bus_scan+0x9a/0xa6
> [<c23b3631>] ? acpi_bus_type_and_status+0x82/0x82
> [<c2b17ec9>] acpi_scan_init+0x51/0x144
> [<c2b252a2>] ? pci_mmcfg_late_init+0x49/0x4b
> [<c2b17cdc>] acpi_init+0x224/0x28c
> [<c2001144>] do_one_initcall+0x34/0x170
> [<c2b17ab8>] ? acpi_sleep_proc_init+0x2e/0x2e
> [<c2aeeb83>] kernel_init_freeable+0x119/0x1b6
> [<c2aee4da>] ? do_early_param+0x74/0x74
> [<c2743f10>] kernel_init+0x10/0xd0
> [<c2765697>] ret_from_kernel_thread+0x1b/0x28
> [<c2743f00>] ? rest_init+0x60/0x60
>
> The most reliable way to trigger this crash seems to be booting a 32-bit
> kernel via the EFI boot stub.
>
> The solution is to use ioremap() instead of phys_to_virt() to map the
> setup data into the kernel address space.
>
> Tested-by: Jani Nikula <[email protected]>
> Cc: Matthew Garrett <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Seth Forshee <[email protected]>
> Cc: Jesse Barnes <[email protected]>
> Cc: <[email protected]>
> Signed-off-by: Matt Fleming <[email protected]>
> ---
> arch/x86/pci/common.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 305c68b..981c2db 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -628,7 +628,9 @@ int pcibios_add_device(struct pci_dev *dev)
>
> pa_data = boot_params.hdr.setup_data;
> while (pa_data) {
> - data = phys_to_virt(pa_data);
> + data = ioremap(pa_data, sizeof(*rom));
> + if (!data)
> + return -ENOMEM;
>
> if (data->type == SETUP_PCI) {
> rom = (struct pci_setup_rom *)data;
> @@ -645,6 +647,7 @@ int pcibios_add_device(struct pci_dev *dev)
> }
> }
> pa_data = data->next;
> + iounmap(data);
> }
> return 0;
> }
> --
> 1.8.1.4
>
> --
> Matt Fleming, Intel Open Source Technology Center

2013-06-08 11:12:33

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Wed, 2013-05-22 at 10:43 +0100, Matt Fleming wrote:
> From: Matt Fleming <[email protected]>
>
> pcibios_add_device() assumes that the physical addresses stored in
> setup_data are accessible via the direct kernel mapping, and that
> calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
> where the direct mapping range is much smaller than on x86-64.
>
> Calling phys_to_virt() on a highmem address results in the following,

Fixes paging request oops for me as well, thanks! Would you please add a
-stable tag to this patch?

--
Best Regards,
Artem Bityutskiy

2013-06-08 11:14:16

by Bityutskiy, Artem

[permalink] [raw]
Subject: Re: [PATCH] x86/PCI: setup data may be in highmem

On Sat, 2013-06-08 at 14:15 +0300, Artem Bityutskiy wrote:
> On Wed, 2013-05-22 at 10:43 +0100, Matt Fleming wrote:
> > From: Matt Fleming <[email protected]>
> >
> > pcibios_add_device() assumes that the physical addresses stored in
> > setup_data are accessible via the direct kernel mapping, and that
> > calling phys_to_virt() is valid. This isn't guaranteed to be true on x86
> > where the direct mapping range is much smaller than on x86-64.
> >
> > Calling phys_to_virt() on a highmem address results in the following,
>
> Fixes paging request oops for me as well, thanks! Would you please add a
> -stable tag to this patch?

Oh, pardon, you did CC -stable.

--
Best Regards,
Artem Bityutskiy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?