linux-kernel - Re: [PATCH 2/2] sbrmi: Add support for APML protocols

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 for Android: free password hash cracker in your pocket

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

Date: Wed, 15 May 2024 11:32:17 +0100
From: Lee Jones <lee@...nel.org>
To: Guenter Roeck <linux@...ck-us.net>
Cc: "Chatradhi, Naveen Krishna" <naveenkrishna.chatradhi@....com>,
	Naveen Krishna Chatradhi <nchatrad@....com>,
	linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
	Akshay Gupta <Akshay.Gupta@....com>, arnd@...db.de,
	gregkh@...uxfoundation.org
Subject: Re: [PATCH 2/2] sbrmi: Add support for APML protocols

On Tue, 14 May 2024, Guenter Roeck wrote:

> On 5/14/24 12:15, Chatradhi, Naveen Krishna wrote:
> > + @Misc and @MFD maintainers in CC
> > 
> > Hi
> > 
> > On 5/3/2024 3:56 AM, Guenter Roeck wrote:
> > > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> > > 
> > > 
> > > On 5/2/24 15:05, Naveen Krishna Chatradhi wrote:
> > > > From: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@....com>
> > > > 
> > > > The present sbrmi module only support reporting power. However, AMD data
> > > > center processors support various system management functionality
> > > > Out-of-band over Advanced Platform Management Link APML.
> > > > 
> > > > Register a miscdevice, which creates a device /dev/sbrmiX with an IOCTL
> > > > interface for the user space to invoke the following protocols.
> > > >    - Mailbox read/write (already defined in sbrmi_mailbox_xfer())
> > > >    - CPUID read
> > > >    - MCAMSR read
> > > > 
> > > 
> > > This is not hardware monitoring functionality and would have to reside
> > > elsewhere, outside the hwmon subsystem.
> > 
> > I thought as much, please provide your opinion on the following approach.
> > 
> > Background: Present sbrmi under hwmon subsystem is probed as an i2c driver and reports power.
> > 
> > However, APML interface defines few other protocols to support OOB system management functionality.
> > 
> > As adding the core functionality of the APML interface in drivers/hwmon/sbrmi is not the correct approach.
> > 
> > We would like do the following
> > 
> > 1. Move the i2c client probe, misc device registration and rmi_mailbox_xfer() function to a drivers/misc.
> > 
> > 2. Add an MFD device with a cell, which probes the hwmon/sbrmi and continues to report power using the symbols exported by the misc/sbrmi.
> > 
> 
> afaik mfd API function must not be used outside drivers/mfd. The mfd maintainer
> is (or at least used to be) pretty strict on that. The core code of a
> multi-function device might better be implemented in drivers/mfd, with
> drivers in drivers/misc (for the misc device) and drivers/hwmon/ (for
> hwmon functionality). The misc and hwmon drivers could then communicate
> with the core using regmap.

Yes, please only use the MFD API from drivers/mfd.

There are 'offenders' that slipped by me, but in general if you need to
create an MFD then it should be located in the MFD subsystem.

> drivers/mailbox/ supports mailbox style communication. I don't know if that
> would apply to the mailbox functionality implemented here.

-- 
Lee Jones [李琼斯]