Re: pci: Arch hook to determine config space size
On Wed, 2005-02-02 at 11:05 +0100, Arnd Bergmann wrote: > How about something along the lines of this patch? Instead of adding a > pointer to the pci data from the device node, it embeds the node inside > a new struct pci_device_node. The patch is not complete and therefore > not expected to work as is, but maybe you want to reuse it. > > The interesting part that is missing is creating and destroying > pci_device_nodes in prom.c, maybe you have an idea how to do that. > > I'm also not sure about eeh. Are the eeh functions known to be called > only for device_nodes of PCI devices? If not, eeh_mode and > eeh_config_addr might have to stay inside of device_node. I'd rather not go that way for now. There are at least PCI and VIO devices concerned by this, and maybe more (depending on how I deal with macio devices for example). We also want, ultimately, to have the DMA routines be function pointers in this auxilliary structure. Ben.
Re: pci: Arch hook to determine config space size
On Dinsdag 01 Februar 2005 05:57, Benjamin Herrenschmidt wrote: > BTW. I'm thinking about moving all those PCI/VIO related fields out of > struct device_node to their own structure and keep only a pointer to > that structure in device_node. That way, we avoid the bloat for every > single non-pci node in the system, and we can have different structures > for different bus types (along with proper iommu function pointers and > that sort-of-thing). How about something along the lines of this patch? Instead of adding a pointer to the pci data from the device node, it embeds the node inside a new struct pci_device_node. The patch is not complete and therefore not expected to work as is, but maybe you want to reuse it. The interesting part that is missing is creating and destroying pci_device_nodes in prom.c, maybe you have an idea how to do that. I'm also not sure about eeh. Are the eeh functions known to be called only for device_nodes of PCI devices? If not, eeh_mode and eeh_config_addr might have to stay inside of device_node. Arnd <>< Signed-off-by: Arnd Bergmann <[EMAIL PROTECTED]> Index: linux-2.6-64/arch/ppc64/kernel/pci.h === --- linux-2.6-64.orig/arch/ppc64/kernel/pci.h 2005-01-24 23:46:37.0 +0100 +++ linux-2.6-64/arch/ppc64/kernel/pci.h2005-02-02 00:11:01.485740824 +0100 @@ -29,8 +29,8 @@ /* PCI device_node operations */ struct device_node; -typedef void *(*traverse_func)(struct device_node *me, void *data); -void *traverse_pci_devices(struct device_node *start, traverse_func pre, +typedef void *(*traverse_func)(struct pci_device_node *me, void *data); +void *traverse_pci_devices(struct pci_device_node *start, traverse_func pre, void *data); void pci_devs_phb_init(void); Index: linux-2.6-64/arch/ppc64/kernel/pSeries_iommu.c === --- linux-2.6-64.orig/arch/ppc64/kernel/pSeries_iommu.c 2005-02-01 22:53:00.673332472 +0100 +++ linux-2.6-64/arch/ppc64/kernel/pSeries_iommu.c 2005-02-02 00:11:01.486740672 +0100 @@ -48,7 +48,7 @@ #define DBG(fmt...) -extern int is_python(struct device_node *); +extern int is_python(struct pci_device_node *); static void tce_build_pSeries(struct iommu_table *tbl, long index, long npages, unsigned long uaddr, @@ -289,7 +289,7 @@ * Currently we hard code these sizes (more or less). */ static void iommu_table_setparms_lpar(struct pci_controller *phb, - struct device_node *dn, + struct pci_device_node *dn, struct iommu_table *tbl, unsigned int *dma_window) { @@ -308,7 +308,7 @@ static void iommu_bus_setup_pSeries(struct pci_bus *bus) { - struct device_node *dn, *pdn; + struct pci_device_node *dn, *pdn; DBG("iommu_bus_setup_pSeries, bus %p, bus->self %p\n", bus, bus->self); @@ -331,7 +331,7 @@ DBG("Python root bus %s\n", bus->name); - iohole = (unsigned int *)get_property(dn, "io-hole", 0); + iohole = (unsigned int *)get_property(>node, "io-hole", 0); if (iohole) { /* On first bus we need to leave room for the @@ -349,7 +349,7 @@ tbl = kmalloc(sizeof(struct iommu_table), GFP_KERNEL); - iommu_table_setparms(dn->phb, dn, tbl); + iommu_table_setparms(dn->phb, >node, tbl); dn->iommu_table = iommu_init_table(tbl); } else { /* 256 MB window by default */ @@ -368,7 +368,7 @@ tbl = kmalloc(sizeof(struct iommu_table), GFP_KERNEL); - iommu_table_setparms(dn->phb, dn, tbl); + iommu_table_setparms(dn->phb, >node, tbl); dn->iommu_table = iommu_init_table(tbl); } else { @@ -382,17 +382,19 @@ static void iommu_bus_setup_pSeriesLP(struct pci_bus *bus) { struct iommu_table *tbl; - struct device_node *dn, *pdn; + struct pci_device_node *dn, *pdn; + struct device_node *n; unsigned int *dma_window = NULL; dn = pci_bus_to_OF_node(bus); /* Find nearest ibm,dma-window, walking up the device tree */ - for (pdn = dn; pdn != NULL; pdn = pdn->parent) { - dma_window = (unsigned int *)get_property(pdn, "ibm,dma-window", NULL); + for (n = >node; n; n = n->parent) { + dma_window = (unsigned int *)get_property(n, "ibm,dma-window", NULL); if (dma_window != NULL) break; } + pdn = to_pci_node(n); if (dma_window == NULL) { DBG("iommu_bus_setup_pSeriesLP: bus %s
Re: pci: Arch hook to determine config space size
On Dinsdag 01 Februar 2005 05:57, Benjamin Herrenschmidt wrote: BTW. I'm thinking about moving all those PCI/VIO related fields out of struct device_node to their own structure and keep only a pointer to that structure in device_node. That way, we avoid the bloat for every single non-pci node in the system, and we can have different structures for different bus types (along with proper iommu function pointers and that sort-of-thing). How about something along the lines of this patch? Instead of adding a pointer to the pci data from the device node, it embeds the node inside a new struct pci_device_node. The patch is not complete and therefore not expected to work as is, but maybe you want to reuse it. The interesting part that is missing is creating and destroying pci_device_nodes in prom.c, maybe you have an idea how to do that. I'm also not sure about eeh. Are the eeh functions known to be called only for device_nodes of PCI devices? If not, eeh_mode and eeh_config_addr might have to stay inside of device_node. Arnd Signed-off-by: Arnd Bergmann [EMAIL PROTECTED] Index: linux-2.6-64/arch/ppc64/kernel/pci.h === --- linux-2.6-64.orig/arch/ppc64/kernel/pci.h 2005-01-24 23:46:37.0 +0100 +++ linux-2.6-64/arch/ppc64/kernel/pci.h2005-02-02 00:11:01.485740824 +0100 @@ -29,8 +29,8 @@ /* PCI device_node operations */ struct device_node; -typedef void *(*traverse_func)(struct device_node *me, void *data); -void *traverse_pci_devices(struct device_node *start, traverse_func pre, +typedef void *(*traverse_func)(struct pci_device_node *me, void *data); +void *traverse_pci_devices(struct pci_device_node *start, traverse_func pre, void *data); void pci_devs_phb_init(void); Index: linux-2.6-64/arch/ppc64/kernel/pSeries_iommu.c === --- linux-2.6-64.orig/arch/ppc64/kernel/pSeries_iommu.c 2005-02-01 22:53:00.673332472 +0100 +++ linux-2.6-64/arch/ppc64/kernel/pSeries_iommu.c 2005-02-02 00:11:01.486740672 +0100 @@ -48,7 +48,7 @@ #define DBG(fmt...) -extern int is_python(struct device_node *); +extern int is_python(struct pci_device_node *); static void tce_build_pSeries(struct iommu_table *tbl, long index, long npages, unsigned long uaddr, @@ -289,7 +289,7 @@ * Currently we hard code these sizes (more or less). */ static void iommu_table_setparms_lpar(struct pci_controller *phb, - struct device_node *dn, + struct pci_device_node *dn, struct iommu_table *tbl, unsigned int *dma_window) { @@ -308,7 +308,7 @@ static void iommu_bus_setup_pSeries(struct pci_bus *bus) { - struct device_node *dn, *pdn; + struct pci_device_node *dn, *pdn; DBG(iommu_bus_setup_pSeries, bus %p, bus-self %p\n, bus, bus-self); @@ -331,7 +331,7 @@ DBG(Python root bus %s\n, bus-name); - iohole = (unsigned int *)get_property(dn, io-hole, 0); + iohole = (unsigned int *)get_property(dn-node, io-hole, 0); if (iohole) { /* On first bus we need to leave room for the @@ -349,7 +349,7 @@ tbl = kmalloc(sizeof(struct iommu_table), GFP_KERNEL); - iommu_table_setparms(dn-phb, dn, tbl); + iommu_table_setparms(dn-phb, dn-node, tbl); dn-iommu_table = iommu_init_table(tbl); } else { /* 256 MB window by default */ @@ -368,7 +368,7 @@ tbl = kmalloc(sizeof(struct iommu_table), GFP_KERNEL); - iommu_table_setparms(dn-phb, dn, tbl); + iommu_table_setparms(dn-phb, dn-node, tbl); dn-iommu_table = iommu_init_table(tbl); } else { @@ -382,17 +382,19 @@ static void iommu_bus_setup_pSeriesLP(struct pci_bus *bus) { struct iommu_table *tbl; - struct device_node *dn, *pdn; + struct pci_device_node *dn, *pdn; + struct device_node *n; unsigned int *dma_window = NULL; dn = pci_bus_to_OF_node(bus); /* Find nearest ibm,dma-window, walking up the device tree */ - for (pdn = dn; pdn != NULL; pdn = pdn-parent) { - dma_window = (unsigned int *)get_property(pdn, ibm,dma-window, NULL); + for (n = dn-node; n; n = n-parent) { + dma_window = (unsigned int *)get_property(n, ibm,dma-window, NULL); if (dma_window != NULL) break; } + pdn = to_pci_node(n); if (dma_window == NULL) { DBG(iommu_bus_setup_pSeriesLP: bus %s seems to have no
Re: pci: Arch hook to determine config space size
On Wed, 2005-02-02 at 11:05 +0100, Arnd Bergmann wrote: How about something along the lines of this patch? Instead of adding a pointer to the pci data from the device node, it embeds the node inside a new struct pci_device_node. The patch is not complete and therefore not expected to work as is, but maybe you want to reuse it. The interesting part that is missing is creating and destroying pci_device_nodes in prom.c, maybe you have an idea how to do that. I'm also not sure about eeh. Are the eeh functions known to be called only for device_nodes of PCI devices? If not, eeh_mode and eeh_config_addr might have to stay inside of device_node. I'd rather not go that way for now. There are at least PCI and VIO devices concerned by this, and maybe more (depending on how I deal with macio devices for example). We also want, ultimately, to have the DMA routines be function pointers in this auxilliary structure. Ben.
Re: pci: Arch hook to determine config space size
Matthew Wilcox wrote: On Mon, Jan 31, 2005 at 10:52:29PM -0600, Brian King wrote: @@ -62,8 +72,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; You should probably delete this redundant test at the same time ... Done. The new patch below also adds some address checking to iSeries config accessor functions. Additionally, this patch should address Arnd's concern, as it now looks for the "ibm,pci-config-space-type" property on the device itself rather than on the bridge. -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. Signed-off-by: Brian King <[EMAIL PROTECTED]> --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/iSeries_pci.c |6 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 20 --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci_dn.c |6 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h|1 4 files changed, 29 insertions(+), 4 deletions(-) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-02-01 13:31:46.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-02-01 13:39:32.0 -0600 @@ -52,6 +52,16 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + if (where < 256) + return 1; + if (where < 4096 && dn->pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -60,10 +70,11 @@ static int rtas_read_config(struct devic if (!dn) return PCIBIOS_DEVICE_NOT_FOUND; - if (where & (size - 1)) + if (!config_access_valid(dn, where)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | + (dn->devfn << 8) | (where & 0xff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, , @@ -108,10 +119,11 @@ static int rtas_write_config(struct devi if (!dn) return PCIBIOS_DEVICE_NOT_FOUND; - if (where & (size - 1)) + if (!config_access_valid(dn, where)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | + (dn->devfn << 8) | (where & 0xff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid >> 32, buid & 0x, size, (ulong) val); diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-02-01 13:31:46.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-02-01 13:31:46.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for pci devices */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ diff -puN arch/ppc64/kernel/pci_dn.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pci_dn.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pci_dn.c~ppc64_pcix_mode2_cfg 2005-02-01 13:31:46.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci_dn.c 2005-02-01 13:31:46.0 -0600 @@ -37,6 +37,7 @@ static void * __devinit update_dn_pci_info(struct device_node *dn, void *data) { struct pci_controller *phb = data; + int *type = (int *)get_property(dn, "ibm,pci-config-space-type", NULL); u32 *regs; dn->phb = phb; @@ -46,6 +47,11 @@ static void * __devinit update_dn_pci_in dn->busno = (regs[0] >> 16) & 0xff; dn->devfn = (regs[0] >> 8) & 0xff; } + + if (type && *type == 1) + dn->pci_ext_config_space = 1; + else + dn->pci_ext_config_space = 0; return NULL; } diff -puN
Re: pci: Arch hook to determine config space size
Grant Grundler wrote: On Mon, Jan 31, 2005 at 01:40:04PM -0600, Brian King wrote: CC'ing the linux-pci mailing list... thanks... This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, ) != PCIBIOS_SUCCESSFUL) goto fail; pci_read_config_dword lands in arch specific code. See drivers/pci/access.c:PCI_OP_READ() macro. I'm missing what pcibios_exp_cfg_space() does that can't be handled by the bus_ops supplied by pci_scan_bus(). I would expect the pci_read_config_dword to fail for being out of bounds. Is that wrong? Or is bus_ops not feasible in this case because pcibios needs access to pci_dev? The current patch for this has become essentially that. It is now a PPC64 specific patch that adds bounds checking in the PPC64 PCI config access functions. -Brian -- Brian King eServer Storage I/O IBM Linux Technology Center
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 10:52:29PM -0600, Brian King wrote: > @@ -62,8 +72,11 @@ static int rtas_read_config(struct devic > return PCIBIOS_DEVICE_NOT_FOUND; > if (where & (size - 1)) > return PCIBIOS_BAD_REGISTER_NUMBER; You should probably delete this redundant test at the same time ... > + if (!config_access_valid(dn, where)) > + return PCIBIOS_BAD_REGISTER_NUMBER; > > - addr = (dn->busno << 16) | (dn->devfn << 8) | where; > + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | > + (dn->devfn << 8) | (where & 0xff); > buid = dn->phb->buid; > if (buid) { > ret = rtas_call(ibm_read_pci_config, 4, 2, , -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain
Re: pci: Arch hook to determine config space size
Grant Grundler wrote: On Mon, Jan 31, 2005 at 01:40:04PM -0600, Brian King wrote: CC'ing the linux-pci mailing list... thanks... This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, status) != PCIBIOS_SUCCESSFUL) goto fail; pci_read_config_dword lands in arch specific code. See drivers/pci/access.c:PCI_OP_READ() macro. I'm missing what pcibios_exp_cfg_space() does that can't be handled by the bus_ops supplied by pci_scan_bus(). I would expect the pci_read_config_dword to fail for being out of bounds. Is that wrong? Or is bus_ops not feasible in this case because pcibios needs access to pci_dev? The current patch for this has become essentially that. It is now a PPC64 specific patch that adds bounds checking in the PPC64 PCI config access functions. -Brian -- Brian King eServer Storage I/O IBM Linux Technology Center
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 10:52:29PM -0600, Brian King wrote: @@ -62,8 +72,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; You should probably delete this redundant test at the same time ... + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, returnval, -- Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception. -- Mark Twain
Re: pci: Arch hook to determine config space size
Matthew Wilcox wrote: On Mon, Jan 31, 2005 at 10:52:29PM -0600, Brian King wrote: @@ -62,8 +72,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; You should probably delete this redundant test at the same time ... Done. The new patch below also adds some address checking to iSeries config accessor functions. Additionally, this patch should address Arnd's concern, as it now looks for the ibm,pci-config-space-type property on the device itself rather than on the bridge. -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/iSeries_pci.c |6 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 20 --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci_dn.c |6 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h|1 4 files changed, 29 insertions(+), 4 deletions(-) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-02-01 13:31:46.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-02-01 13:39:32.0 -0600 @@ -52,6 +52,16 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + if (where 256) + return 1; + if (where 4096 dn-pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -60,10 +70,11 @@ static int rtas_read_config(struct devic if (!dn) return PCIBIOS_DEVICE_NOT_FOUND; - if (where (size - 1)) + if (!config_access_valid(dn, where)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, returnval, @@ -108,10 +119,11 @@ static int rtas_write_config(struct devi if (!dn) return PCIBIOS_DEVICE_NOT_FOUND; - if (where (size - 1)) + if (!config_access_valid(dn, where)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid 32, buid 0x, size, (ulong) val); diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-02-01 13:31:46.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-02-01 13:31:46.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for pci devices */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ diff -puN arch/ppc64/kernel/pci_dn.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pci_dn.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pci_dn.c~ppc64_pcix_mode2_cfg 2005-02-01 13:31:46.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci_dn.c 2005-02-01 13:31:46.0 -0600 @@ -37,6 +37,7 @@ static void * __devinit update_dn_pci_info(struct device_node *dn, void *data) { struct pci_controller *phb = data; + int *type = (int *)get_property(dn, ibm,pci-config-space-type, NULL); u32 *regs; dn-phb = phb; @@ -46,6 +47,11 @@ static void * __devinit update_dn_pci_in dn-busno = (regs[0] 16) 0xff; dn-devfn = (regs[0] 8) 0xff; } + + if (type *type == 1) + dn-pci_ext_config_space = 1; + else + dn-pci_ext_config_space = 0; return NULL; } diff -puN arch/ppc64/kernel/iSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/iSeries_pci.c ---
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 01:40:04PM -0600, Brian King wrote: > CC'ing the linux-pci mailing list... thanks... > > This patch adds an arch hook so > > that individual archs can indicate if the underlying system supports > > expanded config space accesses or not. > >@@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci > > goto fail; > > } > > > >+if (!pcibios_exp_cfg_space(dev)) > >+goto fail; > > if (pci_read_config_dword(dev, 256, ) != PCIBIOS_SUCCESSFUL) > > goto fail; pci_read_config_dword lands in arch specific code. See drivers/pci/access.c:PCI_OP_READ() macro. I'm missing what pcibios_exp_cfg_space() does that can't be handled by the bus_ops supplied by pci_scan_bus(). I would expect the pci_read_config_dword to fail for being out of bounds. Is that wrong? Or is bus_ops not feasible in this case because pcibios needs access to pci_dev? If it's feasible, maybe the right place to add this hook is to pci_read_config_dword which is also handed the pci_dev. And add another function pointer to bus_ops (which could be NULL) to check chipset support for Expanded Config space before calling pci_bus_read_config_dword. Thats cleaner than adding a hook before each use of pci_read_config_dword. hth, grant
Re: pci: Arch hook to determine config space size
On Mon, 2005-01-31 at 22:52 -0600, Brian King wrote: > Assuming I am reading the spec correctly, this is only a property of the > PHB, so I could move it into the pci_controller struct instead. Note that Arnd seems to imply the opposite ... BTW. I'm thinking about moving all those PCI/VIO related fields out of struct device_node to their own structure and keep only a pointer to that structure in device_node. That way, we avoid the bloat for every single non-pci node in the system, and we can have different structures for different bus types (along with proper iommu function pointers and that sort-of-thing). So if you think you really need a per-device info here, feel free to add it to device_node for now, and I'll move it to the new structure along with the rest of the stuff once I find time to do this patch. Ben.
Re: pci: Arch hook to determine config space size
Benjamin Herrenschmidt wrote: On Mon, 2005-01-31 at 16:43 -0600, Brian King wrote: diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-01-31 14:32:01.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for phb's or bridges */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ Grrr... more crap added to the device-node, I don't like that ... This is a PHB only field, can't it be in struct pci_controller instead ? Assuming I am reading the spec correctly, this is only a property of the PHB, so I could move it into the pci_controller struct instead. -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 31 ++- linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/pci-bridge.h |1 2 files changed, 30 insertions(+), 2 deletions(-) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-01-31 22:27:49.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-01-31 22:31:04.0 -0600 @@ -52,6 +52,16 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + if (where < 256) + return 1; + if (where < 4096 && dn->phb->pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -62,8 +72,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | + (dn->devfn << 8) | (where & 0xff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, , @@ -110,8 +123,11 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | + (dn->devfn << 8) | (where & 0xff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid >> 32, buid & 0x, size, (ulong) val); @@ -270,6 +286,16 @@ static int phb_set_bus_ranges(struct dev return 0; } +static char __devinit get_phb_config_space_type(struct pci_controller *phb) +{ + int *type = (int *)get_property(phb->arch_data, + "ibm,pci-config-space-type", NULL); + + if (type && *type == 1) + return 1; + return 0; +} + static int __devinit setup_phb(struct device_node *dev, struct pci_controller *phb, unsigned int addr_size_words) @@ -285,6 +311,7 @@ static int __devinit setup_phb(struct de phb->arch_data = dev; phb->ops = _pci_ops; phb->buid = get_phb_buid(dev); + phb->pci_ext_config_space = get_phb_config_space_type(phb); return 0; } diff -puN include/asm-ppc64/pci-bridge.h~ppc64_pcix_mode2_cfg include/asm-ppc64/pci-bridge.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/pci-bridge.h~ppc64_pcix_mode2_cfg 2005-01-31 22:27:49.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/pci-bridge.h 2005-01-31 22:27:49.0 -0600 @@ -17,6 +17,7 @@ struct pci_controller { struct pci_bus *bus; char is_dynamic; + char pci_ext_config_space; void *arch_data; struct list_head list_node; _
Re: pci: Arch hook to determine config space size
On Mon, 2005-01-31 at 16:43 -0600, Brian King wrote: > diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg > include/asm-ppc64/prom.h > --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg > 2005-01-31 14:32:01.0 -0600 > +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-01-31 > 14:32:01.0 -0600 > @@ -137,6 +137,7 @@ struct device_node { > int devfn; /* for pci devices */ > int eeh_mode; /* See eeh.h for possible EEH_MODEs */ > int eeh_config_addr; > + int pci_ext_config_space; /* for phb's or bridges */ > struct pci_controller *phb;/* for pci devices */ > struct iommu_table *iommu_table; /* for phb's or bridges */ Grrr... more crap added to the device-node, I don't like that ... This is a PHB only field, can't it be in struct pci_controller instead ? Ben.
Re: pci: Arch hook to determine config space size
Brian King <[EMAIL PROTECTED]> schrieb am 31.01.2005, 23:43:30: > > Isn't the config space size a property of the PCI device instead of the > > host bridge? For a PCI device behind a PCIe host bridge, this could > > still lead to an incorrect config space accesses. > > It is a property of both. Accessing config space beyond the first 256 > bytes will only work if both the PCI device and the host bridge support > it. The problem I ran into was generic pci code issuing a config read to > offset 256 after checking that the device supports it when the host > bridge did not support it. If I interpret the spec correctly, the firmware should always store the value we need in the property for every device node, which means that you should look at the host bridge config-space-type attribute only when you want to look at the bridge itself. If the device claims to support a PCIe config space and the bridge doesn't, that sounds to me like a firmware bug. Arnd <><
Re: pci: Arch hook to determine config space size
Arnd Bergmann wrote: On Maandag 31 Januar 2005 22:35, Brian King wrote: Matthew Wilcox wrote: Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus > 255) || (devfn > 255) || (reg > 255)) return -EINVAL; Here is a pure ppc64 implementation that does this. Actually, it doesn't: +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn->phb->arch_data; + + if (where < 256 || hose_dn->pci_ext_config_space) + return 1; This needs a check for (where < 4096) in case of PCIe or PCI-X. Done. @@ -62,6 +72,8 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn->busno << 16) | (dn->devfn << 8) | where; addr is still wrong, see my previous mail. Fixed. @@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de phb->arch_data = dev; phb->ops = _pci_ops; phb->buid = get_phb_buid(dev); + get_phb_config_space_type(dev); return 0; } Isn't the config space size a property of the PCI device instead of the host bridge? For a PCI device behind a PCIe host bridge, this could still lead to an incorrect config space accesses. It is a property of both. Accessing config space beyond the first 256 bytes will only work if both the PCI device and the host bridge support it. The problem I ran into was generic pci code issuing a config read to offset 256 after checking that the device supports it when the host bridge did not support it. PS: I got a permanent fatal error from <[EMAIL PROTECTED]>, does that list actually exist? Sorry about that... Should be fixed on this thread now. I checked the archives and saw a thread related to adding another L: line to the MAINTAINERS file for the linux-pci list. Greg - was some flavor of that patch going in? -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. Signed-off-by: Brian King <[EMAIL PROTECTED]> --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 33 ++- linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h|1 2 files changed, 32 insertions(+), 2 deletions(-) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-01-31 16:17:08.0 -0600 @@ -52,6 +52,18 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn->phb->arch_data; + + if (where < 256) + return 1; + if (where < 4096 && hose_dn->pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -62,8 +74,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | + (dn->devfn << 8) | (where & 0xff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, , @@ -110,8 +125,11 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) | + (dn->devfn << 8) | (where & 0xff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid >> 32, buid & 0x, size, (ulong) val); @@ -270,6 +288,16 @@ static int phb_set_bus_ranges(struct dev return 0; }
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 10:56:44PM +0100, Arnd Bergmann wrote: > PS: I got a permanent fatal error from <[EMAIL PROTECTED]>, does > that list actually exist? No, that is not the email address for the linux-pci mailing list. I don't know who put that in this thread, but next time, someone might want to actually look the address up before blindly guessing... thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci: Arch hook to determine config space size
On Maandag 31 Januar 2005 22:35, Brian King wrote: > Matthew Wilcox wrote: > > Basically, ppc64's config ops are broken and need to check the offset > > being read. Here's i386: > > > > static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, > > u32 v > > alue) > > { > > unsigned long flags; > > > > if ((bus > 255) || (devfn > 255) || (reg > 255)) > > return -EINVAL; > > Here is a pure ppc64 implementation that does this. Actually, it doesn't: > +static int config_access_valid(struct device_node *dn, int where) > +{ > + struct device_node *hose_dn = dn->phb->arch_data; > + > + if (where < 256 || hose_dn->pci_ext_config_space) > + return 1; This needs a check for (where < 4096) in case of PCIe or PCI-X. > @@ -62,6 +72,8 @@ static int rtas_read_config(struct devic > return PCIBIOS_DEVICE_NOT_FOUND; > if (where & (size - 1)) > return PCIBIOS_BAD_REGISTER_NUMBER; > + if (!config_access_valid(dn, where)) > + return PCIBIOS_BAD_REGISTER_NUMBER; > > addr = (dn->busno << 16) | (dn->devfn << 8) | where; addr is still wrong, see my previous mail. > @@ -110,6 +122,8 @@ static int rtas_write_config(struct devi > return PCIBIOS_DEVICE_NOT_FOUND; > if (where & (size - 1)) > return PCIBIOS_BAD_REGISTER_NUMBER; > + if (!config_access_valid(dn, where)) > + return PCIBIOS_BAD_REGISTER_NUMBER; > > addr = (dn->busno << 16) | (dn->devfn << 8) | where; same here > @@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de > phb->arch_data = dev; > phb->ops = _pci_ops; > phb->buid = get_phb_buid(dev); > + get_phb_config_space_type(dev); > > return 0; > } Isn't the config space size a property of the PCI device instead of the host bridge? For a PCI device behind a PCIe host bridge, this could still lead to an incorrect config space accesses. Arnd <>< PS: I got a permanent fatal error from <[EMAIL PROTECTED]>, does that list actually exist? pgpiwTKnAFNtl.pgp Description: signature
Re: pci: Arch hook to determine config space size
Matthew Wilcox wrote: Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus > 255) || (devfn > 255) || (reg > 255)) return -EINVAL; Here is a pure ppc64 implementation that does this. I think all the config ops in ppc64 are broken and need to check for these limits. Also, it does some checks that are already performed by upper layers: if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; is checked for in drivers/pci/access.c I can submit a separate patch to clean that up. -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. Signed-off-by: Brian King <[EMAIL PROTECTED]> --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 25 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h|1 2 files changed, 26 insertions(+) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-01-31 15:09:53.0 -0600 @@ -52,6 +52,16 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn->phb->arch_data; + + if (where < 256 || hose_dn->pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -62,6 +72,8 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn->busno << 16) | (dn->devfn << 8) | where; buid = dn->phb->buid; @@ -110,6 +122,8 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn->busno << 16) | (dn->devfn << 8) | where; buid = dn->phb->buid; @@ -270,6 +284,16 @@ static int phb_set_bus_ranges(struct dev return 0; } +static void __devinit get_phb_config_space_type(struct device_node *dn) +{ + int *type = (int *)get_property(dn, "ibm,pci-config-space-type", NULL); + + if (type && *type == 1) + dn->pci_ext_config_space = 1; + else + dn->pci_ext_config_space = 0; +} + static int __devinit setup_phb(struct device_node *dev, struct pci_controller *phb, unsigned int addr_size_words) @@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de phb->arch_data = dev; phb->ops = _pci_ops; phb->buid = get_phb_buid(dev); + get_phb_config_space_type(dev); return 0; } diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-01-31 14:32:01.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for phb's or bridges */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ _
Re: pci: Arch hook to determine config space size
On Maandag 31 Januar 2005 20:29, Matthew Wilcox wrote: > Thanks for copying linux-pci. I hate this patch. > > Basically, ppc64's config ops are broken and need to check the offset > being read. To make things worse, simply allowing the larger config space will silently access the wrong device. The least that needs to be done is to pass the correct address to the firmware. This patch should do the right thing, though I don't have any PCIe card to test with. Note that at least for the rtas pci config access, the bus/devfn values come from the device tree, which makes it somewhat harder to screw them up, and rtas ought to check for obviously wrong addresses as well. Signed-off-by: Arnd Bergmann <[EMAIL PROTECTED]> --- linux-mm.orig/arch/ppc64/kernel/pSeries_pci.c 2005-01-28 07:21:15.0 -0500 +++ linux-mm/arch/ppc64/kernel/pSeries_pci.c2005-01-31 15:56:10.244983464 -0500 @@ -63,7 +63,8 @@ if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) + | (dn->devfn << 8) | (where & 0x0ff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, , @@ -111,7 +112,8 @@ if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn->busno << 16) | (dn->devfn << 8) | where; + addr = ((where & 0xf00) << 20) | (dn->busno << 16) + | (dn->devfn << 8) | (where & 0x0ff); buid = dn->phb->buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid >> 32, buid & 0x, size, (ulong) val); pgp9VhwvLAPji.pgp Description: signature
Re: pci: Arch hook to determine config space size
Brian King wrote: Greg KH wrote: On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. CC'ing the linux-pci mailing list... -brian How about this? When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. Signed-off-by: Brian King <[EMAIL PROTECTED]> --- linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ia64/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/m68knommu/kernel/comempci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pmc-sierra/yosemite/ht.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/parisc/kernel/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/ppc/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/iSeries_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci.c| 18 ++ linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/mpc1211/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/overdrive/galileo.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh/drivers/pci/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh64/kernel/pcibios.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc/kernel/pcic.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc64/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/v850/kernel/rte_mb_a_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c|2 + linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h|1 21 files changed, 55 insertions(+) diff -puN drivers/pci/probe.c~pci_get_cfg_size_all drivers/pci/probe.c --- linux-2.6.11-rc2-bk9/drivers/pci/probe.c~pci_get_cfg_size_all 2005-01-31 11:16:22.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c 2005-01-31 11:22:07.0 -0600 @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, ) != PCIBIOS_SUCCESSFUL) goto fail; if (status == 0x) diff -puN arch/alpha/kernel/pci.c~pci_get_cfg_size_all arch/alpha/kernel/pci.c --- linux-2.6.11-rc2-bk9/arch/alpha/kernel/pci.c~pci_get_cfg_size_all 2005-01-31 11:16:33.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c 2005-01-31 11:22:27.0 -0600 @@ -202,6 +202,8 @@ pcibios_setup(char *str) return str; } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + #ifdef ALPHA_RESTORE_SRM_SETUP static struct pdev_srm_saved_conf *srm_saved_configs; diff -puN arch/arm/kernel/bios32.c~pci_get_cfg_size_all arch/arm/kernel/bios32.c --- linux-2.6.11-rc2-bk9/arch/arm/kernel/bios32.c~pci_get_cfg_size_all 2005-01-31 11:16:43.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c 2005-01-31 11:22:27.0 -0600 @@ -67,6 +67,8 @@ void pcibios_report_status(u_int status_ } } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + /* * We don't use this to fix the device, but initialisation of it. * It's not the correct use for this, but it works. diff -puN arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all arch/frv/mb93090-mb00/pci-frv.c --- linux-2.6.11-rc2-bk9/arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all 2005-01-31 11:16:55.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c 2005-01-31 11:22:27.0 -0600 @@ -286,3 +286,5 @@ void pcibios_set_master(struct pci_dev * printk(KERN_DEBUG "PCI: Setting latency timer of device %s to %d\n", pci_name(dev), lat); pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat); } + +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } diff -puN arch/i386/pci/common.c~pci_get_cfg_size_all arch/i386/pci/common.c --- linux-2.6.11-rc2-bk9/arch/i386/pci/common.c~pci_get_cfg_size_all 2005-01-31 11:17:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c 2005-01-31 11:22:27.0 -0600 @@ -249,3 +249,5 @@ int pcibios_enable_device(struct
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 01:10:46PM -0600, Brian King wrote: > Greg KH wrote: > >On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: > > > >>>+int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { > >>>return 1; } > >> > >>- prototypes belong to headers > >>- weak linkage is the perfect way for total obsfucation > >> > >>please make this a regular arch hook > > > > > >I agree. Also, when sending PCI related patches, please cc the > >linux-pci mailing list. > > How about this? Thanks for copying linux-pci. I hate this patch. Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus > 255) || (devfn > 255) || (reg > 255)) return -EINVAL; I think all the config ops in ppc64 are broken and need to check for these limits. Also, it does some checks that are already performed by upper layers: if (where & (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; is checked for in drivers/pci/access.c -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 01:10:46PM -0600, Brian King wrote: > +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } > + Kernel functions traditionally return 0 for success and -ESOMETHING for error. Care to fix this up to match that convention? thanks, greg k-h
Re: pci: Arch hook to determine config space size
Greg KH wrote: On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. How about this? -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. Signed-off-by: Brian King <[EMAIL PROTECTED]> --- linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ia64/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/m68knommu/kernel/comempci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pmc-sierra/yosemite/ht.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/parisc/kernel/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/ppc/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/iSeries_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci.c| 18 ++ linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/mpc1211/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/overdrive/galileo.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh/drivers/pci/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh64/kernel/pcibios.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc/kernel/pcic.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc64/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/v850/kernel/rte_mb_a_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c|2 + linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h|1 21 files changed, 55 insertions(+) diff -puN drivers/pci/probe.c~pci_get_cfg_size_all drivers/pci/probe.c --- linux-2.6.11-rc2-bk9/drivers/pci/probe.c~pci_get_cfg_size_all 2005-01-31 11:16:22.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c2005-01-31 11:22:07.0 -0600 @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, ) != PCIBIOS_SUCCESSFUL) goto fail; if (status == 0x) diff -puN arch/alpha/kernel/pci.c~pci_get_cfg_size_all arch/alpha/kernel/pci.c --- linux-2.6.11-rc2-bk9/arch/alpha/kernel/pci.c~pci_get_cfg_size_all 2005-01-31 11:16:33.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c2005-01-31 11:22:27.0 -0600 @@ -202,6 +202,8 @@ pcibios_setup(char *str) return str; } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + #ifdef ALPHA_RESTORE_SRM_SETUP static struct pdev_srm_saved_conf *srm_saved_configs; diff -puN arch/arm/kernel/bios32.c~pci_get_cfg_size_all arch/arm/kernel/bios32.c --- linux-2.6.11-rc2-bk9/arch/arm/kernel/bios32.c~pci_get_cfg_size_all 2005-01-31 11:16:43.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c 2005-01-31 11:22:27.0 -0600 @@ -67,6 +67,8 @@ void pcibios_report_status(u_int status_ } } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + /* * We don't use this to fix the device, but initialisation of it. * It's not the correct use for this, but it works. diff -puN arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all arch/frv/mb93090-mb00/pci-frv.c --- linux-2.6.11-rc2-bk9/arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all 2005-01-31 11:16:55.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c 2005-01-31 11:22:27.0 -0600 @@ -286,3 +286,5 @@ void pcibios_set_master(struct pci_dev * printk(KERN_DEBUG "PCI: Setting latency timer of device %s to %d\n", pci_name(dev), lat); pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat); } + +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } diff -puN arch/i386/pci/common.c~pci_get_cfg_size_all arch/i386/pci/common.c --- linux-2.6.11-rc2-bk9/arch/i386/pci/common.c~pci_get_cfg_size_all 2005-01-31 11:17:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c 2005-01-31
Re: pci: Arch hook to determine config space size
Greg KH wrote: On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. How about this? -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ia64/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/m68knommu/kernel/comempci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pmc-sierra/yosemite/ht.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/parisc/kernel/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/ppc/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/iSeries_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci.c| 18 ++ linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/mpc1211/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/overdrive/galileo.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh/drivers/pci/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh64/kernel/pcibios.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc/kernel/pcic.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc64/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/v850/kernel/rte_mb_a_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c|2 + linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h|1 21 files changed, 55 insertions(+) diff -puN drivers/pci/probe.c~pci_get_cfg_size_all drivers/pci/probe.c --- linux-2.6.11-rc2-bk9/drivers/pci/probe.c~pci_get_cfg_size_all 2005-01-31 11:16:22.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c2005-01-31 11:22:07.0 -0600 @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, status) != PCIBIOS_SUCCESSFUL) goto fail; if (status == 0x) diff -puN arch/alpha/kernel/pci.c~pci_get_cfg_size_all arch/alpha/kernel/pci.c --- linux-2.6.11-rc2-bk9/arch/alpha/kernel/pci.c~pci_get_cfg_size_all 2005-01-31 11:16:33.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c2005-01-31 11:22:27.0 -0600 @@ -202,6 +202,8 @@ pcibios_setup(char *str) return str; } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + #ifdef ALPHA_RESTORE_SRM_SETUP static struct pdev_srm_saved_conf *srm_saved_configs; diff -puN arch/arm/kernel/bios32.c~pci_get_cfg_size_all arch/arm/kernel/bios32.c --- linux-2.6.11-rc2-bk9/arch/arm/kernel/bios32.c~pci_get_cfg_size_all 2005-01-31 11:16:43.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c 2005-01-31 11:22:27.0 -0600 @@ -67,6 +67,8 @@ void pcibios_report_status(u_int status_ } } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + /* * We don't use this to fix the device, but initialisation of it. * It's not the correct use for this, but it works. diff -puN arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all arch/frv/mb93090-mb00/pci-frv.c --- linux-2.6.11-rc2-bk9/arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all 2005-01-31 11:16:55.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c 2005-01-31 11:22:27.0 -0600 @@ -286,3 +286,5 @@ void pcibios_set_master(struct pci_dev * printk(KERN_DEBUG PCI: Setting latency timer of device %s to %d\n, pci_name(dev), lat); pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat); } + +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } diff -puN arch/i386/pci/common.c~pci_get_cfg_size_all arch/i386/pci/common.c --- linux-2.6.11-rc2-bk9/arch/i386/pci/common.c~pci_get_cfg_size_all 2005-01-31 11:17:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c 2005-01-31
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 01:10:46PM -0600, Brian King wrote: +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + Kernel functions traditionally return 0 for success and -ESOMETHING for error. Care to fix this up to match that convention? thanks, greg k-h
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 01:10:46PM -0600, Brian King wrote: Greg KH wrote: On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. How about this? Thanks for copying linux-pci. I hate this patch. Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus 255) || (devfn 255) || (reg 255)) return -EINVAL; I think all the config ops in ppc64 are broken and need to check for these limits. Also, it does some checks that are already performed by upper layers: if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; is checked for in drivers/pci/access.c -- Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception. -- Mark Twain
Re: pci: Arch hook to determine config space size
Brian King wrote: Greg KH wrote: On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. CC'ing the linux-pci mailing list... -brian How about this? When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ia64/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/m68knommu/kernel/comempci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pci/pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/mips/pmc-sierra/yosemite/ht.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/parisc/kernel/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/ppc/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/iSeries_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pci.c| 18 ++ linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/mpc1211/pci.c |1 linux-2.6.11-rc2-bk9-bjking1/arch/sh/boards/overdrive/galileo.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh/drivers/pci/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sh64/kernel/pcibios.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc/kernel/pcic.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/sparc64/kernel/pci.c |2 + linux-2.6.11-rc2-bk9-bjking1/arch/v850/kernel/rte_mb_a_pci.c|2 + linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c|2 + linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h|1 21 files changed, 55 insertions(+) diff -puN drivers/pci/probe.c~pci_get_cfg_size_all drivers/pci/probe.c --- linux-2.6.11-rc2-bk9/drivers/pci/probe.c~pci_get_cfg_size_all 2005-01-31 11:16:22.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/probe.c 2005-01-31 11:22:07.0 -0600 @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, status) != PCIBIOS_SUCCESSFUL) goto fail; if (status == 0x) diff -puN arch/alpha/kernel/pci.c~pci_get_cfg_size_all arch/alpha/kernel/pci.c --- linux-2.6.11-rc2-bk9/arch/alpha/kernel/pci.c~pci_get_cfg_size_all 2005-01-31 11:16:33.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/alpha/kernel/pci.c 2005-01-31 11:22:27.0 -0600 @@ -202,6 +202,8 @@ pcibios_setup(char *str) return str; } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + #ifdef ALPHA_RESTORE_SRM_SETUP static struct pdev_srm_saved_conf *srm_saved_configs; diff -puN arch/arm/kernel/bios32.c~pci_get_cfg_size_all arch/arm/kernel/bios32.c --- linux-2.6.11-rc2-bk9/arch/arm/kernel/bios32.c~pci_get_cfg_size_all 2005-01-31 11:16:43.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/arm/kernel/bios32.c 2005-01-31 11:22:27.0 -0600 @@ -67,6 +67,8 @@ void pcibios_report_status(u_int status_ } } +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + /* * We don't use this to fix the device, but initialisation of it. * It's not the correct use for this, but it works. diff -puN arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all arch/frv/mb93090-mb00/pci-frv.c --- linux-2.6.11-rc2-bk9/arch/frv/mb93090-mb00/pci-frv.c~pci_get_cfg_size_all 2005-01-31 11:16:55.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/frv/mb93090-mb00/pci-frv.c 2005-01-31 11:22:27.0 -0600 @@ -286,3 +286,5 @@ void pcibios_set_master(struct pci_dev * printk(KERN_DEBUG PCI: Setting latency timer of device %s to %d\n, pci_name(dev), lat); pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat); } + +int pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } diff -puN arch/i386/pci/common.c~pci_get_cfg_size_all arch/i386/pci/common.c --- linux-2.6.11-rc2-bk9/arch/i386/pci/common.c~pci_get_cfg_size_all 2005-01-31 11:17:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/i386/pci/common.c 2005-01-31 11:22:27.0 -0600 @@ -249,3 +249,5 @@ int pcibios_enable_device(struct
Re: pci: Arch hook to determine config space size
On Maandag 31 Januar 2005 20:29, Matthew Wilcox wrote: Thanks for copying linux-pci. I hate this patch. Basically, ppc64's config ops are broken and need to check the offset being read. To make things worse, simply allowing the larger config space will silently access the wrong device. The least that needs to be done is to pass the correct address to the firmware. This patch should do the right thing, though I don't have any PCIe card to test with. Note that at least for the rtas pci config access, the bus/devfn values come from the device tree, which makes it somewhat harder to screw them up, and rtas ought to check for obviously wrong addresses as well. Signed-off-by: Arnd Bergmann [EMAIL PROTECTED] --- linux-mm.orig/arch/ppc64/kernel/pSeries_pci.c 2005-01-28 07:21:15.0 -0500 +++ linux-mm/arch/ppc64/kernel/pSeries_pci.c2005-01-31 15:56:10.244983464 -0500 @@ -63,7 +63,8 @@ if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) + | (dn-devfn 8) | (where 0x0ff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, returnval, @@ -111,7 +112,8 @@ if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) + | (dn-devfn 8) | (where 0x0ff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid 32, buid 0x, size, (ulong) val); pgp9VhwvLAPji.pgp Description: signature
Re: pci: Arch hook to determine config space size
Matthew Wilcox wrote: Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus 255) || (devfn 255) || (reg 255)) return -EINVAL; Here is a pure ppc64 implementation that does this. I think all the config ops in ppc64 are broken and need to check for these limits. Also, it does some checks that are already performed by upper layers: if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; is checked for in drivers/pci/access.c I can submit a separate patch to clean that up. -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 25 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h|1 2 files changed, 26 insertions(+) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-01-31 15:09:53.0 -0600 @@ -52,6 +52,16 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn-phb-arch_data; + + if (where 256 || hose_dn-pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -62,6 +72,8 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn-busno 16) | (dn-devfn 8) | where; buid = dn-phb-buid; @@ -110,6 +122,8 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn-busno 16) | (dn-devfn 8) | where; buid = dn-phb-buid; @@ -270,6 +284,16 @@ static int phb_set_bus_ranges(struct dev return 0; } +static void __devinit get_phb_config_space_type(struct device_node *dn) +{ + int *type = (int *)get_property(dn, ibm,pci-config-space-type, NULL); + + if (type *type == 1) + dn-pci_ext_config_space = 1; + else + dn-pci_ext_config_space = 0; +} + static int __devinit setup_phb(struct device_node *dev, struct pci_controller *phb, unsigned int addr_size_words) @@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de phb-arch_data = dev; phb-ops = rtas_pci_ops; phb-buid = get_phb_buid(dev); + get_phb_config_space_type(dev); return 0; } diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-01-31 14:32:01.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for phb's or bridges */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ _
Re: pci: Arch hook to determine config space size
On Maandag 31 Januar 2005 22:35, Brian King wrote: Matthew Wilcox wrote: Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus 255) || (devfn 255) || (reg 255)) return -EINVAL; Here is a pure ppc64 implementation that does this. Actually, it doesn't: +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn-phb-arch_data; + + if (where 256 || hose_dn-pci_ext_config_space) + return 1; This needs a check for (where 4096) in case of PCIe or PCI-X. @@ -62,6 +72,8 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn-busno 16) | (dn-devfn 8) | where; addr is still wrong, see my previous mail. @@ -110,6 +122,8 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn-busno 16) | (dn-devfn 8) | where; same here @@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de phb-arch_data = dev; phb-ops = rtas_pci_ops; phb-buid = get_phb_buid(dev); + get_phb_config_space_type(dev); return 0; } Isn't the config space size a property of the PCI device instead of the host bridge? For a PCI device behind a PCIe host bridge, this could still lead to an incorrect config space accesses. Arnd PS: I got a permanent fatal error from [EMAIL PROTECTED], does that list actually exist? pgpiwTKnAFNtl.pgp Description: signature
Re: pci: Arch hook to determine config space size
Arnd Bergmann wrote: On Maandag 31 Januar 2005 22:35, Brian King wrote: Matthew Wilcox wrote: Basically, ppc64's config ops are broken and need to check the offset being read. Here's i386: static int pci_conf1_write (int seg, int bus, int devfn, int reg, int len, u32 v alue) { unsigned long flags; if ((bus 255) || (devfn 255) || (reg 255)) return -EINVAL; Here is a pure ppc64 implementation that does this. Actually, it doesn't: +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn-phb-arch_data; + + if (where 256 || hose_dn-pci_ext_config_space) + return 1; This needs a check for (where 4096) in case of PCIe or PCI-X. Done. @@ -62,6 +72,8 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; addr = (dn-busno 16) | (dn-devfn 8) | where; addr is still wrong, see my previous mail. Fixed. @@ -285,6 +309,7 @@ static int __devinit setup_phb(struct de phb-arch_data = dev; phb-ops = rtas_pci_ops; phb-buid = get_phb_buid(dev); + get_phb_config_space_type(dev); return 0; } Isn't the config space size a property of the PCI device instead of the host bridge? For a PCI device behind a PCIe host bridge, this could still lead to an incorrect config space accesses. It is a property of both. Accessing config space beyond the first 256 bytes will only work if both the PCI device and the host bridge support it. The problem I ran into was generic pci code issuing a config read to offset 256 after checking that the device supports it when the host bridge did not support it. PS: I got a permanent fatal error from [EMAIL PROTECTED], does that list actually exist? Sorry about that... Should be fixed on this thread now. I checked the archives and saw a thread related to adding another L: line to the MAINTAINERS file for the linux-pci list. Greg - was some flavor of that patch going in? -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 33 ++- linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h|1 2 files changed, 32 insertions(+), 2 deletions(-) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-01-31 16:17:08.0 -0600 @@ -52,6 +52,18 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + struct device_node *hose_dn = dn-phb-arch_data; + + if (where 256) + return 1; + if (where 4096 hose_dn-pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -62,8 +74,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, returnval, @@ -110,8 +125,11 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid 32, buid 0x, size, (ulong) val); @@ -270,6 +288,16 @@ static int phb_set_bus_ranges(struct dev return 0; } +static void __devinit get_phb_config_space_type(struct
Re: pci: Arch hook to determine config space size
Brian King [EMAIL PROTECTED] schrieb am 31.01.2005, 23:43:30: Isn't the config space size a property of the PCI device instead of the host bridge? For a PCI device behind a PCIe host bridge, this could still lead to an incorrect config space accesses. It is a property of both. Accessing config space beyond the first 256 bytes will only work if both the PCI device and the host bridge support it. The problem I ran into was generic pci code issuing a config read to offset 256 after checking that the device supports it when the host bridge did not support it. If I interpret the spec correctly, the firmware should always store the value we need in the property for every device node, which means that you should look at the host bridge config-space-type attribute only when you want to look at the bridge itself. If the device claims to support a PCIe config space and the bridge doesn't, that sounds to me like a firmware bug. Arnd
Re: pci: Arch hook to determine config space size
Benjamin Herrenschmidt wrote: On Mon, 2005-01-31 at 16:43 -0600, Brian King wrote: diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-01-31 14:32:01.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for phb's or bridges */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ Grrr... more crap added to the device-node, I don't like that ... This is a PHB only field, can't it be in struct pci_controller instead ? Assuming I am reading the spec correctly, this is only a property of the PHB, so I could move it into the pci_controller struct instead. -- Brian King eServer Storage I/O IBM Linux Technology Center When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch checks OF to see if the PCI bridge supports PCI-X Mode 2 and fails config accesses beyond 256 bytes if it does not. --- linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c | 31 ++- linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/pci-bridge.h |1 2 files changed, 30 insertions(+), 2 deletions(-) diff -puN arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg arch/ppc64/kernel/pSeries_pci.c --- linux-2.6.11-rc2-bk9/arch/ppc64/kernel/pSeries_pci.c~ppc64_pcix_mode2_cfg 2005-01-31 22:27:49.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/arch/ppc64/kernel/pSeries_pci.c 2005-01-31 22:31:04.0 -0600 @@ -52,6 +52,16 @@ static int s7a_workaround; extern struct mpic *pSeries_mpic; +static int config_access_valid(struct device_node *dn, int where) +{ + if (where 256) + return 1; + if (where 4096 dn-phb-pci_ext_config_space) + return 1; + + return 0; +} + static int rtas_read_config(struct device_node *dn, int where, int size, u32 *val) { int returnval = -1; @@ -62,8 +72,11 @@ static int rtas_read_config(struct devic return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_read_pci_config, 4, 2, returnval, @@ -110,8 +123,11 @@ static int rtas_write_config(struct devi return PCIBIOS_DEVICE_NOT_FOUND; if (where (size - 1)) return PCIBIOS_BAD_REGISTER_NUMBER; + if (!config_access_valid(dn, where)) + return PCIBIOS_BAD_REGISTER_NUMBER; - addr = (dn-busno 16) | (dn-devfn 8) | where; + addr = ((where 0xf00) 20) | (dn-busno 16) | + (dn-devfn 8) | (where 0xff); buid = dn-phb-buid; if (buid) { ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr, buid 32, buid 0x, size, (ulong) val); @@ -270,6 +286,16 @@ static int phb_set_bus_ranges(struct dev return 0; } +static char __devinit get_phb_config_space_type(struct pci_controller *phb) +{ + int *type = (int *)get_property(phb-arch_data, + ibm,pci-config-space-type, NULL); + + if (type *type == 1) + return 1; + return 0; +} + static int __devinit setup_phb(struct device_node *dev, struct pci_controller *phb, unsigned int addr_size_words) @@ -285,6 +311,7 @@ static int __devinit setup_phb(struct de phb-arch_data = dev; phb-ops = rtas_pci_ops; phb-buid = get_phb_buid(dev); + phb-pci_ext_config_space = get_phb_config_space_type(phb); return 0; } diff -puN include/asm-ppc64/pci-bridge.h~ppc64_pcix_mode2_cfg include/asm-ppc64/pci-bridge.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/pci-bridge.h~ppc64_pcix_mode2_cfg 2005-01-31 22:27:49.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/pci-bridge.h 2005-01-31 22:27:49.0 -0600 @@ -17,6 +17,7 @@ struct pci_controller { struct pci_bus *bus; char is_dynamic; + char pci_ext_config_space; void *arch_data; struct list_head list_node; _
Re: pci: Arch hook to determine config space size
On Mon, 2005-01-31 at 22:52 -0600, Brian King wrote: Assuming I am reading the spec correctly, this is only a property of the PHB, so I could move it into the pci_controller struct instead. Note that Arnd seems to imply the opposite ... BTW. I'm thinking about moving all those PCI/VIO related fields out of struct device_node to their own structure and keep only a pointer to that structure in device_node. That way, we avoid the bloat for every single non-pci node in the system, and we can have different structures for different bus types (along with proper iommu function pointers and that sort-of-thing). So if you think you really need a per-device info here, feel free to add it to device_node for now, and I'll move it to the new structure along with the rest of the stuff once I find time to do this patch. Ben.
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 01:40:04PM -0600, Brian King wrote: CC'ing the linux-pci mailing list... thanks... This patch adds an arch hook so that individual archs can indicate if the underlying system supports expanded config space accesses or not. @@ -653,6 +653,8 @@ static int pci_cfg_space_size(struct pci goto fail; } +if (!pcibios_exp_cfg_space(dev)) +goto fail; if (pci_read_config_dword(dev, 256, status) != PCIBIOS_SUCCESSFUL) goto fail; pci_read_config_dword lands in arch specific code. See drivers/pci/access.c:PCI_OP_READ() macro. I'm missing what pcibios_exp_cfg_space() does that can't be handled by the bus_ops supplied by pci_scan_bus(). I would expect the pci_read_config_dword to fail for being out of bounds. Is that wrong? Or is bus_ops not feasible in this case because pcibios needs access to pci_dev? If it's feasible, maybe the right place to add this hook is to pci_read_config_dword which is also handed the pci_dev. And add another function pointer to bus_ops (which could be NULL) to check chipset support for Expanded Config space before calling pci_bus_read_config_dword. Thats cleaner than adding a hook before each use of pci_read_config_dword. hth, grant
Re: pci: Arch hook to determine config space size
On Mon, Jan 31, 2005 at 10:56:44PM +0100, Arnd Bergmann wrote: PS: I got a permanent fatal error from [EMAIL PROTECTED], does that list actually exist? No, that is not the email address for the linux-pci mailing list. I don't know who put that in this thread, but next time, someone might want to actually look the address up before blindly guessing... thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: pci: Arch hook to determine config space size
On Mon, 2005-01-31 at 16:43 -0600, Brian King wrote: diff -puN include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg include/asm-ppc64/prom.h --- linux-2.6.11-rc2-bk9/include/asm-ppc64/prom.h~ppc64_pcix_mode2_cfg 2005-01-31 14:32:01.0 -0600 +++ linux-2.6.11-rc2-bk9-bjking1/include/asm-ppc64/prom.h 2005-01-31 14:32:01.0 -0600 @@ -137,6 +137,7 @@ struct device_node { int devfn; /* for pci devices */ int eeh_mode; /* See eeh.h for possible EEH_MODEs */ int eeh_config_addr; + int pci_ext_config_space; /* for phb's or bridges */ struct pci_controller *phb;/* for pci devices */ struct iommu_table *iommu_table; /* for phb's or bridges */ Grrr... more crap added to the device-node, I don't like that ... This is a PHB only field, can't it be in struct pci_controller instead ? Ben.
Re: [PATCH 1/2] pci: Arch hook to determine config space size
On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: > > +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { > > return 1; } > > - prototypes belong to headers > - weak linkage is the perfect way for total obsfucation > > please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pci: Arch hook to determine config space size
> +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { > return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook > Please read the FAQ at http://www.tux.org/lkml/ ---end quoted text--- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] pci: Arch hook to determine config space size
When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch adds the ability for arch specific code to override part of the config space size determination to fix this. Signed-off-by: Brian King <[EMAIL PROTECTED]> --- linux-2.6.11-rc2-bk5-bjking1/drivers/pci/probe.c |4 1 files changed, 4 insertions(+) diff -puN drivers/pci/probe.c~pci_arch_cfg_space_size drivers/pci/probe.c --- linux-2.6.11-rc2-bk5/drivers/pci/probe.c~pci_arch_cfg_space_size 2005-01-27 16:56:46.0 -0600 +++ linux-2.6.11-rc2-bk5-bjking1/drivers/pci/probe.c2005-01-27 16:56:46.0 -0600 @@ -627,6 +627,8 @@ static void pci_release_dev(struct devic kfree(pci_dev); } +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + /** * pci_cfg_space_size - get the configuration space size of the PCI device. * @@ -653,6 +655,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, ) != PCIBIOS_SUCCESSFUL) goto fail; if (status == 0x) _ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] pci: Arch hook to determine config space size
When working with a PCI-X Mode 2 adapter on a PCI-X Mode 1 PPC64 system, the current code used to determine the config space size of a device results in a PCI Master abort and an EEH error, resulting in the device being taken offline. This patch adds the ability for arch specific code to override part of the config space size determination to fix this. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6.11-rc2-bk5-bjking1/drivers/pci/probe.c |4 1 files changed, 4 insertions(+) diff -puN drivers/pci/probe.c~pci_arch_cfg_space_size drivers/pci/probe.c --- linux-2.6.11-rc2-bk5/drivers/pci/probe.c~pci_arch_cfg_space_size 2005-01-27 16:56:46.0 -0600 +++ linux-2.6.11-rc2-bk5-bjking1/drivers/pci/probe.c2005-01-27 16:56:46.0 -0600 @@ -627,6 +627,8 @@ static void pci_release_dev(struct devic kfree(pci_dev); } +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } + /** * pci_cfg_space_size - get the configuration space size of the PCI device. * @@ -653,6 +655,8 @@ static int pci_cfg_space_size(struct pci goto fail; } + if (!pcibios_exp_cfg_space(dev)) + goto fail; if (pci_read_config_dword(dev, 256, status) != PCIBIOS_SUCCESSFUL) goto fail; if (status == 0x) _ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pci: Arch hook to determine config space size
+int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook Please read the FAQ at http://www.tux.org/lkml/ ---end quoted text--- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pci: Arch hook to determine config space size
On Fri, Jan 28, 2005 at 06:52:34PM +, Christoph Hellwig wrote: +int __attribute__ ((weak)) pcibios_exp_cfg_space(struct pci_dev *dev) { return 1; } - prototypes belong to headers - weak linkage is the perfect way for total obsfucation please make this a regular arch hook I agree. Also, when sending PCI related patches, please cc the linux-pci mailing list. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/