From: David Daney <[email protected]>
Needed to make pci_iomap() work.
Signed-off-by: David Daney <[email protected]>
---
arch/arm64/include/asm/io.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 540f7c0..8ef78d5 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -149,6 +149,11 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
#define PCI_IOBASE ((void __iomem *)PCI_IO_START)
+#define HAVE_ARCH_PIO_SIZE 1
+#define PIO_RESERVED SZ_32M
+#define PIO_OFFSET 0
+#define PIO_MASK (PIO_RESERVED - 1)
+
/*
* String version of I/O memory access operations.
*/
--
1.7.11.7
On Mon, Jul 13, 2015 at 10:31:36PM +0100, David Daney wrote:
> From: David Daney <[email protected]>
>
> Needed to make pci_iomap() work.
Care to elaborate?
AFAICT, mapping an IO bar on arm64 just gives you back a VA into our
PCI_IOBASE region and the ioreadX accessors will just call the readX
macros, so there should be no need for further port adjustment.
Will
On 07/14/2015 04:00 AM, Will Deacon wrote:
> On Mon, Jul 13, 2015 at 10:31:36PM +0100, David Daney wrote:
>> From: David Daney <[email protected]>
>>
>> Needed to make pci_iomap() work.
>
> Care to elaborate?
>
I should have explained what I am doing here a little better.
Systems based on the Cavium ThunderX processor may have up to 8
independent PCIe root complexes. The I/O space on each bus occupies an
independent physical address window.
So, in order to be able to map all of these (semi) contiguously, we need
a lot more virtual address space than is supplied by the default values
for all these constants.
The option I chose here was to unconditionally expand the I/O ranges for
all arm64 systems. If you think this breaks existing systems/drivers, I
will have to look for other options.
David Daney
> AFAICT, mapping an IO bar on arm64 just gives you back a VA into our
> PCI_IOBASE region and the ioreadX accessors will just call the readX
> macros, so there should be no need for further port adjustment.
>
> Will
>
On Tue, Jul 14, 2015 at 05:12:57PM +0100, David Daney wrote:
> On 07/14/2015 04:00 AM, Will Deacon wrote:
> > On Mon, Jul 13, 2015 at 10:31:36PM +0100, David Daney wrote:
> >> From: David Daney <[email protected]>
> >>
> >> Needed to make pci_iomap() work.
> >
> > Care to elaborate?
> >
>
> I should have explained what I am doing here a little better.
Yeah, thanks.
> Systems based on the Cavium ThunderX processor may have up to 8
> independent PCIe root complexes. The I/O space on each bus occupies an
> independent physical address window.
Hmm, so do you have 64k of I/O space per-bus? That gives 8x256x64k = 128M
IIUC, so not sure what your 32MB is for.
> So, in order to be able to map all of these (semi) contiguously, we need
> a lot more virtual address space than is supplied by the default values
> for all these constants.
>
> The option I chose here was to unconditionally expand the I/O ranges for
> all arm64 systems. If you think this breaks existing systems/drivers, I
> will have to look for other options.
Hmm, but pci_iomap winds up calling __pci_ioport_map, which expands to
ioport_map which just does:
return PCI_IOBASE + (port & IO_SPACE_LIMIT);
so I'm struggling to see what your patch achieves.
Will
On 07/14/2015 09:29 AM, Will Deacon wrote:
> On Tue, Jul 14, 2015 at 05:12:57PM +0100, David Daney wrote:
>> On 07/14/2015 04:00 AM, Will Deacon wrote:
>>> On Mon, Jul 13, 2015 at 10:31:36PM +0100, David Daney wrote:
>>>> From: David Daney <[email protected]>
>>>>
>>>> Needed to make pci_iomap() work.
>>>
>>> Care to elaborate?
>>>
>>
>> I should have explained what I am doing here a little better.
>
> Yeah, thanks.
>
>> Systems based on the Cavium ThunderX processor may have up to 8
>> independent PCIe root complexes. The I/O space on each bus occupies an
>> independent physical address window.
>
> Hmm, so do you have 64k of I/O space per-bus? That gives 8x256x64k = 128M
> IIUC, so not sure what your 32MB is for.
I don't understand where your 256 came from there.
Actually, my current implementation has 1M per bus(which is overkill).
For 8 buses I need 8M, which fits within the PCI_IO_SIZE...
>
>> So, in order to be able to map all of these (semi) contiguously, we need
>> a lot more virtual address space than is supplied by the default values
>> for all these constants.
>>
>> The option I chose here was to unconditionally expand the I/O ranges for
>> all arm64 systems. If you think this breaks existing systems/drivers, I
>> will have to look for other options.
>
> Hmm, but pci_iomap winds up calling __pci_ioport_map, which expands to
> ioport_map which just does:
>
> return PCI_IOBASE + (port & IO_SPACE_LIMIT);
>
> so I'm struggling to see what your patch achieves.
Here is ioport_map (from lib/iomap.c):
void __iomem *ioport_map(unsigned long port, unsigned int nr)
{
if (port > PIO_MASK)
return NULL;
return (void __iomem *) (unsigned long) (port + PIO_OFFSET);
}
With the default value of PIO_MASK (64K), I cannot map any I/O ports on
my PCIe RC 1..7
The values I supplied in my patch may be sub-optimal, but I think
something is needed. I will look into this in a little more detail today.
Thanks,
David Daney
>
> Will
>
Hi David,
On Tue, Jul 14, 2015 at 05:58:20PM +0100, David Daney wrote:
> On 07/14/2015 09:29 AM, Will Deacon wrote:
> > On Tue, Jul 14, 2015 at 05:12:57PM +0100, David Daney wrote:
> >> Systems based on the Cavium ThunderX processor may have up to 8
> >> independent PCIe root complexes. The I/O space on each bus occupies an
> >> independent physical address window.
> >
> > Hmm, so do you have 64k of I/O space per-bus? That gives 8x256x64k = 128M
> > IIUC, so not sure what your 32MB is for.
>
> I don't understand where your 256 came from there.
Sorry, sounds like we're mixing up buses and root complexes. Which is it?
> Actually, my current implementation has 1M per bus(which is overkill).
> For 8 buses I need 8M, which fits within the PCI_IO_SIZE...
So there's no problem?
> >> So, in order to be able to map all of these (semi) contiguously, we need
> >> a lot more virtual address space than is supplied by the default values
> >> for all these constants.
> >>
> >> The option I chose here was to unconditionally expand the I/O ranges for
> >> all arm64 systems. If you think this breaks existing systems/drivers, I
> >> will have to look for other options.
> >
> > Hmm, but pci_iomap winds up calling __pci_ioport_map, which expands to
> > ioport_map which just does:
> >
> > return PCI_IOBASE + (port & IO_SPACE_LIMIT);
> >
> > so I'm struggling to see what your patch achieves.
>
> Here is ioport_map (from lib/iomap.c):
>
>
> void __iomem *ioport_map(unsigned long port, unsigned int nr)
> {
> if (port > PIO_MASK)
> return NULL;
> return (void __iomem *) (unsigned long) (port + PIO_OFFSET);
> }
We only get this definition if CONFIG_GENERIC_IOMAP=y. Why is that
selected?
Will
On 07/14/2015 10:04 AM, Will Deacon wrote:
> Hi David,
>
> On Tue, Jul 14, 2015 at 05:58:20PM +0100, David Daney wrote:
[...]
> We only get this definition if CONFIG_GENERIC_IOMAP=y. Why is that
> selected?
>
OK, Good question.
The original patch was against 3.18, but commit 09a5723983e (arm64: Use
include/asm-generic/io.h) changed some of this, so it may need
reconsideration.
Let me reevaluate this patch.
Thanks,
David Daney