[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAErSpo7Z5x=U3vSnxrFpwYUDrUrnrrBCe1F9_GQNYqWWQFWNpQ@mail.gmail.com>
Date: Wed, 12 Nov 2014 10:29:11 -0700
From: Bjorn Helgaas <bhelgaas@...gle.com>
To: Michael Jamet <michael.jamet@...el.com>
Cc: "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
amir.jer.levy@...el.com, dan.alloun@...el.com,
Rafael Wysocki <rjw@...ysocki.net>,
Andreas Noever <andreas.noever@...il.com>
Subject: Re: [PATCH] pci: support Thunderbolt requirements for I/O resources.
[+cc Rafael, Andreas]
On Wed, Nov 12, 2014 at 7:52 PM, Michael Jamet <michael.jamet@...el.com> wrote:
> Every Thunderbolt-based devices or Thunderbolt-connected
> devices should not allocate PCI I/O resources
> per Thunderbolt specs.
Please include a pointer to those specs in the changelog.
> On a Thunderbolt PC, BIOS is responsible to allocate IO
> resources. Kernel shouldn't allocate the PCI I/O resources
> as it interferes with BIOS operation.
> Doing this may cause the devices in the Thunderbolt chain
> not being detected or added, or worse to stuck the
> Thunderbolt Host controller.
These new kernel/firmware coordination requirements need to be
documented. If they're already part of a PCIe ECN or PCI firmware
spec, just provide a pointer.
> To prevent this, we detect a chain contains a Thunderbolt
> device by checking the Thunderbolt PCI device id.
I'm really not happy about checking a list of device IDs to identify
Thunderbolt devices. Surely there's a better way, because a list like
this has to be updated regularly.
Bjorn
> The validation is carried out at the pci_enable_device_flags()
> function that checks the PCI device or bridge is Thunderbolt
> chained and avoid IO resources allocation.
>
> In addition, decode_bar() and pci_bridge_check_ranges()
> function has been internally checked for Thunderbolt
> as well to ensure no IO resources are allocated.
>
> Signed-off-by: Michael Jamet <michael.jamet@...el.com>
> ---
> drivers/pci/pci.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 2 ++
> drivers/pci/probe.c | 3 ++-
> drivers/pci/setup-bus.c | 4 ++--
> include/linux/pci.h | 2 ++
> 5 files changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 625a4ac..41c2619 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -198,6 +198,62 @@ static int __pci_bus_find_cap_start(struct pci_bus *bus,
> }
>
> /**
> + * pci_is_thunderbolt_device - checks PCI device is Thunderbolt-based.
> + * The only existing way is to check the device id is allocated to Thunderbolt.
> + * @dev: the PCI device structure to check against
> + *
> + * Returns true if the PCI device is of the Thunderbolt type.
> + */
> +bool pci_is_thunderbolt_device(struct pci_dev *dev)
> +{
> + if ((dev->vendor == PCI_VENDOR_ID_INTEL) &&
> + ((dev->device == 0xcaca)
> + || (dev->device == 0x1513)
> + || (dev->device == 0xcbca)
> + || (dev->device == 0x151A)
> + || (dev->device == 0x151B)
> + || (dev->device == 0x1549)
> + || (dev->device == 0x1547)
> + || (dev->device == 0x1548)
> + || (dev->device == 0x1566)
> + || (dev->device == 0x1567)
> + || (dev->device == 0x1569)
> + || (dev->device == 0x1568)
> + || (dev->device == 0x156A)
> + || (dev->device == 0x156B)
> + || (dev->device == 0x156C)
> + || (dev->device == 0x156D)
> + || (dev->device == 0x1579)
> + || (dev->device == 0x157A)
> + || (dev->device == 0x157D)
> + || (dev->device == 0x157E)))
> + return true;
> +
> + return false;
> +}
> +EXPORT_SYMBOL(pci_is_thunderbolt_device);
> +
> +/**
> + * pci_is_connected_to_thunderbolt - check if connected to a Thunderbolt chain.
> + * @dev: the PCI device structure to check against
> + *
> + * Returns true if the PCI device s connected is connected to a
> + * Thunderbolt-based in the chain.
> + */
> +bool pci_is_connected_to_thunderbolt(struct pci_dev *dev)
> +{
> + struct pci_dev *bridge;
> +
> + if (pci_is_thunderbolt_device(dev))
> + return true;
> + bridge = pci_upstream_bridge(dev);
> + if (bridge)
> + return pci_is_connected_to_thunderbolt(bridge);
> + return false;
> +}
> +EXPORT_SYMBOL(pci_is_connected_to_thunderbolt);
> +
> +/**
> * pci_find_capability - query for devices' capabilities
> * @dev: PCI device to query
> * @cap: capability code
> @@ -1285,6 +1341,14 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
> if (atomic_inc_return(&dev->enable_cnt) > 1)
> return 0; /* already enabled */
>
> + /*
> + * if IO resource have been requested, but the device is connected
> + * to Thunderbolt chain, don't allocate IO resource
> + */
> + if ((flags & IORESOURCE_IO)
> + && pci_is_connected_to_thunderbolt(dev))
> + flags &= ~IORESOURCE_IO;
> +
> bridge = pci_upstream_bridge(dev);
> if (bridge)
> pci_enable_bridge(bridge);
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 0601890..fb9a3b1 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -4,6 +4,8 @@
> #define PCI_CFG_SPACE_SIZE 256
> #define PCI_CFG_SPACE_EXP_SIZE 4096
>
> +bool pci_is_thunderbolt_device(struct pci_dev *dev);
> +
> extern const unsigned char pcie_link_speed[];
>
> /* Functions internal to the PCI core code */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 5ed9930..d8171af 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -131,7 +131,8 @@ static inline unsigned long decode_bar(struct pci_dev *dev, u32 bar)
>
> if ((bar & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
> flags = bar & ~PCI_BASE_ADDRESS_IO_MASK;
> - flags |= IORESOURCE_IO;
> + if (!pci_is_connected_to_thunderbolt(dev))
> + flags |= IORESOURCE_IO;
> return flags;
> }
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 0482235..79ac38f 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -663,12 +663,12 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
> b_res[1].flags |= IORESOURCE_MEM;
>
> pci_read_config_word(bridge, PCI_IO_BASE, &io);
> - if (!io) {
> + if (!io || pci_is_thunderbolt_device(bridge)) {
> pci_write_config_word(bridge, PCI_IO_BASE, 0xe0f0);
> pci_read_config_word(bridge, PCI_IO_BASE, &io);
> pci_write_config_word(bridge, PCI_IO_BASE, 0x0);
> }
> - if (io)
> + if (io && !pci_is_thunderbolt_device(bridge))
> b_res[0].flags |= IORESOURCE_IO;
>
> /* DECchip 21050 pass 2 errata: the bridge may miss an address
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2a8c405..09ddc2c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -931,6 +931,8 @@ int __must_check pci_enable_device_io(struct pci_dev *dev);
> int __must_check pci_enable_device_mem(struct pci_dev *dev);
> int __must_check pci_reenable_device(struct pci_dev *);
> int __must_check pcim_enable_device(struct pci_dev *pdev);
> +
> +bool pci_is_connected_to_thunderbolt(struct pci_dev *dev);
> void pcim_pin_device(struct pci_dev *pdev);
>
> static inline int pci_is_enabled(struct pci_dev *pdev)
> --
> 1.9.1
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists