IPMI Sensor Read-only role updates.

[Emily Shaffer]

Okay, I guess I didn't understand that the proposal was for the MRW config
yaml, which is different from the config yaml which is sent to the mako
template :) Carry on.

On Tue, Feb 27, 2018 at 10:48 AM Ratan Gupta <ratagupt at linux.vnet.ibm.com>
wrote:

> Hi Emily,
>
> Minor correction inline.
>
> Regards
>
> Ratan
> On Tuesday 27 February 2018 09:06 PM, Emily Shaffer wrote:
>
> Hi Jayanath,
>
> Find my comments inline.
>
> It seems you may be thinking of only changes for a system specific yaml,
> in which case I don't think you need to ask for permission :) But the
> architecture you're discussing is already there today.
>
> Emily
>
> On Tue, Feb 27, 2018, 6:22 AM Ratan Gupta <ratagupt at linux.vnet.ibm.com>
> wrote:
>
>> Hi,
>>
>> Please find my comments inline
>>
>> Regards
>>
>> Ratan Gupta
>>
>>
>>
>> On Tuesday 27 February 2018 04:51 PM, Jayanth Othayoth wrote:
>> > I am working on an issue openbmc/openbmc#2500 (Add support for Turbo
>> > allowed sensor). As part of this issue implementation , there is a
>> > requirement  to change this IPMI sensor to read only for host.
>> > Currently all sensors are read/write by design and doesn’t have way to
>> > change this behavior. Planing to change this behavior by adding
>> > “Mutability” field in the config.yaml.
>>
> ES: The Mutability field already exists in sensor config yaml.
> https://github.com/openbmc/phosphor-host-ipmid/blob/2e12a31f39d95c381a466e277e510faa02c351fb/scripts/writesensor.mako.cpp#L68
>
>     RG>> Here in mako we always set the mutability property to READ as
> config YAML doesn't have the Mutability property.
>
> https://github.com/openbmc/openbmc/blob/master/meta-phosphor/common/recipes-phosphor/ipmi/phosphor-ipmi-sensor-inventory-mrw-config/config.yaml
>
>     RG>>We have the Mutability check in the getSensorReading but we don't
> have that check in the setSensorReading so we can introduce that check
> there with the required changes in the config.YAML.
>
>
> >
>> > As part of this issue am planing to  Initialize  mutability with
>> > Mutability::Write|Mutability::Read for all sensors and set the
>> > required sensor mutability Mutability::Read.
>>
> ES: You're saying you'd like to ensure all sensors are readable? This
> doesn't make sense in some cases, notably fan PWM in some cases where the
> only read response would be DBus parroting back what we wrote (not an
> actual value reflecting reality).
>
>> >       - Minimal impact on testing and not breaking the existing
>> > functionality.
>> RG>>How we would be doing it? If we are planning to do introduce a check
>> for a specific sensor then we should not do like this.
>> In the current design we have not introduced any such checks and we
>> should avoid that.
>> Please correct me if I misunderstood it.
>>
>   RG>>if we add the Mutability property in the config.yaml with configured
> values as READ and WRITE for all other sensor
>             Except this one(turboAllowedSensor) which is required to be
> READ
>             In this sense we are not breaking the current code as the
> interpretation in the code for all the sensor is READ and WRITE
>
> >
>> > Follow-on story is needed to fix this in generic way like
>> >   -Add “mutable” field in the config.yaml and initialize the the field
>>
> ES: Today it's defaulted to read-only for all uninitialized sensors.
>
>> > based on System types also update
>> > sensorhandler.cpp->ipmi_sen_set_sensor() to do set only for write
>> > enabled sensor.
>> >     - This required system specific IPMI sensor behavior review and
>> > updates in corresponding config.yaml.
>> RG>> I agree with this approach.What do you mean by "System types"
>> here,We have already system specfic YAML in the system specfic meta
>> layers.
>>
>> >
>> > Please add your view on this.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20180227/e5a2ef16/attachment.html>