netdev - Re: [PATCH v2 07/35] brcmfmac: pcie: Read Apple OTP information

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: <7b3e7ae0-5791-f4ad-619a-a3cc3f913a44@marcan.st>
Date:   Thu, 6 Jan 2022 22:08:11 +0900
From:   Hector Martin <marcan@...can.st>
To:     Arend van Spriel <arend.vanspriel@...adcom.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        Arend van Spriel <aspriel@...il.com>,
        Franky Lin <franky.lin@...adcom.com>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        Chi-hsien Lin <chi-hsien.lin@...ineon.com>,
        Wright Feng <wright.feng@...ineon.com>,
        Dmitry Osipenko <digetx@...il.com>
Cc:     Sven Peter <sven@...npeter.dev>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Mark Kettenis <kettenis@...nbsd.org>,
        Rafał Miłecki <zajec5@...il.com>,
        Pieter-Paul Giesberts <pieter-paul.giesberts@...adcom.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Hans de Goede <hdegoede@...hat.com>,
        "John W. Linville" <linville@...driver.com>,
        "brian m. carlson" <sandals@...stytoothpaste.net>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, brcm80211-dev-list.pdl@...adcom.com,
        SHA-cyfmac-dev-list@...ineon.com
Subject: Re: [PATCH v2 07/35] brcmfmac: pcie: Read Apple OTP information

On 06/01/2022 21.37, Arend van Spriel wrote:
> On 1/4/2022 8:26 AM, Hector Martin wrote:
>> +static int brcmf_pcie_read_otp(struct brcmf_pciedev_info *devinfo)
>> +{
>> +	const struct pci_dev *pdev = devinfo->pdev;
>> +	struct brcmf_bus *bus = dev_get_drvdata(&pdev->dev);
>> +	u32 coreid, base, words, idx, sromctl;
>> +	u16 *otp;
>> +	struct brcmf_core *core;
>> +	int ret;
>> +
>> +	switch (devinfo->ci->chip) {
>> +	default:
>> +		/* OTP not supported on this chip */
>> +		return 0;
>> +	}
> 
> Does not seem this code is put to work yet. Will dive into it later on.

The specific OTP ranges and cores are added by the subsequent patches
that add support for individual chips, once all the scaffolding is in place.

> 
>> +	core = brcmf_chip_get_core(devinfo->ci, coreid);
>> +	if (!core) {
>> +		brcmf_err(bus, "No OTP core\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	if (coreid == BCMA_CORE_CHIPCOMMON) {
>> +		/* Chips with OTP accessed via ChipCommon need additional
>> +		 * handling to access the OTP
>> +		 */
>> +		brcmf_pcie_select_core(devinfo, coreid);
>> +		sromctl = READCC32(devinfo, sromcontrol);
>> +
>> +		if (!(sromctl & BCMA_CC_SROM_CONTROL_OTP_PRESENT)) {
>> +			/* Chip lacks OTP, try without it... */
>> +			brcmf_err(bus,
>> +				  "OTP unavailable, using default firmware\n");
>> +			return 0;
>> +		}
>> +
>> +		/* Map OTP to shadow area */
>> +		WRITECC32(devinfo, sromcontrol,
>> +			  sromctl | BCMA_CC_SROM_CONTROL_OTPSEL);
>> +	}
>> +
>> +	otp = kzalloc(sizeof(u16) * words, GFP_KERNEL);
>> +
>> +	/* Map bus window to SROM/OTP shadow area in core */
>> +	base = brcmf_pcie_buscore_prep_addr(devinfo->pdev, base + core->base);
> 
> I guess this changes the bar window...
> 
>> +	brcmf_dbg(PCIE, "OTP data:\n");
>> +	for (idx = 0; idx < words; idx++) {
>> +		otp[idx] = brcmf_pcie_read_reg16(devinfo, base + 2 * idx);
>> +		brcmf_dbg(PCIE, "[%8x] 0x%04x\n", base + 2 * idx, otp[idx]);
>> +	}
>> +
>> +	if (coreid == BCMA_CORE_CHIPCOMMON) {
>> +		brcmf_pcie_select_core(devinfo, coreid);
> 
> ... which is why you need to reselect the core. Otherwise it makes no 
> sense to me.

Yes; *technically* with the BCMA_CORE_CHIPCOMMON core the OTP is always
within the first 0x1000 and so I wouldn't have to reselect it, since
it'd end up with the same window, but that is not the case with
BCMA_CORE_GCI used on other chips (where the OTP offset is >0x1000),
although those don't hit this code path. So while this line could be
removed without causing any issues, I find it more orthogonal and safer
to keep the pattern where I select the core before accessing
core-relative fixed registers, and treat brcmf_pcie_buscore_prep_addr as
invalidating the BAR window for all intents and purposes.

-- 
Hector Martin (marcan@...can.st)
Public Key: https://mrcn.st/pub