linux-kernel - Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 21 Aug 2017 22:31:26 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     "Kani, Toshimitsu" <toshi.kani@....com>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list()

On Mon, Aug 21, 2017 at 7:36 PM, Borislav Petkov <bp@...en8.de> wrote:
> On Mon, Aug 21, 2017 at 05:23:37PM +0000, Kani, Toshimitsu wrote:
>> > > 'data' here is private to the caller.  So, I do not think we need
>> > > to define the bits.  Shall I change the name to 'driver_data' to
>> > > make it more explicit?
>> >
>> > You changed it to 'data'. It was a u32-used-as-boolean
>> > is_critical_error before.
>> >
>> > So you can just as well make it into flags and people can extend
>> > those flags if needed. A flag bit should be enough in most cases
>> > anyway. If they really need driver_data, then they can add a void *
>> > member.
>>
>> Hmm.. In patch 2, intel_pstate_platform_pwr_mgmt_exists() uses this
>> field for PSS and PCC, which are enum values.  I think we should allow
>> drivers to set any values here.  I agree that it may need to be void *
>> if we also allow drivers to set a pointer here.
>
> Let's see what Rafael prefers.

I would retain the is_critical_error field and use that for printing
the recoverable / non-recoverable message.  This is kind of orthogonal
to whether or not any extra data is needed and that can be an
additional field.  In that case unsigned long should be sufficient to
accommodate a pointer if need be.

Thanks,
Rafael