linux-kernel - RE: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

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]

Message-ID: <DM6PR19MB26362B2A2CDFE73BE167FD34FA2E0@DM6PR19MB2636.namprd19.prod.outlook.com>
Date:   Tue, 1 Sep 2020 14:22:44 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@...l.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        Divya Bharathi <divya27392@...il.com>,
        "dvhart@...radead.org" <dvhart@...radead.org>
CC:     LKML <linux-kernel@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "Bharathi, Divya" <Divya.Bharathi@...l.com>,
        "Ksr, Prasanth" <Prasanth.Ksr@...l.com>
Subject: RE: [PATCH] Introduce support for Systems Management Driver over WMI
 for Dell Systems

>
> "A read-only attribute enumerating if a reboot is pending on any BIOS attribute
> change."
> does not really seem to make much sense. I guess what this is trying to say is:
>
> "This read-only attribute reads 1 if a reboot is necessary to apply pending BIOS
> attribute changes"?
>
>               0:      All BIOS attributes setting are current
>               1:      A reboot is necessary to get pending pending BIOS attribute
> changes applied
>
> Or some such. I'm not really happy with my own text either, but I think it
> better explains
> what this attribute is about then the original text, right ?

I think that text does read better, Divya and team will reword it.

<snip>

> > +           display_name_language_code:     A file that can be read to obtain
> > +           the language code corresponding to the "display_name" of the <attr>
>
> This needs to be specified better, e.g. this needs to say that this is an
> ISO 639‑1 language code (or some other language-code specification)

Ack.

>
>
> > +
> > +           modifier:       A file that can be read to obtain attribute-level
> > +           dependency rule which has to be met to configure <attr>
>
> What is the difference between modifier and value_modifier ? Also this need to
> be specified in more detail.

Ack.

>
> > +
> > +           possible_value: A file that can be read to obtain the possible
> > +           value of the <attr>
>
> This is an enum, so possible value_s_ ?  I assume that for a enum this will list
> all possible values, this also needs to specify how the possible values will be
> separated (e.g. using semi-colons or newlines or ...).

Yes correct.

>
>
> > +
> > +           value_modifier: A file that can be read to obtain value-level
> > +           dependency on a possible value which has to be met to configure
> <attr>
> > +
> > +What:              /sys/devices/platform/dell-wmi-
> sysman/attributes/integer/<attr>/
> > +Date:              October 2020
> > +KernelVersion:     5.9
> > +Contact:   Divya Bharathi <Divya.Bharathi@...l.com>,
> > +           Mario Limonciello <mario.limonciello@...l.com>,
> > +           Prasanth KSR <prasanth.ksr@...l.com>
> > +Description:
> > +           This directory exposes interfaces for interaction with
> > +           BIOS integer attributes.
> > +
> > +           Integer attributes are settings that accept a range of
> > +           numerical values for inputs. Each BIOS integer has a
> > +           lower bound and an upper bound on the values that it can take.
> > +
> > +           current_value:  A file that can be read to obtain the current
> > +           value of the <attr>
> > +
> > +           This file can also be written to in order to update
> > +           the value of an <attr>.
> > +
> > +           default_value:  A file that can be read to obtain the default
> > +           value of the <attr>
> > +
> > +           display_name:   A file that can be read to obtain a user friendly
> > +           description of the at <attr>
> > +
> > +           display_name_language_code:     A file that can be read to obtain
> > +           the language code corresponding to the "display_name" of the <attr>
> > +
> > +           lower_bound:    A file that can be read to obtain the lower
> > +           bound value of the <attr>
> > +
> > +           modifier:       A file that can be read to obtain attribute-level
> > +           dependency rule which has to be met to configure <attr>
> > +
> > +           scalar_increment:       A file that can be read to obtain the
> > +           resolution of the incremental value this attribute accepts.
> > +
> > +           upper_bound:    A file that can be read to obtain the upper
> > +           bound value of the <attr>
>
> Are these integers or also possibly floats? I guess possibly also floats, right?
> Then at a minimum this should specify which decimal-separator is used (I assume
> we will go with the usual '.' as decimal separator).

In practice they're integers, but I don't see why they couldn't be floats.