Hi Andy, On 1/31/22 16:13, Andy Shevchenko wrote: > From: Jonathan Yong <jonathan.yong@xxxxxxxxx> > > There are already two and at least one more user is coming which > require an access to Primary to Sideband (P2SB) bridge in order > to get IO or MMIO BAR hidden by BIOS. > > Create a library to access P2SB for x86 devices. > > Background information > ====================== > Note, the term "bridge" is used in the documentation and it has nothing > to do with a PCI (host) bridge as per the PCI specifications. > > The P2SB is an interesting device by its nature and hardware design. > First of all, it has several devices in the hardware behind it. These > devices may or may not be represented as ACPI devices by a firmware. > > It also has a hardwired (to 0s) the least significant bits of the > base address register which is represented by the only 64-bit BAR0. > It means that OS mustn't reallocate the BAR. > > On top of that in some cases P2SB is represented by function 0 on PCI > slot (in terms of B:D.F) and according to the PCI specification any > other function can't be seen until function 0 is present and visible. > > In the PCI configuration space of P2SB device the full 32-bit register > is allocated for the only purpose of hiding the entire P2SB device. As > per [3]: > > 3.1.39 P2SB Control (P2SBC)—Offset E0h > > Hide Device (HIDE): When this bit is set, the P2SB will return 1s on > any PCI Configuration Read on IOSF-P. All other transactions including > PCI Configuration Writes on IOSF-P are unaffected by this. This does > not affect reads performed on the IOSF-SB interface. > > This doesn't prevent MMIO accesses, although preventing the OS from > assigning these addresses. The firmware on the affected platforms marks > the region as unusable (by cutting it off from the PCI host bridge > resources) as depicted in the Apollo Lake example below: > > PCI host bridge to bus 0000:00 > pci_bus 0000:00: root bus resource [io 0x0070-0x0077] > pci_bus 0000:00: root bus resource [io 0x0000-0x006f window] > pci_bus 0000:00: root bus resource [io 0x0078-0x0cf7 window] > pci_bus 0000:00: root bus resource [io 0x0d00-0xffff window] > pci_bus 0000:00: root bus resource [mem 0x7c000001-0x7fffffff window] > pci_bus 0000:00: root bus resource [mem 0x7b800001-0x7bffffff window] > pci_bus 0000:00: root bus resource [mem 0x80000000-0xcfffffff window] > pci_bus 0000:00: root bus resource [mem 0xe0000000-0xefffffff window] > pci_bus 0000:00: root bus resource [bus 00-ff] > > The P2SB 16MB BAR is located at 0xd0000000-0xd0ffffff memory window. > > The generic solution > ==================== > The generic solution for all cases when we need to access to the information > behind P2SB device is a library code where users ask for necessary resources > by demand and hence those users take care of not being run on the systems > where this access is not required. > > The library provides the p2sb_bar() API to retrieve the MMIO of the BAR0 of > the device from P2SB device slot. > > P2SB unconditional unhiding awareness > ===================================== > Technically it's possible to unhide the P2SB device and devices on > the same PCI slot and access them at any time as needed. But there are > several potential issues with that: > > - the systems were never tested against such configuration and hence > nobody knows what kind of bugs it may bring, especially when we talk > about SPI NOR case which contains Intel FirmWare Image (IFWI) code > (including BIOS) and already known to be problematic in the past for > end users > > - the PCI by its nature is a hotpluggable bus and in case somebody > attaches a driver to the functions of a P2SB slot device(s) the > end user experience and system behaviour can be unpredictable > > - the kernel code would need some ugly hacks (or code looking as an > ugly hack) under arch/x86/pci in order to enable these devices on > only selected platforms (which may include CPU ID table followed by > a potentially growing number of DMI strings > > The future improvements > ======================= > The future improvements with this code may go in order to gain some kind > of cache, if it's possible at all, to prevent unhiding and hiding many > times to take static information that may be saved once per boot. > > Links > ===== > [1]: https://lab.whitequark.org/notes/2017-11-08/accessing-intel-ich-pch-gpios/ > [2]: https://cdrdv2.intel.com/v1/dl/getContent/332690?wapkw=332690 > [3]: https://cdrdv2.intel.com/v1/dl/getContent/332691?wapkw=332691 > [4]: https://medium.com/@jacksonchen_43335/bios-gpio-p2sb-70e9b829b403 > > Signed-off-by: Jonathan Yong <jonathan.yong@xxxxxxxxx> > Co-developed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Acked-by: Hans de Goede <hdegoede@xxxxxxxxxx> > Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > --- > drivers/platform/x86/intel/Kconfig | 12 + > drivers/platform/x86/intel/Makefile | 1 + > drivers/platform/x86/intel/p2sb.c | 299 +++++++++++++++++++++++++ > include/linux/platform_data/x86/p2sb.h | 27 +++ > 4 files changed, 339 insertions(+) > create mode 100644 drivers/platform/x86/intel/p2sb.c > create mode 100644 include/linux/platform_data/x86/p2sb.h > > diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig > index 8e65086bb6c8..55b499fb8249 100644 > --- a/drivers/platform/x86/intel/Kconfig > +++ b/drivers/platform/x86/intel/Kconfig > @@ -68,6 +68,18 @@ config INTEL_OAKTRAIL > enable/disable the Camera, WiFi, BT etc. devices. If in doubt, say Y > here; it will only load on supported platforms. > > +config P2SB > + bool "Primary to Sideband (P2SB) bridge access support" > + depends on PCI > + help > + The Primary to Sideband (P2SB) bridge is an interface to some > + PCI devices connected through it. In particular, SPI NOR controller > + in Intel Apollo Lake SoC is one of such devices. > + > + The main purpose of this library is to unhide P2SB device in case > + firmware kept it hidden on some platforms in order to access devices > + behind it. > + > config INTEL_BXTWC_PMIC_TMU > tristate "Intel Broxton Whiskey Cove TMU Driver" > depends on INTEL_SOC_PMIC_BXTWC > diff --git a/drivers/platform/x86/intel/Makefile b/drivers/platform/x86/intel/Makefile > index 35f2066578b2..6ce944285730 100644 > --- a/drivers/platform/x86/intel/Makefile > +++ b/drivers/platform/x86/intel/Makefile > @@ -26,6 +26,7 @@ intel_int0002_vgpio-y := int0002_vgpio.o > obj-$(CONFIG_INTEL_INT0002_VGPIO) += intel_int0002_vgpio.o > intel_oaktrail-y := oaktrail.o > obj-$(CONFIG_INTEL_OAKTRAIL) += intel_oaktrail.o > +obj-$(CONFIG_P2SB) += p2sb.o > intel_vsec-y := vsec.o > obj-$(CONFIG_INTEL_VSEC) += intel_vsec.o > > diff --git a/drivers/platform/x86/intel/p2sb.c b/drivers/platform/x86/intel/p2sb.c > new file mode 100644 > index 000000000000..8c12882d0bde > --- /dev/null > +++ b/drivers/platform/x86/intel/p2sb.c > @@ -0,0 +1,299 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Primary to Sideband (P2SB) bridge access support > + * > + * Copyright (c) 2017, 2021-2022 Intel Corporation. > + * > + * Authors: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > + * Jonathan Yong <jonathan.yong@xxxxxxxxx> > + */ > + > +#include <linux/bitops.h> > +#include <linux/export.h> > +#include <linux/pci.h> > +#include <linux/platform_data/x86/p2sb.h> > +#include <linux/printk.h> > + > +#include <asm/cpu_device_id.h> > +#include <asm/intel-family.h> > + > +#define p2sb_printk(level, bus, devfn, fmt, arg...) \ > + printk(level "pci %04x:%02x:%02x.%d: " fmt, \ > + pci_domain_nr(bus), (bus)->number, PCI_SLOT(devfn), PCI_FUNC(devfn), ##arg) > + > +#define p2sb_err(bus, devfn, fmt, arg...) p2sb_printk(KERN_ERR, (bus), devfn, fmt, ##arg) > +#define p2sb_info(bus, devfn, fmt, arg...) p2sb_printk(KERN_INFO, (bus), devfn, fmt, ##arg) > + > +#define P2SBC 0xe0 > +#define P2SBC_HIDE BIT(8) > + > +static const struct x86_cpu_id p2sb_cpu_ids[] = { > + X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT, PCI_DEVFN(13, 0)), > + {} > +}; > + > +static int p2sb_get_devfn(unsigned int *devfn) > +{ > + const struct x86_cpu_id *id; > + > + id = x86_match_cpu(p2sb_cpu_ids); > + if (!id) > + return -ENODEV; > + > + *devfn = (unsigned int)id->driver_data; > + return 0; > +} > + > +/* > + * Here is practically a copy'n'paste of __pci_read_base() and Co, > + * modified to work with PCI bus and devfn instead of PCI device. > + * > + * Note, the PCI core doesn't want to have that being refactored > + * and reused. > + */ > +static u64 pci_size(u64 base, u64 maxbase, u64 mask) > +{ > + u64 size = mask & maxbase; /* Find the significant bits */ > + if (!size) > + return 0; > + > + /* > + * Get the lowest of them to find the decode size, and from that > + * the extent. > + */ > + size = size & ~(size-1); > + > + /* > + * base == maxbase can be valid only if the BAR has already been > + * programmed with all 1s. > + */ > + if (base == maxbase && ((base | (size - 1)) & mask) != mask) > + return 0; > + > + return size; > +} > + > +static inline unsigned long decode_bar(u32 bar) > +{ > + u32 mem_type; > + unsigned long flags; > + > + if ((bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) { > + flags = bar & ~PCI_BASE_ADDRESS_IO_MASK; > + flags |= IORESOURCE_IO; > + return flags; > + } > + > + flags = bar & ~PCI_BASE_ADDRESS_MEM_MASK; > + flags |= IORESOURCE_MEM; > + if (flags & PCI_BASE_ADDRESS_MEM_PREFETCH) > + flags |= IORESOURCE_PREFETCH; > + > + mem_type = bar & PCI_BASE_ADDRESS_MEM_TYPE_MASK; > + switch (mem_type) { > + case PCI_BASE_ADDRESS_MEM_TYPE_32: > + break; > + case PCI_BASE_ADDRESS_MEM_TYPE_1M: > + /* 1M mem BAR treated as 32-bit BAR */ > + break; > + case PCI_BASE_ADDRESS_MEM_TYPE_64: > + flags |= IORESOURCE_MEM_64; > + break; > + default: > + /* mem unknown type treated as 32-bit BAR */ > + break; > + } > + return flags; > +} > + > +/** > + * __pci_bus_read_base - Read a PCI BAR > + * @bus: the PCI bus > + * @devfn: the PCI device and function > + * @res: resource buffer to be filled in > + * @pos: BAR position in the config space > + * > + * Returns 1 if the BAR is 64-bit, or 0 if 32-bit. > + * In case of error resulting @res->flags is 0. > + */ > +static int __pci_bus_read_base(struct pci_bus *bus, unsigned int devfn, > + struct resource *res, unsigned int pos) > +{ Andy, as discussed already, this is quite a big function to mostly duplicate from the PCI-core. AFAIK the last status of the discussion surrounding this was trying to have the P2SB code here create a fake pci_dev struct and use that with the existing function instead. Have you given this approach a try ? Note if that does not work out I'm fine with taking this patch as is. Regards, Hans > + u32 l = 0, sz = 0, mask = ~0; > + u64 l64, sz64, mask64; > + struct pci_bus_region region, inverted_region; > + > + pci_bus_read_config_dword(bus, devfn, pos, &l); > + pci_bus_write_config_dword(bus, devfn, pos, l | mask); > + pci_bus_read_config_dword(bus, devfn, pos, &sz); > + pci_bus_write_config_dword(bus, devfn, pos, l); > + > + /* > + * All bits set in sz means the device isn't working properly. > + * If the BAR isn't implemented, all bits must be 0. If it's a > + * memory BAR or a ROM, bit 0 must be clear; if it's an io BAR, bit > + * 1 must be clear. > + */ > + if (PCI_POSSIBLE_ERROR(sz)) > + sz = 0; > + > + /* > + * I don't know how l can have all bits set. Copied from old code. > + * Maybe it fixes a bug on some ancient platform. > + */ > + if (PCI_POSSIBLE_ERROR(l)) > + l = 0; > + > + res->flags = decode_bar(l); > + res->flags |= IORESOURCE_SIZEALIGN; > + if (res->flags & IORESOURCE_IO) { > + l64 = l & PCI_BASE_ADDRESS_IO_MASK; > + sz64 = sz & PCI_BASE_ADDRESS_IO_MASK; > + mask64 = PCI_BASE_ADDRESS_IO_MASK & (u32)IO_SPACE_LIMIT; > + } else { > + l64 = l & PCI_BASE_ADDRESS_MEM_MASK; > + sz64 = sz & PCI_BASE_ADDRESS_MEM_MASK; > + mask64 = (u32)PCI_BASE_ADDRESS_MEM_MASK; > + } > + > + if (res->flags & IORESOURCE_MEM_64) { > + pci_bus_read_config_dword(bus, devfn, pos + 4, &l); > + pci_bus_write_config_dword(bus, devfn, pos + 4, ~0); > + pci_bus_read_config_dword(bus, devfn, pos + 4, &sz); > + pci_bus_write_config_dword(bus, devfn, pos + 4, l); > + > + l64 |= ((u64)l << 32); > + sz64 |= ((u64)sz << 32); > + mask64 |= ((u64)~0 << 32); > + } > + > + if (!sz64) > + goto fail; > + > + sz64 = pci_size(l64, sz64, mask64); > + if (!sz64) { > + p2sb_info(bus, devfn, FW_BUG "reg 0x%x: invalid BAR (can't size)\n", pos); > + goto fail; > + } > + > + if (res->flags & IORESOURCE_MEM_64) { > + if ((sizeof(pci_bus_addr_t) < 8 || sizeof(resource_size_t) < 8) > + && sz64 > 0x100000000ULL) { > + res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; > + res->start = 0; > + res->end = 0; > + p2sb_err(bus, devfn, > + "reg 0x%x: can't handle BAR larger than 4GB (size %#010llx)\n", > + pos, (unsigned long long)sz64); > + goto out; > + } > + > + if ((sizeof(pci_bus_addr_t) < 8) && l) { > + /* Above 32-bit boundary; try to reallocate */ > + res->flags |= IORESOURCE_UNSET; > + res->start = 0; > + res->end = sz64 - 1; > + p2sb_info(bus, devfn, > + "reg 0x%x: can't handle BAR above 4GB (bus address %#010llx)\n", > + pos, (unsigned long long)l64); > + goto out; > + } > + } > + > + region.start = l64; > + region.end = l64 + sz64 - 1; > + > + pcibios_bus_to_resource(bus, res, ®ion); > + pcibios_resource_to_bus(bus, &inverted_region, res); > + > + /* > + * If "A" is a BAR value (a bus address), "bus_to_resource(A)" is > + * the corresponding resource address (the physical address used by > + * the CPU. Converting that resource address back to a bus address > + * should yield the original BAR value: > + * > + * resource_to_bus(bus_to_resource(A)) == A > + * > + * If it doesn't, CPU accesses to "bus_to_resource(A)" will not > + * be claimed by the device. > + */ > + if (inverted_region.start != region.start) { > + res->flags |= IORESOURCE_UNSET; > + res->start = 0; > + res->end = region.end - region.start; > + p2sb_info(bus, devfn, "reg 0x%x: initial BAR value %#010llx invalid\n", > + pos, (unsigned long long)region.start); > + } > + > + goto out; > + > +fail: > + res->flags = 0; > +out: > + if (res->flags) > + p2sb_info(bus, devfn, "reg 0x%x: %pR\n", pos, res); > + > + return (res->flags & IORESOURCE_MEM_64) ? 1 : 0; > +} > + > +/** > + * p2sb_bar - Get Primary to Sideband (P2SB) bridge device BAR > + * @bus: PCI bus to communicate with > + * @devfn: PCI slot and function to communicate with > + * @mem: memory resource to be filled in > + * > + * The BIOS prevents the P2SB device from being enumerated by the PCI > + * subsystem, so we need to unhide and hide it back to lookup the BAR. > + * > + * if @bus is NULL, the bus 0 in domain 0 will be in use. > + * If @devfn is 0, it will be replaced by devfn of the P2SB device. > + * > + * Caller must provide a valid pointer to @mem. > + * > + * Locking is handled by pci_rescan_remove_lock mutex. > + * > + * Return: > + * 0 on success or appropriate errno value on error. > + */ > +int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem) > +{ > + unsigned int devfn_p2sb; > + u32 value = P2SBC_HIDE; > + int ret; > + > + /* Get devfn for P2SB device itself */ > + ret = p2sb_get_devfn(&devfn_p2sb); > + if (ret) > + return ret; > + > + /* if @pdev is NULL, use bus 0 in domain 0 */ > + bus = bus ?: pci_find_bus(0, 0); > + > + /* If @devfn is 0, replace it with devfn of P2SB device itself */ > + devfn = devfn ?: devfn_p2sb; > + > + pci_lock_rescan_remove(); > + > + /* Unhide the P2SB device, if needed */ > + pci_bus_read_config_dword(bus, devfn_p2sb, P2SBC, &value); > + if ((value & P2SBC_HIDE) == P2SBC_HIDE) > + pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, 0); > + > + /* Read the first BAR of the device in question */ > + __pci_bus_read_base(bus, devfn, mem, PCI_BASE_ADDRESS_0); > + > + /* Hide the P2SB device, if it was hidden */ > + if (value & P2SBC_HIDE) > + pci_bus_write_config_dword(bus, devfn_p2sb, P2SBC, P2SBC_HIDE); > + > + pci_unlock_rescan_remove(); > + > + if (mem->flags == 0) { > + p2sb_err(bus, devfn, "Can't read BAR0: %pR\n", mem); > + return -ENODEV; > + } > + > + p2sb_info(bus, devfn, "BAR: %pR\n", mem); > + return 0; > +} > +EXPORT_SYMBOL_GPL(p2sb_bar); > diff --git a/include/linux/platform_data/x86/p2sb.h b/include/linux/platform_data/x86/p2sb.h > new file mode 100644 > index 000000000000..2f71de65aee4 > --- /dev/null > +++ b/include/linux/platform_data/x86/p2sb.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Primary to Sideband (P2SB) bridge access support > + */ > + > +#ifndef _PLATFORM_DATA_X86_P2SB_H > +#define _PLATFORM_DATA_X86_P2SB_H > + > +#include <linux/errno.h> > + > +struct pci_bus; > +struct resource; > + > +#if IS_BUILTIN(CONFIG_P2SB) > + > +int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem); > + > +#else /* CONFIG_P2SB */ > + > +static inline int p2sb_bar(struct pci_bus *bus, unsigned int devfn, struct resource *mem) > +{ > + return -ENODEV; > +} > + > +#endif /* CONFIG_P2SB is not set */ > + > +#endif /* _PLATFORM_DATA_X86_P2SB_H */