Hi,
Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
causes the following failure on APM mustang board(arm64) when
booting via UEFI and ACPI:
No valid GICC entries exist
ACPI: Failed to initialize GIC IRQ controller
Kernel panic - not syncing: No interrupt controller found.
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
Hardware name: APM X-Gene Mustang board (DT)
Call trace:
[<ffffffc000089b94>] dump_backtrace+0x0/0x12c
[<ffffffc000089cd0>] show_stack+0x10/0x1c
[<ffffffc0005fac18>] dump_stack+0x8c/0xdc
[<ffffffc0005f7218>] panic+0xe4/0x220
[<ffffffc00082631c>] init_IRQ+0x24/0x30
[<ffffffc00082486c>] start_kernel+0x274/0x3d8
---[ end Kernel panic - not syncing: No interrupt controller found.
Thanks,
Ming
On 10/07/15 08:45, Ming Lei wrote:
> Hi,
>
> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
> causes the following failure on APM mustang board(arm64) when
> booting via UEFI and ACPI:
>
> No valid GICC entries exist
> ACPI: Failed to initialize GIC IRQ controller
> Kernel panic - not syncing: No interrupt controller found.
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
> Hardware name: APM X-Gene Mustang board (DT)
> Call trace:
> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
> [<ffffffc000089cd0>] show_stack+0x10/0x1c
> [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
> [<ffffffc0005f7218>] panic+0xe4/0x220
> [<ffffffc00082631c>] init_IRQ+0x24/0x30
> [<ffffffc00082486c>] start_kernel+0x274/0x3d8
> ---[ end Kernel panic - not syncing: No interrupt controller found.
Isn't that addressed by [1] which Catalin has queued for -rc2?
Thanks,
M.
[1] https://lkml.org/lkml/2015/7/6/876
--
Jazz is not dead. It just smells funny...
On Fri, Jul 10, 2015 at 1:28 PM, Marc Zyngier <[email protected]> wrote:
>
> On 10/07/15 08:45, Ming Lei wrote:
> > Hi,
> >
> > Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
> > causes the following failure on APM mustang board(arm64) when
> > booting via UEFI and ACPI:
> >
> > No valid GICC entries exist
> > ACPI: Failed to initialize GIC IRQ controller
> > Kernel panic - not syncing: No interrupt controller found.
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
> > Hardware name: APM X-Gene Mustang board (DT)
> > Call trace:
> > [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
> > [<ffffffc000089cd0>] show_stack+0x10/0x1c
> > [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
> > [<ffffffc0005f7218>] panic+0xe4/0x220
> > [<ffffffc00082631c>] init_IRQ+0x24/0x30
> > [<ffffffc00082486c>] start_kernel+0x274/0x3d8
> > ---[ end Kernel panic - not syncing: No interrupt controller found.
>
> Isn't that addressed by [1] which Catalin has queued for -rc2?
I am also seeing the same . But it fixes after I apply the parking
protocol patch but that is not upstreamed.
>
>
> Thanks,
>
> M.
>
> [1] https://lkml.org/lkml/2015/7/6/876
>
> --
> Jazz is not dead. It just smells funny...
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Thanks,
with regards,
Suman Tripathi
On 10/07/15 09:17, Suman Tripathi wrote:
> On Fri, Jul 10, 2015 at 1:28 PM, Marc Zyngier <[email protected]> wrote:
>>
>> On 10/07/15 08:45, Ming Lei wrote:
>>> Hi,
>>>
>>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>>> causes the following failure on APM mustang board(arm64) when
>>> booting via UEFI and ACPI:
>>>
>>> No valid GICC entries exist
>>> ACPI: Failed to initialize GIC IRQ controller
>>> Kernel panic - not syncing: No interrupt controller found.
>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
>>> Hardware name: APM X-Gene Mustang board (DT)
>>> Call trace:
>>> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
>>> [<ffffffc000089cd0>] show_stack+0x10/0x1c
>>> [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
>>> [<ffffffc0005f7218>] panic+0xe4/0x220
>>> [<ffffffc00082631c>] init_IRQ+0x24/0x30
>>> [<ffffffc00082486c>] start_kernel+0x274/0x3d8
>>> ---[ end Kernel panic - not syncing: No interrupt controller found.
>>
>> Isn't that addressed by [1] which Catalin has queued for -rc2?
>
>
> I am also seeing the same . But it fixes after I apply the parking
> protocol patch but that is not upstreamed.
I'm more interested to find out if the patch I mentioned in my original
email fixes it or not. An additional dependency on something that is not
aimed for mainline yet doesn't really help.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On Fri, Jul 10, 2015 at 1:53 PM, Marc Zyngier <[email protected]> wrote:
> On 10/07/15 09:17, Suman Tripathi wrote:
>> On Fri, Jul 10, 2015 at 1:28 PM, Marc Zyngier <[email protected]> wrote:
>>>
>>> On 10/07/15 08:45, Ming Lei wrote:
>>>> Hi,
>>>>
>>>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>>>> causes the following failure on APM mustang board(arm64) when
>>>> booting via UEFI and ACPI:
>>>>
>>>> No valid GICC entries exist
>>>> ACPI: Failed to initialize GIC IRQ controller
>>>> Kernel panic - not syncing: No interrupt controller found.
>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
>>>> Hardware name: APM X-Gene Mustang board (DT)
>>>> Call trace:
>>>> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
>>>> [<ffffffc000089cd0>] show_stack+0x10/0x1c
>>>> [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
>>>> [<ffffffc0005f7218>] panic+0xe4/0x220
>>>> [<ffffffc00082631c>] init_IRQ+0x24/0x30
>>>> [<ffffffc00082486c>] start_kernel+0x274/0x3d8
>>>> ---[ end Kernel panic - not syncing: No interrupt controller found.
>>>
>>> Isn't that addressed by [1] which Catalin has queued for -rc2?
>>
>>
>> I am also seeing the same . But it fixes after I apply the parking
>> protocol patch but that is not upstreamed.
>
> I'm more interested to find out if the patch I mentioned in my original
> email fixes it or not. An additional dependency on something that is not
> aimed for mainline yet doesn't really help.
Yes the patch mentioned by you fixes the issue.
[1] https://lkml.org/lkml/2015/7/6/876
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
--
Thanks,
with regards,
Suman Tripathi
On Fri, Jul 10, 2015 at 4:23 PM, Marc Zyngier <[email protected]> wrote:
> On 10/07/15 09:17, Suman Tripathi wrote:
>> On Fri, Jul 10, 2015 at 1:28 PM, Marc Zyngier <[email protected]> wrote:
>>>
>>> On 10/07/15 08:45, Ming Lei wrote:
>>>> Hi,
>>>>
>>>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>>>> causes the following failure on APM mustang board(arm64) when
>>>> booting via UEFI and ACPI:
>>>>
>>>> No valid GICC entries exist
>>>> ACPI: Failed to initialize GIC IRQ controller
>>>> Kernel panic - not syncing: No interrupt controller found.
>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
>>>> Hardware name: APM X-Gene Mustang board (DT)
>>>> Call trace:
>>>> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
>>>> [<ffffffc000089cd0>] show_stack+0x10/0x1c
>>>> [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
>>>> [<ffffffc0005f7218>] panic+0xe4/0x220
>>>> [<ffffffc00082631c>] init_IRQ+0x24/0x30
>>>> [<ffffffc00082486c>] start_kernel+0x274/0x3d8
>>>> ---[ end Kernel panic - not syncing: No interrupt controller found.
>>>
>>> Isn't that addressed by [1] which Catalin has queued for -rc2?
>>
>>
>> I am also seeing the same . But it fixes after I apply the parking
>> protocol patch but that is not upstreamed.
>
> I'm more interested to find out if the patch I mentioned in my original
> email fixes it or not. An additional dependency on something that is not
> aimed for mainline yet doesn't really help.
Yes, Al's patches do fix the issue.
Thanks,
Ming
On 07/10/2015 03:58 PM, Marc Zyngier wrote:
> On 10/07/15 08:45, Ming Lei wrote:
>> Hi,
>>
>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>> causes the following failure on APM mustang board(arm64) when
>> booting via UEFI and ACPI:
>>
>> No valid GICC entries exist
>> ACPI: Failed to initialize GIC IRQ controller
>> Kernel panic - not syncing: No interrupt controller found.
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
>> Hardware name: APM X-Gene Mustang board (DT)
>> Call trace:
>> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
>> [<ffffffc000089cd0>] show_stack+0x10/0x1c
>> [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
>> [<ffffffc0005f7218>] panic+0xe4/0x220
>> [<ffffffc00082631c>] init_IRQ+0x24/0x30
>> [<ffffffc00082486c>] start_kernel+0x274/0x3d8
>> ---[ end Kernel panic - not syncing: No interrupt controller found.
>
> Isn't that addressed by [1] which Catalin has queued for -rc2?
>
> Thanks,
>
> M.
>
> [1] https://lkml.org/lkml/2015/7/6/876
Yes, already fixed by Al and Catalin already queued for
4.2-rc2.
Thanks
Hanjun
On 07/10/2015 06:11 PM, Ming Lei wrote:
> On Fri, Jul 10, 2015 at 4:23 PM, Marc Zyngier <[email protected]> wrote:
>> On 10/07/15 09:17, Suman Tripathi wrote:
>>> On Fri, Jul 10, 2015 at 1:28 PM, Marc Zyngier <[email protected]> wrote:
>>>>
>>>> On 10/07/15 08:45, Ming Lei wrote:
>>>>> Hi,
>>>>>
>>>>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>>>>> causes the following failure on APM mustang board(arm64) when
>>>>> booting via UEFI and ACPI:
>>>>>
>>>>> No valid GICC entries exist
>>>>> ACPI: Failed to initialize GIC IRQ controller
>>>>> Kernel panic - not syncing: No interrupt controller found.
>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45
>>>>> Hardware name: APM X-Gene Mustang board (DT)
>>>>> Call trace:
>>>>> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c
>>>>> [<ffffffc000089cd0>] show_stack+0x10/0x1c
>>>>> [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
>>>>> [<ffffffc0005f7218>] panic+0xe4/0x220
>>>>> [<ffffffc00082631c>] init_IRQ+0x24/0x30
>>>>> [<ffffffc00082486c>] start_kernel+0x274/0x3d8
>>>>> ---[ end Kernel panic - not syncing: No interrupt controller found.
>>>>
>>>> Isn't that addressed by [1] which Catalin has queued for -rc2?
>>>
>>>
>>> I am also seeing the same . But it fixes after I apply the parking
>>> protocol patch but that is not upstreamed.
>>
>> I'm more interested to find out if the patch I mentioned in my original
>> email fixes it or not. An additional dependency on something that is not
>> aimed for mainline yet doesn't really help.
>
> Yes, Al's patches do fix the issue.
I should have read ahead :)
Thanks
Hanjun
> -----Original Message-----
> From: Ming Lei [mailto:[email protected]]
> Sent: Friday, July 10, 2015 12:46 AM
> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
> Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner; Jason
> Cooper; Hanjun Guo
> Subject: ACPI: regression: Failed to initialize GIC IRQ controller
>
> Hi,
>
> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
> causes the following failure on APM mustang board(arm64) when booting via
> UEFI and ACPI:
I would be interested to know just what exactly about this change broke things.
Bob
>
> No valid GICC entries exist
> ACPI: Failed to initialize GIC IRQ controller Kernel panic - not syncing:
> No interrupt controller found.
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45 Hardware name:
> APM X-Gene Mustang board (DT) Call trace:
> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c [<ffffffc000089cd0>]
> show_stack+0x10/0x1c [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
> [<ffffffc0005f7218>] panic+0xe4/0x220 [<ffffffc00082631c>]
> init_IRQ+0x24/0x30 [<ffffffc00082486c>] start_kernel+0x274/0x3d8 ---[ end
> Kernel panic - not syncing: No interrupt controller found.
>
>
> Thanks,
> Ming
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Jul 10, 2015 at 10:28 PM, Moore, Robert <[email protected]> wrote:
>
>
>> -----Original Message-----
>> From: Ming Lei [mailto:[email protected]]
>> Sent: Friday, July 10, 2015 12:46 AM
>> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
>> Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner; Jason
>> Cooper; Hanjun Guo
>> Subject: ACPI: regression: Failed to initialize GIC IRQ controller
>>
>> Hi,
>>
>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>> causes the following failure on APM mustang board(arm64) when booting via
>> UEFI and ACPI:
>
>
> I would be interested to know just what exactly about this change broke things.
sizeof(struct acpi_madt_generic_interrupt)
It's nice that someone took a sizeof() on the struct -- so, one would hope that no code actually depended on a particular value, no?
> -----Original Message-----
> From: Ming Lei [mailto:[email protected]]
> Sent: Friday, July 10, 2015 7:43 AM
> To: Moore, Robert
> Cc: Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing List; linux-arm-
> kernel; Thomas Gleixner; Jason Cooper; Hanjun Guo
> Subject: Re: ACPI: regression: Failed to initialize GIC IRQ controller
>
> On Fri, Jul 10, 2015 at 10:28 PM, Moore, Robert <[email protected]>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ming Lei [mailto:[email protected]]
> >> Sent: Friday, July 10, 2015 12:46 AM
> >> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
> >> Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner;
> >> Jason Cooper; Hanjun Guo
> >> Subject: ACPI: regression: Failed to initialize GIC IRQ controller
> >>
> >> Hi,
> >>
> >> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT
> >> table.) causes the following failure on APM mustang board(arm64) when
> >> booting via UEFI and ACPI:
> >
> >
> > I would be interested to know just what exactly about this change broke
> things.
>
> sizeof(struct acpi_madt_generic_interrupt)
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 10/07/15 15:28, Moore, Robert wrote:
>
>
>> -----Original Message-----
>> From: Ming Lei [mailto:[email protected]]
>> Sent: Friday, July 10, 2015 12:46 AM
>> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
>> Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner; Jason
>> Cooper; Hanjun Guo
>> Subject: ACPI: regression: Failed to initialize GIC IRQ controller
>>
>> Hi,
>>
>> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
>> causes the following failure on APM mustang board(arm64) when booting via
>> UEFI and ACPI:
>
>
> I would be interested to know just what exactly about this change broke things.
The gory details are there: https://lkml.org/lkml/2015/7/6/876
Basically, some data structure (acpi_madt_generic_interrupt) grew by 4
bytes from ACPI 5.1 to 6.0, but BAD_MADT_ENTRY only knows about the new
size, and not the old one.
When booting on an old(er) version of ACPI, the interrupt controller
tables are ignore (not the right size), and the machine stops booting
very early.
Thanks,
M.
> Bob
>
>
>
>>
>> No valid GICC entries exist
>> ACPI: Failed to initialize GIC IRQ controller Kernel panic - not syncing:
>> No interrupt controller found.
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45 Hardware name:
>> APM X-Gene Mustang board (DT) Call trace:
>> [<ffffffc000089b94>] dump_backtrace+0x0/0x12c [<ffffffc000089cd0>]
>> show_stack+0x10/0x1c [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
>> [<ffffffc0005f7218>] panic+0xe4/0x220 [<ffffffc00082631c>]
>> init_IRQ+0x24/0x30 [<ffffffc00082486c>] start_kernel+0x274/0x3d8 ---[ end
>> Kernel panic - not syncing: No interrupt controller found.
>>
>>
>> Thanks,
>> Ming
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
Jazz is not dead. It just smells funny...
On Fri, Jul 10, 2015 at 03:28:36PM +0100, Moore, Robert wrote:
>
>
> > -----Original Message-----
> > From: Ming Lei [mailto:[email protected]]
> > Sent: Friday, July 10, 2015 12:46 AM
> > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
> > Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner; Jason
> > Cooper; Hanjun Guo
> > Subject: ACPI: regression: Failed to initialize GIC IRQ controller
> >
> > Hi,
> >
> > Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT table.)
> > causes the following failure on APM mustang board(arm64) when booting via
> > UEFI and ACPI:
>
>
> I would be interested to know just what exactly about this change broke things.
struct acpi_madt_generic_interrupt gained 4 bytes, 4 too many for ACPI
on ARM64 to boot with 5.1 tables.
>From now onwards we have to test acpica changes against the arm64 kernel
tree, this must not happen again.
See below:
https://lkml.org/lkml/2015/7/6/876
Thanks,
Lorenzo
> Bob
>
>
>
> >
> > No valid GICC entries exist
> > ACPI: Failed to initialize GIC IRQ controller Kernel panic - not syncing:
> > No interrupt controller found.
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.2.0-rc1+ #45 Hardware name:
> > APM X-Gene Mustang board (DT) Call trace:
> > [<ffffffc000089b94>] dump_backtrace+0x0/0x12c [<ffffffc000089cd0>]
> > show_stack+0x10/0x1c [<ffffffc0005fac18>] dump_stack+0x8c/0xdc
> > [<ffffffc0005f7218>] panic+0xe4/0x220 [<ffffffc00082631c>]
> > init_IRQ+0x24/0x30 [<ffffffc00082486c>] start_kernel+0x274/0x3d8 ---[ end
> > Kernel panic - not syncing: No interrupt controller found.
> >
> >
> > Thanks,
> > Ming
> N???????????????r??????y?????????b???X????????v???^???)??{.n???+????????????{????????????zX??????????????}???????????z???&j:+v?????????????????????zZ+??????+zf?????????h?????????~????????????i?????????z??? ???w??????????????????????&???)??f??????^j??y???m??????@A???a????????? 0??????h??????i
On Fri, Jul 10, 2015 at 03:45:32PM +0100, Moore, Robert wrote:
> It's nice that someone took a sizeof() on the struct -- so, one would hope that no code actually depended on a particular value, no?
Unfortunately that sizeof has been there forever (x86/ia64),
ia64 code ran into a similar issue, so the check was removed
to cope with lsapic MADT updates, see:
arch/ia64/kernel/acpi.c line 204
/*Skip BAD_MADT_ENTRY check, as lsapic size could vary */
Is checking the subtable length field against a static value
really worthwhile/suitable ?
Thanks,
Lorenzo
> > -----Original Message-----
> > From: Ming Lei [mailto:[email protected]]
> > Sent: Friday, July 10, 2015 7:43 AM
> > To: Moore, Robert
> > Cc: Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing List; linux-arm-
> > kernel; Thomas Gleixner; Jason Cooper; Hanjun Guo
> > Subject: Re: ACPI: regression: Failed to initialize GIC IRQ controller
> >
> > On Fri, Jul 10, 2015 at 10:28 PM, Moore, Robert <[email protected]>
> > wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Ming Lei [mailto:[email protected]]
> > >> Sent: Friday, July 10, 2015 12:46 AM
> > >> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
> > >> Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner;
> > >> Jason Cooper; Hanjun Guo
> > >> Subject: ACPI: regression: Failed to initialize GIC IRQ controller
> > >>
> > >> Hi,
> > >>
> > >> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT
> > >> table.) causes the following failure on APM mustang board(arm64) when
> > >> booting via UEFI and ACPI:
> > >
> > >
> > > I would be interested to know just what exactly about this change broke
> > things.
> >
> > sizeof(struct acpi_madt_generic_interrupt)
> N???????????????r??????y?????????b???X????????v???^???)??{.n???+????????????{????????????zX??????????????}???????????z???&j:+v?????????????????????zZ+??????+zf?????????h?????????~????????????i?????????z??? ???w??????????????????????&???)??f??????^j??y???m??????@A???a????????? 0??????h??????i
> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:[email protected]]
> Sent: Friday, July 10, 2015 8:18 AM
> To: Moore, Robert
> Cc: Ming Lei; Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing List;
> linux-arm-kernel; Thomas Gleixner; Jason Cooper; [email protected]
> Subject: Re: ACPI: regression: Failed to initialize GIC IRQ controller
>
> On Fri, Jul 10, 2015 at 03:45:32PM +0100, Moore, Robert wrote:
> > It's nice that someone took a sizeof() on the struct -- so, one would
> hope that no code actually depended on a particular value, no?
>
> Unfortunately that sizeof has been there forever (x86/ia64),
> ia64 code ran into a similar issue, so the check was removed to cope with
> lsapic MADT updates, see:
>
> arch/ia64/kernel/acpi.c line 204
>
> /*Skip BAD_MADT_ENTRY check, as lsapic size could vary */
>
> Is checking the subtable length field against a static value really
> worthwhile/suitable ?
>
I would at least traverse the subtables via the subtable length given in the table, and not use a sizeof() for each subtable. Then, multiple table/subtable versions are handled automatically; you don't have to use any new fields until necessary.
> Thanks,
> Lorenzo
>
> > > -----Original Message-----
> > > From: Ming Lei [mailto:[email protected]]
> > > Sent: Friday, July 10, 2015 7:43 AM
> > > To: Moore, Robert
> > > Cc: Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing List;
> > > linux-arm- kernel; Thomas Gleixner; Jason Cooper; Hanjun Guo
> > > Subject: Re: ACPI: regression: Failed to initialize GIC IRQ
> > > controller
> > >
> > > On Fri, Jul 10, 2015 at 10:28 PM, Moore, Robert
> > > <[email protected]>
> > > wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Ming Lei [mailto:[email protected]]
> > > >> Sent: Friday, July 10, 2015 12:46 AM
> > > >> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J
> > > >> Cc: Linux Kernel Mailing List; linux-arm-kernel; Thomas Gleixner;
> > > >> Jason Cooper; Hanjun Guo
> > > >> Subject: ACPI: regression: Failed to initialize GIC IRQ
> > > >> controller
> > > >>
> > > >> Hi,
> > > >>
> > > >> Commit 0cff8dc0099f6d4f(ACPICA: ACPI 6.0: Add changes for MADT
> > > >> table.) causes the following failure on APM mustang board(arm64)
> > > >> when booting via UEFI and ACPI:
> > > >
> > > >
> > > > I would be interested to know just what exactly about this change
> > > > broke
> > > things.
> > >
> > > sizeof(struct acpi_madt_generic_interrupt)
> >
> N???????????????r??????y?????????b???X????????v???^???)??{.n???+??????????
> ??{????????????zX??????????????}???????????z???&j:+v?????????
> ????????????zZ+??????+zf?????????h?????????~????????????i?????????z??? ???
> w??????????????????????&???)??f??????^j??y???m??????@A???a?????????
>
> 0??????h??????i
On Fri, Jul 10, 2015 at 04:47:34PM +0100, Moore, Robert wrote:
>
>
> > -----Original Message-----
> > From: Lorenzo Pieralisi [mailto:[email protected]]
> > Sent: Friday, July 10, 2015 8:18 AM
> > To: Moore, Robert
> > Cc: Ming Lei; Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing List;
> > linux-arm-kernel; Thomas Gleixner; Jason Cooper; [email protected]
> > Subject: Re: ACPI: regression: Failed to initialize GIC IRQ controller
> >
> > On Fri, Jul 10, 2015 at 03:45:32PM +0100, Moore, Robert wrote:
> > > It's nice that someone took a sizeof() on the struct -- so, one would
> > hope that no code actually depended on a particular value, no?
> >
> > Unfortunately that sizeof has been there forever (x86/ia64),
> > ia64 code ran into a similar issue, so the check was removed to cope with
> > lsapic MADT updates, see:
> >
> > arch/ia64/kernel/acpi.c line 204
> >
> > /*Skip BAD_MADT_ENTRY check, as lsapic size could vary */
> >
> > Is checking the subtable length field against a static value really
> > worthwhile/suitable ?
> >
>
> I would at least traverse the subtables via the subtable length given in the table, and not use a sizeof() for each subtable. Then, multiple table/subtable versions are handled automatically; you don't have to use any new fields until necessary.
I lost you here, sorry. You are describing how the subtable entries are
parsed in acpi_parse_entries, but that's not what we are debating here.
BAD_MADT_ENTRY checks the subtable length against the ACPICA MADT structs
sized through sizeof to determine if the length field is "correct", I do
not see how you can do it by traversing the tables (how can you determine
where a subtable _really_ ends or to put it differently how to check that
a subtable length is _really_ right ?).
Thanks,
Lorenzo
> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:[email protected]]
> Sent: Friday, July 10, 2015 10:03 AM
> To: Moore, Robert
> Cc: Ming Lei; Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing List;
> linux-arm-kernel; Thomas Gleixner; Jason Cooper; [email protected]
> Subject: Re: ACPI: regression: Failed to initialize GIC IRQ controller
>
> On Fri, Jul 10, 2015 at 04:47:34PM +0100, Moore, Robert wrote:
> >
> >
> > > -----Original Message-----
> > > From: Lorenzo Pieralisi [mailto:[email protected]]
> > > Sent: Friday, July 10, 2015 8:18 AM
> > > To: Moore, Robert
> > > Cc: Ming Lei; Zheng, Lv; Wysocki, Rafael J; Linux Kernel Mailing
> > > List; linux-arm-kernel; Thomas Gleixner; Jason Cooper;
> > > [email protected]
> > > Subject: Re: ACPI: regression: Failed to initialize GIC IRQ
> > > controller
> > >
> > > On Fri, Jul 10, 2015 at 03:45:32PM +0100, Moore, Robert wrote:
> > > > It's nice that someone took a sizeof() on the struct -- so, one
> > > > would
> > > hope that no code actually depended on a particular value, no?
> > >
> > > Unfortunately that sizeof has been there forever (x86/ia64),
> > > ia64 code ran into a similar issue, so the check was removed to cope
> > > with lsapic MADT updates, see:
> > >
> > > arch/ia64/kernel/acpi.c line 204
> > >
> > > /*Skip BAD_MADT_ENTRY check, as lsapic size could vary */
> > >
> > > Is checking the subtable length field against a static value really
> > > worthwhile/suitable ?
> > >
> >
> > I would at least traverse the subtables via the subtable length given in
> the table, and not use a sizeof() for each subtable. Then, multiple
> table/subtable versions are handled automatically; you don't have to use
> any new fields until necessary.
>
> I lost you here, sorry. You are describing how the subtable entries are
> parsed in acpi_parse_entries, but that's not what we are debating here.
> BAD_MADT_ENTRY checks the subtable length against the ACPICA MADT structs
> sized through sizeof to determine if the length field is "correct", I do
> not see how you can do it by traversing the tables (how can you determine
> where a subtable _really_ ends or to put it differently how to check that
> a subtable length is _really_ right ?).
Ok, this is not the actual traversal.
However, the traversal probably uses the subtable length entries already, or it least it should.
I think that the subtable lengths are one field that absolutely *must* be trusted, else lots of things would break. Perhaps the MADT does not have variable-length subtable fields (I forget), but many ACPI tables do.
Once you've got a subtable, you want to probable check it it is at least as long as you expect. Any longer should be treated as OK, since it probably contains fields that you don't know how to decipher anyway.
>
> Thanks,
> Lorenzo