linux-kernel - Re: [RFC PATCH 5/5] fsi/scom: Major overhaul

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:   Mon, 18 Jun 2018 14:09:20 +1000
From:   Alistair Popple <alistair@...ple.id.au>
To:     openbmc@...ts.ozlabs.org
Cc:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 5/5] fsi/scom: Major overhaul

On Sunday, 17 June 2018 11:22:11 AM AEST Benjamin Herrenschmidt wrote:
> On Sun, 2018-06-17 at 11:17 +1000, Benjamin Herrenschmidt wrote:
> > 
> > We have everything that cronus needs and more than pdbg needs afaik :-)

Yep, has what we need and more (such as put scom under mask and indirect scom).
Only other useful thing would be repeated getsom/putscom operations (eg. read
the same scom address n times) as they would help with ADU access which can do
autoincrement or scanscom (although we should just use the scan engine directly
for that so not a big issue).

> +	for (retries = 0; retries < SCOM_MAX_RETRIES; retries++) {
> +		rc = raw_get_scom(scom, value, addr, &status);
> +		if (rc) {
> +			/* Try resetting the bridge if FSI fails */
> +			if (rc != -ENODEV && retries == 0) {
> +				fsi_device_write(scom->fsi_dev, SCOM_FSI2PIB_RESET_REG,
> +						 &dummy, sizeof(uint32_t));
> +				rc = -EBUSY;
> +			} else
> +				return rc;
> +		} else
> +			rc = handle_fsi2pib_status(scom, status);
> +		if (rc && rc != -EBUSY)
> +			break;
> +		if (rc == 0) {
> +			rc = handle_pib_status(scom,
> +					       (status & SCOM_STATUS_PIB_RESP_MASK)
> +					       >> SCOM_STATUS_PIB_RESP_SHIFT);
> +			if (rc && rc != -EBUSY)
> +				break;
> +		}
> +		if (rc == 0)
> +			break;
> +		msleep(1);
> +	}

The rc handling above took me a little while to grok but I didn't come up with a
cleaner alternative and I think it's correct.

- Alistair

> > 
> > That said, cronus does a bunch of other stupid things that I'm still
> > trying to figure out how to fix.
> > 
> > We might need to create a /dev/cfam rather than going through that
> > magic sysfs "raw" file, and I wouldn't mind using a single IDA so that
> > all the devices below a given FSI slace (cfam itself, sbefifo, occ,
> > ...) have the same "number".
> 
> Also while we're at reworking how all this is exposed to our broken
> userspace, I wouldn't mind putting all these dev entries under a
> directory, if I can figure out how to do that (I haven't really looked
> yet).
> 
> /dev/fsi/{cfamN,sbefifoN,occN, ...} and possibly similar by-id and by-
> path that other subsystems use, so we have something more deterministic
> than the "random number" crap we do now.
> 
> We can always keep hacks to do symlinks in our kernel tree until we
> have converted all our userspace users.
> 
> We currently control the only userspace users of this, so now is a good
> time to cleanup how we expose things. This won't always be the case.
> 
> Cheers,
> Ben.
> 
>