linux-kernel - Re: [PATCH v6] PCI hotplug: rpaphp: Error out on busy status from get-sensor-state

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]

Date:   Thu, 28 Apr 2022 17:31:38 -0500
From:   Nathan Lynch <nathanl@...ux.ibm.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     linuxppc-dev <linuxppc-dev@...abs.org>,
        Tyrel Datwyler <tyreld@...ux.ibm.com>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        "Oliver O'Halloran" <oohall@...il.com>,
        linux-pci <linux-pci@...r.kernel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Mahesh Salgaonkar <mahesh@...ux.ibm.com>
Subject: Re: [PATCH v6] PCI hotplug: rpaphp: Error out on busy status from
 get-sensor-state

Bjorn Helgaas <helgaas@...nel.org> writes:
> On Tue, Apr 26, 2022 at 11:07:39PM +0530, Mahesh Salgaonkar wrote:
>> +/*
>> + * RTAS call get-sensor-state(DR_ENTITY_SENSE) return values as per PAPR:
>> + *    -1: Hardware Error
>> + *    -2: RTAS_BUSY
>> + *    -3: Invalid sensor. RTAS Parameter Error.
>> + * -9000: Need DR entity to be powered up and unisolated before RTAS call
>> + * -9001: Need DR entity to be powered up, but not unisolated, before RTAS call
>> + * -9002: DR entity unusable
>> + *  990x: Extended delay - where x is a number in the range of 0-5
>> + */
>> +#define RTAS_HARDWARE_ERROR	(-1)
>> +#define RTAS_INVALID_SENSOR	(-3)
>> +#define SLOT_UNISOLATED		(-9000)
>> +#define SLOT_NOT_UNISOLATED	(-9001)
>
> I would say "isolated" instead of "not unisolated", but I suppose this
> follows language in the spec.  If so, you should follow the spec.

"not unisolated" is the spec language.


>> +#define SLOT_NOT_USABLE		(-9002)
>> +
>> +static int rtas_to_errno(int rtas_rc)
>> +{
>> +	int rc;
>> +
>> +	switch (rtas_rc) {
>> +	case RTAS_HARDWARE_ERROR:
>> +		rc = -EIO;
>> +		break;
>> +	case RTAS_INVALID_SENSOR:
>> +		rc = -EINVAL;
>> +		break;
>> +	case SLOT_UNISOLATED:
>> +	case SLOT_NOT_UNISOLATED:
>> +		rc = -EFAULT;
>> +		break;
>> +	case SLOT_NOT_USABLE:
>> +		rc = -ENODEV;
>> +		break;
>> +	case RTAS_BUSY:
>> +	case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
>> +		rc = -EBUSY;
>> +		break;
>> +	default:
>> +		err("%s: unexpected RTAS error %d\n", __func__, rtas_rc);
>> +		rc = -ERANGE;
>> +		break;
>> +	}
>> +	return rc;
>
> This basically duplicates rtas_error_rc().  Why do we need two copies?

It treats RTAS_BUSY, RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX
differently, which is part of the point of this change.

Aside: rtas_error_rc() (from powerpc's rtas.c) is badly named. Its
conversions make sense for only a handful of RTAS calls. RTAS error
codes have function-specific interpretations.