On Thu, Aug 06, 2020 at 02:57:34PM -0700, Tuan Phan wrote:
> Ampere Altra SOC supports only 32-bit ECAM reading. Therefore,
> add an MCFG quirk for the platform.
This is interesting. So this host bridge supports sub 32-bit config
*writes*, but not reads?
I actually don't know whether that complies with the spec or not. If
config registers are not allowed to have side effects on read, this
*would* be compliant.
PCIe r5.0, sec 7.4, doesn't list any register types with read side
effects, so there shouldn't be any in the registers defined by the
spec. But I would think device-specific registers could do whatever
they wanted, e.g., reading an interrupt status register or something
could clear it.
And I think sec 7.2.2 about ECAM implicitly requires sub 32-bit
accesses because it mentions the access size and byte enables.
Is this a one-off situation where future hardware will allow sub
32-bit reads and writes? We don't want a stream of quirks for future
devices.
> Signed-off-by: Tuan Phan <tuanphan@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/acpi/pci_mcfg.c | 20 ++++++++++++++++++++
> drivers/pci/ecam.c | 10 ++++++++++
> include/linux/pci-ecam.h | 1 +
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index 54b36b7ad47d..e526571e0ebd 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -142,6 +142,26 @@ static struct mcfg_fixup mcfg_quirks[] = {
> XGENE_V2_ECAM_MCFG(4, 0),
> XGENE_V2_ECAM_MCFG(4, 1),
> XGENE_V2_ECAM_MCFG(4, 2),
> +
> +#define ALTRA_ECAM_QUIRK(rev, seg) \
> + { "Ampere", "Altra ", rev, seg, MCFG_BUS_ANY, &pci_32b_read_ops }
> +
> + ALTRA_ECAM_QUIRK(1, 0),
> + ALTRA_ECAM_QUIRK(1, 1),
> + ALTRA_ECAM_QUIRK(1, 2),
> + ALTRA_ECAM_QUIRK(1, 3),
> + ALTRA_ECAM_QUIRK(1, 4),
> + ALTRA_ECAM_QUIRK(1, 5),
> + ALTRA_ECAM_QUIRK(1, 6),
> + ALTRA_ECAM_QUIRK(1, 7),
> + ALTRA_ECAM_QUIRK(1, 8),
> + ALTRA_ECAM_QUIRK(1, 9),
> + ALTRA_ECAM_QUIRK(1, 10),
> + ALTRA_ECAM_QUIRK(1, 11),
> + ALTRA_ECAM_QUIRK(1, 12),
> + ALTRA_ECAM_QUIRK(1, 13),
> + ALTRA_ECAM_QUIRK(1, 14),
> + ALTRA_ECAM_QUIRK(1, 15),
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index 8f065a42fc1a..b54d32a31669 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -168,4 +168,14 @@ const struct pci_ecam_ops pci_32b_ops = {
> .write = pci_generic_config_write32,
> }
> };
> +
> +/* ECAM ops for 32-bit read only (non-compliant) */
> +const struct pci_ecam_ops pci_32b_read_ops = {
> + .bus_shift = 20,
> + .pci_ops = {
> + .map_bus = pci_ecam_map_bus,
> + .read = pci_generic_config_read32,
> + .write = pci_generic_config_write,
> + }
> +};
> #endif
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 1af5cb02ef7f..033ce74f02e8 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -51,6 +51,7 @@ extern const struct pci_ecam_ops pci_generic_ecam_ops;
>
> #if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
> extern const struct pci_ecam_ops pci_32b_ops; /* 32-bit accesses only */
> +extern const struct pci_ecam_ops pci_32b_read_ops; /* 32-bit read only */
> extern const struct pci_ecam_ops hisi_pcie_ops; /* HiSilicon */
> extern const struct pci_ecam_ops thunder_pem_ecam_ops; /* Cavium ThunderX 1.x & 2.x */
> extern const struct pci_ecam_ops pci_thunder_ecam_ops; /* Cavium ThunderX 1.x */
> --
> 2.18.4
>
