linux-kernel - Re: [PATCH 2/5] dt-bindings: soc: hpe: hpe,gxp-srom.yaml

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: <Y8VUOENIhe72sqMO@enigma.ccjz.io>
Date:   Mon, 16 Jan 2023 21:42:16 +0800
From:   Clay Chang <clayc@....com>
To:     Arnd Bergmann <arnd@...db.de>
CC:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        <linux-kernel@...r.kernel.org>, <soc@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <devicetree@...r.kernel.org>,
        "Verdun, Jean-Marie" <verdun@....com>,
        "Hawkins, Nick" <nick.hawkins@....com>,
        Rob Herring <robh+dt@...nel.org>,
        <krzysztof.kozlowski+dt@...aro.org>,
        Russell King <linux@...linux.org.uk>,
        Olof Johansson <olof@...om.net>
Subject: Re: [PATCH 2/5] dt-bindings: soc: hpe: hpe,gxp-srom.yaml

Hi Arnd,

Thank you for taking time to answer my questions.

On Thu, Jan 12, 2023 at 02:37:53PM +0100, Arnd Bergmann wrote:
> On Thu, Jan 12, 2023, at 14:16, Clay Chang wrote:
> > On Tue, Jan 10, 2023 at 10:49:36AM +0100, Krzysztof Kozlowski wrote:
> >> On 10/01/2023 05:25, clayc@....com wrote:
> 
> >> > @@ -0,0 +1,36 @@
> >> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> > +%YAML 1.2
> >> > +---
> >> > +$id: http://devicetree.org/schemas/soc/hpe/hpe,gxp-srom.yaml#  
> >> > +$schema: http://devicetree.org/meta-schemas/core.yaml#  
> >> > +
> >> > +title: HPE GXP SoC SROM Control Register
> >> > +
> >> > +maintainers:
> >> > +  - Clay Chang <clayc@....com>
> >> > +
> >> > +description: |+
> >> > +  The SROM control register can be used to configure LPC related legacy
> >> > +  I/O registers.
> >> 
> >> And why this is a hardware? No, you now add fake devices to be able to
> >> write some stuff from user-space... Otherwise this needs proper hardware
> >> description.
> >
> > Thank you for commenting on this. You are right, this is not a real
> > hardware device, but simply exposes MMIO regions to the user-space.
> > Maybe we should rewrite this as a syscon driver. Is writing a syscon
> > driver a right direction?
> 
> There are two completely separate questions about the DT binding
> and about the user-visible interface.
> 
> The binding needs to properly identify what this device is. I don't
> think anyone without the datasheet can tell you the right answer
> here, it really depends what the other registers do. If there are
> lots of unrelated registers in a small area, a syscon might be 
> the right answer, but if they are all related to an external
> memory bus, then categorizing it as a memory controller may
> be more appropriate.

Our use-cases are more like some register accesses not related to an
external memory bus, so syscon might be a better fit.

> 
> For the user interface side, I don't really like the idea of
> having a hardware register directly exposed as driver in
> drivers/soc, this generally makes it impossible to have portable
> userspace that works across implementations of multiple SoC
> vendors, and it makes it too easy to come up with an ad-hoc
> interface to make a chip work for a particular use case when
> a more general solution would be better.
> 

I agree with you. I have one question though: if we create a 'hpe'
directory under drivers/soc, and put all HPE BMC specific drivers there,
do you think this proper?

> Again, it's hard for me to tell why this even needs to be runtime
> configurable, please try to describe what type of application
> would access the sysfs interface here, and why this can't just
> be set to a fixed value by bootloader or kernel without user
> interaction.

The register is used for communication and synchronization between the
BMC and the host. During runtime, user-space daemons configures the
value of the register for interactions.

> 
>        Arnd

Thanks,
Clay