Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753885Ab2K3F3G (ORCPT ); Fri, 30 Nov 2012 00:29:06 -0500 Received: from mail-pa0-f46.google.com ([209.85.220.46]:64586 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753747Ab2K3F3D (ORCPT ); Fri, 30 Nov 2012 00:29:03 -0500 Message-ID: <50B84418.5070804@numascale-asia.com> Date: Fri, 30 Nov 2012 13:28:56 +0800 From: Daniel J Blueman Organization: Numascale Asia User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Bjorn Helgaas CC: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Steffen Persvold Subject: Re: [PATCH v2 RESEND] Add NumaChip remote PCI support References: <1353487196-10204-1-git-send-email-daniel@numascale-asia.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5651 Lines: 159 Hi Bjorn, On 29/11/2012 07:08, Bjorn Helgaas wrote: > On Wed, Nov 21, 2012 at 1:39 AM, Daniel J Blueman > 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 >> --- >> 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 >> + * >> + */ >> + >> +#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 >> #include >> >> +#include >> #include >> #include >> #include >> @@ -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.o > > It 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 >> + * >> + * PCI accessor functions derived from mmconfig_64.c >> + * >> + */ >> + >> +#include >> +#include >> + >> +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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/