Subject: Re: [discuss] Re: Please pull x86-64 bug fixes - msg#00025

Previous Message by Date: click to view message preview

Re: [discuss] Re: Please pull x86-64 bug fixes

On Thu, 5 Oct 2006, Jeff Garzik wrote: > Andi Kleen wrote: > > If the choice is between a secret NDA only card with dubious > > functionality and booting on lots of modern boards I know what to choose. > > That's a strawman argument. There is no need to choose. You can clearly boot > on lots of modern boards with mmconfig just fine. We just need to narrow down > which ones. Jeff, _that_ is the strawman argument. The thing is, nobody has been able to so far come up with a way to narrow down which ones. I think Andi's response was quite on the mark: if you have a patch to narrow it down, please share. Until then, the fact is, we don't know _how_, and you're barking up the wrong tree. It is entirely possible that the only reasonable solution is to potentially _never_ use MMIO as the main config mechanism (where "never" is "in the next few years" - at some point we may be able to just say "screw it, we don't care any more"), and that the drivers that want to use MMIO for config accesses will have to use special operations for that. In other words, right now we have int pci_read_config_byte(struct pci_dev *dev, int where, u8 *val) and maybe we will simply have to add a totally new function like int pci_read_mmio_config_byte(struct pci_dev *dev, int where, u8 *val) for drivers that literally _require_ the mmio accesses for one reason or another. Alternatively, we could just have a "ioremap()" kind of thing, and use readl/writel on the end result, ie maybe we could do void __iomem *cfg = mmiocfg_remap(dev); if (!cfg) { printk("MMIO PCI configuration cycles not supported\n"); return -EIO; } /* Read the extended error register or some-such crud.. */ val = readl(cfg + 0x180); .. See? The thign is, at least right now the _advantages_ of MMIOCFG are basically zero for all regular hardware, and as such the disadvantages (in the form of machines that hang on device discovery) are way way _way_ bigger, and no way will do start using MMIO config cycles by default just because some unreleased hardware might want to. But using them explicitly in drivers - that's another thing. What do you think about this simple solution? Linus

Next Message by Date: click to view message preview

Re: [discuss] Re: Please pull x86-64 bug fixes

On Thu, 5 Oct 2006, Andi Kleen wrote: > > Ok. Please drop them then. Ok, done. Since they weren't the last commits in your branch, I had to do a certain amount of hand-waving: I merged up to the commit preceding the AC flag changes, and then I cherry-picked the one later commit separately. So it's not a normal merge, since I wanted to avoid those two commits. I've pushed out the result, you should check it out (and drop the top three commits on your head once you're ok with my result, since they won't exist in my tree). Linus

Previous Message by Thread: click to view message preview

Re: [discuss] Re: Please pull x86-64 bug fixes

On Thu, 5 Oct 2006, Jeff Garzik wrote: > Andi Kleen wrote: > > If the choice is between a secret NDA only card with dubious > > functionality and booting on lots of modern boards I know what to choose. > > That's a strawman argument. There is no need to choose. You can clearly boot > on lots of modern boards with mmconfig just fine. We just need to narrow down > which ones. Jeff, _that_ is the strawman argument. The thing is, nobody has been able to so far come up with a way to narrow down which ones. I think Andi's response was quite on the mark: if you have a patch to narrow it down, please share. Until then, the fact is, we don't know _how_, and you're barking up the wrong tree. It is entirely possible that the only reasonable solution is to potentially _never_ use MMIO as the main config mechanism (where "never" is "in the next few years" - at some point we may be able to just say "screw it, we don't care any more"), and that the drivers that want to use MMIO for config accesses will have to use special operations for that. In other words, right now we have int pci_read_config_byte(struct pci_dev *dev, int where, u8 *val) and maybe we will simply have to add a totally new function like int pci_read_mmio_config_byte(struct pci_dev *dev, int where, u8 *val) for drivers that literally _require_ the mmio accesses for one reason or another. Alternatively, we could just have a "ioremap()" kind of thing, and use readl/writel on the end result, ie maybe we could do void __iomem *cfg = mmiocfg_remap(dev); if (!cfg) { printk("MMIO PCI configuration cycles not supported\n"); return -EIO; } /* Read the extended error register or some-such crud.. */ val = readl(cfg + 0x180); .. See? The thign is, at least right now the _advantages_ of MMIOCFG are basically zero for all regular hardware, and as such the disadvantages (in the form of machines that hang on device discovery) are way way _way_ bigger, and no way will do start using MMIO config cycles by default just because some unreleased hardware might want to. But using them explicitly in drivers - that's another thing. What do you think about this simple solution? Linus

Next Message by Thread: click to view message preview

Re: [discuss] Re: Please pull x86-64 bug fixes

On Fri, 6 Oct 2006, Andi Kleen wrote: > > > In other words, right now we have > > > > int pci_read_config_byte(struct pci_dev *dev, int where, u8 *val) > > > > and maybe we will simply have to add a totally new function like > > > > int pci_read_mmio_config_byte(struct pci_dev *dev, int where, u8 *val) > > > > for drivers that literally _require_ the mmio accesses for one reason or > > another. > > That's easy to decide: if (where >= 256) mmconfig is required. > > I'm just afraid it probably won't help if the MCFG is totally broken and > points to some other devices (like on the Intel boards). Then these drivers > will > just hang and all of Alan's warning messages won't help with that. Sure. I'd actually prefer a separate interface partly for that reason, and partly also because I think the whole "pci_read_config_xxx()" interface has always been horrible. If we had the void __iomem *cfg = mmiocfg_remap(dev); interface, we could (fairly easily) blacklist known-bad motherboards if we needed to, and also, it would allow drivers to check whether mmiocfg is available. It's possible that some drivers might want it if it exists, but it wouldn't necessarily be somethign that they _require_, so they could gracefully handle the case of getting a NULL config space handle back. For example, for some devices, maybe they'd lose some error handling capability, but they'd still be able to work otherwise. We _can_ do the same thing with checking the error return value from "pci_read_config_xxxx()", and use the "use different access method if the index is >= 256", but I have to say, that just makes my gag reflex trigger. Having the same function just silently do two different things depending on the offset just sounds like a recipe for disaster. I dunno. I'm not likely to care _that_ deeply about this all, but I do think that machines that hang on device discovery is just about the worst possible thing, so I'd much rather have ten machines that can't use their very rare devices without some explicit kernel command line than have even _one_ machine that just hangs because MMIOCFG is buggered. (And we should probably have the "pci=mmiocfg" kernel command line entry that forces MMIOCFG regardless of any e820 issues, even for normal accesses). Linus