2014-02-14 10:38:47

by Conrad Kostecki

[permalink] [raw]
Subject: [PATCH] x86: HPET force enable for Soekris net6501

Hello,
as the Soekris net6501 does not have any ACPI implementation, HPET won't get enabled.
This patch enables HPET on such platforms.

[ 0.430149] pci 0000:00:01.0: Force enabled HPET at 0xfed00000
[ 0.644838] HPET: 3 timers in total, 0 timers will be used for per-cpu timer

Original patch by Peter Neubauer, slightly modified by me.
-> http://www.mail-archive.com/[email protected]/msg06462.html

Cheers
Conrad

Signed-off-by: Peter Neubauer <[email protected]>
Signed-off-by: Conrad Kostecki <[email protected]>

--- a/arch/x86/kernel/quirks.c 2014-02-14 11:13:27.703432732 +0100
+++ b/arch/x86/kernel/quirks.c 2014-02-14 11:14:32.327496474 +0100
@@ -498,6 +498,25 @@ void force_hpet_resume(void)
}

/*
+ * Soekris net6501, based on Atom E6xx series, does not have ACPI.
+ * HPET should be force enabled on such platforms.
+ */
+static void e6xx_force_enable_hpet(struct pci_dev *dev)
+{
+ if (hpet_address || force_hpet_address)
+ return;
+
+ force_hpet_address = 0xFED00000;
+ force_hpet_resume_type = NONE_FORCE_HPET_RESUME;
+ dev_printk(KERN_DEBUG, &dev->dev, "Force enabled HPET at "
+ "0x%lx\n", force_hpet_address);
+ return;
+}
+
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_E6XX_CU,
+ e6xx_force_enable_hpet);
+
+/*
* HPET MSI on some boards (ATI SB700/SB800) has side effect on
* floppy DMA. Disable HPET MSI on such platforms.
* See erratum #27 (Misinterpreted MSI Requests May Result in
--- a/include/linux/pci_ids.h 2014-02-14 11:13:00.575408953 +0100
+++ b/include/linux/pci_ids.h 2014-02-14 11:13:37.819442066 +0100
@@ -2854,6 +2854,7 @@
#define PCI_DEVICE_ID_INTEL_82372FB_1 0x7601
#define PCI_DEVICE_ID_INTEL_SCH_LPC 0x8119
#define PCI_DEVICE_ID_INTEL_SCH_IDE 0x811a
+#define PCI_DEVICE_ID_INTEL_E6XX_CU 0x8183
#define PCI_DEVICE_ID_INTEL_ITC_LPC 0x8186
#define PCI_DEVICE_ID_INTEL_82454GX 0x84c4
#define PCI_DEVICE_ID_INTEL_82450GX 0x84c5


2014-02-14 17:47:12

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 02:23 AM, Conrad Kostecki wrote:
> Hello,
> as the Soekris net6501 does not have any ACPI implementation, HPET won't get enabled.
> This patch enables HPET on such platforms.
>
> [ 0.430149] pci 0000:00:01.0: Force enabled HPET at 0xfed00000
> [ 0.644838] HPET: 3 timers in total, 0 timers will be used for per-cpu timer
>
> Original patch by Peter Neubauer, slightly modified by me.
> -> http://www.mail-archive.com/[email protected]/msg06462.html
>
> Cheers
> Conrad

Are you sure this won't break any other E6xx platforms?

-hpa

2014-02-14 18:06:54

by Conrad Kostecki

[permalink] [raw]
Subject: AW: [PATCH] x86: HPET force enable for Soekris net6501

> On 02/14/2014 02:23 AM, Conrad Kostecki wrote:
> > Hello,
> > as the Soekris net6501 does not have any ACPI implementation, HPET
> won't get enabled.
> > This patch enables HPET on such platforms.
> >
> > [ 0.430149] pci 0000:00:01.0: Force enabled HPET at 0xfed00000
> > [ 0.644838] HPET: 3 timers in total, 0 timers will be used for per-cpu timer
> >
> > Original patch by Peter Neubauer, slightly modified by me.
> > -> http://www.mail-archive.com/soekris-
> [email protected]/msg06462.html
> >
> > Cheers
> > Conrad
>
> Are you sure this won't break any other E6xx platforms?

Hm. I am not. The Soekris is my only device with an E6xx CPU. This one is special, as Soekris does not implement ACPI.
I don't know, how other E6xx systems do work. I guess, there will have ACPI. There seem not to be many out there.
This patch is now running for about six months without any problems for me.

I was also searching for some other systems. I found here only one manual.
There are at least the same vendor/device id shown, as used also in Soekris.
-> http://www.ekf.de/p/pc2/pc2_ug.pdf

Is there maybe any possibility to check for the specific Soekris model? At least MPTABLE identifies the Soekris:
[ 0.000000] MPTABLE: OEM ID: Soekris
[ 0.000000] MPTABLE: Product ID: net6501
[ 0.000000] MPTABLE: APIC at: 0xFEE00000

Cheers
Conrad

2014-02-14 18:09:17

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 10:06 AM, Conrad Kostecki wrote:
>
> Hm. I am not. The Soekris is my only device with an E6xx CPU. This one is special, as Soekris does not implement ACPI.
> I don't know, how other E6xx systems do work. I guess, there will have ACPI. There seem not to be many out there.
> This patch is now running for about six months without any problems for me.
>
> I was also searching for some other systems. I found here only one manual.
> There are at least the same vendor/device id shown, as used also in Soekris.
> -> http://www.ekf.de/p/pc2/pc2_ug.pdf
>
> Is there maybe any possibility to check for the specific Soekris model? At least MPTABLE identifies the Soekris:
> [ 0.000000] MPTABLE: OEM ID: Soekris
> [ 0.000000] MPTABLE: Product ID: net6501
> [ 0.000000] MPTABLE: APIC at: 0xFEE00000
>

Does it have DMI?

-hpa

2014-02-14 18:13:29

by Conrad Kostecki

[permalink] [raw]
Subject: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

> On 02/14/2014 10:06 AM, Conrad Kostecki wrote:
> >
> > Hm. I am not. The Soekris is my only device with an E6xx CPU. This one is
> special, as Soekris does not implement ACPI.
> > I don't know, how other E6xx systems do work. I guess, there will have
> ACPI. There seem not to be many out there.
> > This patch is now running for about six months without any problems for
> me.
> >
> > I was also searching for some other systems. I found here only one manual.
> > There are at least the same vendor/device id shown, as used also in
> Soekris.
> > -> http://www.ekf.de/p/pc2/pc2_ug.pdf
> >
> > Is there maybe any possibility to check for the specific Soekris model? At
> least MPTABLE identifies the Soekris:
> > [ 0.000000] MPTABLE: OEM ID: Soekris
> > [ 0.000000] MPTABLE: Product ID: net6501
> > [ 0.000000] MPTABLE: APIC at: 0xFEE00000
> >
>
> Does it have DMI?

Unfortunately not.

# dmidecode 2.12
# No SMBIOS nor DMI entry point found, sorry.

Cheers
Conrad

2014-02-14 18:17:09

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 10:13 AM, Conrad Kostecki wrote:
>>
>> Does it have DMI?
>
> Unfortunately not.
>
> # dmidecode 2.12
> # No SMBIOS nor DMI entry point found, sorry.
>

Sigh. Does anyone have contacts at Soekris who can complain about this
stuff?

-hpa

2014-02-14 18:21:15

by Thomas Gleixner

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Fri, 14 Feb 2014, H. Peter Anvin wrote:

> On 02/14/2014 10:13 AM, Conrad Kostecki wrote:
> >>
> >> Does it have DMI?
> >
> > Unfortunately not.
> >
> > # dmidecode 2.12
> > # No SMBIOS nor DMI entry point found, sorry.
> >
>
> Sigh. Does anyone have contacts at Soekris who can complain about this
> stuff?

Well, lots of embedded stuff comes with a crippled BIOS. And there is
often no way to get any BIOS fixes from the vendor through the
subvendors and supplier chain.

I wish we could just use devicetree for such cases and fix the crud
ourself.

Thanks,

tglx

2014-02-14 18:22:30

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 10:21 AM, Thomas Gleixner wrote:
> On Fri, 14 Feb 2014, H. Peter Anvin wrote:
>
>> On 02/14/2014 10:13 AM, Conrad Kostecki wrote:
>>>>
>>>> Does it have DMI?
>>>
>>> Unfortunately not.
>>>
>>> # dmidecode 2.12
>>> # No SMBIOS nor DMI entry point found, sorry.
>>>
>>
>> Sigh. Does anyone have contacts at Soekris who can complain about this
>> stuff?
>
> Well, lots of embedded stuff comes with a crippled BIOS. And there is
> often no way to get any BIOS fixes from the vendor through the
> subvendors and supplier chain.
>
> I wish we could just use devicetree for such cases and fix the crud
> ourself.
>

We'd have to identify the platform, which is the main problem. Right
now we support quirking for DMI or PCI, but I don't think we do for MPTABLE.

-hpa

2014-02-14 18:28:57

by Conrad Kostecki

[permalink] [raw]
Subject: AW: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

> On 02/14/2014 10:13 AM, Conrad Kostecki wrote:
> >>
> >> Does it have DMI?
> >
> > Unfortunately not.
> >
> > # dmidecode 2.12
> > # No SMBIOS nor DMI entry point found, sorry.
> >
>
> Sigh. Does anyone have contacts at Soekris who can complain about this
> stuff?

I don't think, that Soekris will fix this. No model of Soekris ever had implemented DMI.
Their BIOS (called comBIOS) is completely written by them. Output is via serial port only.
At least I know, that the technical engineers at Soekris respond on [email protected].

Maybe the patch could be extended, that HPET would be only enabled if there is no ACPI present?

2014-02-14 18:38:21

by Thomas Gleixner

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> On 02/14/2014 10:21 AM, Thomas Gleixner wrote:
> > I wish we could just use devicetree for such cases and fix the crud
> > ourself.
> >
>
> We'd have to identify the platform, which is the main problem. Right
> now we support quirking for DMI or PCI, but I don't think we do for MPTABLE.

My point is that device tree support for some basic stuff like
hpet/ioapic and such would allow people like Conrad to avoid the
stupid hackery of quirks.

Building your own DT requires to read a datasheet as does hacking a
quirk, but its definitely simpler. And we can collect the DTs for
known boards either in the kernel or in some external repository.

People who are dealing with embedded stuff are not those who are
frightened by datasheets and building a custom kernel with some extra
blob.

I bet Conrad is also stuck with PIC on the E6xx CPU and that's a major
PITA. I have such a board as well and it simply sucks.

Now you can't hack an ioapic quirk because that's way to complex, but
we have proven with the ce4100 that it is reasonably simple to get
that stuff working nicely when you can read a datasheet. If we could
generalize that for a few crucial devices that would help a lot.

When I asked the board vendor why there are no acpi tables in the
device, I got the answer, that this is an embedded board and the
"BIOS" built with BLDK does not support that. We all know that's not
true, but how does that help?

The people who brought up the initial target OS (WinCE) on that board
worked around the lack of ACPI by hacking HPET support into the CE
preloader and switched all device drivers to use MSI because CE failed
to handle the PIC properly. That avoided that they needed to hack the
ioapic into submission as well.

That's the sad reality. And we have to cope with these boards whether
we like it or not.

Thanks,

tglx







2014-02-14 18:40:01

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

We could also just add an ACPI table... same concept. Still need to find it.

On February 14, 2014 10:38:24 AM PST, Thomas Gleixner <[email protected]> wrote:
>On Fri, 14 Feb 2014, H. Peter Anvin wrote:
>> On 02/14/2014 10:21 AM, Thomas Gleixner wrote:
>> > I wish we could just use devicetree for such cases and fix the crud
>> > ourself.
>> >
>>
>> We'd have to identify the platform, which is the main problem. Right
>> now we support quirking for DMI or PCI, but I don't think we do for
>MPTABLE.
>
>My point is that device tree support for some basic stuff like
>hpet/ioapic and such would allow people like Conrad to avoid the
>stupid hackery of quirks.
>
>Building your own DT requires to read a datasheet as does hacking a
>quirk, but its definitely simpler. And we can collect the DTs for
>known boards either in the kernel or in some external repository.
>
>People who are dealing with embedded stuff are not those who are
>frightened by datasheets and building a custom kernel with some extra
>blob.
>
>I bet Conrad is also stuck with PIC on the E6xx CPU and that's a major
>PITA. I have such a board as well and it simply sucks.
>
>Now you can't hack an ioapic quirk because that's way to complex, but
>we have proven with the ce4100 that it is reasonably simple to get
>that stuff working nicely when you can read a datasheet. If we could
>generalize that for a few crucial devices that would help a lot.
>
>When I asked the board vendor why there are no acpi tables in the
>device, I got the answer, that this is an embedded board and the
>"BIOS" built with BLDK does not support that. We all know that's not
>true, but how does that help?
>
>The people who brought up the initial target OS (WinCE) on that board
>worked around the lack of ACPI by hacking HPET support into the CE
>preloader and switched all device drivers to use MSI because CE failed
>to handle the PIC properly. That avoided that they needed to hack the
>ioapic into submission as well.
>
>That's the sad reality. And we have to cope with these boards whether
>we like it or not.
>
>Thanks,
>
> tglx

--
Sent from my mobile phone. Please pardon brevity and lack of formatting.

2014-02-14 19:15:36

by Thomas Gleixner

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501



On Fri, 14 Feb 2014, H. Peter Anvin wrote:

> We could also just add an ACPI table... same concept. Still need to find it.

I'm fine with ACPI tables if we can provide simple means for embedded
users to load one via grub or just attach it to the kernel image.

Sure, the user needs to know how to prepare one, but for similar
platforms, e.g. e6xx based stuff the tables would look all the
same. We probably could just recycle those from the BLDK.

Thanks,

tglx

2014-02-14 19:27:02

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
>
>
> On Fri, 14 Feb 2014, H. Peter Anvin wrote:
>
>> We could also just add an ACPI table... same concept. Still need to find it.
>
> I'm fine with ACPI tables if we can provide simple means for embedded
> users to load one via grub or just attach it to the kernel image.

That already exists, see Documentation/acpi/initrd_table_override.txt.

> Sure, the user needs to know how to prepare one, but for similar
> platforms, e.g. e6xx based stuff the tables would look all the
> same. We probably could just recycle those from the BLDK.

Yes, that might be a good way to do it.

-hpa

2014-02-14 19:59:16

by Thomas Gleixner

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
> > I'm fine with ACPI tables if we can provide simple means for embedded
> > users to load one via grub or just attach it to the kernel image.
>
> That already exists, see Documentation/acpi/initrd_table_override.txt.

That requires, that you have already ACPI tables.

ACPI_SIG_RSDP cannot be overridden and that's the base table you need
to get ACPI going in the first place. So we need support for that and
probably for storing the tables at some non canonical place.

Thanks,

tglx

2014-02-14 20:06:40

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 11:59 AM, Thomas Gleixner wrote:
> On Fri, 14 Feb 2014, H. Peter Anvin wrote:
>> On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
>>> I'm fine with ACPI tables if we can provide simple means for embedded
>>> users to load one via grub or just attach it to the kernel image.
>>
>> That already exists, see Documentation/acpi/initrd_table_override.txt.
>
> That requires, that you have already ACPI tables.
>
> ACPI_SIG_RSDP cannot be overridden and that's the base table you need
> to get ACPI going in the first place. So we need support for that and
> probably for storing the tables at some non canonical place.
>

Well, the RSDP and RSDT/XSDT are nothing but pointers to other tables,
so if explicitly overridden I'm not sure if one actually would need
them. That doesn't mean our current code will work without them, though.

Thomas, Rafael, do you have any insights?

-hpa

2014-02-14 21:16:40

by Thomas Gleixner

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> On 02/14/2014 11:59 AM, Thomas Gleixner wrote:
> > On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> >> On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
> >>> I'm fine with ACPI tables if we can provide simple means for embedded
> >>> users to load one via grub or just attach it to the kernel image.
> >>
> >> That already exists, see Documentation/acpi/initrd_table_override.txt.
> >
> > That requires, that you have already ACPI tables.
> >
> > ACPI_SIG_RSDP cannot be overridden and that's the base table you need
> > to get ACPI going in the first place. So we need support for that and
> > probably for storing the tables at some non canonical place.
> >
>
> Well, the RSDP and RSDT/XSDT are nothing but pointers to other tables,
> so if explicitly overridden I'm not sure if one actually would need
> them. That doesn't mean our current code will work without them, though.

I tried once to overload all of the tables, but failed miserably in
the ACPI dungeon. RSDP was the major pain point IIRC.

Thanks,

tglx

2014-02-14 21:18:46

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 01:16 PM, Thomas Gleixner wrote:
>>
>> Well, the RSDP and RSDT/XSDT are nothing but pointers to other tables,
>> so if explicitly overridden I'm not sure if one actually would need
>> them. That doesn't mean our current code will work without them, though.
>
> I tried once to overload all of the tables, but failed miserably in
> the ACPI dungeon. RSDP was the major pain point IIRC.
>

I suspect we need to handle the RSDP and RSDT/XSDT specially, since,
again, they are really just pointers to where to find the other tables.
They are part of the discovery, not the data.

-hpa

2014-02-14 21:47:30

by Thomas Gleixner

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Fri, 14 Feb 2014, H. Peter Anvin wrote:

> On 02/14/2014 01:16 PM, Thomas Gleixner wrote:
> >>
> >> Well, the RSDP and RSDT/XSDT are nothing but pointers to other tables,
> >> so if explicitly overridden I'm not sure if one actually would need
> >> them. That doesn't mean our current code will work without them, though.
> >
> > I tried once to overload all of the tables, but failed miserably in
> > the ACPI dungeon. RSDP was the major pain point IIRC.
> >
>
> I suspect we need to handle the RSDP and RSDT/XSDT specially, since,
> again, they are really just pointers to where to find the other tables.
> They are part of the discovery, not the data.

I'm aware of that and I tried to hack around it but failed miserably
due to lack of masochism. It was simpler to abuse the DT stuff to get
things done. :)

Thanks,

tglx

2014-02-14 21:48:43

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/14/2014 01:47 PM, Thomas Gleixner wrote:
>
> I'm aware of that and I tried to hack around it but failed miserably
> due to lack of masochism. It was simpler to abuse the DT stuff to get
> things done. :)
>

Right... we should fix that, though.

-hpa

2014-02-17 16:28:11

by Thomas Renninger

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Friday, February 14, 2014 10:16:41 PM Thomas Gleixner wrote:
> On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> > On 02/14/2014 11:59 AM, Thomas Gleixner wrote:
> > > On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> > >> On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
> > >>> I'm fine with ACPI tables if we can provide simple means for embedded
> > >>> users to load one via grub or just attach it to the kernel image.
> > >>
> > >> That already exists, see Documentation/acpi/initrd_table_override.txt.
> > >
> > > That requires, that you have already ACPI tables.
> > >
> > > ACPI_SIG_RSDP cannot be overridden and that's the base table you need
> > > to get ACPI going in the first place. So we need support for that and
> > > probably for storing the tables at some non canonical place.
> >
> > Well, the RSDP and RSDT/XSDT are nothing but pointers to other tables,
> > so if explicitly overridden I'm not sure if one actually would need
> > them. That doesn't mean our current code will work without them, though.
>
> I tried once to overload all of the tables, but failed miserably in
> the ACPI dungeon. RSDP was the major pain point IIRC.

What exactly do you try to achieve?
I cannot imagine a use-case where RSDP and XSDT overriding would help you.

Have you tried the current mechanism to override tables?
What is missing and for what do you need it for?

I need more context, maybe I can help then.

Thomas

2014-02-17 17:19:41

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

What I gather is that they want to add tables where there are none, and that the ACPI code doesn't play along because there is no RSDP nor any RSDT/XSDT.

On February 17, 2014 8:28:05 AM PST, Thomas Renninger <[email protected]> wrote:
>On Friday, February 14, 2014 10:16:41 PM Thomas Gleixner wrote:
>> On Fri, 14 Feb 2014, H. Peter Anvin wrote:
>> > On 02/14/2014 11:59 AM, Thomas Gleixner wrote:
>> > > On Fri, 14 Feb 2014, H. Peter Anvin wrote:
>> > >> On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
>> > >>> I'm fine with ACPI tables if we can provide simple means for
>embedded
>> > >>> users to load one via grub or just attach it to the kernel
>image.
>> > >>
>> > >> That already exists, see
>Documentation/acpi/initrd_table_override.txt.
>> > >
>> > > That requires, that you have already ACPI tables.
>> > >
>> > > ACPI_SIG_RSDP cannot be overridden and that's the base table you
>need
>> > > to get ACPI going in the first place. So we need support for that
>and
>> > > probably for storing the tables at some non canonical place.
>> >
>> > Well, the RSDP and RSDT/XSDT are nothing but pointers to other
>tables,
>> > so if explicitly overridden I'm not sure if one actually would need
>> > them. That doesn't mean our current code will work without them,
>though.
>>
>> I tried once to overload all of the tables, but failed miserably in
>> the ACPI dungeon. RSDP was the major pain point IIRC.
>
>What exactly do you try to achieve?
>I cannot imagine a use-case where RSDP and XSDT overriding would help
>you.
>
>Have you tried the current mechanism to override tables?
>What is missing and for what do you need it for?
>
>I need more context, maybe I can help then.
>
> Thomas

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

2014-02-17 18:23:17

by Thomas Renninger

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Monday, February 17, 2014 09:19:03 AM H. Peter Anvin wrote:
> What I gather is that they want to add tables where there are none, and that
> the ACPI code doesn't play along because there is no RSDP nor any
> RSDT/XSDT.
Yep, this does currently not work.

Easiest I can think of instead of trying to modify RSDP or similar, is
to still pass the tables via unzipped, glued cpio which the kernel
can access early. The same way it is done for current ACPI table overriding
and early microcode passing.

Also find them via:
file = find_cpio_data(cpio_path, data, size, &offset);
(compare with drivers/acpi/osl.c)

and add them to (compare with drivers/acpi/acpica/acglobal.h):
/*
* acpi_gbl_root_table_list is the master list of ACPI tables that were
* found in the RSDT/XSDT.
*/
ACPI_EXTERN struct acpi_table_list acpi_gbl_root_table_list;

But right now, this is acpica internal only.
Most elegant way should be that ACPICA people would add another OS
callback:

acpi_status
acpi_os_physical_table_add(acpi_physical_address *address,
u32 *table_length));

which is called right after acpi_os_physical_table_override.

Implementation of it should be in osl.c again where it can easily
be tracked whether a table has been replaced already and need not to
be added, for example something like:
__initdata struct cpio_data overridden_tables[ACPI_OVERRIDE_TABLES];

ACPICA part should not be hard as well. It's some time ago, but it
may be a simple call to:
/* Add the table to the global root table list */

status = acpi_tb_store_table(table_desc->address, table_desc->pointer,
table_desc->length, table_desc->flags,
table_index);

if acpi_os_physical_table_add() has returned a valid address/length.
This would then add the table to acpica internal:

/*
* acpi_gbl_root_table_list is the master list of ACPI tables that were
* found in the RSDT/XSDT.
*/
ACPI_EXTERN struct acpi_table_list acpi_gbl_root_table_list;

and later all the tables in there get loaded and set up as if they
were passed from BIOS.

Thomas


> On February 17, 2014 8:28:05 AM PST, Thomas Renninger <[email protected]> wrote:
> >On Friday, February 14, 2014 10:16:41 PM Thomas Gleixner wrote:
> >> On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> >> > On 02/14/2014 11:59 AM, Thomas Gleixner wrote:
> >> > > On Fri, 14 Feb 2014, H. Peter Anvin wrote:
> >> > >> On 02/14/2014 11:15 AM, Thomas Gleixner wrote:
> >> > >>> I'm fine with ACPI tables if we can provide simple means for
> >
> >embedded
> >
> >> > >>> users to load one via grub or just attach it to the kernel
> >
> >image.
> >
> >> > >> That already exists, see
> >
> >Documentation/acpi/initrd_table_override.txt.
> >
> >> > > That requires, that you have already ACPI tables.
> >> > >
> >> > > ACPI_SIG_RSDP cannot be overridden and that's the base table you
> >
> >need
> >
> >> > > to get ACPI going in the first place. So we need support for that
> >
> >and
> >
> >> > > probably for storing the tables at some non canonical place.
> >> >
> >> > Well, the RSDP and RSDT/XSDT are nothing but pointers to other
> >
> >tables,
> >
> >> > so if explicitly overridden I'm not sure if one actually would need
> >> > them. That doesn't mean our current code will work without them,
> >
> >though.
> >
> >> I tried once to overload all of the tables, but failed miserably in
> >> the ACPI dungeon. RSDP was the major pain point IIRC.
> >
> >What exactly do you try to achieve?
> >I cannot imagine a use-case where RSDP and XSDT overriding would help
> >you.
> >
> >Have you tried the current mechanism to override tables?
> >What is missing and for what do you need it for?
> >
> >I need more context, maybe I can help then.
> >
> > Thomas

2014-02-17 18:50:37

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/17/2014 10:23 AM, Thomas Renninger wrote:
>
> Easiest I can think of instead of trying to modify RSDP or similar, is
> to still pass the tables via unzipped, glued cpio which the kernel
> can access early. The same way it is done for current ACPI table overriding
> and early microcode passing.
>

We shouldn't *need* an RSDP and RSDT/XSDT, though, since all they are
are pointers to the actual tables. Rather than hacking around it we
should probably just fix the fundamental problem.

-hpa

2014-02-17 19:25:25

by Thomas Renninger

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On Monday, February 17, 2014 10:47:50 AM H. Peter Anvin wrote:
> On 02/17/2014 10:23 AM, Thomas Renninger wrote:
> > Easiest I can think of instead of trying to modify RSDP or similar, is
> > to still pass the tables via unzipped, glued cpio which the kernel
> > can access early. The same way it is done for current ACPI table
> > overriding
> > and early microcode passing.
>
> We shouldn't *need* an RSDP and RSDT/XSDT, though, since all they are
> are pointers to the actual tables. Rather than hacking around it we
> should probably just fix the fundamental problem.

That is what I described.
If something similar what I wrote gets implemented, you can simply
add ACPI tables to initrd as described here:
Documentation/acpi/initrd_table_override.txt
and if BIOS already provides them they get overridden, otherwise
they get added.

Thomas

2014-02-17 19:43:04

by H. Peter Anvin

[permalink] [raw]
Subject: Re: AW: AW: [PATCH] x86: HPET force enable for Soekris net6501

On 02/17/2014 11:25 AM, Thomas Renninger wrote:
> On Monday, February 17, 2014 10:47:50 AM H. Peter Anvin wrote:
>> On 02/17/2014 10:23 AM, Thomas Renninger wrote:
>>> Easiest I can think of instead of trying to modify RSDP or similar, is
>>> to still pass the tables via unzipped, glued cpio which the kernel
>>> can access early. The same way it is done for current ACPI table
>>> overriding
>>> and early microcode passing.
>>
>> We shouldn't *need* an RSDP and RSDT/XSDT, though, since all they are
>> are pointers to the actual tables. Rather than hacking around it we
>> should probably just fix the fundamental problem.
>
> That is what I described.
> If something similar what I wrote gets implemented, you can simply
> add ACPI tables to initrd as described here:
> Documentation/acpi/initrd_table_override.txt
> and if BIOS already provides them they get overridden, otherwise
> they get added.
>

Right, that was the idea. Thomas Gleixner said it didn't work.

-hpa

2014-02-18 18:22:57

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 4/4] ACPI: Add new table signatures that can be overridden/added.

Signed-off-by: Thomas Renninger <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
drivers/acpi/osl.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 07439b4..deba2f0 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -38,6 +38,7 @@
#include <linux/delay.h>
#include <linux/workqueue.h>
#include <linux/nmi.h>
+# define ACPI_UNDEFINED_TABLES 1
#include <linux/acpi.h>
#include <linux/efi.h>
#include <linux/ioport.h>
@@ -559,7 +560,8 @@ static const char * const table_sigs[] = {
ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
- ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
+ ACPI_SIG_WDRT, ACPI_SIG_BGRT, ACPI_SIG_ATKG, ACPI_SIG_GSCI,
+ ACPI_SIG_IEIT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };

#define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
--
1.7.6.1

2014-02-18 18:22:55

by Thomas Renninger

[permalink] [raw]
Subject: ACPI: Also allow ACPI table adding via initrd not only overriding

The ACPICA patches have to go into separate acpica repository first.
It should also be checked in acpica whether the table signature the OS likes
to add already exists (from BIOS). In this case the table must not be added.

Worked here by trying to override a DSDT and addind a BGRT table.

Thomas

2014-02-18 18:23:36

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 3/4] ACPICA: Add BGRT signature to known signatures

In Linux there even exists a driver already making use of this table:
drivers/acpi/bgrt.c:MODULE_DESCRIPTION("BGRT boot graphic support");

Signed-off-by: Thomas Renninger <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
include/acpi/actbl2.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 094a906..9ed1c20 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -83,6 +83,7 @@
#define ACPI_SIG_WDAT "WDAT" /* Watchdog Action Table */
#define ACPI_SIG_WDDT "WDDT" /* Watchdog Timer Description Table */
#define ACPI_SIG_WDRT "WDRT" /* Watchdog Resource Table */
+#define ACPI_SIG_BGRT "BGRT" /* Boot Graphic Support */

#ifdef ACPI_UNDEFINED_TABLES
/*
--
1.7.6.1

2014-02-18 18:23:35

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 2/4] ACPICA: Introduce new acpi_os_physical_table_add OS callback

This one allows OS to add arbitrary ACPI tables.

ToDo: It should get checked whether a table with the same signature already
exists and if this is the case, adding should not happen.

Signed-off-by: Thomas Renninger <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
drivers/acpi/acpica/tbutils.c | 24 ++++++++++++++++++++++++
include/acpi/acpiosxf.h | 6 ++++++
2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/acpica/tbutils.c b/drivers/acpi/acpica/tbutils.c
index 6412d3c..c763816 100644
--- a/drivers/acpi/acpica/tbutils.c
+++ b/drivers/acpi/acpica/tbutils.c
@@ -453,6 +453,8 @@ static acpi_status acpi_tb_validate_xsdt(acpi_physical_address xsdt_address)
*
******************************************************************************/

+#define ACPI_MAX_TABLE_ADD 64
+
acpi_status __init acpi_tb_parse_root_table(acpi_physical_address rsdp_address)
{
struct acpi_table_rsdp *rsdp;
@@ -623,5 +625,27 @@ acpi_status __init acpi_tb_parse_root_table(acpi_physical_address rsdp_address)
}
}

+ /*
+ * ACPI Table Add:
+ * Allow the OS to add additional tables to the global root table list
+ */
+ for (i = 0; i < ACPI_MAX_TABLE_ADD; i++) {
+ int tmp;
+ table_entry_size = 0;
+ address = 0;
+ status = acpi_os_physical_table_add(&address,
+ &table_entry_size);
+ if (status == AE_OK && table_entry_size && address) {
+ table = acpi_os_map_memory(address, table_entry_size);
+ ACPI_INFO((AE_INFO, "Add OS provided table:"));
+ acpi_tb_print_table_header(address, table);
+ status = acpi_tb_store_table(address,
+ table,
+ table_entry_size,
+ ACPI_TABLE_ORIGIN_MAPPED,
+ &tmp);
+ } else
+ break;
+ }
return_ACPI_STATUS(AE_OK);
}
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 01e6c6d..70c00ed 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -111,6 +111,12 @@ acpi_os_physical_table_override(struct acpi_table_header *existing_table,
u32 *new_table_length);
#endif

+#ifndef ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_physical_table_add
+acpi_status
+acpi_os_physical_table_add(acpi_physical_address * new_address,
+ u32 *new_table_length);
+#endif
+
/*
* Spinlock primitives
*/
--
1.7.6.1

2014-02-18 18:23:32

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

This is done the same way as the previous ACPI physical table override
mechanism.
How to override or add tables via initrd, please look up:
Documentation/acpi/initrd_table_override.txt

SSDTs can only be overridden, not added.

Overriding only happens if the OEM id of the table header matches the one
with the BIOS provided one.
All table types (SSDTs are an exception), must only show up once.
So either you:
- Add a fresh new table for adding of which type (signature) none exists
in the BIOS -> OS ACPI table adding happens.
or
- Add a table which already exists in BIOS, but the OEM id must match the
one of the table of the same type (signature) that exists in BIOS already
-> OS ACPI table overriding happens
Typically one copies away the original ACPI table, disassembles,
modifies (for example adding debug strings), compiles it and provides
the table via initrd for overriding (will have the same OEM id).
But this is not necessary, one could also come up with a selfmade
table for overriding, by taking care that the signature and OEM id is
the same as the one provided by BIOS

In ACPI table overriding case you see in dmesg:
ACPI: Override [DSDT- BXDSDT], this is unsafe: tainting kernel
Disabling lock debugging due to kernel taint

In ACPI table adding case you see in dmesg (BGRT table got added):
ACPI: Add [BGRT-SLIC-WKS], this is unsafe: tainting kernel
ACPI: BGRT 000000007fffd1ba 000038 (v00 HPQOEM SLIC-WKS 01072009
INTL 20130823)

Signed-off-by: Thomas Renninger <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
drivers/acpi/osl.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 88 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index fc1aa79..07439b4 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -566,6 +566,8 @@ static const char * const table_sigs[] = {

#define ACPI_OVERRIDE_TABLES 64
static struct cpio_data __initdata acpi_initrd_files[ACPI_OVERRIDE_TABLES];
+/* Remember physical address of overriden or added tables */
+static acpi_physical_address acpi_table_overridden[ACPI_OVERRIDE_TABLES];

#define MAP_CHUNK_SIZE (NR_FIX_BTMAPS << PAGE_SHIFT)

@@ -715,7 +717,7 @@ acpi_os_physical_table_override(struct acpi_table_header *existing_table,
*address = 0;
return AE_OK;
#else
- int table_offset = 0;
+ int no, table_offset = 0;
struct acpi_table_header *table;

*table_length = 0;
@@ -759,6 +761,12 @@ acpi_os_physical_table_override(struct acpi_table_header *existing_table,
*table_length = table->length;
acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
*address = acpi_tables_addr + table_offset;
+ for (no = 0; no < ACPI_OVERRIDE_TABLES; no++) {
+ if (acpi_table_overridden[no] == 0) {
+ acpi_table_overridden[no] = *address;
+ break;
+ }
+ }
break;
} while (table_offset + ACPI_HEADER_SIZE < all_tables_size);

@@ -768,6 +776,85 @@ acpi_os_physical_table_override(struct acpi_table_header *existing_table,
#endif
}

+acpi_status
+acpi_os_physical_table_add(acpi_physical_address *address,
+ u32 *table_length)
+{
+#ifndef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
+ *table_length = 0;
+ *address = 0;
+ return AE_OK;
+#else
+ int no, table_offset = 0;
+ struct acpi_table_header *table;
+
+ *table_length = 0;
+ *address = 0;
+
+ if (!acpi_tables_addr)
+ return AE_OK;
+
+ do {
+ if (table_offset + ACPI_HEADER_SIZE > all_tables_size) {
+ WARN_ON(1);
+ return AE_OK;
+ }
+
+ table = acpi_os_map_memory(acpi_tables_addr + table_offset,
+ ACPI_HEADER_SIZE);
+
+ if (table_offset + table->length > all_tables_size) {
+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
+ WARN_ON(1);
+ return AE_OK;
+ }
+
+ table_offset += table->length;
+
+ /* Do not add SSDTs for now, they might be intended to get
+ overridden when an SSDT gets loaded dynamically in ACPI
+ context at any time later */
+ if (!memcmp("SSDT", table->signature, 4)) {
+ acpi_os_unmap_memory(table,
+ ACPI_HEADER_SIZE);
+ continue;
+ }
+
+ /* Only add tables that have not been overridden already */
+ for (no = 0; no < ACPI_OVERRIDE_TABLES; no++) {
+ if (acpi_table_overridden[no] == 0)
+ break;
+ if (acpi_table_overridden[no] ==
+ acpi_tables_addr + table_offset - table->length)
+ break;
+ }
+ /* All tables have been added or overridden */
+ if (acpi_table_overridden[no] != 0) {
+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
+ continue;
+ }
+ /* Max table override/add limit reached */
+ if (no == ACPI_OVERRIDE_TABLES) {
+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
+ return AE_ERROR;
+ }
+
+ table_offset -= table->length;
+ *table_length = table->length;
+ *address = acpi_tables_addr + table_offset;
+ /* do not add this table again */
+ acpi_table_overridden[no] = *address;
+ pr_warn(PREFIX
+ "Add [%4.4s-%8.8s], this is unsafe: tainting kernel\n",
+ table->signature, table->oem_table_id);
+ add_taint(TAINT_OVERRIDDEN_ACPI_TABLE, LOCKDEP_NOW_UNRELIABLE);
+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
+ return AE_OK;
+ } while (table_offset + ACPI_HEADER_SIZE < all_tables_size);
+ return AE_OK;
+#endif
+}
+
static irqreturn_t acpi_irq(int irq, void *dev_id)
{
u32 handled;
--
1.7.6.1

2014-02-18 18:30:37

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

Why can't you add SSDTs? It would be particularly useful.

On February 18, 2014 10:22:40 AM PST, Thomas Renninger <[email protected]> wrote:
>This is done the same way as the previous ACPI physical table override
>mechanism.
>How to override or add tables via initrd, please look up:
>Documentation/acpi/initrd_table_override.txt
>
>SSDTs can only be overridden, not added.
>
>Overriding only happens if the OEM id of the table header matches the
>one
>with the BIOS provided one.
>All table types (SSDTs are an exception), must only show up once.
>So either you:
>- Add a fresh new table for adding of which type (signature) none
>exists
> in the BIOS -> OS ACPI table adding happens.
>or
>- Add a table which already exists in BIOS, but the OEM id must match
>the
>one of the table of the same type (signature) that exists in BIOS
>already
> -> OS ACPI table overriding happens
> Typically one copies away the original ACPI table, disassembles,
> modifies (for example adding debug strings), compiles it and provides
> the table via initrd for overriding (will have the same OEM id).
> But this is not necessary, one could also come up with a selfmade
> table for overriding, by taking care that the signature and OEM id is
> the same as the one provided by BIOS
>
>In ACPI table overriding case you see in dmesg:
> ACPI: Override [DSDT- BXDSDT], this is unsafe: tainting kernel
> Disabling lock debugging due to kernel taint
>
>In ACPI table adding case you see in dmesg (BGRT table got added):
> ACPI: Add [BGRT-SLIC-WKS], this is unsafe: tainting kernel
> ACPI: BGRT 000000007fffd1ba 000038 (v00 HPQOEM SLIC-WKS 01072009
> INTL 20130823)
>
>Signed-off-by: Thomas Renninger <[email protected]>
>CC: [email protected]
>CC: [email protected]
>CC: [email protected]
>CC: [email protected]
>CC: [email protected]
>CC: [email protected]
>CC: [email protected]
>CC: [email protected]
>---
>drivers/acpi/osl.c | 89
>+++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 88 insertions(+), 1 deletions(-)
>
>diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>index fc1aa79..07439b4 100644
>--- a/drivers/acpi/osl.c
>+++ b/drivers/acpi/osl.c
>@@ -566,6 +566,8 @@ static const char * const table_sigs[] = {
>
> #define ACPI_OVERRIDE_TABLES 64
>static struct cpio_data __initdata
>acpi_initrd_files[ACPI_OVERRIDE_TABLES];
>+/* Remember physical address of overriden or added tables */
>+static acpi_physical_address
>acpi_table_overridden[ACPI_OVERRIDE_TABLES];
>
> #define MAP_CHUNK_SIZE (NR_FIX_BTMAPS << PAGE_SHIFT)
>
>@@ -715,7 +717,7 @@ acpi_os_physical_table_override(struct
>acpi_table_header *existing_table,
> *address = 0;
> return AE_OK;
> #else
>- int table_offset = 0;
>+ int no, table_offset = 0;
> struct acpi_table_header *table;
>
> *table_length = 0;
>@@ -759,6 +761,12 @@ acpi_os_physical_table_override(struct
>acpi_table_header *existing_table,
> *table_length = table->length;
> acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
> *address = acpi_tables_addr + table_offset;
>+ for (no = 0; no < ACPI_OVERRIDE_TABLES; no++) {
>+ if (acpi_table_overridden[no] == 0) {
>+ acpi_table_overridden[no] = *address;
>+ break;
>+ }
>+ }
> break;
> } while (table_offset + ACPI_HEADER_SIZE < all_tables_size);
>
>@@ -768,6 +776,85 @@ acpi_os_physical_table_override(struct
>acpi_table_header *existing_table,
> #endif
> }
>
>+acpi_status
>+acpi_os_physical_table_add(acpi_physical_address *address,
>+ u32 *table_length)
>+{
>+#ifndef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
>+ *table_length = 0;
>+ *address = 0;
>+ return AE_OK;
>+#else
>+ int no, table_offset = 0;
>+ struct acpi_table_header *table;
>+
>+ *table_length = 0;
>+ *address = 0;
>+
>+ if (!acpi_tables_addr)
>+ return AE_OK;
>+
>+ do {
>+ if (table_offset + ACPI_HEADER_SIZE > all_tables_size) {
>+ WARN_ON(1);
>+ return AE_OK;
>+ }
>+
>+ table = acpi_os_map_memory(acpi_tables_addr + table_offset,
>+ ACPI_HEADER_SIZE);
>+
>+ if (table_offset + table->length > all_tables_size) {
>+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
>+ WARN_ON(1);
>+ return AE_OK;
>+ }
>+
>+ table_offset += table->length;
>+
>+ /* Do not add SSDTs for now, they might be intended to get
>+ overridden when an SSDT gets loaded dynamically in ACPI
>+ context at any time later */
>+ if (!memcmp("SSDT", table->signature, 4)) {
>+ acpi_os_unmap_memory(table,
>+ ACPI_HEADER_SIZE);
>+ continue;
>+ }
>+
>+ /* Only add tables that have not been overridden already */
>+ for (no = 0; no < ACPI_OVERRIDE_TABLES; no++) {
>+ if (acpi_table_overridden[no] == 0)
>+ break;
>+ if (acpi_table_overridden[no] ==
>+ acpi_tables_addr + table_offset - table->length)
>+ break;
>+ }
>+ /* All tables have been added or overridden */
>+ if (acpi_table_overridden[no] != 0) {
>+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
>+ continue;
>+ }
>+ /* Max table override/add limit reached */
>+ if (no == ACPI_OVERRIDE_TABLES) {
>+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
>+ return AE_ERROR;
>+ }
>+
>+ table_offset -= table->length;
>+ *table_length = table->length;
>+ *address = acpi_tables_addr + table_offset;
>+ /* do not add this table again */
>+ acpi_table_overridden[no] = *address;
>+ pr_warn(PREFIX
>+ "Add [%4.4s-%8.8s], this is unsafe: tainting kernel\n",
>+ table->signature, table->oem_table_id);
>+ add_taint(TAINT_OVERRIDDEN_ACPI_TABLE, LOCKDEP_NOW_UNRELIABLE);
>+ acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
>+ return AE_OK;
>+ } while (table_offset + ACPI_HEADER_SIZE < all_tables_size);
>+ return AE_OK;
>+#endif
>+}
>+
> static irqreturn_t acpi_irq(int irq, void *dev_id)
> {
> u32 handled;

--
Sent from my mobile phone. Please pardon brevity and lack of formatting.

2014-02-18 18:38:59

by Moore, Robert

[permalink] [raw]
Subject: RE: [Devel] ACPI: Also allow ACPI table adding via initrd not only overriding

acpi_load_table won't work?



> -----Original Message-----
> From: Devel [mailto:[email protected]] On Behalf Of Thomas
> Renninger
> Sent: Tuesday, February 18, 2014 10:23 AM
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected]
> Subject: [Devel] ACPI: Also allow ACPI table adding via initrd not only
> overriding
>
> The ACPICA patches have to go into separate acpica repository first.
> It should also be checked in acpica whether the table signature the OS
> likes to add already exists (from BIOS). In this case the table must not
> be added.
>
> Worked here by trying to override a DSDT and addind a BGRT table.
>
> Thomas
>
> _______________________________________________
> Devel mailing list
> [email protected]
> https://lists.acpica.org/mailman/listinfo/devel

2014-02-18 18:44:32

by Thomas Renninger

[permalink] [raw]
Subject: Re: [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

On Tuesday, February 18, 2014 10:27:23 AM H. Peter Anvin wrote:
> Why can't you add SSDTs? It would be particularly useful.

There are 2 ways how ACPI tables get added:
- Via pointer from a root table (XSDT or RSDT iirc)
- Via load statement inside of ACPI context when ACPI BIOS
code gets executed (iirc the physical address is passed).

The latter is only for SSDTs.
The problem is that you if you add an SSDT early, it might
have been intended for overriding when an SSDT gets dynamically
loaded later when the system is up which is particular useful as
well if you want to debug this specific BIOS table.

This could be workarounded via a boot param:
acpi=allow_ssdt_adding
But this is not nice. Maybe someone has a more elegant idea.
Something could still be added if someone is really needing this.

Thomas

2014-02-18 18:52:50

by Thomas Renninger

[permalink] [raw]
Subject: Re: [Devel] ACPI: Also allow ACPI table adding via initrd not only overriding

On Tuesday, February 18, 2014 06:38:45 PM Moore, Robert wrote:
> acpi_load_table won't work?

I guess not, there is:
/* Must acquire the interpreter lock during this operation */
status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);

So the interpreter must be running already?
That would mean the adding can only happen much later.
This probably would eleminate most use cases for adding arbitrary
ACPI tables. Most tables (beside DSDT and SSDT) are needed really
early, otherwise the info could have been added as AML as well.

Thomas

> > -----Original Message-----
> > From: Devel [mailto:[email protected]] On Behalf Of Thomas
> > Renninger
> > Sent: Tuesday, February 18, 2014 10:23 AM
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected]; [email protected]
> > Subject: [Devel] ACPI: Also allow ACPI table adding via initrd not only
> > overriding
> >
> > The ACPICA patches have to go into separate acpica repository first.
> > It should also be checked in acpica whether the table signature the OS
> > likes to add already exists (from BIOS). In this case the table must not
> > be added.
> >
> > Worked here by trying to override a DSDT and addind a BGRT table.
> >
> > Thomas
> >
> > _______________________________________________
> > Devel mailing list
> > [email protected]
> > https://lists.acpica.org/mailman/listinfo/devel

2014-02-18 19:59:20

by Moore, Robert

[permalink] [raw]
Subject: RE: [Devel] ACPI: Also allow ACPI table adding via initrd not only overriding

Maybe not exactly "running", but at the very least, ACPICA must be initialized.

All of this of course depends on how early the table needs to be loaded.


> -----Original Message-----
> From: Thomas Renninger [mailto:[email protected]]
> Sent: Tuesday, February 18, 2014 10:53 AM
> To: Moore, Robert
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [Devel] ACPI: Also allow ACPI table adding via initrd not
> only overriding
>
> On Tuesday, February 18, 2014 06:38:45 PM Moore, Robert wrote:
> > acpi_load_table won't work?
>
> I guess not, there is:
> /* Must acquire the interpreter lock during this operation */
> status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);
>
> So the interpreter must be running already?
> That would mean the adding can only happen much later.
> This probably would eleminate most use cases for adding arbitrary ACPI
> tables. Most tables (beside DSDT and SSDT) are needed really early,
> otherwise the info could have been added as AML as well.
>
> Thomas
>
> > > -----Original Message-----
> > > From: Devel [mailto:[email protected]] On Behalf Of Thomas
> > > Renninger
> > > Sent: Tuesday, February 18, 2014 10:23 AM
> > > Cc: [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]
> > > Subject: [Devel] ACPI: Also allow ACPI table adding via initrd not
> > > only overriding
> > >
> > > The ACPICA patches have to go into separate acpica repository first.
> > > It should also be checked in acpica whether the table signature the
> > > OS likes to add already exists (from BIOS). In this case the table
> > > must not be added.
> > >
> > > Worked here by trying to override a DSDT and addind a BGRT table.
> > >
> > > Thomas
> > >
> > > _______________________________________________
> > > Devel mailing list
> > > [email protected]
> > > https://lists.acpica.org/mailman/listinfo/devel

2014-02-18 20:53:42

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

On 02/18/2014 10:44 AM, Thomas Renninger wrote:
> On Tuesday, February 18, 2014 10:27:23 AM H. Peter Anvin wrote:
>> Why can't you add SSDTs? It would be particularly useful.
>
> There are 2 ways how ACPI tables get added:
> - Via pointer from a root table (XSDT or RSDT iirc)
> - Via load statement inside of ACPI context when ACPI BIOS
> code gets executed (iirc the physical address is passed).
>
> The latter is only for SSDTs.
> The problem is that you if you add an SSDT early, it might
> have been intended for overriding when an SSDT gets dynamically
> loaded later when the system is up which is particular useful as
> well if you want to debug this specific BIOS table.
>
> This could be workarounded via a boot param:
> acpi=allow_ssdt_adding
> But this is not nice. Maybe someone has a more elegant idea.
> Something could still be added if someone is really needing this.

Since adding SSDTs is one of the things I really can imagine one would
do, I think we need to figure out how to do that. I would think that
overriding would be the exception case.

-hpa

2014-02-19 11:14:44

by Thomas Renninger

[permalink] [raw]
Subject: Re: [Devel] ACPI: Also allow ACPI table adding via initrd not only overriding

On Tuesday, February 18, 2014 07:59:17 PM Moore, Robert wrote:
> Maybe not exactly "running", but at the very least, ACPICA must be
> initialized.
>
> All of this of course depends on how early the table needs to be loaded.

I'd say as early as possible.
Not sure about Thomas' use case.
I expect most of the extra tables are needed before ACPICA gets
initialized otherwise it could have been specified to put the info
into DSDT/SSDT via ASL.
Sooner or later people will hit the "I need it earlier" limit and
patch:
ACPICA: Introduce new acpi_os_physical_table_add OS callback
shows that it can be done early and that it integrates nicely
with only some additional lines.

Thomas

2014-02-19 11:23:03

by Thomas Renninger

[permalink] [raw]
Subject: Re: [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

On Tuesday, February 18, 2014 12:51:07 PM H. Peter Anvin wrote:
> On 02/18/2014 10:44 AM, Thomas Renninger wrote:
> > On Tuesday, February 18, 2014 10:27:23 AM H. Peter Anvin wrote:
> >> Why can't you add SSDTs? It would be particularly useful.
> >
> > There are 2 ways how ACPI tables get added:
> > - Via pointer from a root table (XSDT or RSDT iirc)
> > - Via load statement inside of ACPI context when ACPI BIOS
> >
> > code gets executed (iirc the physical address is passed).
> >
> > The latter is only for SSDTs.
> > The problem is that you if you add an SSDT early, it might
> > have been intended for overriding when an SSDT gets dynamically
> > loaded later when the system is up which is particular useful as
> > well if you want to debug this specific BIOS table.
> >
> > This could be workarounded via a boot param:
> > acpi=allow_ssdt_adding
> > But this is not nice. Maybe someone has a more elegant idea.
> > Something could still be added if someone is really needing this.
>
> Since adding SSDTs is one of the things I really can imagine one would
> do, I think we need to figure out how to do that. I would think that
> overriding would be the exception case.

You can always paste new code into the DSDT. It is effectively the same
than adding a new SSDT table.
The other way around, modifying or deleting broken code in a BIOS
provided SSDT needs overriding.

So in fact adding SSDTs is not necessary at all.
It would be a nice add-on, but it's not even worth introducing
an extra boot param or whatever confusion.
A hint in Documentation that adding additional ASL (ACPI Source Language)
code to the DSDT would be the same than creating and adding a new
SSDT table which is not possible should be enough.

The whole thing will always taint the kernel and is meant for
debugging only anyway.

Thomas

2014-02-19 13:02:59

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [Devel] ACPI: Also allow ACPI table adding via initrd not only overriding

On Wed, 19 Feb 2014, Thomas Renninger wrote:

> On Tuesday, February 18, 2014 07:59:17 PM Moore, Robert wrote:
> > Maybe not exactly "running", but at the very least, ACPICA must be
> > initialized.
> >
> > All of this of course depends on how early the table needs to be loaded.
>
> I'd say as early as possible.
> Not sure about Thomas' use case.

The case is a BIOS with complete lack of ACPI tables. So we have no
data about IOAPIC, HPET and other stuff.

The current acpi_initrd_override() call is sufficient, because its
called before acpi_boot_table_init(), which is the first function
accessing any of the tables.

Thanks,

tglx

2014-02-21 07:24:35

by Zheng, Lv

[permalink] [raw]
Subject: RE: [Devel] [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

Hi, Thomas

> From: Devel [mailto:[email protected]] On Behalf Of Thomas Renninger
> Sent: Wednesday, February 19, 2014 7:23 PM
>
> On Tuesday, February 18, 2014 12:51:07 PM H. Peter Anvin wrote:
> > On 02/18/2014 10:44 AM, Thomas Renninger wrote:
> > > On Tuesday, February 18, 2014 10:27:23 AM H. Peter Anvin wrote:
> > >> Why can't you add SSDTs? It would be particularly useful.
> > >
> > > There are 2 ways how ACPI tables get added:
> > > - Via pointer from a root table (XSDT or RSDT iirc)
> > > - Via load statement inside of ACPI context when ACPI BIOS
> > >
> > > code gets executed (iirc the physical address is passed).
> > >
> > > The latter is only for SSDTs.
> > > The problem is that you if you add an SSDT early, it might
> > > have been intended for overriding when an SSDT gets dynamically
> > > loaded later when the system is up which is particular useful as
> > > well if you want to debug this specific BIOS table.
> > >
> > > This could be workarounded via a boot param:
> > > acpi=allow_ssdt_adding
> > > But this is not nice. Maybe someone has a more elegant idea.
> > > Something could still be added if someone is really needing this.
> >
> > Since adding SSDTs is one of the things I really can imagine one would
> > do, I think we need to figure out how to do that. I would think that
> > overriding would be the exception case.
>
> You can always paste new code into the DSDT. It is effectively the same
> than adding a new SSDT table.
> The other way around, modifying or deleting broken code in a BIOS
> provided SSDT needs overriding.
>
> So in fact adding SSDTs is not necessary at all.
> It would be a nice add-on, but it's not even worth introducing
> an extra boot param or whatever confusion.
> A hint in Documentation that adding additional ASL (ACPI Source Language)
> code to the DSDT would be the same than creating and adding a new
> SSDT table which is not possible should be enough.

IMO, we need to load tables from RSDT/XSDT, they look like "static tables".
Given that we can live in the environment where table reloading is properly implemented by ACPICA, we won't face issues.
And the reloading feature is also required by ACPI specification.
If a table contains same "signature, oem_id, oem_table_id", it could be able to replace the old loaded one if the revision field is greater than the old one.

Actually I'm currently working on the parallel reloading support and all functionalities have been done.
This report is a bit hurry than I expected.
I'll try to prepare fixes (which seems to be dozens of patches) for the testers to validate.

Fortunately, specific to this bug, I don't think the reload requirement must be implemented as the new one doesn't contain a greater revision number.
So there might just be dead lock issues for this bug.

Thanks and best regards
-Lv

>
> The whole thing will always taint the kernel and is meant for
> debugging only anyway.
>
> Thomas
> _______________________________________________
> Devel mailing list
> [email protected]
> https://lists.acpica.org/mailman/listinfo/devel

2014-02-21 07:28:08

by Zheng, Lv

[permalink] [raw]
Subject: RE: [Devel] [PATCH 1/4] ACPI: Provide support for ACPI table adding via OS

Hi, Thomas

> From: Devel [mailto:[email protected]] On Behalf Of Thomas Renninger
> Sent: Wednesday, February 19, 2014 2:44 AM
>
> On Tuesday, February 18, 2014 10:27:23 AM H. Peter Anvin wrote:
> > Why can't you add SSDTs? It would be particularly useful.
>
> There are 2 ways how ACPI tables get added:
> - Via pointer from a root table (XSDT or RSDT iirc)
> - Via load statement inside of ACPI context when ACPI BIOS
> code gets executed (iirc the physical address is passed).
>
> The latter is only for SSDTs.
> The problem is that you if you add an SSDT early, it might
> have been intended for overriding when an SSDT gets dynamically
> loaded later when the system is up which is particular useful as
> well if you want to debug this specific BIOS table.
>
> This could be workarounded via a boot param:
> acpi=allow_ssdt_adding
> But this is not nice. Maybe someone has a more elegant idea.
> Something could still be added if someone is really needing this.

I'm not sure if you are talking about the issue that:
If a system booted using customized DSDT embedded with SSDT, it also requires dynamic SSDT loading be prevented in ACPICA.

Thanks and best regards
-Lv


>
> Thomas
> _______________________________________________
> Devel mailing list
> [email protected]
> https://lists.acpica.org/mailman/listinfo/devel