| 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
| ||
|
Date: Tue, 12 Jun 2018 17:53:16 +0530
From: poza@...eaurora.org
To: Ray Jui <ray.jui@...adcom.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Bjorn Helgaas <helgaas@...nel.org>,
linux-kernel@...r.kernel.org,
bcm-kernel-feedback-list@...adcom.com, linux-pci@...r.kernel.org,
Ray Jui <rjui@...adcom.com>, linux-pci-owner@...r.kernel.org
Subject: Re: [PATCH v2 2/5] PCI: iproc: Fix up corrupted PAXC root complex
config registers
On 2018-06-12 13:57, poza@...eaurora.org wrote:
> On 2018-06-12 05:51, Ray Jui wrote:
>> On certain versions of Broadcom PAXC based root complexes, certain
>> regions of the configuration space are corrupted. As a result, it
>> prevents the Linux PCIe stack from traversing the linked list of the
>> capability registers completely and therefore the root complex is
>> not advertised as "PCIe capable". This prevents the correct PCIe RID
>> from being parsed in the kernel PCIe stack. A correct RID is required
>> for mapping to a stream ID from the SMMU or the device ID from the
>> GICv3 ITS
>>
>> This patch fixes up the issue by manually populating the related
>> PCIe capabilities
>>
>> Signed-off-by: Ray Jui <rjui@...adcom.com>
>> ---
>> drivers/pci/host/pcie-iproc.c | 65
>> +++++++++++++++++++++++++++++++++++++++----
>> drivers/pci/host/pcie-iproc.h | 3 ++
>> 2 files changed, 62 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-iproc.c
>> b/drivers/pci/host/pcie-iproc.c
>> index 3c76c5f..680f6b1 100644
>> --- a/drivers/pci/host/pcie-iproc.c
>> +++ b/drivers/pci/host/pcie-iproc.c
>> @@ -85,6 +85,8 @@
>> #define IMAP_VALID_SHIFT 0
>> #define IMAP_VALID BIT(IMAP_VALID_SHIFT)
>>
>> +#define IPROC_PCI_PM_CAP 0x48
>> +#define IPROC_PCI_PM_CAP_MASK 0xffff
>> #define IPROC_PCI_EXP_CAP 0xac
>>
>> #define IPROC_PCIE_REG_INVALID 0xffff
>> @@ -375,6 +377,17 @@ static const u16 iproc_pcie_reg_paxc_v2[] = {
>> [IPROC_PCIE_CFG_DATA] = 0x1fc,
>> };
>>
>> +/*
>> + * List of device IDs of controllers that have corrupted capability
>> list that
>> + * require SW fixup
>> + */
>> +static const u16 iproc_pcie_corrupt_cap_did[] = {
>> + 0x16cd,
>> + 0x16f0,
>> + 0xd802,
>> + 0xd804
>> +};
>> +
>> static inline struct iproc_pcie *iproc_data(struct pci_bus *bus)
>> {
>> struct iproc_pcie *pcie = bus->sysdata;
>> @@ -495,6 +508,49 @@ static unsigned int iproc_pcie_cfg_retry(void
>> __iomem *cfg_data_p)
>> return data;
>> }
>>
>> +static void iproc_pcie_fix_cap(struct iproc_pcie *pcie, int where,
>> u32 *val)
>> +{
>> + u32 i, dev_id;
>> +
>> + switch (where & ~0x3) {
>> + case PCI_VENDOR_ID:
>> + dev_id = *val >> 16;
>> +
>> + /*
>> + * Activate fixup for those controllers that have corrupted
>> + * capability list registers
>> + */
>> + for (i = 0; i < ARRAY_SIZE(iproc_pcie_corrupt_cap_did); i++)
>> + if (dev_id == iproc_pcie_corrupt_cap_did[i])
>> + pcie->fix_paxc_cap = true;
>
> and I think this code will try to fix up every time config space is
> read.
> Does this get corrupted often, randomly ?
> Can it not be solved by using one time Quirk ?
> and if not Quirk, you dont want to be setting pcie->fix_paxc_cap =
> false somewhere
>
> besides, pcie->fix_paxc_cap = true; is set if PCI_VENDOR_ID is read
> first.
> and rest cases stay with the assumption that PCI_VENDOR_ID will be read
> first.
> which is infact read first during enumeration
> (that is the assumption code is making), but that is safe assumption
> to make I think.
>
ok I see that Bjorn has suggested to fix it this way instead of Quirks.
will just mark
Reviewed-by: Oza Pawandeep <poza@...eaurora.org>
>> + break;
>> +
>> + case IPROC_PCI_PM_CAP:
>> + if (pcie->fix_paxc_cap) {
>> + /* advertise PM, force next capability to PCIe */
>> + *val &= ~IPROC_PCI_PM_CAP_MASK;
>> + *val |= IPROC_PCI_EXP_CAP << 8 | PCI_CAP_ID_PM;
>> + }
>> + break;
>> +
>> + case IPROC_PCI_EXP_CAP:
>> + if (pcie->fix_paxc_cap) {
>> + /* advertise root port, version 2, terminate here */
>> + *val = (PCI_EXP_TYPE_ROOT_PORT << 4 | 2) << 16 |
>> + PCI_CAP_ID_EXP;
>> + }
>> + break;
>> +
>> + case IPROC_PCI_EXP_CAP + PCI_EXP_RTCTL:
>> + /* Don't advertise CRS SV support */
>> + *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16);
>> + break;
>> +
>> + default:
>> + break;
>> + }
>> +}
>> +
>> static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int
>> devfn,
>> int where, int size, u32 *val)
>> {
>> @@ -509,13 +565,10 @@ static int iproc_pcie_config_read(struct pci_bus
>> *bus, unsigned int devfn,
>> /* root complex access */
>> if (busno == 0) {
>> ret = pci_generic_config_read32(bus, devfn, where, size, val);
>> - if (ret != PCIBIOS_SUCCESSFUL)
>> - return ret;
>> + if (ret == PCIBIOS_SUCCESSFUL)
>> + iproc_pcie_fix_cap(pcie, where, val);
>>
>> - /* Don't advertise CRS SV support */
>> - if ((where & ~0x3) == IPROC_PCI_EXP_CAP + PCI_EXP_RTCTL)
>> - *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16);
>> - return PCIBIOS_SUCCESSFUL;
>> + return ret;
>> }
>>
>> cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn,
>> where);
>> diff --git a/drivers/pci/host/pcie-iproc.h
>> b/drivers/pci/host/pcie-iproc.h
>> index 814b600..9d5cfee 100644
>> --- a/drivers/pci/host/pcie-iproc.h
>> +++ b/drivers/pci/host/pcie-iproc.h
>> @@ -60,6 +60,8 @@ struct iproc_msi;
>> * @ep_is_internal: indicates an internal emulated endpoint device is
>> connected
>> * @has_apb_err_disable: indicates the controller can be configured
>> to prevent
>> * unsupported request from being forwarded as an APB bus error
>> + * @fix_paxc_cap: indicates the controller has corrupted capability
>> list in its
>> + * config space registers and requires SW based fixup
>> *
>> * @need_ob_cfg: indicates SW needs to configure the outbound mapping
>> window
>> * @ob: outbound mapping related parameters
>> @@ -85,6 +87,7 @@ struct iproc_pcie {
>> int (*map_irq)(const struct pci_dev *, u8, u8);
>> bool ep_is_internal;
>> bool has_apb_err_disable;
>> + bool fix_paxc_cap;
>>
>> bool need_ob_cfg;
>> struct iproc_pcie_ob ob;
Powered by blists - more mailing lists