linux-kernel - Re: [PATCH 1/2] phy: marvell: comphy: Convert internal SMCC firmware return codes to errno

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: <20200902172029.GG3050651@lunn.ch>
Date:   Wed, 2 Sep 2020 19:20:29 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Pali Rohár <pali@...nel.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Marek Behún <marek.behun@....cz>,
        Tomasz Maciej Nowak <tmn505@...il.com>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] phy: marvell: comphy: Convert internal SMCC firmware
 return codes to errno

On Wed, Sep 02, 2020 at 07:05:25PM +0200, Pali Rohár wrote:
> On Wednesday 02 September 2020 19:00:10 Andrew Lunn wrote:
> > > > > +	switch (ret) {
> > > > > +	case SMCCC_RET_SUCCESS:
> > > > > +		return 0;
> > > > > +	case SMCCC_RET_NOT_SUPPORTED:
> > > > > +		return -EOPNOTSUPP;
> > > > > +	default:
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > >  }
> > > > 
> > > > Hi Pali
> > > > 
> > > > Maybe this should be a global helper translating SMCCC_RET_* into a
> > > > standard errno value?
> > > > 
> > > > 	 Andrew
> > > 
> > > Hello Andrew!
> > > 
> > > Well, I'm not sure if some standard global helper is the correct way for
> > > marvell comphy handler. It returns 0 for success and -1 on error when
> > > handler is not supported.
> > 
> > No, i was meaning just 
> > 
> > switch (ret) {
> > case SMCCC_RET_SUCCESS:
> > 	return 0;
> > case SMCCC_RET_NOT_SUPPORTED:
> > 	return -EOPNOTSUPP;
> > default:
> > 	return -EINVAL;
> > }
> 
> But this is not a complete generic helper. There are more generic SMCC
> return codes and generic helper should define and translate all of them.

/*
 * Return codes defined in ARM DEN 0070A
 * ARM DEN 0070A is now merged/consolidated into ARM DEN 0028 C
 */
#define SMCCC_RET_SUCCESS			0
#define SMCCC_RET_NOT_SUPPORTED			-1
#define SMCCC_RET_NOT_REQUIRED			-2
#define SMCCC_RET_INVALID_PARAMETER		-3

I only see problems with SMCCC_RET_NOT_REQUIRED and what value to use
for it. Do you have any idea what is actually means? A parameter was
passed which was not required? Or that the call itself is not
required? Looking at the uses of it currently in the kernel, it does
not seem to be an actual error. So maybe just return 0?

	Andrew