linux-kernel - Re: [PATCH v2 2/6] firmware: arm_scmi: add SCMIv3.0 Sensors descriptors extensions

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: <a51fecaa-4495-6d8e-9b16-598bbe47b1cd@opensynergy.com>
Date:   Tue, 10 Nov 2020 18:50:04 +0100
From:   Peter Hilber <peter.hilber@...nsynergy.com>
To:     Cristian Marussi <cristian.marussi@....com>
CC:     <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <sudeep.holla@....com>,
        <lukasz.luba@....com>, <james.quinlan@...adcom.com>,
        <Jonathan.Cameron@...wei.com>, <egranata@...gle.com>,
        <jbhayana@...gle.com>, <mikhail.golubev@...nsynergy.com>,
        <Igor.Skalkin@...nsynergy.com>
Subject: Re: [PATCH v2 2/6] firmware: arm_scmi: add SCMIv3.0 Sensors
 descriptors extensions

Hi Cristian,

sorry, I mistakenly used the wrong sender ("Mailing Lists") for the
original comment mail. Please see below for my reply.

On 10.11.20 18:21, Cristian Marussi wrote:
> On Tue, Nov 10, 2020 at 05:00:05PM +0100, Mailing Lists wrote:
>> On 26.10.20 21:10, Cristian Marussi wrote:
>>> Add support for new SCMIv3.0 Sensors extensions related to new sensors'
>>> features, like multiple axis and update intervals, while keeping
>>> compatibility with SCMIv2.0 features.
>>> While at that, refactor and simplify all the internal helpers macros and
>>> move struct scmi_sensor_info to use only non-fixed-size typing.
>>>
>>> Signed-off-by: Cristian Marussi <cristian.marussi@....com>
>>> ---
>>> v1 --> v2
>>> - restrict segmented intervals descriptors to single triplet
>>> - add proper usage of scmi_reset_rx_to_maxsz
>>> ---
>>>  drivers/firmware/arm_scmi/sensors.c | 391 ++++++++++++++++++++++++++--
>>>  include/linux/scmi_protocol.h       | 219 +++++++++++++++-
>>>  2 files changed, 584 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
>>> index 6aaff478d032..5a18f8c84bef 100644
>>> --- a/drivers/firmware/arm_scmi/sensors.c
>>> +++ b/drivers/firmware/arm_scmi/sensors.c
>>> @@ -7,16 +7,21 @@
>>>
>>>  #define pr_fmt(fmt) "SCMI Notifications SENSOR - " fmt
>>>
>>> +#include <linux/bitfield.h>
>>>  #include <linux/scmi_protocol.h>
>>>
>>>  #include "common.h"
>>>  #include "notify.h"
>>>
>>> +#define SCMI_MAX_NUM_SENSOR_AXIS   64
>>
>> IMHO the related 6 bit wide fields, like SENSOR_DESCRIPTION_GET "Number
>> of axes", should determine the maximum value, so 64 -> 63.
>>
> 
> Yes, bits [21:16] 'Number of Axes' in sensor_attributes_high, but this
> #define was meant to represent the maximum number of sensor axis (64...ranging
> from 0 to 63) not the highest possible numbered (63).
> 

But in my understanding the actual maximum number of sensor axes is 63
due to the maximum value 63 of 'Number of Axes', 64 would overflow
already. The ids would range from 0 to 62.

That said, in my understanding there is no problem with retaining a
higher value ATM.

Best regards,

Peter