Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1370608 > unrolled thread

Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd

Started byOctavian Purdila <octavian.purdila@intel.com>
First post2016-04-04 15:10 +0200
Last post2016-04-06 08:20 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd Octavian Purdila <octavian.purdila@intel.com> - 2016-04-04 15:10 +0200
    RE: [RFC PATCH 02/10] acpi: install SSDT tables from initrd "Zheng, Lv" <lv.zheng@intel.com> - 2016-04-05 03:00 +0200
      Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd Octavian Purdila <octavian.purdila@intel.com> - 2016-04-05 09:30 +0200
        RE: [RFC PATCH 02/10] acpi: install SSDT tables from initrd "Zheng, Lv" <lv.zheng@intel.com> - 2016-04-06 08:20 +0200

#1370608 — Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd

FromOctavian Purdila <octavian.purdila@intel.com>
Date2016-04-04 15:10 +0200
SubjectRe: [RFC PATCH 02/10] acpi: install SSDT tables from initrd
Message-ID<rkcqu-3rR-21@gated-at.bofh.it>
On Fri, Apr 1, 2016 at 1:11 PM, Octavian Purdila
<octavian.purdila@intel.com> wrote:
> On Fri, Apr 1, 2016 at 8:05 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
>> Hi,
>>
>> IMO, there is already a similar function upstreamed:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c85cc81
>> Could it work for your use case?
>
> Yes, it is basically the same.
>
> The only difference is on how we handle taint. I think we should use a
> new taint for overlays and that we don't need to disable lockdep.
>
> BTW, why is lockdep disabled when we override?

The other thing I forgot to mention is that I think we should allow
installing new tables even if CONFIG_ACPI_INITRD_TABLE_OVERRIDE is not
selected. IMO the override and overlay functionality is different,
with the latter being more then a debug option.

I will prepare a patch for the next version of the series to decouple
installing new tables from CONFIG_ACPI_INITRD_TABLE_OVERRIDE.

[toc] | [next] | [standalone]


#1371072

From"Zheng, Lv" <lv.zheng@intel.com>
Date2016-04-05 03:00 +0200
Message-ID<rknvA-364-7@gated-at.bofh.it>
In reply to#1370608
Hi,

> From: Octavian Purdila [mailto:octavian.purdila@intel.com]
> Subject: Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd
> 
> On Fri, Apr 1, 2016 at 1:11 PM, Octavian Purdila
> <octavian.purdila@intel.com> wrote:
> > On Fri, Apr 1, 2016 at 8:05 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> >> Hi,
> >>
> >> IMO, there is already a similar function upstreamed:
> >>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c85c
> c81
> >> Could it work for your use case?
> >
> > Yes, it is basically the same.
> >
> > The only difference is on how we handle taint. I think we should use a
> > new taint for overlays and that we don't need to disable lockdep.
> >
> > BTW, why is lockdep disabled when we override?
> 
> The other thing I forgot to mention is that I think we should allow
> installing new tables even if CONFIG_ACPI_INITRD_TABLE_OVERRIDE is not
> selected. IMO the override and overlay functionality is different,
> with the latter being more then a debug option.
[Lv Zheng] 
I don't think so. The initrd override mechanism is not dependent on CONFIG_ACPI_DEBUG.
According to the spec, we can allow a higher versioning same table (same table sig, table id, table oem id) to override the old tables as a patching functionality.
So both the functionalities are not debug options and serve for the same purpose from this point of view.
And IMO that's why the initrd override mechanism needn't be dependent on CONFIG_ACPI_DEBUG.

I'm really OK with removing the acpi_table_taint() for CONFIG_ACPI_INITRD_TABLE_OVERRIDE but leaving some info messages indicating the table upgrades.
I don't think this mechanism is unsafe.
It happens during a initialization step occurring before the table is loaded and hence should be safe even the synchronization is not so robust in ACPICA.
And with the revision support added, we should be able to allow vendors to update the buggy tables.
That means the tables may be originated from the safe sources - the vendors.

> 
> I will prepare a patch for the next version of the series to decouple
> installing new tables from CONFIG_ACPI_INITRD_TABLE_OVERRIDE.
[Lv Zheng] 
I don't think they need to be decoupled.
The use case is:
If there is an ACPI table in initrd image and:
1. if the table's revision is higher than the existing one, override the existing one;
2. if the table is a brand new one, install it.

Thanks and best regards
-Lv

[toc] | [prev] | [next] | [standalone]


#1371282

FromOctavian Purdila <octavian.purdila@intel.com>
Date2016-04-05 09:30 +0200
Message-ID<rktB0-858-11@gated-at.bofh.it>
In reply to#1371072
On Tue, Apr 5, 2016 at 3:49 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> Hi,
>
>> From: Octavian Purdila [mailto:octavian.purdila@intel.com]
>> Subject: Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd
>>
>> On Fri, Apr 1, 2016 at 1:11 PM, Octavian Purdila
>> <octavian.purdila@intel.com> wrote:
>> > On Fri, Apr 1, 2016 at 8:05 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
>> >> Hi,
>> >>
>> >> IMO, there is already a similar function upstreamed:
>> >>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c85c
>> c81
>> >> Could it work for your use case?
>> >
>> > Yes, it is basically the same.
>> >
>> > The only difference is on how we handle taint. I think we should use a
>> > new taint for overlays and that we don't need to disable lockdep.
>> >
>> > BTW, why is lockdep disabled when we override?
>>
>> The other thing I forgot to mention is that I think we should allow
>> installing new tables even if CONFIG_ACPI_INITRD_TABLE_OVERRIDE is not
>> selected. IMO the override and overlay functionality is different,
>> with the latter being more then a debug option.
> [Lv Zheng]
> I don't think so. The initrd override mechanism is not dependent on CONFIG_ACPI_DEBUG.
> According to the spec, we can allow a higher versioning same table (same table sig, table id, table oem id) to override the old tables as a patching functionality.
> So both the functionalities are not debug options and serve for the same purpose from this point of view.
> And IMO that's why the initrd override mechanism needn't be dependent on CONFIG_ACPI_DEBUG.
>

The problem is that CONFIG_ACPI_INITRD_TABLE_OVERRIDE is presented as
a debug option in Documentation/initrd_table_override.txt and most
distributions are not selecting it which makes it hard to use it in
practice.

> I'm really OK with removing the acpi_table_taint() for CONFIG_ACPI_INITRD_TABLE_OVERRIDE but leaving some info messages indicating the table upgrades.
> I don't think this mechanism is unsafe.
> It happens during a initialization step occurring before the table is loaded and hence should be safe even the synchronization is not so robust in ACPICA.
> And with the revision support added, we should be able to allow vendors to update the buggy tables.
> That means the tables may be originated from the safe sources - the vendors.
>
>>
>> I will prepare a patch for the next version of the series to decouple
>> installing new tables from CONFIG_ACPI_INITRD_TABLE_OVERRIDE.
> [Lv Zheng]
> I don't think they need to be decoupled.
> The use case is:
> If there is an ACPI table in initrd image and:
> 1. if the table's revision is higher than the existing one, override the existing one;
> 2. if the table is a brand new one, install it.
>

The implementation will stay the same of course, I was just suggesting
to move CONFIG_ACPI_INITRD_TABLE_OVERRIDE in
acpi_os_physical_table_override to allow new tables to be loaded even
if the option is not selected.

[toc] | [prev] | [next] | [standalone]


#1372218

From"Zheng, Lv" <lv.zheng@intel.com>
Date2016-04-06 08:20 +0200
Message-ID<rkOYO-880-17@gated-at.bofh.it>
In reply to#1371282
Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Octavian Purdila
> Subject: Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd
> 
> On Tue, Apr 5, 2016 at 3:49 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> > Hi,
> >
> >> From: Octavian Purdila [mailto:octavian.purdila@intel.com]
> >> Subject: Re: [RFC PATCH 02/10] acpi: install SSDT tables from initrd
> >>
> >> On Fri, Apr 1, 2016 at 1:11 PM, Octavian Purdila
> >> <octavian.purdila@intel.com> wrote:
> >> > On Fri, Apr 1, 2016 at 8:05 AM, Zheng, Lv <lv.zheng@intel.com> wrote:
> >> >> Hi,
> >> >>
> >> >> IMO, there is already a similar function upstreamed:
> >> >>
> >>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=c85c
> >> c81
> >> >> Could it work for your use case?
> >> >
> >> > Yes, it is basically the same.
> >> >
> >> > The only difference is on how we handle taint. I think we should use a
> >> > new taint for overlays and that we don't need to disable lockdep.
> >> >
> >> > BTW, why is lockdep disabled when we override?
> >>
> >> The other thing I forgot to mention is that I think we should allow
> >> installing new tables even if CONFIG_ACPI_INITRD_TABLE_OVERRIDE is not
> >> selected. IMO the override and overlay functionality is different,
> >> with the latter being more then a debug option.
> > [Lv Zheng]
> > I don't think so. The initrd override mechanism is not dependent on
> CONFIG_ACPI_DEBUG.
> > According to the spec, we can allow a higher versioning same table (same
> table sig, table id, table oem id) to override the old tables as a patching
> functionality.
> > So both the functionalities are not debug options and serve for the same
> purpose from this point of view.
> > And IMO that's why the initrd override mechanism needn't be dependent on
> CONFIG_ACPI_DEBUG.
> >
> 
> The problem is that CONFIG_ACPI_INITRD_TABLE_OVERRIDE is presented as
> a debug option in Documentation/initrd_table_override.txt and most
> distributions are not selecting it which makes it hard to use it in
> practice.
> 
> > I'm really OK with removing the acpi_table_taint() for
> CONFIG_ACPI_INITRD_TABLE_OVERRIDE but leaving some info messages
> indicating the table upgrades.
> > I don't think this mechanism is unsafe.
> > It happens during a initialization step occurring before the table is loaded and
> hence should be safe even the synchronization is not so robust in ACPICA.
> > And with the revision support added, we should be able to allow vendors to
> update the buggy tables.
> > That means the tables may be originated from the safe sources - the vendors.
> >
> >>
> >> I will prepare a patch for the next version of the series to decouple
> >> installing new tables from CONFIG_ACPI_INITRD_TABLE_OVERRIDE.
> > [Lv Zheng]
> > I don't think they need to be decoupled.
> > The use case is:
> > If there is an ACPI table in initrd image and:
> > 1. if the table's revision is higher than the existing one, override the existing
> one;
> > 2. if the table is a brand new one, install it.
> >
> 
> The implementation will stay the same of course, I was just suggesting
> to move CONFIG_ACPI_INITRD_TABLE_OVERRIDE in
> acpi_os_physical_table_override to allow new tables to be loaded even
> if the option is not selected.
[Lv Zheng] 
This sounds reasonable.
Or you can rename it to CONFIG_ACPI_TABLE_UPGRADE and make it default 'y'.
Also you need to remove acpi_table_taint() which is not so useful now.

Thanks and best regards
-Lv

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web