Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1306421 > unrolled thread

Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI  hostbridge init based on ACPI Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-01-11 16:40 +0100
    Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI  hostbridge init based on ACPI Sinan Kaya <okaya@codeaurora.org> - 2016-01-11 17:00 +0100
      Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI Arnd Bergmann <arnd@arndb.de> - 2016-01-12 15:40 +0100
        Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI  hostbridge init based on ACPI Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-01-12 19:40 +0100
          Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI Arnd Bergmann <arnd@arndb.de> - 2016-01-12 22:40 +0100

#1306421 — Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI

On Mon, Dec 21, 2015 at 03:26:14PM +0000, Okaya@codeaurora.org wrote:
> > On Monday 21 December 2015, Tomasz Nowicki wrote:
> >> On 21.12.2015 13:10, Lorenzo Pieralisi wrote:
> >> > On Fri, Dec 18, 2015 at 06:56:39PM +0000, okaya@codeaurora.org wrote:
> >
> >> >> I have multiple root ports with the same IO port configuration in the
> >> >> current ACPI table.
> >> >>
> >> >> Root port 0 = IO range 0x1000-0x10FFF
> >> >> Root port 1 = IO range 0x1000-0x10FFF
> >> >> Root port 2 = IO range 0x1000-0x10FFF
> >> >
> >> > It is fine. You end up mapping for each of those a 4k window of the
> >> > virtual address space allocated to IO and that's what you will have in
> >> > the kernel PCI resources (not in the HW BARs though). If that was a
> >> problem
> >> > it would be even for the current DT host controllers eg:
> >> >
> >> > arch/arm64/boot/dts/apm/apm-storm.dtsi
> >> >
> >> > it should not be (again I will let Arnd comment on this since he may
> >> be
> >> > aware of issues encountered on other arches/platforms).
> >> >
> >>
> >> Root port 0 = IO range 0x1000-0x10FFF
> >> Root port 1 = IO range 0x1000-0x10FFF
> >> Root port 2 = IO range 0x1000-0x10FFF
> >>
> >> If above ranges are mapped into different CPU windows, then yes, it is
> >> fine.
> >
> > Ideally, they should all be the same CPU address so we only have to map
> > the window
> > once, each device gets an address below 64K, and you can have legacy port
> > numbers
> > (below 4K) on any bus, which is required to make certain GPUs work.
> >
> > I haven't actually seen anyone do that on ARM though, every implementation
> > so
> > far has a separate mapping per host bridge, and we can cope with that too,
> > and we can live with either overlapping bus addresses or unique bus
> > addresses,
> > any of them can be expressed by the PCI core in Linux, we just have to
> > make sure
> > that we correctly translate the firmware tables into our internal
> > structures.
> >
> > 	Arnd
> > --
> > 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
> >
> 
> Thanks, I won't be touching the acpi tables then and I will assume the
> hack had a problem. It was trying to remap the io range of the second root
> port  to the first port io address map.
> 
> I was getting a warning from resource.c
> 
> Btw, when I tested the io ranges before, kernel didn't accept anything
> below 1k like 0. That is why my range starts at 1k.

And that's what you should not do. ACPI tables have to have a 1:1
correspondence with HW resources and you must not change them to
make the kernel (which version by the way, given that ARM64 ACPI PCI
support is not in the mainline) work. I already said that: we must not
interpret ACPI tables, we must define them according to specifications,
and that's what everyone should follow to write FW.

So, why does your PCI IO range starts at 1k ? Please define what you
mean by "kernel didn't accept anything below 1k", I want to understand
what you are referring to.

Thanks,
Lorenzo

[toc] | [next] | [standalone]

#1306435

On 1/11/2016 10:39 AM, Lorenzo Pieralisi wrote:
>> Thanks, I won't be touching the acpi tables then and I will assume the
>> > hack had a problem. It was trying to remap the io range of the second root
>> > port  to the first port io address map.
>> > 
>> > I was getting a warning from resource.c
>> > 
>> > Btw, when I tested the io ranges before, kernel didn't accept anything
>> > below 1k like 0. That is why my range starts at 1k.
> And that's what you should not do. ACPI tables have to have a 1:1
> correspondence with HW resources and you must not change them to
> make the kernel (which version by the way, given that ARM64 ACPI PCI
> support is not in the mainline) work. I already said that: we must not
> interpret ACPI tables, we must define them according to specifications,
> and that's what everyone should follow to write FW.
> 

What confused me was the kernel view of IO addresses vs. PCI IO addresses. I looked at
Mark Salter's patch and I realized that the kernel is maintaining global IO addresses with offsets 
to real PCI IO addresses.

I was expecting to see the PCI addresses to match kernel IO addresses. I wondered if somebody put some
restriction into Linux or not which happened to me below.

_#_cat_/proc/ioports
00000000-0000efff : PCI Bus 0000:00
  00001000-00001fff : PCI Bus 0000:01
0000f000-0001dfff : PCI Bus 0002:00
  0000f000-0000ffff : PCI Bus 0002:01
0001e000-0002cfff : PCI Bus 0006:00
  0001e000-0001efff : PCI Bus 0006:01
    0001e000-0001e01f : 0006:01:00.0
    0001e020-0001e03f : 0006:01:00.1
/ #

#_dmesg_|_grep_resource
[    2.945762] pci_bus 0000:00: root bus resource [io  0x0000-0xefff window] (bus address [0x1000-0xffff])
[    3.652201] pci_bus 0002:00: root bus resource [io  0xf000-0x1dfff window] (bus address [0x1000-0xffff])
[    6.546716] pci_bus 0006:00: root bus resource [io  0x1e000-0x2cfff window] (bus address [0x1000-0xffff])
/ #

Since we are talking about what ACPI dictates vs. what kernel does. Here is something that got me 
while testing.

Somebody sneaked in a 0x10003 upper limit on PCI addresses for some reason below. There is nothing magic
about 0x10003 and I'm wonding why we have this limit.

static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
				      u8 io_decode, u8 translation_type)
{
	res->flags = IORESOURCE_IO;

	if (!acpi_dev_resource_len_valid(res->start, res->end, len, true))
		res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;

	if (res->end >= 0x10003)
		res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;



I did a debug session with Tomasz last week. He fixed the issue. The range for
IO resources were not being registered properly. The second root port was causing
a bug check in the kernel because the IO range was overlapping with the first root port.
The issue is fixed now. 


> So, why does your PCI IO range starts at 1k ? Please define what you
> mean by "kernel didn't accept anything below 1k", I want to understand
> what you are referring to.

I created my own version of ACPI PCI root port patch by porting ia64 to ARMv7 two years ago
and wrote the ACPI table on an ARMv7 platform. I have been reusing the same tables since
then. 

The issue was what Arnd described in his email to this thread. (PCIBIOS_MIN_IO) macro. 
I have tested the IO range starting from 0 on Tomasz's patches. I'm not seeing any problems.


-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1307481 — Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI

On Monday 11 January 2016 10:56:30 Sinan Kaya wrote:
> 
> #_dmesg_|_grep_resource
> [    2.945762] pci_bus 0000:00: root bus resource [io  0x0000-0xefff window] (bus address [0x1000-0xffff])
> [    3.652201] pci_bus 0002:00: root bus resource [io  0xf000-0x1dfff window] (bus address [0x1000-0xffff])
> [    6.546716] pci_bus 0006:00: root bus resource [io  0x1e000-0x2cfff window] (bus address [0x1000-0xffff])
> / #

This is bad. We normally want to stay out of the first 0x1000 bytes of the Linux space,
to prevent drivers from poking into the ISA registers.

We can have one of the buses be the "primary" bus that has its first 0x1000 bytes of
I/O space mapped into the respective Linux addresses, but mapping the second 0x1000
bytes into the reserved space is the worst possible outcome here, as legacy ISA
drivers will now poke at random other devices that are intentionally moved to
high addresses to stay of of that range.
 
> Since we are talking about what ACPI dictates vs. what kernel does. Here is something that got me 
> while testing.
> 
> Somebody sneaked in a 0x10003 upper limit on PCI addresses for some reason below. There is nothing magic
> about 0x10003 and I'm wonding why we have this limit.

I/O ports are at aligned addresses, the highest 4-byte address you can access
is 0xfffc with the default 0x10000 port limit per bus. This limit is generally
seen as sufficient because that is what x86 has. Most PCI devices have no I/O ports
at all, and the ones that have them have only a couple of bytes of address space
in it.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1307740

On Tue, Jan 12, 2016 at 03:30:25PM +0100, Arnd Bergmann wrote:
> On Monday 11 January 2016 10:56:30 Sinan Kaya wrote:
> > 
> > #_dmesg_|_grep_resource
> > [    2.945762] pci_bus 0000:00: root bus resource [io  0x0000-0xefff window] (bus address [0x1000-0xffff])
> > [    3.652201] pci_bus 0002:00: root bus resource [io  0xf000-0x1dfff window] (bus address [0x1000-0xffff])
> > [    6.546716] pci_bus 0006:00: root bus resource [io  0x1e000-0x2cfff window] (bus address [0x1000-0xffff])
> > / #
> 
> This is bad. We normally want to stay out of the first 0x1000 bytes of
> the Linux space, to prevent drivers from poking into the ISA
> registers.

You are referring to:

pci_bus 0000:00: root bus resource [io  0x0000-0xefff window]
                                        ^^^^^^
here, right ? [0x0 - PCIBIOS_MIN_IO] is not assigned by the PCI
code that reassigns resources anyway, so devices with IO BARs won't
get assigned [0x0 - PCIBIOS_MIN_IO] address space (Linux space).

Are you saying we should disallow the [0x0 - 0x1000] in the PCI busses
IO resource (Linux space) ?

In pci_address_to_pio() the offset (Linux IO resource) we assign starts
from 0x0, so we always allocate that chunk of IO address space (that is
an offset into the Linux virtual address space), am I correct ?

> We can have one of the buses be the "primary" bus that has its first
> 0x1000 bytes of I/O space mapped into the respective Linux addresses,
> but mapping the second 0x1000 bytes into the reserved space is the
> worst possible outcome here, as legacy ISA drivers will now poke at
> random other devices that are intentionally moved to high addresses to
> stay of of that range.

And you are referring to:

root bus resource [io  0x0000-0xefff window] (bus address [0x1000-0xffff])
		       ^^^^^^                              ^^^^^^

here ? If ISA drivers poke at addresses in the [0x0 - 0x1000]
range (Linux space IO offset) they end up on the PCI bus with addresses
above 0x1000, is that what you are saying when you refer to "moved to
high addresses to stay out of that range" ?

Thanks,
Lorenzo

[toc] | [prev] | [next] | [standalone]

#1307848 — Re: [PATCH V2 00/23] MMCONFIG refactoring and support for ARM64 PCI hostbridge init based on ACPI

On Tuesday 12 January 2016 18:38:54 Lorenzo Pieralisi wrote:
> On Tue, Jan 12, 2016 at 03:30:25PM +0100, Arnd Bergmann wrote:
> > On Monday 11 January 2016 10:56:30 Sinan Kaya wrote:
> > > 
> > > #_dmesg_|_grep_resource
> > > [    2.945762] pci_bus 0000:00: root bus resource [io  0x0000-0xefff window] (bus address [0x1000-0xffff])
> > > [    3.652201] pci_bus 0002:00: root bus resource [io  0xf000-0x1dfff window] (bus address [0x1000-0xffff])
> > > [    6.546716] pci_bus 0006:00: root bus resource [io  0x1e000-0x2cfff window] (bus address [0x1000-0xffff])
> > > / #
> > 
> > This is bad. We normally want to stay out of the first 0x1000 bytes of
> > the Linux space, to prevent drivers from poking into the ISA
> > registers.
> 
> You are referring to:
> 
> pci_bus 0000:00: root bus resource [io  0x0000-0xefff window]
>                                         ^^^^^^
> here, right ? [0x0 - PCIBIOS_MIN_IO] is not assigned by the PCI
> code that reassigns resources anyway, so devices with IO BARs won't
> get assigned [0x0 - PCIBIOS_MIN_IO] address space (Linux space).
> 
> Are you saying we should disallow the [0x0 - 0x1000] in the PCI busses
> IO resource (Linux space) ?
> 
> In pci_address_to_pio() the offset (Linux IO resource) we assign starts
> from 0x0, so we always allocate that chunk of IO address space (that is
> an offset into the Linux virtual address space), am I correct ?

I think we can assign the address zero of the Linux I/O port range, but
we should never assign it to a bus port range that does not also start
at zero.

If we encounter a firmware description that has bus range which excludes
the first 1k, we should probably assign it to somewhere after 0x10000
(65536), so we can later assign a primary I/O space to a bus that has an
ISA or LPC bridge with actual devices below 0x1000 (4096).

> > We can have one of the buses be the "primary" bus that has its first
> > 0x1000 bytes of I/O space mapped into the respective Linux addresses,
> > but mapping the second 0x1000 bytes into the reserved space is the
> > worst possible outcome here, as legacy ISA drivers will now poke at
> > random other devices that are intentionally moved to high addresses to
> > stay of of that range.
> 
> And you are referring to:
> 
> root bus resource [io  0x0000-0xefff window] (bus address [0x1000-0xffff])
> 		       ^^^^^^                              ^^^^^^
> 
> here ? If ISA drivers poke at addresses in the [0x0 - 0x1000]
> range (Linux space IO offset) they end up on the PCI bus with addresses
> above 0x1000, is that what you are saying when you refer to "moved to
> high addresses to stay out of that range" ?

Correct.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web