Hi Bjorn, On 29/11/2012 07:08, Bjorn Helgaas wrote:
On Wed, Nov 21, 2012 at 1:39 AM, Daniel J Blueman <daniel@xxxxxxxxxxxxxxxxxx> wrote:Add NumaChip-specific PCI access mechanism via MMCONFIG cycles, but preventing access to AMD Northbridges which shouldn't respond. v2: Use PCI_DEVFN in precomputed constant limit; drop unneeded includes Signed-off-by: Daniel J Blueman <daniel@xxxxxxxxxxxxxxxxxx> --- arch/x86/include/asm/numachip/numachip.h | 20 +++++ arch/x86/kernel/apic/apic_numachip.c | 2 + arch/x86/pci/Makefile | 1 + arch/x86/pci/numachip.c | 134 ++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+) create mode 100644 arch/x86/include/asm/numachip/numachip.h create mode 100644 arch/x86/pci/numachip.c diff --git a/arch/x86/include/asm/numachip/numachip.h b/arch/x86/include/asm/numachip/numachip.h new file mode 100644 index 0000000..d35e71a --- /dev/null +++ b/arch/x86/include/asm/numachip/numachip.h @@ -0,0 +1,20 @@ +/* + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * Numascale NumaConnect-specific header file + * + * Copyright (C) 2012 Numascale AS. All rights reserved. + * + * Send feedback to <support@xxxxxxxxxxxxx> + * + */ + +#ifndef _ASM_X86_NUMACHIP_NUMACHIP_H +#define _ASM_X86_NUMACHIP_NUMACHIP_H + +extern int __init pci_numachip_init(void); + +#endif /* _ASM_X86_NUMACHIP_NUMACHIP_H */ + diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c index a65829a..9c2aa89 100644 --- a/arch/x86/kernel/apic/apic_numachip.c +++ b/arch/x86/kernel/apic/apic_numachip.c @@ -22,6 +22,7 @@ #include <linux/hardirq.h> #include <linux/delay.h> +#include <asm/numachip/numachip.h> #include <asm/numachip/numachip_csr.h> #include <asm/smp.h> #include <asm/apic.h> @@ -179,6 +180,7 @@ static int __init numachip_system_init(void) return 0; x86_cpuinit.fixup_cpu_id = fixup_cpu_id; + x86_init.pci.arch_init = pci_numachip_init; map_csrs(); diff --git a/arch/x86/pci/Makefile b/arch/x86/pci/Makefile index 3af5a1e..ee0af58 100644 --- a/arch/x86/pci/Makefile +++ b/arch/x86/pci/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_STA2X11) += sta2x11-fixup.o obj-$(CONFIG_X86_VISWS) += visws.o obj-$(CONFIG_X86_NUMAQ) += numaq_32.o +obj-$(CONFIG_X86_NUMACHIP) += numachip.oIt looks like this depends on CONFIG_PCI_MMCONFIG for pci_mmconfig_lookup(). Are there config constraints that force CONFIG_PCI_MMCONFIG=y when CONFIG_X86_NUMACHIP=y?
I'll revise the patch with this constraint after we work out the best approach for below.
obj-$(CONFIG_X86_INTEL_MID) += mrst.o diff --git a/arch/x86/pci/numachip.c b/arch/x86/pci/numachip.c new file mode 100644 index 0000000..3773e05 --- /dev/null +++ b/arch/x86/pci/numachip.c @@ -0,0 +1,129 @@ +/* + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * Numascale NumaConnect-specific PCI code + * + * Copyright (C) 2012 Numascale AS. All rights reserved. + * + * Send feedback to <support@xxxxxxxxxxxxx> + * + * PCI accessor functions derived from mmconfig_64.c + * + */ + +#include <linux/pci.h> +#include <asm/pci_x86.h> + +static u8 limit __read_mostly; + +static inline char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn) +{ + struct pci_mmcfg_region *cfg = pci_mmconfig_lookup(seg, bus); + + if (cfg && cfg->virt) + return cfg->virt + (PCI_MMCFG_BUS_OFFSET(bus) | (devfn << 12)); + return NULL; +}Most of this file is copied directly from mmconfig_64.c (as you mentioned above). I wonder if we could avoid the code duplication by making the pci_dev_base() implementation in mmconfig_64.c a weak definition. Then you could just supply a non-weak pci_dev_base() here that would override that default version. Your version would look something like: char __iomem *pci_dev_base(unsigned int seg, unsigned int bus, unsigned int devfn) { struct pci_mmcfg_region *cfg = pci_mmconfig_lookup(seg, bus); if (cfg && cfg->virt && devfn < limit) return cfg->virt + (PCI_MMCFG_BUS_OFFSET(bus) | (devfn << 12)); return NULL; } That would be different from what you have in this patch because reads & writes to devices above "limit" would return -EINVAL rather than 0 as you do here. Would that be a problem?
That would work nicely (pointer lookup and inlining etc aside) if there was the runtime ability to override pci_dev_base only if the NumaChip signature was detected.
We could expose pci_dev_base via struct x86_init_pci; the extra complexity and performance tradeoff may not be worth it for a single case perhaps?
Thanks, Daniel -- Daniel J Blueman Principal Software Engineer, Numascale Asia -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
