All of lore.kernel.org
 help / color / mirror / Atom feed
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: ACPI: regression: Failed to initialize GIC IRQ controller
Date: Fri, 10 Jul 2015 18:02:54 +0100	[thread overview]
Message-ID: <20150710170254.GC14257@red-moon> (raw)
In-Reply-To: <94F2FBAB4432B54E8AACC7DFDE6C92E37D30B373@ORSMSX112.amr.corp.intel.com>

On Fri, Jul 10, 2015 at 04:47:34PM +0100, Moore, Robert wrote:
> 
> 
> > -----Original Message-----
> > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi at arm.com]
> > 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; hanjun.guo at linaro.org
> > 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

WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: "Moore, Robert" <robert.moore@intel.com>
Cc: Ming Lei <ming.lei@canonical.com>,
	"Zheng, Lv" <lv.zheng@intel.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	"hanjun.guo@linaro.org" <hanjun.guo@linaro.org>
Subject: Re: ACPI: regression: Failed to initialize GIC IRQ controller
Date: Fri, 10 Jul 2015 18:02:54 +0100	[thread overview]
Message-ID: <20150710170254.GC14257@red-moon> (raw)
In-Reply-To: <94F2FBAB4432B54E8AACC7DFDE6C92E37D30B373@ORSMSX112.amr.corp.intel.com>

On Fri, Jul 10, 2015 at 04:47:34PM +0100, Moore, Robert wrote:
> 
> 
> > -----Original Message-----
> > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com]
> > 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; hanjun.guo@linaro.org
> > 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

  reply	other threads:[~2015-07-10 17:02 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-10  7:45 ACPI: regression: Failed to initialize GIC IRQ controller Ming Lei
2015-07-10  7:45 ` Ming Lei
2015-07-10  7:58 ` Marc Zyngier
2015-07-10  7:58   ` Marc Zyngier
2015-07-10  8:17   ` Suman Tripathi
2015-07-10  8:17     ` Suman Tripathi
2015-07-10  8:23     ` Marc Zyngier
2015-07-10  8:23       ` Marc Zyngier
2015-07-10  9:37       ` Suman Tripathi
2015-07-10  9:37         ` Suman Tripathi
2015-07-10 10:11       ` Ming Lei
2015-07-10 10:11         ` Ming Lei
2015-07-10 11:10         ` Hanjun Guo
2015-07-10 11:10           ` Hanjun Guo
2015-07-10 10:49   ` Hanjun Guo
2015-07-10 10:49     ` Hanjun Guo
2015-07-10 14:28 ` Moore, Robert
2015-07-10 14:28   ` Moore, Robert
2015-07-10 14:43   ` Ming Lei
2015-07-10 14:43     ` Ming Lei
2015-07-10 14:45     ` Moore, Robert
2015-07-10 14:45       ` Moore, Robert
2015-07-10 15:17       ` Lorenzo Pieralisi
2015-07-10 15:17         ` Lorenzo Pieralisi
2015-07-10 15:47         ` Moore, Robert
2015-07-10 15:47           ` Moore, Robert
2015-07-10 17:02           ` Lorenzo Pieralisi [this message]
2015-07-10 17:02             ` Lorenzo Pieralisi
2015-07-10 17:07             ` Moore, Robert
2015-07-10 17:07               ` Moore, Robert
2015-07-10 14:47   ` Marc Zyngier
2015-07-10 14:47     ` Marc Zyngier
2015-07-10 14:48   ` Lorenzo Pieralisi
2015-07-10 14:48     ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150710170254.GC14257@red-moon \
    --to=lorenzo.pieralisi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.