Hi,
This patch adds support for PCI to AArch64. It is based on the patch posted
here [1] that adds support for creating generic host bridge structure from
device tree. With that in place, I was able to boot an FPGA platform that
has PCIe host bridge support and use a PCIe network card.
The API used is different from the one used by ARM architecture. There is
no pci_common_init_dev() function and no hw_pci structure, as that is no
longer needed. Once the last signature is added to the legal agreement, I
will post the host bridge driver code that I am using. Meanwhile, here
is an example of what the probe function looks like, posted as an example:
static int myhostbridge_probe(struct platform_device *pdev)
{
int err;
struct device_node *dev;
struct pci_host_bridge *bridge;
struct resource bus_range;
struct myhostbridge_port *pp;
LIST_HEAD(resources);
dev = pdev->dev.of_node;
if (!of_device_is_available(dev)) {
pr_warn("%s: disabled\n", dev->full_name);
return -ENODEV;
}
pp = kzalloc(sizeof(struct myhostbridge_port), GFP_KERNEL);
if (!pp)
return -ENOMEM;
err = of_pci_parse_bus_range(dev, &bus_range);
if (err) {
bus_range.start = 0;
bus_range.end = 255;
bus_range.flags = IORESOURCE_BUS;
}
pci_add_resource(&resources, &bus_range);
bridge = pci_host_bridge_of_init(&pdev->dev, 0, &myhostbridge_ops, pp, &resources);
if (!bridge) {
err = -EINVAL;
goto bridge_init_fail;
}
err = myhostbridge_setup(bridge->bus);
if (err)
goto bridge_init_fail;
/*
* Add flags here, this is just an example
*/
pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
pci_add_flags(PCI_REASSIGN_ALL_BUS | PCI_REASSIGN_ALL_RSRC);
bus_range.end = pci_scan_child_bus(bridge->bus);
pci_bus_update_busn_res_end(bridge->bus, bus_range.end);
pci_assign_unassigned_bus_resources(bridge->bus);
pci_bus_add_devices(bridge->bus);
return 0;
bridge_init_fail:
kfree(pp);
pci_free_resource_list(&resources);
return err;
}
Best regards,
Liviu
[1] http://marc.info/?l=linux-pci&m=139145254821334&w=2
Liviu Dudau (1):
arm64: Add support for PCI
arch/arm64/Kconfig | 17 +++++++
arch/arm64/include/asm/Kbuild | 1 +
arch/arm64/include/asm/io.h | 4 ++
arch/arm64/include/asm/pci.h | 35 +++++++++++++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/pci.c | 112 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 170 insertions(+)
create mode 100644 arch/arm64/include/asm/pci.h
create mode 100644 arch/arm64/kernel/pci.c
--
1.8.5.3
Use the generic host bridge functions to provide support for
PCI Express on arm64. There is no support for ISA memory.
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
arch/arm64/Kconfig | 17 +++++++
arch/arm64/include/asm/Kbuild | 1 +
arch/arm64/include/asm/io.h | 4 ++
arch/arm64/include/asm/pci.h | 35 +++++++++++++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/pci.c | 112 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 170 insertions(+)
create mode 100644 arch/arm64/include/asm/pci.h
create mode 100644 arch/arm64/kernel/pci.c
And modify this location: There is no particular reason to have the I/O space
mapped exactly 2MB below the loadable modules, as virtual address space is
essentially free.
None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.
Not sure if we want to treat this one as architecture specific though.
It certainly won't be portable to x86, but it could be shared with
a couple of others. We may also want to redesign the interface.
I've been thinking we could make this function allocate space in the
Linux virtual I/O space aperture, and pass two resources into it
(physical I/O aperture and bus I/O range), and get the actual
io_offset as the return value, or a negative error number.
That way, you could have an arbitrary number of host bridges in the
system and each one gets a share of the virtual aperture until
it's full.
Arnd
Maybe I got it the wrong way around, but the comment in include/linux/io.h says:
/*
* Some systems do not have legacy ISA devices.
* /dev/port is not a valid interface on these systems.
* So for those archs, <asm/io.h> should define the following symbol.
*/
So ... defining it should mean no legacy ISA devices, right?
quoted
#define IO_SPACE_LIMIT 0xffff
You probably want to increase this a bit, to allow multiple host bridges
to have their own I/O space.
And modify this location: There is no particular reason to have the I/O space
mapped exactly 2MB below the loadable modules, as virtual address space is
essentially free.
None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.
Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)
Not sure if we want to treat this one as architecture specific though.
It certainly won't be portable to x86, but it could be shared with
a couple of others. We may also want to redesign the interface.
I've been thinking we could make this function allocate space in the
Linux virtual I/O space aperture, and pass two resources into it
(physical I/O aperture and bus I/O range), and get the actual
io_offset as the return value, or a negative error number.
Not sure I completely follow your idea.
That way, you could have an arbitrary number of host bridges in the
system and each one gets a share of the virtual aperture until
it's full.
One still needs to fix the pci_request_region use that checks against
ioport_resource. But it is an interesting idea.
Arnd
Thanks for reviewing this patch!
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
kodiak furr liked your message with Boxer. On February 3, 2014 at 1:18:38 PM CST, Liviu Dudau wrote:On Mon, Feb 03, 2014 at 06:58:56PM +0000, Arnd Bergmann wrote:> On Monday 03 February 2014 18:43:48 Liviu Dudau wrote:> > diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h> > index 4cc813e..ce5bad2 100644> > --- a/arch/arm64/include/asm/io.h> > +++ b/arch/arm64/include/asm/io.h> > @@ -120,9 +120,13 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)> > /*> > * I/O port access primitives.> > */> > +#define arch_has_dev_port() (0)> > Why not?Maybe I got it the wrong way around, but the comment in include/linux/io.h says:/* * Some systems do not have legacy ISA devices. * /dev/port is not a valid interface on these systems. * So for those archs, should define the following symbol. */So ... defining it should mean no legacy ISA devices, right?> > > #define IO_SPACE_LIMIT 0xffff> > You probably want to increase this a bit, to allow multiple host bridges> to have their own I/O space.OK, but to what size?> > > #define PCI_IOBASE ((void __iomem *)(MODULES_VADDR - SZ_2M))> > And modify this location: There is no particular reason to have the I/O space> mapped exactly 2MB below the loadable modules, as virtual address space is> essentially free.Will talk with Catalin about where to place this.> > > +#define ioport_map(port, nr) (PCI_IOBASE + ((port) & IO_SPACE_LIMIT))> > +#define ioport_unmap(addr)> > inline functions?Will do, thanks!> > > static inline u8 inb(unsigned long addr)> > {> > return readb(addr + PCI_IOBASE);> > diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h> > new file mode 100644> > index 0000000..dd084a3> > --- /dev/null> > +++ b/arch/arm64/include/asm/pci.h> > @@ -0,0 +1,35 @@> > +#ifndef __ASM_PCI_H> > +#define __ASM_PCI_H> > +#ifdef __KERNEL__> > +> > +#include > > +#include > > +#include > > +> > +#include > > +#include > > +#include > > +> > +#define PCIBIOS_MIN_IO 0> > +#define PCIBIOS_MIN_MEM 0> > PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.:) No ISA support! (Die ISA, die!!) > > > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c> > new file mode 100644> > index 0000000..7b652cf> > --- /dev/null> > +++ b/arch/arm64/kernel/pci.c> > @@ -0,0 +1,112 @@> > None of this looks really arm64 specific, nor should it be. I think> we should try a little harder to move this as a default implementation> into common code, even if we start out by having all architectures> override it.Agree. This is the RFC version. I didn't dare to post a patch with fixesfor all architectures. :)> > > +int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)> > +{> > + BUG_ON(offset + SZ_64K - 1 > IO_SPACE_LIMIT);> > +> > + return ioremap_page_range((unsigned long)PCI_IOBASE + offset,> > + (unsigned long)PCI_IOBASE + offset + SZ_64K,> > + phys_addr,> > + __pgprot(PROT_DEVICE_nGnRE));> > +}> > Not sure if we want to treat this one as architecture specific though.> It certainly won't be portable to x86, but it could be shared with> a couple of others. We may also want to redesign the interface.> I've been thinking we could make this function allocate space in the> Linux virtual I/O space aperture, and pass two resources into it> (physical I/O aperture and bus I/O range), and get the actual> io_offset as the return value, or a negative error number.Not sure I completely follow your idea.> > That way, you could have an arbitrary number of host bridges in the> system and each one gets a share of the virtual aperture until> it's full.One still needs to fix the pci_request_region use that checks againstioport_resource. But it is an interesting idea.> > Arnd> > Thanks for reviewing this patch!Liviu--
Maybe I got it the wrong way around, but the comment in include/linux/io.h says:
/*
* Some systems do not have legacy ISA devices.
* /dev/port is not a valid interface on these systems.
* So for those archs, <asm/io.h> should define the following symbol.
*/
So ... defining it should mean no legacy ISA devices, right?
I would read that comment as referring to systems that don't have
any I/O space. If you have PCI, you can by definition have ISA
compatible devices behind a bridge. A typical example would be
a VGA card that supports the 03c0-03df port range.
quoted
quoted
#define IO_SPACE_LIMIT 0xffff
You probably want to increase this a bit, to allow multiple host bridges
to have their own I/O space.
OK, but to what size?
2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
take up too much virtual space. On arm64 it should be at least as big.
Could be more than that, although I don't see a reason why it should be,
unless we expect to see systems with tons of host bridges, or buses
that exceed 64KB of I/O space.
I suppose you can actually use the generic implementation from
asm-generic/io.h, and fix it by using the definition you have
above, since it's currently broken.
None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.
Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)
No need to change the other architectures. You can make it opt-in for
now and just put the code into a common location.
An interesting question however is what the transition plan is to
have the code shared between arm32 and arm64: We will certainly need
to share at least the dw-pcie and the generic SBSA compliant pci
implementation.
Not sure if we want to treat this one as architecture specific though.
It certainly won't be portable to x86, but it could be shared with
a couple of others. We may also want to redesign the interface.
I've been thinking we could make this function allocate space in the
Linux virtual I/O space aperture, and pass two resources into it
(physical I/O aperture and bus I/O range), and get the actual
io_offset as the return value, or a negative error number.
Not sure I completely follow your idea.
Something like this (coded in mail client, don't try to compile):
#define IO_SPACE_PAGES (IO_SPACE_LIMIT + 1) / PAGE_SIZE)
static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
unsigned long pci_ioremap_io(const struct resource *bus, const struct resource phys)
{
unsigned long start, len, virt_start;
int error;
/* use logical address == bus address if possible */
start = bus->start / PAGE_SIZE;
if (start > IO_SPACE_LIMIT / PAGE_SIZE)
start = 0;
/*
* try finding free space for the whole size first,
* fall back to 64K if not available
*/
len = min(resource_size(bus), resource_size(phys);
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
if (start == IO_SPACE_PAGES && len > SZ_64K)
len = SZ_64K;
start = 0;
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
}
/* no 64K area found */
if (start == IO_SPACE_PAGES)
return -ENOMEM;
/* ioremap physical aperture to virtual aperture */
virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
error = ioremap_page_range(virt_start, virt_start + len,
phys->start, __pgprot(PROT_DEVICE_nGnRE));
if (error)
return error;
bitmap_set(start, len / PAGE_SIZE);
/* return io_offset */
return start * PAGE_SIZE - bus->start;
}
EXPORT_SYMBOL_GPL(pci_ioremap_io);
Arnd
Maybe I got it the wrong way around, but the comment in include/linux/io.h says:
/*
* Some systems do not have legacy ISA devices.
* /dev/port is not a valid interface on these systems.
* So for those archs, <asm/io.h> should define the following symbol.
*/
So ... defining it should mean no legacy ISA devices, right?
I would read that comment as referring to systems that don't have
any I/O space. If you have PCI, you can by definition have ISA
compatible devices behind a bridge. A typical example would be
a VGA card that supports the 03c0-03df port range.
But if you have PCI and don't want to support ISA do you have /dev/port? I guess yes.
In that case ISA support is irrelevant here. Will update the value when I send v2.
quoted
quoted
quoted
#define IO_SPACE_LIMIT 0xffff
You probably want to increase this a bit, to allow multiple host bridges
to have their own I/O space.
OK, but to what size?
2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
take up too much virtual space. On arm64 it should be at least as big.
Could be more than that, although I don't see a reason why it should be,
unless we expect to see systems with tons of host bridges, or buses
that exceed 64KB of I/O space.
I suppose you can actually use the generic implementation from
asm-generic/io.h, and fix it by using the definition you have
above, since it's currently broken.
Not exactly broken, but it makes the assumption that your IO space starts at
physical address zero and you have not remapped it. It does guard the
definition with #ifndef CONFIG_GENERIC_IOMAP after all, so it does not
expect to be generic :)
PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.
:) No ISA support! (Die ISA, die!!)
If only it were that easy.
Lets try! :)
I wonder how many active devices that have an ISA slot are still supported
by mainline kernel.
I will update PCIBIOS_MIN_xxxx to match arch/arm for v2.
None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.
Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)
No need to change the other architectures. You can make it opt-in for
now and just put the code into a common location.
An interesting question however is what the transition plan is to
have the code shared between arm32 and arm64: We will certainly need
to share at least the dw-pcie and the generic SBSA compliant pci
implementation.
My vote would be for updating the host controllers to the new API and
to offer the CONFIG option to choose between arch APIs. The alternative
is to use the existing API to wrap the generic implementation.
My main concern with the existing API is the requirement to have a subsys_initcall
in your host bridge or mach code, due to the way the initialisation is done (you
need the DT code to probe your driver, but you cannot start the scanning of the
PCI bus until the arch code is initialised, so it gets deferred via
subsys_initcall when it calls pci_common_init). I bet that if one tries to
instantiate a Tegra PCI host bridge controller on a Marvell platform things will
break pretty quickly (random example here).
Not sure if we want to treat this one as architecture specific though.
It certainly won't be portable to x86, but it could be shared with
a couple of others. We may also want to redesign the interface.
I've been thinking we could make this function allocate space in the
Linux virtual I/O space aperture, and pass two resources into it
(physical I/O aperture and bus I/O range), and get the actual
io_offset as the return value, or a negative error number.
Not sure I completely follow your idea.
Something like this (coded in mail client, don't try to compile):
#define IO_SPACE_PAGES (IO_SPACE_LIMIT + 1) / PAGE_SIZE)
static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
unsigned long pci_ioremap_io(const struct resource *bus, const struct resource phys)
{
unsigned long start, len, virt_start;
int error;
/* use logical address == bus address if possible */
start = bus->start / PAGE_SIZE;
if (start > IO_SPACE_LIMIT / PAGE_SIZE)
start = 0;
/*
* try finding free space for the whole size first,
* fall back to 64K if not available
*/
len = min(resource_size(bus), resource_size(phys);
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
if (start == IO_SPACE_PAGES && len > SZ_64K)
len = SZ_64K;
start = 0;
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
}
/* no 64K area found */
if (start == IO_SPACE_PAGES)
return -ENOMEM;
/* ioremap physical aperture to virtual aperture */
virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
error = ioremap_page_range(virt_start, virt_start + len,
phys->start, __pgprot(PROT_DEVICE_nGnRE));
if (error)
return error;
bitmap_set(start, len / PAGE_SIZE);
/* return io_offset */
return start * PAGE_SIZE - bus->start;
}
EXPORT_SYMBOL_GPL(pci_ioremap_io);
Arnd
I see. I need to think how this will change the existing code. Current users of pci_ioremap_io
ask for multiples of SZ_64K offsets regardless of the actual need.
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
I'm not sure that this will work. The in[bwl], out[bwl] macros in
arch/arm64/include/asm/io.h already add the PCI_IOBASE offset.
Instead of these two #defines, why not just enforce that
GENERIC_PCI_IOMAP is enabled? Or at least wrap these defines with 'if
(!config_enabled(CONFIG_GENERIC_PCI_IOMAP))' or similar.
From: Rob Herring <hidden> Date: 2014-02-03 23:07:51
On Mon, Feb 3, 2014 at 2:05 PM, Arnd Bergmann [off-list ref] wrote:
On Monday 03 February 2014 19:18:38 Liviu Dudau wrote:
quoted
On Mon, Feb 03, 2014 at 06:58:56PM +0000, Arnd Bergmann wrote:
quoted
On Monday 03 February 2014 18:43:48 Liviu Dudau wrote:
[snip]
quoted
Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)
No need to change the other architectures. You can make it opt-in for
now and just put the code into a common location.
An interesting question however is what the transition plan is to
have the code shared between arm32 and arm64: We will certainly need
to share at least the dw-pcie and the generic SBSA compliant pci
implementation.
You might want to re-read the SBSA. Unless ARM provides an IP block or
there is some other standard such as EHCI or AHCI, there is no generic
implementation. You only have to go look at the Linux EHCI or AHCI
drivers and see how meaningless and inadequate "use EHCI" is. For PCI,
the text is so brief in the SBSA there will be no way PCI is going to
just work given all the variations of root complexes, bridges, address
windows, etc. we typically see on ARM platforms. I could be wrong and
some AML magic will solve all the problems. :)
Rob
From: Jason Gunthorpe <hidden> Date: 2014-02-03 23:31:46
On Mon, Feb 03, 2014 at 05:07:48PM -0600, Rob Herring wrote:
quoted
An interesting question however is what the transition plan is to
have the code shared between arm32 and arm64: We will certainly need
to share at least the dw-pcie and the generic SBSA compliant pci
implementation.
You might want to re-read the SBSA. Unless ARM provides an IP block or
there is some other standard such as EHCI or AHCI, there is no generic
implementation. You only have to go look at the Linux EHCI or AHCI
drivers and see how meaningless and inadequate "use EHCI" is. For PCI,
the text is so brief in the SBSA there will be no way PCI is going to
just work given all the variations of root complexes, bridges, address
windows, etc. we typically see on ARM platforms. I could be wrong and
some AML magic will solve all the problems. :)
The biggest hinderance I've seen while looking at ARM PCI drivers is
quite simply - they don't follow the PCI-E spec. There is a spec, and
it is not followed.
This fixup in the X-Gene is a solid example, the purpose of the BAR in
a PCI-PCI bridge is very clear: using it to specify the system
DRAM aperature is completely wrong.
Not having working aperture windows in the root complex's bridges is
completely wrong.
Lacking any ability to generate 8 and 16 bit config write TLP's is
wrong.
Starting with an end-port PCI-E core and re-tasking it to be a root
port bridge and ignoring all the requirements with the bridge's config
space is utterly and completely WRONG.
Specifying 'use EHCI, AHCI, etc' - which are all PCI based standards
without clearly specifying exactly how PCI is suppose to work is
completely bonkers.
What is needed is a spec that says:
1) Here is how you generate config TLPs. A MMIO region that
conforms to the already specified x86 ECAM would
be perfect
2) Here is a dword by dword break down of the entire config space in
a SOC. Here is where a on-board AHCI controller must show up in
config space. Here is how an external PCI-E port must show
up. Etc. Most of this is already specified, but it clearly needs
to be layed out explicitly for ARM SOCs to actually follow it.
3) Here is how you specify the aperture(s) associated with PCI BAR's
and bridge windows in config space. And yes: The CONFIG SPACE
BARS MUST WORK.
4) Here is how MSI works, these are the values you put in the
address/data and here is how you collect the interrupt.
5) Here is how Legacy INTx must be mapped into the GIC.
This is what x86 does, and they have been doing it well for 10
years. If you want to play in the server game you have to properly
implement PCI.
Jason
On Monday 03 February 2014 21:36:58 Liviu Dudau wrote:
On Mon, Feb 03, 2014 at 08:05:56PM +0000, Arnd Bergmann wrote:
quoted
On Monday 03 February 2014 19:18:38 Liviu Dudau wrote:
quoted
So ... defining it should mean no legacy ISA devices, right?
I would read that comment as referring to systems that don't have
any I/O space. If you have PCI, you can by definition have ISA
compatible devices behind a bridge. A typical example would be
a VGA card that supports the 03c0-03df port range.
But if you have PCI and don't want to support ISA do you have /dev/port? I guess yes.
Right, that is my interpretation. You could still have a tool
that tries to poke /dev/port from user space for any I/O BAR
the same way you'd use /dev/mem for memory BARs of PCI devices.
It's discouraged, but it's often the easiest solution for
a quick hack, and I'd expect tools to use this.
quoted
quoted
quoted
quoted
#define IO_SPACE_LIMIT 0xffff
You probably want to increase this a bit, to allow multiple host bridges
to have their own I/O space.
OK, but to what size?
2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
take up too much virtual space. On arm64 it should be at least as big.
Could be more than that, although I don't see a reason why it should be,
unless we expect to see systems with tons of host bridges, or buses
that exceed 64KB of I/O space.
I will increase the size to 2MB for v2.
Thinking about this some more, I'd go a little higher. In case of
pci_mv, we already register a 1MB region for one logical host
(which has multiple I/O spaces behind an emulated bridge), so
going to 16MB or more would let us handle multiple 1MB windows
for some amount of future proofing.
Maybe Catalin can assign us some virtual address space to use,
with the constraints that it should be 16MB or more in an
area that doesn't require additional kernel page table pages.
I suppose you can actually use the generic implementation from
asm-generic/io.h, and fix it by using the definition you have
above, since it's currently broken.
Not exactly broken, but it makes the assumption that your IO space starts at
physical address zero and you have not remapped it. It does guard the
definition with #ifndef CONFIG_GENERIC_IOMAP after all, so it does not
expect to be generic :)
Well, I/O space never starts at physical zero in reality, so it is
broken in practice. The CONFIG_GENERIC_IOMAP option tries to solve
the problem of I/O spaces that are not memory mapped, which is
actually quite rare (x86, ia64, some rare powerpc bridges, and possibly
Alpha). The norm is that if you have I/O space, it is memory mapped
and you don't need GENERIC_IOMAP. I think most of the architectures
selecting GENERIC_IOMAP have incorrectly copied that from x86.
PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.
:) No ISA support! (Die ISA, die!!)
If only it were that easy.
Lets try! :)
I wonder how many active devices that have an ISA slot are still supported
by mainline kernel.
This is missing the point, but any architecture that has a PCI
slot can have an ISA device behind a bridge like this:
http://www.altera.com/products/ip/iup/pci/m-eur-pci-to-isa.html
The real reason is that a lot of PCI cards for practical reasons
expose some non-relocatable memory and I/O BARs in ISA-compatible
locations. Looking at /proc/ioports on my PC, I can spot a couple
of things that may well show up on any ARM machine:
0000-03af : PCI Bus 0000:00
02f8-02ff : serial
03c0-03df : PCI Bus 0000:40
03c0-03df : PCI Bus 0000:00
03c0-03df : vga+
03e0-0cf7 : PCI Bus 0000:00
03e8-03ef : serial
03f8-03ff : serial
0b00-0b0f : pnp 00:08
0b00-0b07 : piix4_smbus
0b20-0b3f : pnp 00:08
0b20-0b27 : piix4_smbus
0ca2-0ca3 : pnp 00:08
0ca2-0ca2 : ipmi_si
0ca3-0ca3 : ipmi_si
0cf8-0cff : PCI conf1
Nothing wrong with the above. Now, it's also possible that
someone decides to build an ARM server by using a PC south
bridge with integrated legacy PC peripherals, such as these:
0000-03af : PCI Bus 0000:00
0000-001f : dma1
0020-0021 : pic1
0040-0043 : timer0
0050-0053 : timer1
0060-0060 : keyboard
0064-0064 : keyboard
0070-0071 : rtc0
0080-008f : dma page reg
00a0-00a1 : pic2
00b0-00b0 : APEI ERST
00c0-00df : dma2
00f0-00ff : fpu
There is some hope that it won't happen, but these things
exist and come with a regular PCIe front-end bus in some
cases.
Finally, there is the LPC bus, which can give you additional
ISAPnP compatible devices.
I will update PCIBIOS_MIN_xxxx to match arch/arm for v2.
None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.
Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)
No need to change the other architectures. You can make it opt-in for
now and just put the code into a common location.
An interesting question however is what the transition plan is to
have the code shared between arm32 and arm64: We will certainly need
to share at least the dw-pcie and the generic SBSA compliant pci
implementation.
My vote would be for updating the host controllers to the new API and
to offer the CONFIG option to choose between arch APIs. The alternative
is to use the existing API to wrap the generic implementation.
The problem with an either/or CONFIG option is that it breaks
multiplatform support. A lot of the arm32 PCI implementations
are only used on platforms that are not multiplatform enabled
though, so we could get away by requiring all multiplatform
configurations to use the new API.
My main concern with the existing API is the requirement to have a subsys_initcall
in your host bridge or mach code, due to the way the initialisation is done (you
need the DT code to probe your driver, but you cannot start the scanning of the
PCI bus until the arch code is initialised, so it gets deferred via
subsys_initcall when it calls pci_common_init). I bet that if one tries to
instantiate a Tegra PCI host bridge controller on a Marvell platform things will
break pretty quickly (random example here).
I'm not following here. All the new host controller drivers should
be platform drivers that only bind to the host devices in DT
that are present. Both mvebu and tegra use a normal "module_platform_driver"
for initialization, not a "subsys_initcall".
quoted
Something like this (coded in mail client, don't try to compile):
#define IO_SPACE_PAGES (IO_SPACE_LIMIT + 1) / PAGE_SIZE)
static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
unsigned long pci_ioremap_io(const struct resource *bus, const struct resource phys)
{
unsigned long start, len, virt_start;
int error;
/* use logical address == bus address if possible */
start = bus->start / PAGE_SIZE;
if (start > IO_SPACE_LIMIT / PAGE_SIZE)
start = 0;
/*
* try finding free space for the whole size first,
* fall back to 64K if not available
*/
len = min(resource_size(bus), resource_size(phys);
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
if (start == IO_SPACE_PAGES && len > SZ_64K)
len = SZ_64K;
start = 0;
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
}
/* no 64K area found */
if (start == IO_SPACE_PAGES)
return -ENOMEM;
/* ioremap physical aperture to virtual aperture */
virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
error = ioremap_page_range(virt_start, virt_start + len,
phys->start, __pgprot(PROT_DEVICE_nGnRE));
if (error)
return error;
bitmap_set(start, len / PAGE_SIZE);
/* return io_offset */
return start * PAGE_SIZE - bus->start;
}
EXPORT_SYMBOL_GPL(pci_ioremap_io);
Arnd
I see. I need to think how this will change the existing code. Current users
of pci_ioremap_io ask for multiples of SZ_64K offsets regardless of the
actual need.
Right. I guess we can support both interfaces on ARM32 for the forseeable
future (renaming the new one) and just change the existing implementation
to update the bitmap. Any cross-platform host controller driver would
have to use the new interface however.
Arnd
On Monday 03 February 2014 17:07:48 Rob Herring wrote:
On Mon, Feb 3, 2014 at 2:05 PM, Arnd Bergmann [off-list ref] wrote:
You might want to re-read the SBSA. Unless ARM provides an IP block or
there is some other standard such as EHCI or AHCI, there is no generic
implementation. You only have to go look at the Linux EHCI or AHCI
drivers and see how meaningless and inadequate "use EHCI" is. For PCI,
the text is so brief in the SBSA there will be no way PCI is going to
just work given all the variations of root complexes, bridges, address
windows, etc. we typically see on ARM platforms. I could be wrong and
some AML magic will solve all the problems.
I don't think you need any AML, and SBSA seems to cover the PCI case
just fine, though I have to agree that the EHCI/AHCI/xHCI part is
rather ambiguous. What the existing PCI host controller drivers do is
essentially:
1. provide a config space access method
2. set up the I/O and memory address windows
3. set up clocks, PHYs etc
4. work around any deviations from the PCI standard
5. provide an MSI/MSI-X controller
For all I can tell, any SBSA compliant system should handle
those four like this:
1. config space is ECAM compliant, we only need to pass the
location in DT. (SBSA 8.1)
2. all address windows are set up by the boot loader, we only
need to know the location (IMHO this should be the
preferred way to do things regardless of SBSA).
3. any external hardware dependencies are set up statically
by the boot loader and are operational as we enter the
kernel.
4. deviations from PCI are not allowed (SBSA 8.8)
5. MSI has to be handled by GICv3 (SBSA 8.3.2)
So I definitely expect SBSA compliant systems to work fine with a
very simple generic PCI host bridge driver (which is likely what
Liviu has implemented and waiting for approval to submit).
The more important question is what systems will actually be
compliant with the above. X-Gene manages to get all five wrong,
for instance, and so would any system that reuses the existing
PCI hardware (alphabetically: exynos, imx6, mvebu, rcar-gen2,
tegra, designware), although points 2 and 3 are a question of
the boot loader, and it's possible that the designware based ones
get point 4 right.
Arnd
On Monday 03 February 2014 16:31:37 Jason Gunthorpe wrote:
Specifying 'use EHCI, AHCI, etc' - which are all PCI based standards
without clearly specifying exactly how PCI is suppose to work is
completely bonkers.
What is needed is a spec that says:
1) Here is how you generate config TLPs. A MMIO region that
conforms to the already specified x86 ECAM would
be perfect
2) Here is a dword by dword break down of the entire config space in
a SOC. Here is where a on-board AHCI controller must show up in
config space. Here is how an external PCI-E port must show
up. Etc. Most of this is already specified, but it clearly needs
to be layed out explicitly for ARM SOCs to actually follow it.
3) Here is how you specify the aperture(s) associated with PCI BAR's
and bridge windows in config space. And yes: The CONFIG SPACE
BARS MUST WORK.
4) Here is how MSI works, these are the values you put in the
address/data and here is how you collect the interrupt.
5) Here is how Legacy INTx must be mapped into the GIC.
This is what x86 does, and they have been doing it well for 10
years. If you want to play in the server game you have to properly
implement PCI.
I'm pretty sure the authors of the SBSA actually thought that was
what they wrote, by referring to external specifications (pci-3.0,
ehci, ahci, ...). However, it seems they were either foolish enough
to believe that hardware designers would follow these specs, or
they were intentionally misled and got talked into putting ambiguous
terminology in because there were already hardware designs that
are not exactly in line with the spirit of the SBSA but can be
argued to be conforming to the text for a lax interpretation.
I think EHCI is a much better example than PCI here, because the
spec has exactly one line to say about it, where it spends a whole
chapter on PCI.
Here is how a sane person would read SBSA to create a compliant
implementation:
I have to use EHCI version 1.1 to provide USB-2.0 support. EHCI
is a PCI device, so I'll put it behind a PCIe port that complies
to the PCIe section of the SBSA. Since EHCI by itself only provides
high-speed USB, and USB-2.0 mandates I provide low-speed and
full-speed as well, I have to add a USB hub device. It would have
been easier to just use OHCI for these, but SBSA says I can't.
Now I want to integrate the EHCI into my SoC and not waste one
of my precious PCIe root ports, so I have to create another PCI
domain with its own ECAM compliant config space to put it into.
Fortunately SBSA lets me add an arbitrary number of PCI domains,
as long as they are all strictly compliant. To software it will
look exactly as if it was on an external card, I just have to
ensure the boot loader correctly sets up the clocks and the phy
before an SBSA compliant OS gets loaded, all runtime power
management will get handled through the EHCI-1.1 energy-efficiency
extensions.
Here is how a crazy person would read the same sentence in the SBSA:
I have an IP block that implements the EHCI register set, that
should be good enough. It's not a fast device, so I can put it
on a non-coherent bus. Since the SoC will be used for networking,
I'll put the registers into big-endian configuration to make it
easier for the OS to access. I'm not allowed to have USB-1.1
according to SBSA, so I can get away without a hub or an extra
OHCI. I can't support MSI because it's not a PCI device, and
the GIC is pretty full, so I'll just connect the IRQ line to
the GPIO controller. In order to do better power management,
I'll design a fancy PHY that the device driver will manage
for implementing autosuspend. I should also give the OS
fine-grained control over the clocks, but it will have to share
the clock domain with the other devices on the same edge of the
chip. The EHCI device is not part of PCI, which measn I don't
have to use the standard SMMU. However, my EHCI implementation
can only do 32-bit DMA, and I'll have to design my own IOMMU
to let it access the entire memory range. USB-OTG is a great
feature and we already paid for having this in our EHCI
implementation, better make sure it comes up in endpoint mode
after waking up from power saving.
Arnd
On Tue, Feb 04, 2014 at 08:44:36AM +0000, Arnd Bergmann wrote:
On Monday 03 February 2014 21:36:58 Liviu Dudau wrote:
quoted
On Mon, Feb 03, 2014 at 08:05:56PM +0000, Arnd Bergmann wrote:
quoted
On Monday 03 February 2014 19:18:38 Liviu Dudau wrote:
quoted
So ... defining it should mean no legacy ISA devices, right?
I would read that comment as referring to systems that don't have
any I/O space. If you have PCI, you can by definition have ISA
compatible devices behind a bridge. A typical example would be
a VGA card that supports the 03c0-03df port range.
But if you have PCI and don't want to support ISA do you have /dev/port? I guess yes.
Right, that is my interpretation. You could still have a tool
that tries to poke /dev/port from user space for any I/O BAR
the same way you'd use /dev/mem for memory BARs of PCI devices.
It's discouraged, but it's often the easiest solution for
a quick hack, and I'd expect tools to use this.
quoted
quoted
quoted
quoted
quoted
#define IO_SPACE_LIMIT 0xffff
You probably want to increase this a bit, to allow multiple host bridges
to have their own I/O space.
OK, but to what size?
2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
take up too much virtual space. On arm64 it should be at least as big.
Could be more than that, although I don't see a reason why it should be,
unless we expect to see systems with tons of host bridges, or buses
that exceed 64KB of I/O space.
I will increase the size to 2MB for v2.
Thinking about this some more, I'd go a little higher. In case of
pci_mv, we already register a 1MB region for one logical host
(which has multiple I/O spaces behind an emulated bridge), so
going to 16MB or more would let us handle multiple 1MB windows
for some amount of future proofing.
Maybe Catalin can assign us some virtual address space to use,
with the constraints that it should be 16MB or more in an
area that doesn't require additional kernel page table pages.
I suppose you can actually use the generic implementation from
asm-generic/io.h, and fix it by using the definition you have
above, since it's currently broken.
Not exactly broken, but it makes the assumption that your IO space starts at
physical address zero and you have not remapped it. It does guard the
definition with #ifndef CONFIG_GENERIC_IOMAP after all, so it does not
expect to be generic :)
Well, I/O space never starts at physical zero in reality, so it is
broken in practice. The CONFIG_GENERIC_IOMAP option tries to solve
the problem of I/O spaces that are not memory mapped, which is
actually quite rare (x86, ia64, some rare powerpc bridges, and possibly
Alpha). The norm is that if you have I/O space, it is memory mapped
and you don't need GENERIC_IOMAP. I think most of the architectures
selecting GENERIC_IOMAP have incorrectly copied that from x86.
If you are talking about CPU addresses for I/O space, then you are (mostly?) right.
I've seen some code in powerpc that tries to handle the case where I/O starts at zero.
For MMIO, yes, it would be crazy to start at CPU address zero. But, the ioport_map
takes a port number, and those do start at zero, right?
PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.
:) No ISA support! (Die ISA, die!!)
If only it were that easy.
Lets try! :)
I wonder how many active devices that have an ISA slot are still supported
by mainline kernel.
This is missing the point, but any architecture that has a PCI
slot can have an ISA device behind a bridge like this:
http://www.altera.com/products/ip/iup/pci/m-eur-pci-to-isa.html
The real reason is that a lot of PCI cards for practical reasons
expose some non-relocatable memory and I/O BARs in ISA-compatible
locations. Looking at /proc/ioports on my PC, I can spot a couple
of things that may well show up on any ARM machine:
0000-03af : PCI Bus 0000:00
02f8-02ff : serial
03c0-03df : PCI Bus 0000:40
03c0-03df : PCI Bus 0000:00
03c0-03df : vga+
03e0-0cf7 : PCI Bus 0000:00
03e8-03ef : serial
03f8-03ff : serial
0b00-0b0f : pnp 00:08
0b00-0b07 : piix4_smbus
0b20-0b3f : pnp 00:08
0b20-0b27 : piix4_smbus
0ca2-0ca3 : pnp 00:08
0ca2-0ca2 : ipmi_si
0ca3-0ca3 : ipmi_si
0cf8-0cff : PCI conf1
Nothing wrong with the above. Now, it's also possible that
someone decides to build an ARM server by using a PC south
bridge with integrated legacy PC peripherals, such as these:
0000-03af : PCI Bus 0000:00
0000-001f : dma1
0020-0021 : pic1
0040-0043 : timer0
0050-0053 : timer1
0060-0060 : keyboard
0064-0064 : keyboard
0070-0071 : rtc0
0080-008f : dma page reg
00a0-00a1 : pic2
00b0-00b0 : APEI ERST
00c0-00df : dma2
00f0-00ff : fpu
There is some hope that it won't happen, but these things
exist and come with a regular PCIe front-end bus in some
cases.
Finally, there is the LPC bus, which can give you additional
ISAPnP compatible devices.
quoted
I will update PCIBIOS_MIN_xxxx to match arch/arm for v2.
None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.
Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)
No need to change the other architectures. You can make it opt-in for
now and just put the code into a common location.
An interesting question however is what the transition plan is to
have the code shared between arm32 and arm64: We will certainly need
to share at least the dw-pcie and the generic SBSA compliant pci
implementation.
My vote would be for updating the host controllers to the new API and
to offer the CONFIG option to choose between arch APIs. The alternative
is to use the existing API to wrap the generic implementation.
The problem with an either/or CONFIG option is that it breaks
multiplatform support. A lot of the arm32 PCI implementations
are only used on platforms that are not multiplatform enabled
though, so we could get away by requiring all multiplatform
configurations to use the new API.
I was thinking more in terms of catching uses of the old API in the new host bridge
driver. The added functions should be able to live beside the old API for the
generic PCI framework, not sure how arm arch code should handle it.
quoted
My main concern with the existing API is the requirement to have a subsys_initcall
in your host bridge or mach code, due to the way the initialisation is done (you
need the DT code to probe your driver, but you cannot start the scanning of the
PCI bus until the arch code is initialised, so it gets deferred via
subsys_initcall when it calls pci_common_init). I bet that if one tries to
instantiate a Tegra PCI host bridge controller on a Marvell platform things will
break pretty quickly (random example here).
I'm not following here. All the new host controller drivers should
be platform drivers that only bind to the host devices in DT
that are present. Both mvebu and tegra use a normal "module_platform_driver"
for initialization, not a "subsys_initcall".
I was actually looking at mach-dove, I thought that was Marvell as well.
But both mvebu and tegra call pci_common_init_dev. The busnr gets assigned based on
the registration order. I wonder if any of the host bridge code copes with having
assigned a bus number other than zero for its "root bus".
quoted
quoted
Something like this (coded in mail client, don't try to compile):
#define IO_SPACE_PAGES (IO_SPACE_LIMIT + 1) / PAGE_SIZE)
static DECLARE_BITMAP(pci_iospace, IO_SPACE_PAGES);
unsigned long pci_ioremap_io(const struct resource *bus, const struct resource phys)
{
unsigned long start, len, virt_start;
int error;
/* use logical address == bus address if possible */
start = bus->start / PAGE_SIZE;
if (start > IO_SPACE_LIMIT / PAGE_SIZE)
start = 0;
/*
* try finding free space for the whole size first,
* fall back to 64K if not available
*/
len = min(resource_size(bus), resource_size(phys);
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
if (start == IO_SPACE_PAGES && len > SZ_64K)
len = SZ_64K;
start = 0;
start = bitmap_find_next_zero_area(pci_iospace, IO_SPACE_PAGES,
start, len / PAGE_SIZE, 0);
}
/* no 64K area found */
if (start == IO_SPACE_PAGES)
return -ENOMEM;
/* ioremap physical aperture to virtual aperture */
virt_start = start * PAGE_SIZE + (unsigned long)PCI_IOBASE;
error = ioremap_page_range(virt_start, virt_start + len,
phys->start, __pgprot(PROT_DEVICE_nGnRE));
if (error)
return error;
bitmap_set(start, len / PAGE_SIZE);
/* return io_offset */
return start * PAGE_SIZE - bus->start;
}
EXPORT_SYMBOL_GPL(pci_ioremap_io);
Arnd
I see. I need to think how this will change the existing code. Current users
of pci_ioremap_io ask for multiples of SZ_64K offsets regardless of the
actual need.
Right. I guess we can support both interfaces on ARM32 for the forseeable
future (renaming the new one) and just change the existing implementation
to update the bitmap. Any cross-platform host controller driver would
have to use the new interface however.
Arnd
OK, I can try to add the function to my patch. Call it pci_ioremap_iores?
Best regards,
Liviu
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
On Tuesday 04 February 2014 11:09:22 Liviu Dudau wrote:
On Tue, Feb 04, 2014 at 08:44:36AM +0000, Arnd Bergmann wrote:
quoted
Well, I/O space never starts at physical zero in reality, so it is
broken in practice. The CONFIG_GENERIC_IOMAP option tries to solve
the problem of I/O spaces that are not memory mapped, which is
actually quite rare (x86, ia64, some rare powerpc bridges, and possibly
Alpha). The norm is that if you have I/O space, it is memory mapped
and you don't need GENERIC_IOMAP. I think most of the architectures
selecting GENERIC_IOMAP have incorrectly copied that from x86.
If you are talking about CPU addresses for I/O space, then you are (mostly?) right.
I've seen some code in powerpc that tries to handle the case where I/O starts at zero.
For MMIO, yes, it would be crazy to start at CPU address zero. But,
the ioport_map takes a port number, and those do start at zero, right?
What I meant is that asm-generic/io.h assumes that the I/O ports are
mapped at /virtual/ address zero, which is even more crazy, since that
is normally in user space. Sorry for confusing it with physical address
zero.
Now the GENERIC_IOMAP uses a similar fiction by defining that virtual
address token 0x10000-0x1ffff are used to access I/O space when calling
inb/outb, but that is something you only need to do when you have
no memory mapped I/O port.
Some older ARM platforms (PXA for instance) also defined the I/O space
to start at virtual address zero, and use a per-bus io_offset that was
equal to the ioremapped I/O window. This actually works, but it means
that the logical port numbers are all high, and you have to set
IO_SPACE_LIMIT to ULONG_MAX, and it breaks horribly for any driver that
tries to store a port number in a type that is shorter than 'unsigned
long'. We definitely don't want to do this for new code.
quoted
quoted
My main concern with the existing API is the requirement to have a subsys_initcall
in your host bridge or mach code, due to the way the initialisation is done (you
need the DT code to probe your driver, but you cannot start the scanning of the
PCI bus until the arch code is initialised, so it gets deferred via
subsys_initcall when it calls pci_common_init). I bet that if one tries to
instantiate a Tegra PCI host bridge controller on a Marvell platform things will
break pretty quickly (random example here).
I'm not following here. All the new host controller drivers should
be platform drivers that only bind to the host devices in DT
that are present. Both mvebu and tegra use a normal "module_platform_driver"
for initialization, not a "subsys_initcall".
I was actually looking at mach-dove, I thought that was Marvell as well.
mach-dove is going away soon, it will get merged into mach-mvebu and
then use drivers/pci/host/pci-mvebu.c
But both mvebu and tegra call pci_common_init_dev. The busnr gets assigned based on
the registration order. I wonder if any of the host bridge code copes with having
assigned a bus number other than zero for its "root bus".
I think all "new" host bridges now use nr_controllers=1, which means
that you always start at but number zero and use PCI domain if
you have multiple independent root bridges.
quoted
Right. I guess we can support both interfaces on ARM32 for the forseeable
future (renaming the new one) and just change the existing implementation
to update the bitmap. Any cross-platform host controller driver would
have to use the new interface however.
OK, I can try to add the function to my patch. Call it pci_ioremap_iores?
Sounds ok, I can't think of anything better at least.
Arnd
I'm not sure that this will work. The in[bwl], out[bwl] macros in
arch/arm64/include/asm/io.h already add the PCI_IOBASE offset.
Instead of these two #defines, why not just enforce that
GENERIC_PCI_IOMAP is enabled? Or at least wrap these defines with 'if
(!config_enabled(CONFIG_GENERIC_PCI_IOMAP))' or similar.
GENERIC_PCI_IOMAP *is* enabled for AArch64. We have select GENERIC_IOMAP in
arch/arm64/Kconfig which in turn selects GENERIC_PCI_IOMAP in lib/Kconfig.
Best regards,
Liviu
I'm not sure that this will work. The in[bwl], out[bwl] macros in
arch/arm64/include/asm/io.h already add the PCI_IOBASE offset.
Instead of these two #defines, why not just enforce that
GENERIC_PCI_IOMAP is enabled? Or at least wrap these defines with 'if
(!config_enabled(CONFIG_GENERIC_PCI_IOMAP))' or similar.
GENERIC_PCI_IOMAP *is* enabled for AArch64. We have select GENERIC_IOMAP in
arch/arm64/Kconfig which in turn selects GENERIC_PCI_IOMAP in lib/Kconfig.
Sorry, it was intent to suggest using the ioport_[map|unmap] functions
in lib/iomap.c instead of defining something similar here. I guess you
will also need to also define CONFIG_HAS_IOPORT for this to happen.
Andrew Murray
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
From: Rob Herring <hidden> Date: 2014-02-04 13:57:49
On Tue, Feb 4, 2014 at 3:44 AM, Arnd Bergmann [off-list ref] wrote:
On Monday 03 February 2014 16:31:37 Jason Gunthorpe wrote:
quoted
Specifying 'use EHCI, AHCI, etc' - which are all PCI based standards
without clearly specifying exactly how PCI is suppose to work is
completely bonkers.
What is needed is a spec that says:
1) Here is how you generate config TLPs. A MMIO region that
conforms to the already specified x86 ECAM would
be perfect
2) Here is a dword by dword break down of the entire config space in
a SOC. Here is where a on-board AHCI controller must show up in
config space. Here is how an external PCI-E port must show
up. Etc. Most of this is already specified, but it clearly needs
to be layed out explicitly for ARM SOCs to actually follow it.
3) Here is how you specify the aperture(s) associated with PCI BAR's
and bridge windows in config space. And yes: The CONFIG SPACE
BARS MUST WORK.
4) Here is how MSI works, these are the values you put in the
address/data and here is how you collect the interrupt.
5) Here is how Legacy INTx must be mapped into the GIC.
This is what x86 does, and they have been doing it well for 10
years. If you want to play in the server game you have to properly
implement PCI.
I'm pretty sure the authors of the SBSA actually thought that was
what they wrote, by referring to external specifications (pci-3.0,
ehci, ahci, ...). However, it seems they were either foolish enough
to believe that hardware designers would follow these specs, or
they were intentionally misled and got talked into putting ambiguous
terminology in because there were already hardware designs that
are not exactly in line with the spirit of the SBSA but can be
argued to be conforming to the text for a lax interpretation.
I think EHCI is a much better example than PCI here, because the
spec has exactly one line to say about it, where it spends a whole
chapter on PCI.
Here is how a sane person would read SBSA to create a compliant
implementation:
s/sane/software/
I have to use EHCI version 1.1 to provide USB-2.0 support. EHCI
is a PCI device, so I'll put it behind a PCIe port that complies
to the PCIe section of the SBSA. Since EHCI by itself only provides
high-speed USB, and USB-2.0 mandates I provide low-speed and
full-speed as well, I have to add a USB hub device. It would have
been easier to just use OHCI for these, but SBSA says I can't.
Now I want to integrate the EHCI into my SoC and not waste one
of my precious PCIe root ports, so I have to create another PCI
domain with its own ECAM compliant config space to put it into.
Fortunately SBSA lets me add an arbitrary number of PCI domains,
as long as they are all strictly compliant. To software it will
look exactly as if it was on an external card, I just have to
ensure the boot loader correctly sets up the clocks and the phy
before an SBSA compliant OS gets loaded, all runtime power
management will get handled through the EHCI-1.1 energy-efficiency
extensions.
Here is how a crazy person would read the same sentence in the SBSA:
s/crazy/hardware/
I have an IP block that implements the EHCI register set, that
should be good enough. It's not a fast device, so I can put it
on a non-coherent bus. Since the SoC will be used for networking,
I'll put the registers into big-endian configuration to make it
easier for the OS to access. I'm not allowed to have USB-1.1
according to SBSA, so I can get away without a hub or an extra
OHCI. I can't support MSI because it's not a PCI device, and
the GIC is pretty full, so I'll just connect the IRQ line to
the GPIO controller. In order to do better power management,
I'll design a fancy PHY that the device driver will manage
for implementing autosuspend. I should also give the OS
fine-grained control over the clocks, but it will have to share
the clock domain with the other devices on the same edge of the
chip. The EHCI device is not part of PCI, which measn I don't
have to use the standard SMMU. However, my EHCI implementation
can only do 32-bit DMA, and I'll have to design my own IOMMU
to let it access the entire memory range. USB-OTG is a great
feature and we already paid for having this in our EHCI
implementation, better make sure it comes up in endpoint mode
after waking up from power saving.
My money is on the latter. I think non-PCI implementations of xHCI
interfaces will be common. This was certainly the case at Calxeda in
what was believed to be a SBSA compliant SOC. However, I think PCI
device or not is the least of the issues and all the other examples
you list are the difficult ones to deal with.
Rob
I'm not sure that this will work. The in[bwl], out[bwl] macros in
arch/arm64/include/asm/io.h already add the PCI_IOBASE offset.
Instead of these two #defines, why not just enforce that
GENERIC_PCI_IOMAP is enabled? Or at least wrap these defines with 'if
(!config_enabled(CONFIG_GENERIC_PCI_IOMAP))' or similar.
GENERIC_PCI_IOMAP is enabled for AArch64. We have select GENERIC_IOMAP in
arch/arm64/Kconfig which in turn selects GENERIC_PCI_IOMAP in lib/Kconfig.
Sorry, it was intent to suggest using the ioport_[map|unmap] functions
in lib/iomap.c instead of defining something similar here. I guess you
will also need to also define CONFIG_HAS_IOPORT for this to happen.
We do want CONFIG_HAS_IOPORT, but I would say that we should not use
CONFIG_GENERIC_IOMAP. As I explained in another reply, enabling this
was probably a simple mistake, and we only need this if we want I/O
spaces that are /not/ memory mapped. IMHO any ARM64 system that doesn't
map its PCI I/O space into MMIO space can live without I/O port
access.
Arnd
On Tue, Feb 04, 2014 at 08:44:36AM +0000, Arnd Bergmann wrote:
On Monday 03 February 2014 21:36:58 Liviu Dudau wrote:
quoted
On Mon, Feb 03, 2014 at 08:05:56PM +0000, Arnd Bergmann wrote:
quoted
2 MB was a compromise on arm32 to allow up to 32 PCI host bridges but not
take up too much virtual space. On arm64 it should be at least as big.
Could be more than that, although I don't see a reason why it should be,
unless we expect to see systems with tons of host bridges, or buses
that exceed 64KB of I/O space.
I will increase the size to 2MB for v2.
Thinking about this some more, I'd go a little higher. In case of
pci_mv, we already register a 1MB region for one logical host
(which has multiple I/O spaces behind an emulated bridge), so
going to 16MB or more would let us handle multiple 1MB windows
for some amount of future proofing.
Maybe Catalin can assign us some virtual address space to use,
with the constraints that it should be 16MB or more in an
area that doesn't require additional kernel page table pages.
See below, 16MB and could be extended further if needed.
------------>8--------------------
From 00521f109ac8a2589c9089a5feaaaa1be7f26108 Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Tue, 4 Feb 2014 16:37:59 +0000
Subject: [PATCH] arm64: Extend the PCI I/O space to 16MB
The patch moves the PCI I/O space (currently at 64K) before the
earlyprintk mapping and extends it to 16MB.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
Documentation/arm64/memory.txt | 16 ++++++++++------
arch/arm64/include/asm/io.h | 2 +-
2 files changed, 11 insertions(+), 7 deletions(-)
From: Jason Gunthorpe <hidden> Date: 2014-02-04 18:15:27
On Tue, Feb 04, 2014 at 10:44:52AM +0100, Arnd Bergmann wrote:
Now I want to integrate the EHCI into my SoC and not waste one
of my precious PCIe root ports, so I have to create another PCI
domain with its own ECAM compliant config space to put it into.
Fortunately SBSA lets me add an arbitrary number of PCI domains,
as long as they are all strictly compliant. To software it will
Just to touch on this for others who might be reading..
IMHO any simple SOC that requires multiple domains is *broken*. A
single domain covers all reasonable needs until you get up to
mega-scale NUMA systems, encouraging people to design with multiple
domains only complicates the kernel :(
SOC internal peripherals should all show up in the bus 0 config space
of the only domain and SOC PCI-E physical ports should show up on bus
0 as PCI-PCI bridges. This is all covered in the PCI-E specs regarding
the root complex.
Generally I would expect the internal peripherals to still be
internally connected with AXI, but also connected through the ECAM
space for configuration, control, power management and address
assignment.
2. all address windows are set up by the boot loader, we only
need to know the location (IMHO this should be the
preferred way to do things regardless of SBSA).
Linux does a full address map re-assignment on boot, IIRC. You need
more magics to inhibit that if your BAR's and bridge windows don't
work.
Hot plug is a whole other thing..
it's possible that the designware based ones get point 4 right.
The designware one's also appear to be re-purposed end point cores, so
their config handling is somewhat bonkers. Tegra got theirs sort of
close because they re-used knowledge/IP from their x86 south bridges -
but even then they didn't really implement ECAM properly for an ARM
environment.
Since config space is where everyone to date has fallen down, I think
the SBSA would have been wise to list dword by dword what a typical
ECAM config space should look like.
Jason
On Tuesday 04 February 2014 11:15:14 Jason Gunthorpe wrote:
On Tue, Feb 04, 2014 at 10:44:52AM +0100, Arnd Bergmann wrote:
quoted
Now I want to integrate the EHCI into my SoC and not waste one
of my precious PCIe root ports, so I have to create another PCI
domain with its own ECAM compliant config space to put it into.
Fortunately SBSA lets me add an arbitrary number of PCI domains,
as long as they are all strictly compliant. To software it will
Just to touch on this for others who might be reading..
IMHO any simple SOC that requires multiple domains is *broken*. A
single domain covers all reasonable needs until you get up to
mega-scale NUMA systems, encouraging people to design with multiple
domains only complicates the kernel :(
Well, the way I see it, we already have support for arbitrary
PCI domains in the kernel, and that works fine, so we can just
as well use it. That way we don't have to partition the available
256 buses among the host bridges, and anything that needs a separate
PCI config space can live in its own world. Quite often when you
have multiple PCI hosts, they actually have different ways to
get at the config space and don't even share the same driver.
On x86, any kind of HT/PCI/PCIe/PCI-x bridge is stuffed into a
single domain so they can support OSs that only know the
traditional config space access methods, but I don't see
any real advantage to that for other architectures.
SOC internal peripherals should all show up in the bus 0 config space
of the only domain and SOC PCI-E physical ports should show up on bus
0 as PCI-PCI bridges. This is all covered in the PCI-E specs regarding
the root complex.
Generally I would expect the internal peripherals to still be
internally connected with AXI, but also connected through the ECAM
space for configuration, control, power management and address
assignment.
That would of course be very nice from a software perspective,
but I think that is much less likely for any practical
implementation.
quoted
2. all address windows are set up by the boot loader, we only
need to know the location (IMHO this should be the
preferred way to do things regardless of SBSA).
Linux does a full address map re-assignment on boot, IIRC. You need
more magics to inhibit that if your BAR's and bridge windows don't
work.
Hot plug is a whole other thing..
I meant the I/O and memory space windows of the host bridge here,
which typically don't get reassigned (except on mvebu). For the
device resources, there is a per-host PCI_REASSIGN_ALL_RSRC
flag and pcibios_assign_all_busses() function that we typically
set on embedded systems where we don't trust the boot loader
to set this up correctly, or at all.
On server systems, I would expect to have the firmware assign
all resources and the kernel to leave them alone. On sparc
and powerpc servers, there is even a third method, which
is to trust firmware to put the correct resources for each
device into DT, overriding what is written in the BAR.
quoted
it's possible that the designware based ones get point 4 right.
The designware one's also appear to be re-purposed end point cores, so
their config handling is somewhat bonkers. Tegra got theirs sort of
close because they re-used knowledge/IP from their x86 south bridges -
but even then they didn't really implement ECAM properly for an ARM
environment.
Since config space is where everyone to date has fallen down, I think
the SBSA would have been wise to list dword by dword what a typical
ECAM config space should look like.
From: Jason Gunthorpe <hidden> Date: 2014-02-04 19:11:00
On Tue, Feb 04, 2014 at 07:34:50PM +0100, Arnd Bergmann wrote:
Well, the way I see it, we already have support for arbitrary
PCI domains in the kernel, and that works fine, so we can just
as well use it. That way we don't have to partition the available
256 buses among the host bridges, and anything that needs a separate
PCI config space can live in its own world. Quite often when you
have multiple PCI hosts, they actually have different ways to
get at the config space and don't even share the same driver.
On x86, any kind of HT/PCI/PCIe/PCI-x bridge is stuffed into a
single domain so they can support OSs that only know the
traditional config space access methods, but I don't see
any real advantage to that for other architectures.
Supporting a standard configration interface is a solid reason, but
there is alot more going on.
For instance to support peer-to-peer IO you need to have a consisent,
non-overlapping set of bus/device/function/tag to uniquely route TLPs
within the chip. Cross domain TLP routing in HW is non-trivial.
IOMMUs (and SR-IOv) rely on the BDF to identify the originating device
for each TLP. Multiple domains means a much more complex IOMMU
environment.
Failure to integrate on-chip devices into the PCI world also means
thing like SR-IOv won't work sanely with on-chip devices.
The only reason we should see multi-domain on a SOC is because the HW
design was lazy. Being lazy misses the Big Picture where PCI is the
cornerstone of many important Server/Enterprise technologies.
quoted
SOC internal peripherals should all show up in the bus 0 config space
of the only domain and SOC PCI-E physical ports should show up on bus
0 as PCI-PCI bridges. This is all covered in the PCI-E specs regarding
the root complex.
Generally I would expect the internal peripherals to still be
internally connected with AXI, but also connected through the ECAM
space for configuration, control, power management and address
assignment.
That would of course be very nice from a software perspective,
but I think that is much less likely for any practical
implementation.
Well, all x86 implementations do this already.. It actually isn't that
big a deal from a HW perspective, you just have to think about it
fully, understand PCI, and position your registers accordingly.
Jason
On Tuesday 04 February 2014 12:10:55 Jason Gunthorpe wrote:
For instance to support peer-to-peer IO you need to have a consisent,
non-overlapping set of bus/device/function/tag to uniquely route TLPs
within the chip. Cross domain TLP routing in HW is non-trivial.
Yes, that is a good reason.
IOMMUs (and SR-IOv) rely on the BDF to identify the originating device
for each TLP. Multiple domains means a much more complex IOMMU
environment.
I fear we already have to support complex IOMMU setups on ARM,
whether there are multiple PCI domains or not. But it would be
nice in theory not to require it.
Failure to integrate on-chip devices into the PCI world also means
thing like SR-IOv won't work sanely with on-chip devices.
I'd consider this a feature ;)
But you are probably right: people will do SR-IOV whether we like
it or not, and they will try to do it on non-PCI devices too,
and great suffering will be involved.
Arnd
Here is how a sane person would read SBSA to create a compliant
implementation:
s/sane/software/
quoted
Here is how a crazy person would read the same sentence in the SBSA:
s/crazy/hardware/
Not much of a difference, apparently ...
My money is on the latter. I think non-PCI implementations of xHCI
interfaces will be common. This was certainly the case at Calxeda in
what was believed to be a SBSA compliant SOC.
I just looked up the EHCI and xHCI specs and am shocked to see that
the PCI config space access for both is an optional part of the
spec, so that assertion may well have been correct.
On the other hand, it does seem impossible to create a compliant
AHCI implementation without making it show up as a PCI function,
so any SBSA compliant SoC that contains AHCI already has to have
all the bits for doing the same on USB.
However, I think PCI
device or not is the least of the issues and all the other examples
you list are the difficult ones to deal with.
Agreed. But if they get the difficult problems right, it's
trivial to also do the PCI config space either in the way
that Jason described, or as a separate PCI domain. In the worst
case, it could still be faked up by a secure-mode firmware
catching all config space accesses.
Arnd
From: Yijing Wang <hidden> Date: 2014-02-18 06:34:13
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+#include <asm/pci-bridge.h>
+
+
+/*
+ * Return the domain number for this bus
+ */
+int pci_domain_nr(struct pci_bus *bus)
+{
+ struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
Here bus is specific to root bus ? or, what about use find_pci_host_bridge() to get the pci_host_bridge
instead.
+
+ if (bridge)
+ return bridge->domain_nr;
+
+ return 0;
+}
+
+int pci_proc_domain(struct pci_bus *bus)
+{
+ return pci_domain_nr(bus);
+}
+
+/*
+ * Called after each bus is probed, but before its children are examined
+ */
+void pcibios_fixup_bus(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+ struct resource *res;
+ int i;
+
+ if (bus->self != NULL) {
What about use !pci_is_root_bus() ?
+ pci_read_bridge_bases(bus);
+
+ pci_bus_for_each_resource(bus, res, i) {
+ if (!res || !res->flags || res->parent)
+ continue;
+
+ /*
+ * If we are going to reassign everything, we can
+ * shrink the P2P resource to have zero size to
+ * save space
+ */
+ if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
+ res->flags |= IORESOURCE_UNSET;
+ res->start = 0;
+ res->end = -1;
+ continue;
+ }
+ }
+ }
+
On Tue, Feb 18, 2014 at 02:33:53PM +0800, Yijing Wang wrote:
quoted
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+#include <asm/pci-bridge.h>
+
+
+/*
+ * Return the domain number for this bus
+ */
+int pci_domain_nr(struct pci_bus *bus)
+{
+ struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
Hi Yijing,
Here bus is specific to root bus ? or, what about use find_pci_host_bridge() to get the pci_host_bridge
instead.
Yes, the call should be to find_pci_host_bridge(). I sort of implied that the
bus is always the root bus, which is obviously not correct. Thanks for pointing it out.
quoted
+
+ if (bridge)
+ return bridge->domain_nr;
+
+ return 0;
+}
+
+int pci_proc_domain(struct pci_bus *bus)
+{
+ return pci_domain_nr(bus);
+}
+
+/*
+ * Called after each bus is probed, but before its children are examined
+ */
+void pcibios_fixup_bus(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+ struct resource *res;
+ int i;
+
+ if (bus->self != NULL) {
What about use !pci_is_root_bus() ?
Again, good call, will change.
quoted
+ pci_read_bridge_bases(bus);
+
+ pci_bus_for_each_resource(bus, res, i) {
+ if (!res || !res->flags || res->parent)
+ continue;
+
+ /*
+ * If we are going to reassign everything, we can
+ * shrink the P2P resource to have zero size to
+ * save space
+ */
+ if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
+ res->flags |= IORESOURCE_UNSET;
+ res->start = 0;
+ res->end = -1;
+ continue;
+ }
+ }
+ }
+
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html