Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tuesday 29 January 2008 05:19:55 am Greg KH wrote: > Hahahaha, oh, that's a good one... > > But what about the thousands of implementations out there that are > buggy? > > I'm with Arjan here, I'm very skeptical. Ugg, let's look at the actual data (again); I'm really not sure why people are jumping to such dire conclusions about the current state of things. AIUI we only have 3 issues so far (remember mmconfig has been enabled in -mm for a long time): 1) host bridge decode problems (disabling decode to avoid overlaps can cause some bridges to stop decoding RAM addrs, but we have a fix for that) 2) config space retry on ATI (I think willy already debunked this one?) 3) some FUD about SMM or other firmware interrupts coming in during BAR sizing while decode is disabled (this one is just pure FUD; if we want to solve it properly we need a new platform hook to disable SMM/NMI/etc. around PCI probing) What else was there? What reason do we have to think that things are so disastrous? So I really prefer willy's approach to Arjan's alternative... Jesse -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Wed, Jan 30, 2008 at 07:42:49AM -0800, Arjan van de Ven wrote: > Xorg doesn't do pci express .. Xorg core provides a set of PCI config access functions (via sysfs) for the graphics drivers. These functions do work correctly with offsets > 256 bytes. Can you guarantee that none of PCI-E video drivers use that, including proprietary nvidia and ati ones? > (newer ones actually have gotten out of the "do the PCI layer ourselves" > business entirely) Unfortunately, not completely true. Though it has nothing to do with extended config space. Ivan. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 29, 2008 at 08:45:55PM -0700, Matthew Wilcox wrote: > On Tue, Jan 29, 2008 at 05:19:55AM -0800, Greg KH wrote: > > Matthew, with Arjan's patch, is anything that currently works now > > broken? Why do you feel it is somehow "wrong"? > > lspci is broken. It used to be able to access extended config space, and > now can't unless it is patched to know about the sysfs flag to enable it. There is also likely damage to Xorg for the very same reason. Ivan. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 29, 2008 at 08:45:55PM -0700, Matthew Wilcox wrote: On Tue, Jan 29, 2008 at 05:19:55AM -0800, Greg KH wrote: Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow wrong? lspci is broken. It used to be able to access extended config space, and now can't unless it is patched to know about the sysfs flag to enable it. There is also likely damage to Xorg for the very same reason. Ivan. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Wed, Jan 30, 2008 at 07:42:49AM -0800, Arjan van de Ven wrote: Xorg doesn't do pci express .. Xorg core provides a set of PCI config access functions (via sysfs) for the graphics drivers. These functions do work correctly with offsets 256 bytes. Can you guarantee that none of PCI-E video drivers use that, including proprietary nvidia and ati ones? (newer ones actually have gotten out of the do the PCI layer ourselves business entirely) Unfortunately, not completely true. Though it has nothing to do with extended config space. Ivan. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tuesday 29 January 2008 05:19:55 am Greg KH wrote: Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Ugg, let's look at the actual data (again); I'm really not sure why people are jumping to such dire conclusions about the current state of things. AIUI we only have 3 issues so far (remember mmconfig has been enabled in -mm for a long time): 1) host bridge decode problems (disabling decode to avoid overlaps can cause some bridges to stop decoding RAM addrs, but we have a fix for that) 2) config space retry on ATI (I think willy already debunked this one?) 3) some FUD about SMM or other firmware interrupts coming in during BAR sizing while decode is disabled (this one is just pure FUD; if we want to solve it properly we need a new platform hook to disable SMM/NMI/etc. around PCI probing) What else was there? What reason do we have to think that things are so disastrous? So I really prefer willy's approach to Arjan's alternative... Jesse -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 29, 2008 at 05:19:55AM -0800, Greg KH wrote: > On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: > > I'm more optimistic because we've so severely restricted the use of > > mmconf after these patches that it's unlikely to cause problems. I also > > hear Vista is now using mmconf, so fewer implementations are going to > > be buggy at this point. > > Hahahaha, oh, that's a good one... Thanks Greg. What happened to "Can't we all try to get along"? > But what about the thousands of implementations out there that are > buggy? > > I'm with Arjan here, I'm very skeptical. Maybe I'm insufficiently imaginative. Can you come up with a plausible way in which the two patches I posted will succumb to bugs? After those patches we only use mmconf if: 1. conf1 has failed to work OR 2. user has compiled their own kernel without support for conf1 OR 3. kernel probes config space 0x100 to see if it can access extended config space (requires the device to be PCIe or PCI-X2) OR 4. root attempts to lspci - or lspci -v OR 5. device driver tries to access extended config space With Arjan's patch, I believe only case 3 changes. In cases 4 and 5, either lspci or the device driver will jump through the hoop to enable access to extended config space. > Matthew, with Arjan's patch, is anything that currently works now > broken? Why do you feel it is somehow "wrong"? lspci is broken. It used to be able to access extended config space, and now can't unless it is patched to know about the sysfs flag to enable it. If you're determined to implement something to disable extended config space by default, it can be done in a much better way than Arjan's patch -- less code (both source and object). -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Matthew Wilcox wrote: On Tue, Jan 29, 2008 at 07:29:51AM -0800, Arjan van de Ven wrote: Right now, that isn't a lot of people in x86 land, but your patch encumbers drivers for non-x86 archs with an additional call to access space that they've never had a problem with. lets say s/x86/x86, IA64 and architectures that use intel, amd or via chipsets/ Umm .. ia64 already does exactly what I'm proposing for x86. It uses one SAL interface for bytes below 256 and a different SAL interface for bytes 256-4095. Not exactly. :) The interface is the same, ia64_sal_pci_config_write() and ia64_sal_pci_config_read(), but a flag bit in the mode argument is used to tell the SAL interface whether to translate the offset component of the config address as having 8 or 12 bits of of displacement. In my estimation, Ivan's patch, in his implementation of Loic's suggestion, is even more elegant, since there is no need to flag whether the access is for offsets below 256. Ivan's code automatically uses Port IO (or equivalent with Matthew's patch) for offsets below 256 and MMCONFIG for offsets from 256 to 4096. And even better, it removes the bitmap that tracks MMCONFIG-unfriendly devices for the first 16 buses, a solution that assumes systems with bus numbers higher than 16 will get MMCONFIG right, which turned out to be a very wrong assumption. Furthermore, the config address is translated by the Northbridge. The delivery mechanism to the Northbridge, whether Port IO or MMCONFIG, is utterly opaque to the devices on the bus, since all they see is PCI config cycles, not Port IO or MMCONFIG cycles. The test only needed to be made at the Northbridge level, not at the device level. Ivan's patch removes all this cruft. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 29, 2008 at 07:29:51AM -0800, Arjan van de Ven wrote: > > Right now, that isn't a lot of people in x86 land, but your patch > > encumbers drivers for non-x86 archs with an additional call to access > > space that they've never had a problem with. > > lets say s/x86/x86, IA64 and architectures that use intel, amd or via > chipsets/ Umm .. ia64 already does exactly what I'm proposing for x86. It uses one SAL interface for bytes below 256 and a different SAL interface for bytes 256-4095. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Tue, 29 Jan 2008 10:15:45 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: specific to legacy x86 hardware is, IMNSHO, a kludge. in addition to pci_enable(), pci_enable_msi(), pci_enable_busmaster() they already need to do to enable various features? These calls are related to generic aspects of the PCI* landscape itself and are not related to any arch-specific hardware, nor were they devised to address chipset-specific or BIOS-specific problems. For the good of all, we should endeavor to avoid putting arch-specific fixes into the generic code whenever possible. And in this case, not only is it possible, it's been done and tested. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, 29 Jan 2008 10:15:45 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > Arjan van de Ven wrote: > > On Tue, 29 Jan 2008 09:15:02 -0500 > > Tony Camuso <[EMAIL PROTECTED]> wrote: > > > >> Greg, > >> > >> The problem with Arjan's patch, if I understand it correctly, is > >> that it requires drivers to make a call to access extended PCI > >> config space. > >> > >> And, IIRC, Arjan's patch encumbers drivers for all arch's, even > >> those that have no MMCONFIG problems. > >> > >> The patches proposed by Loic, Ivan, Matthew, and myself, all > >> address the problem in an x86-specific manner that is transparent > >> to the drivers. > > > > this is not quite correct; the patches from Loic, Ivan, Matthew and > > you are for a different problem statement. > > > > Your patch problem statement is "need to fix mmconfig", my patch > > problem statement is "need to not make users who don't need it > > suffer". These are orthogonal problems. > > > > > > Yes, but your patch also makes users who need extended PCI config > space suffer. > > Right now, that isn't a lot of people in x86 land, but your patch > encumbers drivers for non-x86 archs with an additional call to access > space that they've never had a problem with. lets say s/x86/x86, IA64 and architectures that use intel, amd or via chipsets/ > As more PCI express drivers start to take advantage of AER and other > advanced express capabilities, the extra call to address a condition > specific to legacy x86 hardware is, IMNSHO, a kludge. in addition to pci_enable(), pci_enable_msi(), pci_enable_busmaster() they already need to do to enable various features? -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Tue, 29 Jan 2008 09:15:02 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: Greg, The problem with Arjan's patch, if I understand it correctly, is that it requires drivers to make a call to access extended PCI config space. And, IIRC, Arjan's patch encumbers drivers for all arch's, even those that have no MMCONFIG problems. The patches proposed by Loic, Ivan, Matthew, and myself, all address the problem in an x86-specific manner that is transparent to the drivers. this is not quite correct; the patches from Loic, Ivan, Matthew and you are for a different problem statement. Your patch problem statement is "need to fix mmconfig", my patch problem statement is "need to not make users who don't need it suffer". These are orthogonal problems. Yes, but your patch also makes users who need extended PCI config space suffer. Right now, that isn't a lot of people in x86 land, but your patch encumbers drivers for non-x86 archs with an additional call to access space that they've never had a problem with. As more PCI express drivers start to take advantage of AER and other advanced express capabilities, the extra call to address a condition specific to legacy x86 hardware is, IMNSHO, a kludge. The patches submitted by the others fix the problems with MMCONFIG without encumbering the drivers to be aware of any difference between legacy config space and extended config space. I have tested these patches on a number of systems exhibiting various MMCONFIG- related pathologies, and they work. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, 29 Jan 2008 09:15:02 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > Greg KH wrote: > > On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: > >> I'm more optimistic because we've so severely restricted the use of > >> mmconf after these patches that it's unlikely to cause problems. > >> I also hear Vista is now using mmconf, so fewer implementations > >> are going to be buggy at this point. > > > > Hahahaha, oh, that's a good one... > > > > But what about the thousands of implementations out there that are > > buggy? > > > > I'm with Arjan here, I'm very skeptical. > > > > Matthew, with Arjan's patch, is anything that currently works now > > broken? Why do you feel it is somehow "wrong"? > > > > thanks, > > > > greg k-h > > Greg, > > The problem with Arjan's patch, if I understand it correctly, is that > it requires drivers to make a call to access extended PCI config > space. > > And, IIRC, Arjan's patch encumbers drivers for all arch's, even those > that have no MMCONFIG problems. > > The patches proposed by Loic, Ivan, Matthew, and myself, all address > the problem in an x86-specific manner that is transparent to the > drivers. this is not quite correct; the patches from Loic, Ivan, Matthew and you are for a different problem statement. Your patch problem statement is "need to fix mmconfig", my patch problem statement is "need to not make users who don't need it suffer". These are orthogonal problems. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Greg KH wrote: On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow "wrong"? thanks, greg k-h Greg, The problem with Arjan's patch, if I understand it correctly, is that it requires drivers to make a call to access extended PCI config space. And, IIRC, Arjan's patch encumbers drivers for all arch's, even those that have no MMCONFIG problems. The patches proposed by Loic, Ivan, Matthew, and myself, all address the problem in an x86-specific manner that is transparent to the drivers. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: > On Mon, Jan 28, 2008 at 07:05:05PM -0800, Arjan van de Ven wrote: > > I think there's only one fundamental disagreement; and that is: > > do we think that things are now totally fixed and no new major issues > > will arrive after the "fix yet another mmconfig thing" patches are merged. > > > > If the answer is no, then imho my patch is the right approach; it will > > limit the damage and doesn't make > > the people suffer who don't need extended config space. > > If the answer is yet, then my patch is not needed. > > > > This is a judgment call; I'm skeptical, others are more optimistic that > > after 2 years of messing around > > they have finally found the last golden fix. > > I'm more optimistic because we've so severely restricted the use of > mmconf after these patches that it's unlikely to cause problems. I also > hear Vista is now using mmconf, so fewer implementations are going to > be buggy at this point. Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow "wrong"? 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, 29 Jan 2008 09:15:02 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Greg KH wrote: On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow wrong? thanks, greg k-h Greg, The problem with Arjan's patch, if I understand it correctly, is that it requires drivers to make a call to access extended PCI config space. And, IIRC, Arjan's patch encumbers drivers for all arch's, even those that have no MMCONFIG problems. The patches proposed by Loic, Ivan, Matthew, and myself, all address the problem in an x86-specific manner that is transparent to the drivers. this is not quite correct; the patches from Loic, Ivan, Matthew and you are for a different problem statement. Your patch problem statement is need to fix mmconfig, my patch problem statement is need to not make users who don't need it suffer. These are orthogonal problems. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Tue, 29 Jan 2008 10:15:45 -0500 Tony Camuso [EMAIL PROTECTED] wrote: specific to legacy x86 hardware is, IMNSHO, a kludge. in addition to pci_enable(), pci_enable_msi(), pci_enable_busmaster() they already need to do to enable various features? These calls are related to generic aspects of the PCI* landscape itself and are not related to any arch-specific hardware, nor were they devised to address chipset-specific or BIOS-specific problems. For the good of all, we should endeavor to avoid putting arch-specific fixes into the generic code whenever possible. And in this case, not only is it possible, it's been done and tested. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, 29 Jan 2008 10:15:45 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Arjan van de Ven wrote: On Tue, 29 Jan 2008 09:15:02 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Greg, The problem with Arjan's patch, if I understand it correctly, is that it requires drivers to make a call to access extended PCI config space. And, IIRC, Arjan's patch encumbers drivers for all arch's, even those that have no MMCONFIG problems. The patches proposed by Loic, Ivan, Matthew, and myself, all address the problem in an x86-specific manner that is transparent to the drivers. this is not quite correct; the patches from Loic, Ivan, Matthew and you are for a different problem statement. Your patch problem statement is need to fix mmconfig, my patch problem statement is need to not make users who don't need it suffer. These are orthogonal problems. Yes, but your patch also makes users who need extended PCI config space suffer. Right now, that isn't a lot of people in x86 land, but your patch encumbers drivers for non-x86 archs with an additional call to access space that they've never had a problem with. lets say s/x86/x86, IA64 and architectures that use intel, amd or via chipsets/ As more PCI express drivers start to take advantage of AER and other advanced express capabilities, the extra call to address a condition specific to legacy x86 hardware is, IMNSHO, a kludge. in addition to pci_enable(), pci_enable_msi(), pci_enable_busmaster() they already need to do to enable various features? -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Greg KH wrote: On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow wrong? thanks, greg k-h Greg, The problem with Arjan's patch, if I understand it correctly, is that it requires drivers to make a call to access extended PCI config space. And, IIRC, Arjan's patch encumbers drivers for all arch's, even those that have no MMCONFIG problems. The patches proposed by Loic, Ivan, Matthew, and myself, all address the problem in an x86-specific manner that is transparent to the drivers. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: On Mon, Jan 28, 2008 at 07:05:05PM -0800, Arjan van de Ven wrote: I think there's only one fundamental disagreement; and that is: do we think that things are now totally fixed and no new major issues will arrive after the fix yet another mmconfig thing patches are merged. If the answer is no, then imho my patch is the right approach; it will limit the damage and doesn't make the people suffer who don't need extended config space. If the answer is yet, then my patch is not needed. This is a judgment call; I'm skeptical, others are more optimistic that after 2 years of messing around they have finally found the last golden fix. I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. Hahahaha, oh, that's a good one... But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow wrong? 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Tue, 29 Jan 2008 09:15:02 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Greg, The problem with Arjan's patch, if I understand it correctly, is that it requires drivers to make a call to access extended PCI config space. And, IIRC, Arjan's patch encumbers drivers for all arch's, even those that have no MMCONFIG problems. The patches proposed by Loic, Ivan, Matthew, and myself, all address the problem in an x86-specific manner that is transparent to the drivers. this is not quite correct; the patches from Loic, Ivan, Matthew and you are for a different problem statement. Your patch problem statement is need to fix mmconfig, my patch problem statement is need to not make users who don't need it suffer. These are orthogonal problems. Yes, but your patch also makes users who need extended PCI config space suffer. Right now, that isn't a lot of people in x86 land, but your patch encumbers drivers for non-x86 archs with an additional call to access space that they've never had a problem with. As more PCI express drivers start to take advantage of AER and other advanced express capabilities, the extra call to address a condition specific to legacy x86 hardware is, IMNSHO, a kludge. The patches submitted by the others fix the problems with MMCONFIG without encumbering the drivers to be aware of any difference between legacy config space and extended config space. I have tested these patches on a number of systems exhibiting various MMCONFIG- related pathologies, and they work. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 29, 2008 at 07:29:51AM -0800, Arjan van de Ven wrote: Right now, that isn't a lot of people in x86 land, but your patch encumbers drivers for non-x86 archs with an additional call to access space that they've never had a problem with. lets say s/x86/x86, IA64 and architectures that use intel, amd or via chipsets/ Umm .. ia64 already does exactly what I'm proposing for x86. It uses one SAL interface for bytes below 256 and a different SAL interface for bytes 256-4095. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Matthew Wilcox wrote: On Tue, Jan 29, 2008 at 07:29:51AM -0800, Arjan van de Ven wrote: Right now, that isn't a lot of people in x86 land, but your patch encumbers drivers for non-x86 archs with an additional call to access space that they've never had a problem with. lets say s/x86/x86, IA64 and architectures that use intel, amd or via chipsets/ Umm .. ia64 already does exactly what I'm proposing for x86. It uses one SAL interface for bytes below 256 and a different SAL interface for bytes 256-4095. Not exactly. :) The interface is the same, ia64_sal_pci_config_write() and ia64_sal_pci_config_read(), but a flag bit in the mode argument is used to tell the SAL interface whether to translate the offset component of the config address as having 8 or 12 bits of of displacement. In my estimation, Ivan's patch, in his implementation of Loic's suggestion, is even more elegant, since there is no need to flag whether the access is for offsets below 256. Ivan's code automatically uses Port IO (or equivalent with Matthew's patch) for offsets below 256 and MMCONFIG for offsets from 256 to 4096. And even better, it removes the bitmap that tracks MMCONFIG-unfriendly devices for the first 16 buses, a solution that assumes systems with bus numbers higher than 16 will get MMCONFIG right, which turned out to be a very wrong assumption. Furthermore, the config address is translated by the Northbridge. The delivery mechanism to the Northbridge, whether Port IO or MMCONFIG, is utterly opaque to the devices on the bus, since all they see is PCI config cycles, not Port IO or MMCONFIG cycles. The test only needed to be made at the Northbridge level, not at the device level. Ivan's patch removes all this cruft. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 29, 2008 at 05:19:55AM -0800, Greg KH wrote: On Mon, Jan 28, 2008 at 08:18:04PM -0700, Matthew Wilcox wrote: I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. Hahahaha, oh, that's a good one... Thanks Greg. What happened to Can't we all try to get along? But what about the thousands of implementations out there that are buggy? I'm with Arjan here, I'm very skeptical. Maybe I'm insufficiently imaginative. Can you come up with a plausible way in which the two patches I posted will succumb to bugs? After those patches we only use mmconf if: 1. conf1 has failed to work OR 2. user has compiled their own kernel without support for conf1 OR 3. kernel probes config space 0x100 to see if it can access extended config space (requires the device to be PCIe or PCI-X2) OR 4. root attempts to lspci - or lspci -v OR 5. device driver tries to access extended config space With Arjan's patch, I believe only case 3 changes. In cases 4 and 5, either lspci or the device driver will jump through the hoop to enable access to extended config space. Matthew, with Arjan's patch, is anything that currently works now broken? Why do you feel it is somehow wrong? lspci is broken. It used to be able to access extended config space, and now can't unless it is patched to know about the sysfs flag to enable it. If you're determined to implement something to disable extended config space by default, it can be done in a much better way than Arjan's patch -- less code (both source and object). -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 07:05:05PM -0800, Arjan van de Ven wrote: > I think there's only one fundamental disagreement; and that is: > do we think that things are now totally fixed and no new major issues > will arrive after the "fix yet another mmconfig thing" patches are merged. > > If the answer is no, then imho my patch is the right approach; it will limit > the damage and doesn't make > the people suffer who don't need extended config space. > If the answer is yet, then my patch is not needed. > > This is a judgment call; I'm skeptical, others are more optimistic that after > 2 years of messing around > they have finally found the last golden fix. I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 28 Jan 2008 12:44:31 -0800 Greg KH <[EMAIL PROTECTED]> wrote: > On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: > > Greg, > > > > Have you given Grant's suggestion any further consideration? > > > > I'd like to know how the MMCONFIG issues discussed in this thread > > are going to be handled upstream. I have a patch implemented in > > RHEL 5.2, but I would rather have the upstream patch implemented, > > whatever it is. > > Well, everyone still doesn't seem to agree on the proper way forward > here, so for me to just "pick one" isn't very appropriate. > > So, can we try again? I think there's only one fundamental disagreement; and that is: do we think that things are now totally fixed and no new major issues will arrive after the "fix yet another mmconfig thing" patches are merged. If the answer is no, then imho my patch is the right approach; it will limit the damage and doesn't make the people suffer who don't need extended config space. If the answer is yet, then my patch is not needed. This is a judgment call; I'm skeptical, others are more optimistic that after 2 years of messing around they have finally found the last golden fix. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 02:53:34PM -0800, Greg KH wrote: > Please send me patches, in a form that can be merged, along with a > proper changelog entry, in the order in which you wish them to be > applied, so I know exactly what changes you are referring to. I'll send each patch as a reply to this email. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 03:31:42PM -0700, Matthew Wilcox wrote: > On Mon, Jan 28, 2008 at 12:44:31PM -0800, Greg KH wrote: > > On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: > > > Greg, > > > > > > Have you given Grant's suggestion any further consideration? > > > > > > I'd like to know how the MMCONFIG issues discussed in this thread are > > > going > > > to be handled upstream. I have a patch implemented in RHEL 5.2, but I > > > would > > > rather have the upstream patch implemented, whatever it is. > > > > Well, everyone still doesn't seem to agree on the proper way forward > > here, so for me to just "pick one" isn't very appropriate. > > > > So, can we try again? > > > > Can people submit, what they think the change should be? Right now I > > have Arjan's patch in my kernel tree, but will not send it to Linus for > > .25 for now, unless everyone thinks that is the best solution at the > > moment (which, for me, I'm leaning toward right now...) > > My opinion is that Ivan's patch followed by my patch is the best way > forward. I see Arjan's patch as a good prototype, but it introduces a lot > of unnecessary infrastructure (and a userspace interface that I dislike). > > I would like to see Ivan's patch merged ASAP as it does fix one of > my machines. akpm has the patch from me to disable io decoding, and > intends to send it to Linus during this merge window ... that patch > becomes unnecessary if we merge Ivan's patch. > > My patch is an incremental improvement that adds some of the features > of Arjan's patch without the extra infrastructure. I don't think it's > urgent, but it does make some of our internal interfaces cleaner. Please send me patches, in a form that can be merged, along with a proper changelog entry, in the order in which you wish them to be applied, so I know exactly what changes you are referring to. 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 12:44:31PM -0800, Greg KH wrote: > On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: > > Greg, > > > > Have you given Grant's suggestion any further consideration? > > > > I'd like to know how the MMCONFIG issues discussed in this thread are going > > to be handled upstream. I have a patch implemented in RHEL 5.2, but I would > > rather have the upstream patch implemented, whatever it is. > > Well, everyone still doesn't seem to agree on the proper way forward > here, so for me to just "pick one" isn't very appropriate. > > So, can we try again? > > Can people submit, what they think the change should be? Right now I > have Arjan's patch in my kernel tree, but will not send it to Linus for > .25 for now, unless everyone thinks that is the best solution at the > moment (which, for me, I'm leaning toward right now...) My opinion is that Ivan's patch followed by my patch is the best way forward. I see Arjan's patch as a good prototype, but it introduces a lot of unnecessary infrastructure (and a userspace interface that I dislike). I would like to see Ivan's patch merged ASAP as it does fix one of my machines. akpm has the patch from me to disable io decoding, and intends to send it to Linus during this merge window ... that patch becomes unnecessary if we merge Ivan's patch. My patch is an incremental improvement that adds some of the features of Arjan's patch without the extra infrastructure. I don't think it's urgent, but it does make some of our internal interfaces cleaner. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: > Greg, > > Have you given Grant's suggestion any further consideration? > > I'd like to know how the MMCONFIG issues discussed in this thread are going > to be handled upstream. I have a patch implemented in RHEL 5.2, but I would > rather have the upstream patch implemented, whatever it is. Well, everyone still doesn't seem to agree on the proper way forward here, so for me to just "pick one" isn't very appropriate. So, can we try again? Can people submit, what they think the change should be? Right now I have Arjan's patch in my kernel tree, but will not send it to Linus for .25 for now, unless everyone thinks that is the best solution at the moment (which, for me, I'm leaning toward right now...) thanks, greg "can't we all just get along?" 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Greg, Have you given Grant's suggestion any further consideration? I'd like to know how the MMCONFIG issues discussed in this thread are going to be handled upstream. I have a patch implemented in RHEL 5.2, but I would rather have the upstream patch implemented, whatever it is. Grant Grundler wrote: On Tue, Jan 15, 2008 at 10:56:41AM -0700, Matthew Wilcox wrote: On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. Agreed. Greg, I think at least two better alternatives were proposed already. Please review the thread again. grant -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: Greg, Have you given Grant's suggestion any further consideration? I'd like to know how the MMCONFIG issues discussed in this thread are going to be handled upstream. I have a patch implemented in RHEL 5.2, but I would rather have the upstream patch implemented, whatever it is. Well, everyone still doesn't seem to agree on the proper way forward here, so for me to just pick one isn't very appropriate. So, can we try again? Can people submit, what they think the change should be? Right now I have Arjan's patch in my kernel tree, but will not send it to Linus for .25 for now, unless everyone thinks that is the best solution at the moment (which, for me, I'm leaning toward right now...) thanks, greg can't we all just get along? 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 12:44:31PM -0800, Greg KH wrote: On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: Greg, Have you given Grant's suggestion any further consideration? I'd like to know how the MMCONFIG issues discussed in this thread are going to be handled upstream. I have a patch implemented in RHEL 5.2, but I would rather have the upstream patch implemented, whatever it is. Well, everyone still doesn't seem to agree on the proper way forward here, so for me to just pick one isn't very appropriate. So, can we try again? Can people submit, what they think the change should be? Right now I have Arjan's patch in my kernel tree, but will not send it to Linus for .25 for now, unless everyone thinks that is the best solution at the moment (which, for me, I'm leaning toward right now...) My opinion is that Ivan's patch followed by my patch is the best way forward. I see Arjan's patch as a good prototype, but it introduces a lot of unnecessary infrastructure (and a userspace interface that I dislike). I would like to see Ivan's patch merged ASAP as it does fix one of my machines. akpm has the patch from me to disable io decoding, and intends to send it to Linus during this merge window ... that patch becomes unnecessary if we merge Ivan's patch. My patch is an incremental improvement that adds some of the features of Arjan's patch without the extra infrastructure. I don't think it's urgent, but it does make some of our internal interfaces cleaner. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 03:31:42PM -0700, Matthew Wilcox wrote: On Mon, Jan 28, 2008 at 12:44:31PM -0800, Greg KH wrote: On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: Greg, Have you given Grant's suggestion any further consideration? I'd like to know how the MMCONFIG issues discussed in this thread are going to be handled upstream. I have a patch implemented in RHEL 5.2, but I would rather have the upstream patch implemented, whatever it is. Well, everyone still doesn't seem to agree on the proper way forward here, so for me to just pick one isn't very appropriate. So, can we try again? Can people submit, what they think the change should be? Right now I have Arjan's patch in my kernel tree, but will not send it to Linus for .25 for now, unless everyone thinks that is the best solution at the moment (which, for me, I'm leaning toward right now...) My opinion is that Ivan's patch followed by my patch is the best way forward. I see Arjan's patch as a good prototype, but it introduces a lot of unnecessary infrastructure (and a userspace interface that I dislike). I would like to see Ivan's patch merged ASAP as it does fix one of my machines. akpm has the patch from me to disable io decoding, and intends to send it to Linus during this merge window ... that patch becomes unnecessary if we merge Ivan's patch. My patch is an incremental improvement that adds some of the features of Arjan's patch without the extra infrastructure. I don't think it's urgent, but it does make some of our internal interfaces cleaner. Please send me patches, in a form that can be merged, along with a proper changelog entry, in the order in which you wish them to be applied, so I know exactly what changes you are referring to. 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 02:53:34PM -0800, Greg KH wrote: Please send me patches, in a form that can be merged, along with a proper changelog entry, in the order in which you wish them to be applied, so I know exactly what changes you are referring to. I'll send each patch as a reply to this email. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 28 Jan 2008 12:44:31 -0800 Greg KH [EMAIL PROTECTED] wrote: On Mon, Jan 28, 2008 at 01:32:06PM -0500, Tony Camuso wrote: Greg, Have you given Grant's suggestion any further consideration? I'd like to know how the MMCONFIG issues discussed in this thread are going to be handled upstream. I have a patch implemented in RHEL 5.2, but I would rather have the upstream patch implemented, whatever it is. Well, everyone still doesn't seem to agree on the proper way forward here, so for me to just pick one isn't very appropriate. So, can we try again? I think there's only one fundamental disagreement; and that is: do we think that things are now totally fixed and no new major issues will arrive after the fix yet another mmconfig thing patches are merged. If the answer is no, then imho my patch is the right approach; it will limit the damage and doesn't make the people suffer who don't need extended config space. If the answer is yet, then my patch is not needed. This is a judgment call; I'm skeptical, others are more optimistic that after 2 years of messing around they have finally found the last golden fix. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 28, 2008 at 07:05:05PM -0800, Arjan van de Ven wrote: I think there's only one fundamental disagreement; and that is: do we think that things are now totally fixed and no new major issues will arrive after the fix yet another mmconfig thing patches are merged. If the answer is no, then imho my patch is the right approach; it will limit the damage and doesn't make the people suffer who don't need extended config space. If the answer is yet, then my patch is not needed. This is a judgment call; I'm skeptical, others are more optimistic that after 2 years of messing around they have finally found the last golden fix. I'm more optimistic because we've so severely restricted the use of mmconf after these patches that it's unlikely to cause problems. I also hear Vista is now using mmconf, so fewer implementations are going to be buggy at this point. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Greg, Have you given Grant's suggestion any further consideration? I'd like to know how the MMCONFIG issues discussed in this thread are going to be handled upstream. I have a patch implemented in RHEL 5.2, but I would rather have the upstream patch implemented, whatever it is. Grant Grundler wrote: On Tue, Jan 15, 2008 at 10:56:41AM -0700, Matthew Wilcox wrote: On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. Agreed. Greg, I think at least two better alternatives were proposed already. Please review the thread again. grant -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 10:56:41AM -0700, Matthew Wilcox wrote: > On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: > > But so far, we have a zillion patches floating around, claiming > > different things, some with signed-off-bys and others without, so for > > now, I'll just stick with Arjan's patch in -mm and see if anyone > > complains about those releases... > > I complain about Arjan's patch. For reasons which have been adequately > gone into already in this thread. Agreed. Greg, I think at least two better alternatives were proposed already. Please review the thread again. grant -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 10:56:41AM -0700, Matthew Wilcox wrote: On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. Agreed. Greg, I think at least two better alternatives were proposed already. Please review the thread again. grant -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/15/2008 2:38 PM, Linus Torvalds wrote: On Tue, 15 Jan 2008, Tony Camuso wrote: Linus is confident that conf1 is not going away for at least the next five years. Not on PC's. Small birds tell me that there can be all these non-PC x86 subarchitectures that may or may not have conf1. Linus But is there a ACPI-compliant/architecture that only offers mmconfig for configuration-space access and no other fallback method (i.e. no conf1, no bios,...)? 2.6.24 supports mmconfig for: - ACPI-system with MCFG - a couple chipset discovered by conf1 If a system has no conf1, but does not have e820+ACPI+MCFG, or does have some other method than mmconfig, it was already irrelevant in the discussion of Ivan's initial patch in december (because that system was either never supported or not impacted, and we were trying to fix bugs, not introduce support for new class of systems). Maybe Arjan could share his knowledge, and tell us what system he was thinking about (and whether it needed to be supported by 2.6.24) when saying: "When (and I'm saying "when" not "if") systems arrive that only have MMCONFIG for some of the devices." Anyway Ivan's patch + Matthew's extensions are handling that non-PC arch. That combination is advocated by at least: Ivan Kokshaysky Matthew Wilcox Tony Camuso Loic Prylli even Arjan's said that while he prefers his patch (saying it's more conservative), he does not see a existing problem with the Ivan/Matthew combination. [ simpler, less ambitious fixes can be forgotten if nothing can be done for 2.6.24, I can understand that choice ] The list of problems I see with Arjan's patch are: - no word on whether the existing Linux driver/pci/pcie/aer code should be converted to opt-in? - mmconfig still needs to be revisited to sort-out the mix of mmconfig+conf1+third-method access. - you cannot test if ext-conf-space is available without taking risks: when pci_enable_ext_config() is called, even legacy-conf-space is switched to the new method. So some administrator action (lspci -v +maybe-other-flag) or some driver action (that can optionally use ext-conf-space but does not *rely* on it) could cause some devices to totally disappear (if some pci hierarchy is handled by mmconfig as a 0x section as seen on many amd machines). Matthew/Ivan will simply in the worst case detect that ext-conf-space is not available in pci_cfg_space_size()), legacy-conf-space will still work (and that 0x section is perfectly *safe* to query, tell me if you need more details of why). - introduce a new user-api, and a new kernel API, while in practice there is no evidence that brings any benefits compared to Ivan/Matthew. IMHO, making "pci=nommconf" the default behaviour is better than Arjan's patch: for the exaggerated 99.99% users he claims don't need ext-conf-space, that's obviously as good. And many of the others would benefit from the ability to test and optionally use ext-conf-space is available without taking the risk of crashing something, so something else is better for them. With Arjan's patch, in 10 years, we might still have to use an extra option (or some other action) when using lspci to display extended caps, and we would still run the risk of crashing some old machine when doing so (unless maybe a blacklist of some sort will be added, making the newly introduced API completely useless soon, or unless we keep the painful bitmaps in mmconfig potentially ending-up with 3 set of pci-ops). Loic -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 11:38:42AM -0800, Linus Torvalds wrote: > On Tue, 15 Jan 2008, Tony Camuso wrote: > > Linus is confident that conf1 is not going away for at least the > > next five years. > > Not on PC's. Small birds tell me that there can be all these non-PC x86 > subarchitectures that may or may not have conf1. Right -- hence my patch on top of Ivan's which removes all the assumptions about conf1 from mmconfig (there are still *references* to conf1 in the mmconfig code, but they'll only be used if conf1 is functional). -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, 15 Jan 2008, Tony Camuso wrote: > > Linus is confident that conf1 is not going away for at least the > next five years. Not on PC's. Small birds tell me that there can be all these non-PC x86 subarchitectures that may or may not have conf1. Linus -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
I agree with Matthew. My preference is Ivan's patch using Loic's proposal. My patch would have tested MMCONFIG before using it, but it didn't fix the problem where the decode of large displacement devices can overlap the MMCONFIG region. Ivan's patch fixes that, and the problem of Northbridges that don't respond to MMCONFIG and as a bonus cleans out some code rendered unnecessary by his patch. Linus is confident that conf1 is not going away for at least the next five years. Matthew Wilcox wrote: On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: > But so far, we have a zillion patches floating around, claiming > different things, some with signed-off-bys and others without, so for > now, I'll just stick with Arjan's patch in -mm and see if anyone > complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 11:00:37AM -0500, Loic Prylli wrote: > > > On 1/14/2008 6:04 PM, Adrian Bunk wrote: >>> I thought so, but due to the way that things are initialised, mmconfig >>> happens before conf1. conf1 is known to be usable, but hasn't set >>> raw_pci_ops at this point. Confusing, and not ideal, but fixing this >>> isn't in scope for 2.6.24. >>> ... >>> >> >> *ahem* >> >> I don't think anything of what was discussed in this thread would be in >> scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs >> release 2.6.24). >> >> cu >> Adrian >> > > > Why not put in 2.6.24 a simple fix for the last known remaining mmconfig > problems in 2.6.24? Heh, no, because it is _way_ too late for such a patch that hasn't been tested in any trees, sorry. 2.6.25 is the earliest I'll take such a fix, and if it's really as simple as you say, I'll consider it for the -stable releases for .24 if needed. But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/14/2008 6:04 PM, Adrian Bunk wrote:
I thought so, but due to the way that things are initialised, mmconfig
happens before conf1. conf1 is known to be usable, but hasn't set
raw_pci_ops at this point. Confusing, and not ideal, but fixing this
isn't in scope for 2.6.24.
...
*ahem*
I don't think anything of what was discussed in this thread would be in
scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs
release 2.6.24).
cu
Adrian
Why not put in 2.6.24 a simple fix for the last known remaining mmconfig
problems in 2.6.24? There has mostly been three bugs related to mmconfig:
- BIOS/hardware: exaggerated MCFG claims: solved long ago
- hardware: buggy CRS+mmconfig chipset: fix included last month
- Linux code: mmconfig incompatible with live BAR-probing: *not fixed*
It would be ironic to not fix the only one that is really confined to
the Linux code.
Everybody more or less agrees *any* patches submitted so far does solve
the known problems, and will not cause regressions. The only long
discussion is about how to best prevent the effect of an "imaginary"
fourth bug, and by nature that's a controversial topic.
For 2.6.24, if nothing more than a few lines can be done, either make
pci=nommconf the default and add a pci=mmconf option, or/and apply one
of the easiest patch to review i.e.Tony's one, so small I copy it again
below (using 0x40 or 0x100 for the comparison does not really matter,
personally I would change it to 0x100 to be like Ivan's patch, but
either is much better than nothing). Replacing some mmconfig access by
conf1 cannot cause any regression.
Loic
P.S.: with that patch, conf1-less x86 systems requiring mmconfig would
not be supported. But they are like UFOs. They are plenty of them in the
galaxy, but earth sightings are not convincing enough for 2.6.24
support, they can wait 2.6.25.
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 1bf5816..4474979 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -73,7 +73,7 @@ static int pci_mmcfg_read(unsigned int seg, unsigned
int bus,
}
base = get_base_addr(seg, bus, devfn);
-if (!base)
+if ((!base) || (reg < 0x40))
return pci_conf1_read(seg,bus,devfn,reg,len,value);
spin_lock_irqsave(_config_lock, flags);
@@ -106,7 +106,7 @@ static int pci_mmcfg_write(unsigned int seg,
unsigned int bus,
return -EINVAL;
base = get_base_addr(seg, bus, devfn);
-if (!base)
+if ((!base) || (reg < 0x40))
return pci_conf1_write(seg,bus,devfn,reg,len,value);
spin_lock_irqsave(_config_lock, flags);
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index 4095e4d..4ad1fcb 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -61,7 +61,7 @@ static int pci_mmcfg_read(unsigned int seg, unsigned
int bus,
}
addr = pci_dev_base(seg, bus, devfn);
-if (!addr)
+if ((!addr) || (reg < 0x40))
return pci_conf1_read(seg,bus,devfn,reg,len,value);
switch (len) {
@@ -89,7 +89,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned
int bus,
return -EINVAL;
addr = pci_dev_base(seg, bus, devfn);
-if (!addr)
+if ((!addr) || (reg < 0x40))
return pci_conf1_write(seg,bus,devfn,reg,len,value);
switch (len) {
--
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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
I just thought this might be interesting to the discussion. I recently bought another 2 GB memory for my computer. My hardware is as following: Asus Commando (Intel P965 chipset) Intel Core2 Q6600 4x1 GB Geil PC6400 memory nVidia 8800 gts (old g80 core, 640 mb mem) Without booting with pci=nommeconf i have severe stability issues and often when its not crashing i get slowdowns with the error: kern.log:Jan 15 13:19:40 bilbo kernel: [ 132.046715] NVRM: Xid (0001:00): 6, PE0001 ... repeated x times. In addition the nVidia framebuffer seems to "leak" or not update since i get loads of graphics artifacts. The system works perfectly fine with 2 GB memory and not the pci=nommconf. It works like a charm when using pci=nommconf and 4 GB memory. In adition i have to enable the Northbridge->PCI Memory remap feature in the BIOS to avoid the kernel panicing when trying to access > 3 gb but that is understandable :) My software is Kubuntu 7.10 stock x86_64 kernel, but i do use the binary driver by nVidia. It works like a charm when using pci=nommconf If you guys need any more info about hardware/software from me, please let me know. -- Øyvind Vågen Jægtnes +47 96 22 03 08 (i reject your diurnal rhythm and subsitute my own) -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
I just thought this might be interesting to the discussion. I recently bought another 2 GB memory for my computer. My hardware is as following: Asus Commando (Intel P965 chipset) Intel Core2 Q6600 4x1 GB Geil PC6400 memory nVidia 8800 gts (old g80 core, 640 mb mem) Without booting with pci=nommeconf i have severe stability issues and often when its not crashing i get slowdowns with the error: kern.log:Jan 15 13:19:40 bilbo kernel: [ 132.046715] NVRM: Xid (0001:00): 6, PE0001 ... repeated x times. In addition the nVidia framebuffer seems to leak or not update since i get loads of graphics artifacts. The system works perfectly fine with 2 GB memory and not the pci=nommconf. It works like a charm when using pci=nommconf and 4 GB memory. In adition i have to enable the Northbridge-PCI Memory remap feature in the BIOS to avoid the kernel panicing when trying to access 3 gb but that is understandable :) My software is Kubuntu 7.10 stock x86_64 kernel, but i do use the binary driver by nVidia. It works like a charm when using pci=nommconf If you guys need any more info about hardware/software from me, please let me know. -- Øyvind Vågen Jægtnes +47 96 22 03 08 (i reject your diurnal rhythm and subsitute my own) -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/14/2008 6:04 PM, Adrian Bunk wrote:
I thought so, but due to the way that things are initialised, mmconfig
happens before conf1. conf1 is known to be usable, but hasn't set
raw_pci_ops at this point. Confusing, and not ideal, but fixing this
isn't in scope for 2.6.24.
...
*ahem*
I don't think anything of what was discussed in this thread would be in
scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs
release 2.6.24).
cu
Adrian
Why not put in 2.6.24 a simple fix for the last known remaining mmconfig
problems in 2.6.24? There has mostly been three bugs related to mmconfig:
- BIOS/hardware: exaggerated MCFG claims: solved long ago
- hardware: buggy CRS+mmconfig chipset: fix included last month
- Linux code: mmconfig incompatible with live BAR-probing: *not fixed*
It would be ironic to not fix the only one that is really confined to
the Linux code.
Everybody more or less agrees *any* patches submitted so far does solve
the known problems, and will not cause regressions. The only long
discussion is about how to best prevent the effect of an imaginary
fourth bug, and by nature that's a controversial topic.
For 2.6.24, if nothing more than a few lines can be done, either make
pci=nommconf the default and add a pci=mmconf option, or/and apply one
of the easiest patch to review i.e.Tony's one, so small I copy it again
below (using 0x40 or 0x100 for the comparison does not really matter,
personally I would change it to 0x100 to be like Ivan's patch, but
either is much better than nothing). Replacing some mmconfig access by
conf1 cannot cause any regression.
Loic
P.S.: with that patch, conf1-less x86 systems requiring mmconfig would
not be supported. But they are like UFOs. They are plenty of them in the
galaxy, but earth sightings are not convincing enough for 2.6.24
support, they can wait 2.6.25.
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 1bf5816..4474979 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -73,7 +73,7 @@ static int pci_mmcfg_read(unsigned int seg, unsigned
int bus,
}
base = get_base_addr(seg, bus, devfn);
-if (!base)
+if ((!base) || (reg 0x40))
return pci_conf1_read(seg,bus,devfn,reg,len,value);
spin_lock_irqsave(pci_config_lock, flags);
@@ -106,7 +106,7 @@ static int pci_mmcfg_write(unsigned int seg,
unsigned int bus,
return -EINVAL;
base = get_base_addr(seg, bus, devfn);
-if (!base)
+if ((!base) || (reg 0x40))
return pci_conf1_write(seg,bus,devfn,reg,len,value);
spin_lock_irqsave(pci_config_lock, flags);
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index 4095e4d..4ad1fcb 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -61,7 +61,7 @@ static int pci_mmcfg_read(unsigned int seg, unsigned
int bus,
}
addr = pci_dev_base(seg, bus, devfn);
-if (!addr)
+if ((!addr) || (reg 0x40))
return pci_conf1_read(seg,bus,devfn,reg,len,value);
switch (len) {
@@ -89,7 +89,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned
int bus,
return -EINVAL;
addr = pci_dev_base(seg, bus, devfn);
-if (!addr)
+if ((!addr) || (reg 0x40))
return pci_conf1_write(seg,bus,devfn,reg,len,value);
switch (len) {
--
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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 11:00:37AM -0500, Loic Prylli wrote: On 1/14/2008 6:04 PM, Adrian Bunk wrote: I thought so, but due to the way that things are initialised, mmconfig happens before conf1. conf1 is known to be usable, but hasn't set raw_pci_ops at this point. Confusing, and not ideal, but fixing this isn't in scope for 2.6.24. ... *ahem* I don't think anything of what was discussed in this thread would be in scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs release 2.6.24). cu Adrian Why not put in 2.6.24 a simple fix for the last known remaining mmconfig problems in 2.6.24? Heh, no, because it is _way_ too late for such a patch that hasn't been tested in any trees, sorry. 2.6.25 is the earliest I'll take such a fix, and if it's really as simple as you say, I'll consider it for the -stable releases for .24 if needed. But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
I agree with Matthew. My preference is Ivan's patch using Loic's proposal. My patch would have tested MMCONFIG before using it, but it didn't fix the problem where the decode of large displacement devices can overlap the MMCONFIG region. Ivan's patch fixes that, and the problem of Northbridges that don't respond to MMCONFIG and as a bonus cleans out some code rendered unnecessary by his patch. Linus is confident that conf1 is not going away for at least the next five years. Matthew Wilcox wrote: On Tue, Jan 15, 2008 at 09:46:43AM -0800, Greg KH wrote: But so far, we have a zillion patches floating around, claiming different things, some with signed-off-bys and others without, so for now, I'll just stick with Arjan's patch in -mm and see if anyone complains about those releases... I complain about Arjan's patch. For reasons which have been adequately gone into already in this thread. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, Jan 15, 2008 at 11:38:42AM -0800, Linus Torvalds wrote: On Tue, 15 Jan 2008, Tony Camuso wrote: Linus is confident that conf1 is not going away for at least the next five years. Not on PC's. Small birds tell me that there can be all these non-PC x86 subarchitectures that may or may not have conf1. Right -- hence my patch on top of Ivan's which removes all the assumptions about conf1 from mmconfig (there are still *references* to conf1 in the mmconfig code, but they'll only be used if conf1 is functional). -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Tue, 15 Jan 2008, Tony Camuso wrote: Linus is confident that conf1 is not going away for at least the next five years. Not on PC's. Small birds tell me that there can be all these non-PC x86 subarchitectures that may or may not have conf1. Linus -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/15/2008 2:38 PM, Linus Torvalds wrote: On Tue, 15 Jan 2008, Tony Camuso wrote: Linus is confident that conf1 is not going away for at least the next five years. Not on PC's. Small birds tell me that there can be all these non-PC x86 subarchitectures that may or may not have conf1. Linus But is there a ACPI-compliant/architecture that only offers mmconfig for configuration-space access and no other fallback method (i.e. no conf1, no bios,...)? 2.6.24 supports mmconfig for: - ACPI-system with MCFG - a couple chipset discovered by conf1 If a system has no conf1, but does not have e820+ACPI+MCFG, or does have some other method than mmconfig, it was already irrelevant in the discussion of Ivan's initial patch in december (because that system was either never supported or not impacted, and we were trying to fix bugs, not introduce support for new class of systems). Maybe Arjan could share his knowledge, and tell us what system he was thinking about (and whether it needed to be supported by 2.6.24) when saying: When (and I'm saying when not if) systems arrive that only have MMCONFIG for some of the devices. Anyway Ivan's patch + Matthew's extensions are handling that non-PC arch. That combination is advocated by at least: Ivan Kokshaysky Matthew Wilcox Tony Camuso Loic Prylli even Arjan's said that while he prefers his patch (saying it's more conservative), he does not see a existing problem with the Ivan/Matthew combination. [ simpler, less ambitious fixes can be forgotten if nothing can be done for 2.6.24, I can understand that choice ] The list of problems I see with Arjan's patch are: - no word on whether the existing Linux driver/pci/pcie/aer code should be converted to opt-in? - mmconfig still needs to be revisited to sort-out the mix of mmconfig+conf1+third-method access. - you cannot test if ext-conf-space is available without taking risks: when pci_enable_ext_config() is called, even legacy-conf-space is switched to the new method. So some administrator action (lspci -v +maybe-other-flag) or some driver action (that can optionally use ext-conf-space but does not *rely* on it) could cause some devices to totally disappear (if some pci hierarchy is handled by mmconfig as a 0x section as seen on many amd machines). Matthew/Ivan will simply in the worst case detect that ext-conf-space is not available in pci_cfg_space_size()), legacy-conf-space will still work (and that 0x section is perfectly *safe* to query, tell me if you need more details of why). - introduce a new user-api, and a new kernel API, while in practice there is no evidence that brings any benefits compared to Ivan/Matthew. IMHO, making pci=nommconf the default behaviour is better than Arjan's patch: for the exaggerated 99.99% users he claims don't need ext-conf-space, that's obviously as good. And many of the others would benefit from the ability to test and optionally use ext-conf-space is available without taking the risk of crashing something, so something else is better for them. With Arjan's patch, in 10 years, we might still have to use an extra option (or some other action) when using lspci to display extended caps, and we would still run the risk of crashing some old machine when doing so (unless maybe a blacklist of some sort will be added, making the newly introduced API completely useless soon, or unless we keep the painful bitmaps in mmconfig potentially ending-up with 3 set of pci-ops). Loic -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. This ignores the "lets make it not matter for the 99% of the users" case. . If the system is capable of conf1, then PCI config access at offsets < 256 should be confined to conf1. This solution is most effective for existing and legacy systems. not "conf1" but "what the platform thinks is the best method for < 256". We have this nice abstraction for the platform to select the best method... we should use it. And still, it's another attempt to get this fixed (well.. it's been 2 years in the coming so far, maybe this will be the last one, maybe it will not be... we'll see I suppose, but it sucks to be a user who doesn't need any of the functionality that the extended config space provides in theory but gets to suffer more of the issues) There actually haven't been that many attempts to "get this fixed". It's been more a) people complaining about it and nothing being done about the problems and b) adding hacks to blindly disable it because of reported problems without root-causing why those problems were showing up. With such approaches no wonder it has not been reliable to try and use MMCONFIG in the past.. -- Robert Hancock Saskatoon, SK, Canada To email, remove "nospam" from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 14, 2008 at 03:52:26PM -0700, Matthew Wilcox wrote: > On Sun, Jan 13, 2008 at 09:01:08AM -0800, Arjan van de Ven wrote: >... > > > - pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0xce, 2, ); > > > + pci_direct_conf1.read(0, 0, PCI_DEVFN(0,0), 0xce, 2, ); > > > > couldn't this (at least in some next patch) use the vector if it exists? > > I thought so, but due to the way that things are initialised, mmconfig > happens before conf1. conf1 is known to be usable, but hasn't set > raw_pci_ops at this point. Confusing, and not ideal, but fixing this > isn't in scope for 2.6.24. >... *ahem* I don't think anything of what was discussed in this thread would be in scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs release 2.6.24). cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, Jan 13, 2008 at 09:01:08AM -0800, Arjan van de Ven wrote:
> would be nice the "reg > 256 && raw_pci_Ext_ops==NULL" case would just
> call the raw_pci_ops-> pointer, to give that a chance of refusal
> (but I guess that shouldn't really happen)
We don't have a situation where that can happen -- all the other current
config methods on x86 are limited to <256 bytes. If we get another
method, we can revisit this.
> > - pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0xce, 2, );
> > + pci_direct_conf1.read(0, 0, PCI_DEVFN(0,0), 0xce, 2, );
>
> couldn't this (at least in some next patch) use the vector if it exists?
I thought so, but due to the way that things are initialised, mmconfig
happens before conf1. conf1 is known to be usable, but hasn't set
raw_pci_ops at this point. Confusing, and not ideal, but fixing this
isn't in scope for 2.6.24.
> > printk(KERN_INFO "PCI: Using MMCONFIG\n");
> > raw_pci_ops = _mmcfg;
> > + raw_pci_ext_ops = _mmcfg;
>
> why set BOTH vectors? you probably ONLY want to set the ext one, so
> that calls to the lower 256 go to the original
I had misunderstood how the x86 pci init happened -- I thought conf1
would override this. It doesn't.
The following patch has been tested on ia64, x86 and x86_64.
It successfully avoids the hang on my G33 machine (ie BAR probing
problem), when applied *after* Ivan's patch.
Greg, please apply Ivan's patch and then this one.
---
PCI: Rationalise raw_pci_ops
Replace raw_pci_ops with raw_pci_read() and raw_pci_write(). This is
a better interface for ACPI, ia64 and now x86.
Make pci_raw_ops private to the x86 arch, and use it to implement
raw_pci_read/write. Add a raw_pci_ext_ops for extended config space.
Signed-off-by: Matthew Wilcox <[EMAIL PROTECTED]>
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 488e48a..8fd7e82 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -43,8 +43,7 @@
#define PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg) \
(((u64) seg << 28) | (bus << 20) | (devfn << 12) | (reg))
-static int
-pci_sal_read (unsigned int seg, unsigned int bus, unsigned int devfn,
+int raw_pci_read(unsigned int seg, unsigned int bus, unsigned int devfn,
int reg, int len, u32 *value)
{
u64 addr, data = 0;
@@ -68,8 +67,7 @@ pci_sal_read (unsigned int seg, unsigned int bus, unsigned
int devfn,
return 0;
}
-static int
-pci_sal_write (unsigned int seg, unsigned int bus, unsigned int devfn,
+int raw_pci_write(unsigned int seg, unsigned int bus, unsigned int devfn,
int reg, int len, u32 value)
{
u64 addr;
@@ -91,24 +89,17 @@ pci_sal_write (unsigned int seg, unsigned int bus, unsigned
int devfn,
return 0;
}
-static struct pci_raw_ops pci_sal_ops = {
- .read = pci_sal_read,
- .write =pci_sal_write
-};
-
-struct pci_raw_ops *raw_pci_ops = _sal_ops;
-
-static int
-pci_read (struct pci_bus *bus, unsigned int devfn, int where, int size, u32
*value)
+static int pci_read(struct pci_bus *bus, unsigned int devfn, int where,
+ int size, u32 *value)
{
- return raw_pci_ops->read(pci_domain_nr(bus), bus->number,
+ return raw_pci_read(pci_domain_nr(bus), bus->number,
devfn, where, size, value);
}
-static int
-pci_write (struct pci_bus *bus, unsigned int devfn, int where, int size, u32
value)
+static int pci_write(struct pci_bus *bus, unsigned int devfn, int where,
+ int size, u32 value)
{
- return raw_pci_ops->write(pci_domain_nr(bus), bus->number,
+ return raw_pci_write(pci_domain_nr(bus), bus->number,
devfn, where, size, value);
}
diff --git a/arch/ia64/sn/pci/tioce_provider.c
b/arch/ia64/sn/pci/tioce_provider.c
index e1a3e19..999f14f 100644
--- a/arch/ia64/sn/pci/tioce_provider.c
+++ b/arch/ia64/sn/pci/tioce_provider.c
@@ -752,13 +752,13 @@ tioce_kern_init(struct tioce_common *tioce_common)
* Determine the secondary bus number of the port2 logical PPB.
* This is used to decide whether a given pci device resides on
* port1 or port2. Note: We don't have enough plumbing set up
-* here to use pci_read_config_xxx() so use the raw_pci_ops vector.
+* here to use pci_read_config_xxx() so use raw_pci_read().
*/
seg = tioce_common->ce_pcibus.bs_persist_segment;
bus = tioce_common->ce_pcibus.bs_persist_busnum;
- raw_pci_ops->read(seg, bus, PCI_DEVFN(2, 0), PCI_SECONDARY_BUS, 1,);
+ raw_pci_read(seg, bus, PCI_DEVFN(2, 0), PCI_SECONDARY_BUS, 1,);
tioce_kern->ce_port1_secondary = (u8) tmp;
/*
@@ -799,11 +799,11 @@ tioce_kern_init(struct tioce_common *tioce_common)
/* mem base/limit */
- raw_pci_ops->read(seg, bus, PCI_DEVFN(dev, 0),
+ raw_pci_read(seg, bus,
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: it's not pci_enable_mmconf(), it's pci_enable_extended_config_space... it's independent of the mechanism! Arjan, you would be foisting this call on device drivers running on arches that don't need any such distinction between extended config space and < 256 bytes. I still think it's a bad policy. Let's endeavor to confine arch-specific quirks to the arch-specific code. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008 10:23:14 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > Arjan van de Ven wrote: > > On Mon, 14 Jan 2008 08:01:01 -0500 > > Tony Camuso <[EMAIL PROTECTED]> wrote: > >> > >> If we're going to differentiate MMCONFIG from > >> some other access mechanism, it only needs to be done at the > >> Northbridge level. Devices are electrically ignorant of the > >> protocol used between CPU and Northbridge to get the Northbridge > >> to assert config cycles on the bus. > > > > Again this is about having systems that don't need extended config > > space not use it. At all. The only way to do that is have the > > drivers say they need it, and not use it otherwise. It has NOTHING > > to do with how things are wired up. It's pure a kernel level policy > > decision about whether to use extended config space AT ALL. > > > > The problem with compelling device drivers to determine the PCI > config mechanism is that it must be forced upon arches that > have no PCI configuration quirks or don't even use the same > PCI config mechanisms as x86. it's not pci_enable_mmconf(), it's pci_enable_extended_config_space... it's independent of the mechanism! > > I don't think that's a good policy. > > Better to confine arch-specific quirks to the arch-specific code > whenever possible. > -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Mon, 14 Jan 2008 08:01:01 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: >> If we're going to differentiate MMCONFIG from some other access mechanism, it only needs to be done at the Northbridge level. Devices are electrically ignorant of the protocol used between CPU and Northbridge to get the Northbridge to assert config cycles on the bus. Again this is about having systems that don't need extended config space not use it. At all. The only way to do that is have the drivers say they need it, and not use it otherwise. It has NOTHING to do with how things are wired up. It's pure a kernel level policy decision about whether to use extended config space AT ALL. The problem with compelling device drivers to determine the PCI config mechanism is that it must be forced upon arches that have no PCI configuration quirks or don't even use the same PCI config mechanisms as x86. I don't think that's a good policy. Better to confine arch-specific quirks to the arch-specific code whenever possible. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008 08:01:01 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > Arjan van de Ven wrote: > > On Sun, 13 Jan 2008 22:29:23 -0500 > > Tony Camuso <[EMAIL PROTECTED]> wrote: > > > >> . There is no need to provide different PCI config access > >>mechanisms at device granularity, since the PCI config access > >>mechanism between the CPU and the Northbridge is opaque to > >>the devices. PCI config mechanisms only need to differ at > >>the Northbridge level. > > > > This ignores the "lets make it not matter for the 99% of the users" > > case. > > I don't understand. That;s clear :) > If we're going to differentiate MMCONFIG from > some other access mechanism, it only needs to be done at the > Northbridge level. Devices are electrically ignorant of the protocol > used between CPU and Northbridge to get the Northbridge to assert > config cycles on the bus. Again this is about having systems that don't need extended config space not use it. At all. The only way to do that is have the drivers say they need it, and not use it otherwise. It has NOTHING to do with how things are wired up. It's pure a kernel level policy decision about whether to use extended config space AT ALL. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. This ignores the "lets make it not matter for the 99% of the users" case. I don't understand. If we're going to differentiate MMCONFIG from some other access mechanism, it only needs to be done at the Northbridge level. Devices are electrically ignorant of the protocol used between CPU and Northbridge to get the Northbridge to assert config cycles on the bus. . If the system is capable of conf1, then PCI config access at offsets < 256 should be confined to conf1. This solution is most effective for existing and legacy systems. not "conf1" but "what the platform thinks is the best method for < 256". We have this nice abstraction for the platform to select the best method... we should use it. Agreed. So we have Loic and Ivan's patch limiting MMCONFIG accesses to offsets >= 256. And we have Matthew's patch that abstracts the method for config accesses to offsets < 256. I beleive Matthew has already tested these patches for functionality on x86. All that's needed is to test for regressions on other arches. Is there any interest in providing the following? 1. The ability to use MMCONFIG for all accesses on systems that have no problems with MMCONFIG. 2. For systems using both PCI and PCI express, testing each bus for MMCONFIG compliance, to determine whether MMCONFIG can be used for all config accesses or whether the bus must be limited all to the method abstracted for offsets < 256. Or does that introduce unnecessary complications? -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
> > even when designed for Redmond products. > > I find it hard to believe that even they have their drivers do PCI config > access via ports directly from the drivers, > and especially in driver reference code... Microsoft may not but the standard of Taiwanese driver code (and by reference I mean vendor reference not OS supplier reference) is not always great. When you have weeks to write a driver for a product with a 6 month sales lifetime I guess there are other pressures on driver authors. Easy enough for Intel to analyse though. Alan -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
even when designed for Redmond products. I find it hard to believe that even they have their drivers do PCI config access via ports directly from the drivers, and especially in driver reference code... Microsoft may not but the standard of Taiwanese driver code (and by reference I mean vendor reference not OS supplier reference) is not always great. When you have weeks to write a driver for a product with a 6 month sales lifetime I guess there are other pressures on driver authors. Easy enough for Intel to analyse though. Alan -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso [EMAIL PROTECTED] wrote: . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. This ignores the lets make it not matter for the 99% of the users case. I don't understand. If we're going to differentiate MMCONFIG from some other access mechanism, it only needs to be done at the Northbridge level. Devices are electrically ignorant of the protocol used between CPU and Northbridge to get the Northbridge to assert config cycles on the bus. . If the system is capable of conf1, then PCI config access at offsets 256 should be confined to conf1. This solution is most effective for existing and legacy systems. not conf1 but what the platform thinks is the best method for 256. We have this nice abstraction for the platform to select the best method... we should use it. Agreed. So we have Loic and Ivan's patch limiting MMCONFIG accesses to offsets = 256. And we have Matthew's patch that abstracts the method for config accesses to offsets 256. I beleive Matthew has already tested these patches for functionality on x86. All that's needed is to test for regressions on other arches. Is there any interest in providing the following? 1. The ability to use MMCONFIG for all accesses on systems that have no problems with MMCONFIG. 2. For systems using both PCI and PCI express, testing each bus for MMCONFIG compliance, to determine whether MMCONFIG can be used for all config accesses or whether the bus must be limited all to the method abstracted for offsets 256. Or does that introduce unnecessary complications? -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008 08:01:01 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Arjan van de Ven wrote: On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso [EMAIL PROTECTED] wrote: . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. This ignores the lets make it not matter for the 99% of the users case. I don't understand. That;s clear :) If we're going to differentiate MMCONFIG from some other access mechanism, it only needs to be done at the Northbridge level. Devices are electrically ignorant of the protocol used between CPU and Northbridge to get the Northbridge to assert config cycles on the bus. Again this is about having systems that don't need extended config space not use it. At all. The only way to do that is have the drivers say they need it, and not use it otherwise. It has NOTHING to do with how things are wired up. It's pure a kernel level policy decision about whether to use extended config space AT ALL. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: it's not pci_enable_mmconf(), it's pci_enable_extended_config_space... it's independent of the mechanism! Arjan, you would be foisting this call on device drivers running on arches that don't need any such distinction between extended config space and 256 bytes. I still think it's a bad policy. Let's endeavor to confine arch-specific quirks to the arch-specific code. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008 10:23:14 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Arjan van de Ven wrote: On Mon, 14 Jan 2008 08:01:01 -0500 Tony Camuso [EMAIL PROTECTED] wrote: If we're going to differentiate MMCONFIG from some other access mechanism, it only needs to be done at the Northbridge level. Devices are electrically ignorant of the protocol used between CPU and Northbridge to get the Northbridge to assert config cycles on the bus. Again this is about having systems that don't need extended config space not use it. At all. The only way to do that is have the drivers say they need it, and not use it otherwise. It has NOTHING to do with how things are wired up. It's pure a kernel level policy decision about whether to use extended config space AT ALL. The problem with compelling device drivers to determine the PCI config mechanism is that it must be forced upon arches that have no PCI configuration quirks or don't even use the same PCI config mechanisms as x86. it's not pci_enable_mmconf(), it's pci_enable_extended_config_space... it's independent of the mechanism! I don't think that's a good policy. Better to confine arch-specific quirks to the arch-specific code whenever possible. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, Jan 13, 2008 at 09:01:08AM -0800, Arjan van de Ven wrote:
would be nice the reg 256 raw_pci_Ext_ops==NULL case would just
call the raw_pci_ops- pointer, to give that a chance of refusal
(but I guess that shouldn't really happen)
We don't have a situation where that can happen -- all the other current
config methods on x86 are limited to 256 bytes. If we get another
method, we can revisit this.
- pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0xce, 2, win);
+ pci_direct_conf1.read(0, 0, PCI_DEVFN(0,0), 0xce, 2, win);
couldn't this (at least in some next patch) use the vector if it exists?
I thought so, but due to the way that things are initialised, mmconfig
happens before conf1. conf1 is known to be usable, but hasn't set
raw_pci_ops at this point. Confusing, and not ideal, but fixing this
isn't in scope for 2.6.24.
printk(KERN_INFO PCI: Using MMCONFIG\n);
raw_pci_ops = pci_mmcfg;
+ raw_pci_ext_ops = pci_mmcfg;
why set BOTH vectors? you probably ONLY want to set the ext one, so
that calls to the lower 256 go to the original
I had misunderstood how the x86 pci init happened -- I thought conf1
would override this. It doesn't.
The following patch has been tested on ia64, x86 and x86_64.
It successfully avoids the hang on my G33 machine (ie BAR probing
problem), when applied *after* Ivan's patch.
Greg, please apply Ivan's patch and then this one.
---
PCI: Rationalise raw_pci_ops
Replace raw_pci_ops with raw_pci_read() and raw_pci_write(). This is
a better interface for ACPI, ia64 and now x86.
Make pci_raw_ops private to the x86 arch, and use it to implement
raw_pci_read/write. Add a raw_pci_ext_ops for extended config space.
Signed-off-by: Matthew Wilcox [EMAIL PROTECTED]
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 488e48a..8fd7e82 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -43,8 +43,7 @@
#define PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg) \
(((u64) seg 28) | (bus 20) | (devfn 12) | (reg))
-static int
-pci_sal_read (unsigned int seg, unsigned int bus, unsigned int devfn,
+int raw_pci_read(unsigned int seg, unsigned int bus, unsigned int devfn,
int reg, int len, u32 *value)
{
u64 addr, data = 0;
@@ -68,8 +67,7 @@ pci_sal_read (unsigned int seg, unsigned int bus, unsigned
int devfn,
return 0;
}
-static int
-pci_sal_write (unsigned int seg, unsigned int bus, unsigned int devfn,
+int raw_pci_write(unsigned int seg, unsigned int bus, unsigned int devfn,
int reg, int len, u32 value)
{
u64 addr;
@@ -91,24 +89,17 @@ pci_sal_write (unsigned int seg, unsigned int bus, unsigned
int devfn,
return 0;
}
-static struct pci_raw_ops pci_sal_ops = {
- .read = pci_sal_read,
- .write =pci_sal_write
-};
-
-struct pci_raw_ops *raw_pci_ops = pci_sal_ops;
-
-static int
-pci_read (struct pci_bus *bus, unsigned int devfn, int where, int size, u32
*value)
+static int pci_read(struct pci_bus *bus, unsigned int devfn, int where,
+ int size, u32 *value)
{
- return raw_pci_ops-read(pci_domain_nr(bus), bus-number,
+ return raw_pci_read(pci_domain_nr(bus), bus-number,
devfn, where, size, value);
}
-static int
-pci_write (struct pci_bus *bus, unsigned int devfn, int where, int size, u32
value)
+static int pci_write(struct pci_bus *bus, unsigned int devfn, int where,
+ int size, u32 value)
{
- return raw_pci_ops-write(pci_domain_nr(bus), bus-number,
+ return raw_pci_write(pci_domain_nr(bus), bus-number,
devfn, where, size, value);
}
diff --git a/arch/ia64/sn/pci/tioce_provider.c
b/arch/ia64/sn/pci/tioce_provider.c
index e1a3e19..999f14f 100644
--- a/arch/ia64/sn/pci/tioce_provider.c
+++ b/arch/ia64/sn/pci/tioce_provider.c
@@ -752,13 +752,13 @@ tioce_kern_init(struct tioce_common *tioce_common)
* Determine the secondary bus number of the port2 logical PPB.
* This is used to decide whether a given pci device resides on
* port1 or port2. Note: We don't have enough plumbing set up
-* here to use pci_read_config_xxx() so use the raw_pci_ops vector.
+* here to use pci_read_config_xxx() so use raw_pci_read().
*/
seg = tioce_common-ce_pcibus.bs_persist_segment;
bus = tioce_common-ce_pcibus.bs_persist_busnum;
- raw_pci_ops-read(seg, bus, PCI_DEVFN(2, 0), PCI_SECONDARY_BUS, 1,tmp);
+ raw_pci_read(seg, bus, PCI_DEVFN(2, 0), PCI_SECONDARY_BUS, 1,tmp);
tioce_kern-ce_port1_secondary = (u8) tmp;
/*
@@ -799,11 +799,11 @@ tioce_kern_init(struct tioce_common *tioce_common)
/* mem base/limit */
- raw_pci_ops-read(seg, bus, PCI_DEVFN(dev, 0),
+ raw_pci_read(seg, bus, PCI_DEVFN(dev, 0),
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, Jan 14, 2008 at 03:52:26PM -0700, Matthew Wilcox wrote: On Sun, Jan 13, 2008 at 09:01:08AM -0800, Arjan van de Ven wrote: ... - pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0xce, 2, win); + pci_direct_conf1.read(0, 0, PCI_DEVFN(0,0), 0xce, 2, win); couldn't this (at least in some next patch) use the vector if it exists? I thought so, but due to the way that things are initialised, mmconfig happens before conf1. conf1 is known to be usable, but hasn't set raw_pci_ops at this point. Confusing, and not ideal, but fixing this isn't in scope for 2.6.24. ... *ahem* I don't think anything of what was discussed in this thread would be in scope for 2.6.24 (unless Linus wants to let the bunny that brings eggs release 2.6.24). cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso [EMAIL PROTECTED] wrote: . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. This ignores the lets make it not matter for the 99% of the users case. . If the system is capable of conf1, then PCI config access at offsets 256 should be confined to conf1. This solution is most effective for existing and legacy systems. not conf1 but what the platform thinks is the best method for 256. We have this nice abstraction for the platform to select the best method... we should use it. And still, it's another attempt to get this fixed (well.. it's been 2 years in the coming so far, maybe this will be the last one, maybe it will not be... we'll see I suppose, but it sucks to be a user who doesn't need any of the functionality that the extended config space provides in theory but gets to suffer more of the issues) There actually haven't been that many attempts to get this fixed. It's been more a) people complaining about it and nothing being done about the problems and b) adding hacks to blindly disable it because of reported problems without root-causing why those problems were showing up. With such approaches no wonder it has not been reliable to try and use MMCONFIG in the past.. -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008, Alan Cox wrote: > > > > As someone gently pointed out to me, you are in a position to know this, > > so I probably am wrong. > > I suspect Arjan is wrong. It might be some Intel agenda but I still see > fairly new driver reference code that is hardcoding port accesses even > when designed for Redmond products. Agreed. I suspect that the likelihood of conf1 accesses going away in the next five years is slim to none. Linus -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > . There is no need to provide different PCI config access >mechanisms at device granularity, since the PCI config access >mechanism between the CPU and the Northbridge is opaque to >the devices. PCI config mechanisms only need to differ at >the Northbridge level. This ignores the "lets make it not matter for the 99% of the users" case. > > . If the system is capable of conf1, then PCI config access >at offsets < 256 should be confined to conf1. This solution >is most effective for existing and legacy systems. not "conf1" but "what the platform thinks is the best method for < 256". We have this nice abstraction for the platform to select the best method... we should use it. And still, it's another attempt to get this fixed (well.. it's been 2 years in the coming so far, maybe this will be the last one, maybe it will not be... we'll see I suppose, but it sucks to be a user who doesn't need any of the functionality that the extended config space provides in theory but gets to suffer more of the issues) I'm all in favor of making this more reliable, but really.. we've thought it was fixed time and time again over the last two years. Please consider limiting the scope of the damage as well. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
To all ... Well, here is what I perceive we've got so far. . Some PCI Northbridges do not work with MMCONFIG. . Some PCI BARs can overlap the MMCONFIG area during bus sizing. It is hoped that new BIOSes will locate MMCONFIG in an area safely out of the way of bus sizing code, but there can be no guarantees. . conf1 is going away in newer x86 implementations in the not too distant future. . The PCI express spec requires platforms to provide access to the extended config area, and there are express devices today using that area for AER. . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. . We have a flurry of patches all claiming to solve all or some of these problems. Arjan, I realize it may not be possible for you to answer this question, but I feel compelled to ask it anyway. Is it possible that future x86 architectures will be implementing a SAL-like interface to abstract PCI config access altogether? Or can we condense these patches down to a set that does the following? . If the system is capable of conf1, then PCI config access at offsets < 256 should be confined to conf1. This solution is most effective for existing and legacy systems. . If the system does not support MMCONFIG, of if MMCONFIG is not working, then accesses to offsets > 256 return -1 and an error status. . For systems, where the conf1 mechanism is NOT available, then MMCONFIG should be the PCI access mechanism for all offsets. For such systems, we must assume that the BIOS has become smart enough to locate MMCONFIG in a region safe from encroachment by bus sizing code. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008 00:54:34 + Alan Cox <[EMAIL PROTECTED]> wrote: > On Sun, 13 Jan 2008 16:28:08 -0500 > Tony Camuso <[EMAIL PROTECTED]> wrote: > > > Arjan van de Ven wrote: > > > > >> The PCI spec provides for conf1 as an architected solution. It's > > >> not going away, and especially not in x86 land where Port IO is > > >> built-in to the CPU. > > > I suspect Arjan is wrong. It might be some Intel agenda but I still > see fairly new driver reference code that is hardcoding port accesses > even when designed for Redmond products. I find it hard to believe that even they have their drivers do PCI config access via ports directly from the drivers, and especially in driver reference code... -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 16:28:08 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > Arjan van de Ven wrote: > > >> The PCI spec provides for conf1 as an architected solution. It's not > >> going away, and especially not in x86 land where Port IO is built-in > >> to the CPU. > > > > again sadly you're wrong. > > > > As someone gently pointed out to me, you are in a position to know this, > so I probably am wrong. I suspect Arjan is wrong. It might be some Intel agenda but I still see fairly new driver reference code that is hardcoding port accesses even when designed for Redmond products. Alan -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. again sadly you're wrong. As someone gently pointed out to me, you are in a position to know this, so I probably am wrong. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/13/2008 3:43 PM, Matthew Wilcox wrote: On Sun, Jan 13, 2008 at 10:41:24AM -0800, Arjan van de Ven wrote: Note: There is not a 100% overlap between "need" and "will not be used in the patches that use legacy for < 256". In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an "lspci -vx" from automated scripts). I believe you to be mistaken in this belief. If you take Ivan's patch, conf1 is used for all accesses below 256 bytes. lspci -x only dumps config space up to 64 bytes; lspci - is needed to show extended pci config space. I agree with Arjan about that "not a 100% overlap". It is about the extra ext-conf-space access done while probing in drivers/pci/probe.c: dev->cfg_size = pci_cfg_space_size(dev); (and lspci -v will also query/show the list of extended-caps for pci-x/pcie-x devices that have some, provided the kernel can access ext-conf-space). With Ivan's patch, that line would still cause one extended-conf-space access at offset 256 for pcie/pci-x2 devices (to check the ability to query ext-space). Arjan "opt-in" patch would prevent that extra access. IMHO that access is OK and harmless in all cases, we are already protected by MCFG/e820 checks, but I agree one can express a different opinion based on trying to prevent "never-seen/potential" hardware/BIOS bugs. FWIW it is also there that I was suggested to exclude PCI-X2 devices (when restricted to pcie, that access while probing cannot even cause the harmless master-abort/0x), but there is a small trade-off. Loic -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/13/2008 1:41 PM, Arjan van de Ven wrote: On Sun, 13 Jan 2008 13:23:35 -0500 Loic Prylli <[EMAIL PROTECTED]> wrote: Matthew pointed a patch that basically does what you suggested; only one comment on your mail left after that: 2) the pci_enable_ext_config() function and dev->ext_cfg_space field, sysfs interface should be removed from the patch. There has never been a problem reporting crashes or any undefined behaviour while trying to access ext-conf-space, all the problems where *using mmconfig to access legacy-conf-space*. This entirely misses the point of why I made the patch. The point is NOT that devices are buggy. The point is that right now, 99.99% of the machines out there do NOT need extended config space (no matter how it gets accessed), The point of my patch was to make people who don't need extended config space, not have to deal with it anymore. I think I got your point the first time, and I agree it is sound. But in my subjective and biased opinion, I just think ext-conf-space is already useful and widespread enough (being used is not the same as being strictly required for basic operation) for your proposed tradeoff to not be optimal (protecting against "future/non-proven" hardware bugs, i.e. bringing non-proven benefits, at the expense of making life harder for ext-conf-space users while bringing additional extra API/code). To take an example from the linux tree: the driver/pci/pcie/aer code uses ext-conf-space for every pcie-root (currently several distributions enable it by default), does it mean opt-in would be automatically activated for most pcie hierarchies (defeating most of the benefits of being opt-in), or we just disable that code by default? Does lspci -v will automatically opt-in all pcie (right now by default it tries to list the extended-capabilities for pcie and pcix), or do we now require manual explicit sysfs operations to get the whole thing? Is is an additional flag to lspci (if so will that flag also apply to pcix, possibly causing a crash for lspci -v - on some machines). Note: There is not a 100% overlap between "need" and "will not be used in the patches that use legacy for < 256". In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an "lspci -vx" from automated scripts). To go one step your direction, I have already argued in a couple of emails that I would prefer to not implement ext-conf-space access for any PCI-X devices (removing PCI-X2 from pci_ext_cfg_size), because there we are trying to support devices that we don't really know exists or will ever exists. And protecting against "unproven bugs" makes more sense when it only removes "unproven benefits". Is that a problem? We've had 2 years of mess, with one not-enough patch after another. There still are problems TODAY (eg im 2.6.24-rc7). The patch that falls back to an alternative method for below 256 is no doubt a step in the right direction. (although I'm not all that happy about mixing access types, it's not provably incorrect) Is it enough? I'm not sure. FWIW, I have in my tree a patch almost identical to Ivan's dated "December 2005". Because of the constant activity on the mmconfig front (that I thought would make it obsolete), I never took the effort of suggesting it before one month ago (I am not a regular user of linux-kernel). I admit nobody else should view it that way, but for me rather than the last attempt at fixing mmconfig, it's a patch first used two years ago that would have arguably prevented all problems that have been reported since then. Besides, recent mails show that hypothetically, we could even not change anything to the existing conf-space code, since the only known bug remaining is the one associated with bar probing and could be adressed by: http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24-rc6/2.6.24-rc6-mm1/broken-out/pci-disable-decoding-during-sizing-of-bars.patch [ Thanks to Robert hancok and Grant Grundler for explaining to me the history of bar-probing last month ] Even if that bar-probing patch was applied (maybe it needs to be more combat-proven), by default, it still seems better to not use mmconfig for legacy-conf-space access, but going two extra precaution steps beyond what seems necessary might be excessive. Only time can tell I suppose, but the risk side is that if it is not enough, users who don't need the extended config space for functionality will suffer the bugs AGAIN. You can indeed never exclude 100% that possibility, but if they see a problem again, it is likely to be a new category of hardware/BIOS bugs never seen before. Loic -- 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
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, Jan 13, 2008 at 10:41:24AM -0800, Arjan van de Ven wrote: > Note: There is not a 100% overlap between "need" and "will not be used in > the patches that use legacy for < 256". In the other patches posted, > extended config space will be used in cases where it won't be with my > patch. (Most obvious one is an "lspci -vx" from automated scripts). I believe you to be mistaken in this belief. If you take Ivan's patch, conf1 is used for all accesses below 256 bytes. lspci -x only dumps config space up to 64 bytes; lspci - is needed to show extended pci config space. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 13:23:35 -0500 Loic Prylli <[EMAIL PROTECTED]> wrote: Matthew pointed a patch that basically does what you suggested; only one comment on your mail left after that: > > 2) the pci_enable_ext_config() function and dev->ext_cfg_space field, > sysfs interface should be removed from the patch. There has never > been a problem reporting crashes or any undefined behaviour while > trying to access ext-conf-space, all the problems where *using > mmconfig to access legacy-conf-space*. This entirely misses the point of why I made the patch. The point is NOT that devices are buggy. The point is that right now, 99.99% of the machines out there do NOT need extended config space (no matter how it gets accessed), yet at the same time they suffered from it's issues for... what 2 years now? The point of my patch was to make people who don't need extended config space, not have to deal with it anymore. Note: There is not a 100% overlap between "need" and "will not be used in the patches that use legacy for < 256". In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an "lspci -vx" from automated scripts). Is that a problem? We've had 2 years of mess, with one not-enough patch after another. There still are problems TODAY (eg im 2.6.24-rc7). The patch that falls back to an alternative method for below 256 is no doubt a step in the right direction. (although I'm not all that happy about mixing access types, it's not provably incorrect) Is it enough? I'm not sure. Only time can tell I suppose, but the risk side is that if it is not enough, users who don't need the extended config space for functionality will suffer the bugs AGAIN. So in short, my approach was NOT about "fix PCI", it is about "fix the user experience". It's a stopgap for sure, until the underlying mechanism gets reliable. It's been 2 years. maybe this next step is "it", maybe it isn't. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/12/2008 12:45 PM, Arjan van de Ven wrote: On Sat, 12 Jan 2008 17:40:30 +0300 Ivan Kokshaysky <[EMAIL PROTECTED]> wrote: + if (reg < 256) + return pci_conf1_read(seg,bus,devfn,reg,len,value); + btw this is my main objection to your patch; it intertwines the conf1 and mmconfig code even more. When (and I'm saying "when" not "if") systems arrive that only have MMCONFIG for some of the devices, we'll have to detangle this again, and I'm really not looking forward to that. conf1 has been a hardcoded dependencies of mmconfig for years. Ivan's patch does not make it worse (in fact it considerably simplifies that code, making it easier to untangle later). IMHO, either your patch or Ivan's can be a good base, but: 1) For your remark above to be given any consideration, your patch should be modified to remove the hardcoded conf1 from the *current* mmconfig code, otherwise we end up with 3 set of ops (mmconfig + conf1+ a possible third set of operations) intertwined in a confusing manner. And removing that dependency is not a straightforward operation unless you also do 2): 2) the pci_enable_ext_config() function and dev->ext_cfg_space field, sysfs interface should be removed from the patch. There has never been a problem reporting crashes or any undefined behaviour while trying to access ext-conf-space, all the problems where *using mmconfig to access legacy-conf-space*. The "if (dev->cfg_space_ext > 0)" checks can instead be replaced by "if (reg >= 256)". Otherwise when using per-device explicit enabling, just *checking* whether ext-conf-space is available by calling pci_enable_ext_config(), will make some of the old problems of *loosing legacy conf-space* come back: you would have introduced a new user-space and kernel API while only solving half the problems, not a good deal. if you do 1) and 2), then you really support the good properties you claimed: - You can use mmconfig for ext-space and something else for legacy-space. - You can use mmconfig for everything (for instance if conf1 is not implemented). Of course it is as straightforward to modify Ivan's patch to also have the same properties. Loic Loic -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 07:43:11 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: > Arjan van de Ven wrote: > > On Sat, 12 Jan 2008 20:36:59 -0500 > > Tony Camuso <[EMAIL PROTECTED]> wrote: > > > > > > Just about NOBODY has devices that need the extended config space. > > At all. > > The PCI express spec requires the platform to provide access to this > space for express-compliance. PLATFORM not OS :) Windows isn't using it in the server space, and only in the client space it recently started considering it. > More devices will be using this space > as express becomes the dominant IO bus technology. sure in like 2009 maybe. > Which is why Loic's proposal and Ivan's implementation of it is so > elegant. It solves all these problems in one sweep, and eliminates > the code rendered cruft by Ivan's patch. A two-fer, by my reckoning. > > >> In other words, for x86, I don't think we need to worry about Port > >> IO config access ever going away at all. > > > > You're wrong there. Sad to say, but you're wrong there. > > > > The PCI spec provides for conf1 as an architected solution. It's not > going away, and especially not in x86 land where Port IO is built-in > to the CPU. again sadly you're wrong. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
as a general thing I like where this patch is going
On Sun, 13 Jan 2008 00:24:15 -0700
Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> +
> +int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int
> devfn,
> + int reg, int len,
> u32 *val) +{
> + if (reg < 256)
> + return raw_pci_ops->read(domain, bus, devfn, reg,
> len, val);
> + if (raw_pci_ext_ops)
> + return raw_pci_ext_ops->read(domain, bus, devfn,
> reg, len, val);
> + return -EINVAL;
would be nice the "reg > 256 && raw_pci_Ext_ops==NULL" case would just
call the raw_pci_ops-> pointer, to give that a chance of refusal
(but I guess that shouldn't really happen)
> --- a/arch/x86/pci/mmconfig-shared.c
> +++ b/arch/x86/pci/mmconfig-shared.c
> @@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resources_inserted;
> static const char __init *pci_mmcfg_e7520(void)
> {
> u32 win;
> - pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0xce, 2, );
> + pci_direct_conf1.read(0, 0, PCI_DEVFN(0,0), 0xce, 2, );
couldn't this (at least in some next patch) use the vector if it exists?
\
> @@ -140,5 +134,6 @@ int __init pci_mmcfg_arch_init(void)
> {
> printk(KERN_INFO "PCI: Using MMCONFIG\n");
> raw_pci_ops = _mmcfg;
> + raw_pci_ext_ops = _mmcfg;
why set BOTH vectors? you probably ONLY want to set the ext one, so
that calls to the lower 256 go to the original
--
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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Sat, 12 Jan 2008 20:36:59 -0500 Tony Camuso <[EMAIL PROTECTED]> wrote: Just about NOBODY has devices that need the extended config space. At all. The PCI express spec requires the platform to provide access to this space for express-compliance. More devices will be using this space as express becomes the dominant IO bus technology. As far as the device is concerned, after the Northbridge translates the config access into PCI bus cycles, the device has no idea what mechanism drove the Northbridge to the translation. Wanne bet there'll be devices that screw this up? THere's devices that even screwed up the 64-256 region after all. There may have been devices that incorrectly applied the PCI spec to various fields in the header, I'll grant you that. However, there is no way a device can determine electrically whether the Northbridge received Port IO or MMCONFIG cycles. This is between the CPU and the Northbridge and is utterly opaque to the devices on the bus. The patch I devised concerned itself with Northbridges and separated MMCONFIG-compliant buses from those that could not handle MMCONFIG. THis kind of patchup has been going on for the better part of a year (well 2 years) by now and it's STILL NOT ENOUGH, as you can see by the more patchups that have been proposed as "alternative" to my approach. Which is why Loic's proposal and Ivan's implementation of it is so elegant. It solves all these problems in one sweep, and eliminates the code rendered cruft by Ivan's patch. A two-fer, by my reckoning. In other words, for x86, I don't think we need to worry about Port IO config access ever going away at all. You're wrong there. Sad to say, but you're wrong there. The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: On Sat, 12 Jan 2008 20:36:59 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Just about NOBODY has devices that need the extended config space. At all. The PCI express spec requires the platform to provide access to this space for express-compliance. More devices will be using this space as express becomes the dominant IO bus technology. As far as the device is concerned, after the Northbridge translates the config access into PCI bus cycles, the device has no idea what mechanism drove the Northbridge to the translation. Wanne bet there'll be devices that screw this up? THere's devices that even screwed up the 64-256 region after all. There may have been devices that incorrectly applied the PCI spec to various fields in the header, I'll grant you that. However, there is no way a device can determine electrically whether the Northbridge received Port IO or MMCONFIG cycles. This is between the CPU and the Northbridge and is utterly opaque to the devices on the bus. The patch I devised concerned itself with Northbridges and separated MMCONFIG-compliant buses from those that could not handle MMCONFIG. THis kind of patchup has been going on for the better part of a year (well 2 years) by now and it's STILL NOT ENOUGH, as you can see by the more patchups that have been proposed as alternative to my approach. Which is why Loic's proposal and Ivan's implementation of it is so elegant. It solves all these problems in one sweep, and eliminates the code rendered cruft by Ivan's patch. A two-fer, by my reckoning. In other words, for x86, I don't think we need to worry about Port IO config access ever going away at all. You're wrong there. Sad to say, but you're wrong there. The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
as a general thing I like where this patch is going
On Sun, 13 Jan 2008 00:24:15 -0700
Matthew Wilcox [EMAIL PROTECTED] wrote:
+
+int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int
devfn,
+ int reg, int len,
u32 *val) +{
+ if (reg 256)
+ return raw_pci_ops-read(domain, bus, devfn, reg,
len, val);
+ if (raw_pci_ext_ops)
+ return raw_pci_ext_ops-read(domain, bus, devfn,
reg, len, val);
+ return -EINVAL;
would be nice the reg 256 raw_pci_Ext_ops==NULL case would just
call the raw_pci_ops- pointer, to give that a chance of refusal
(but I guess that shouldn't really happen)
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -28,7 +28,7 @@ static int __initdata pci_mmcfg_resources_inserted;
static const char __init *pci_mmcfg_e7520(void)
{
u32 win;
- pci_conf1_read(0, 0, PCI_DEVFN(0,0), 0xce, 2, win);
+ pci_direct_conf1.read(0, 0, PCI_DEVFN(0,0), 0xce, 2, win);
couldn't this (at least in some next patch) use the vector if it exists?
\
@@ -140,5 +134,6 @@ int __init pci_mmcfg_arch_init(void)
{
printk(KERN_INFO PCI: Using MMCONFIG\n);
raw_pci_ops = pci_mmcfg;
+ raw_pci_ext_ops = pci_mmcfg;
why set BOTH vectors? you probably ONLY want to set the ext one, so
that calls to the lower 256 go to the original
--
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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 07:43:11 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Arjan van de Ven wrote: On Sat, 12 Jan 2008 20:36:59 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Just about NOBODY has devices that need the extended config space. At all. The PCI express spec requires the platform to provide access to this space for express-compliance. PLATFORM not OS :) Windows isn't using it in the server space, and only in the client space it recently started considering it. More devices will be using this space as express becomes the dominant IO bus technology. sure in like 2009 maybe. Which is why Loic's proposal and Ivan's implementation of it is so elegant. It solves all these problems in one sweep, and eliminates the code rendered cruft by Ivan's patch. A two-fer, by my reckoning. In other words, for x86, I don't think we need to worry about Port IO config access ever going away at all. You're wrong there. Sad to say, but you're wrong there. The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. again sadly you're wrong. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/12/2008 12:45 PM, Arjan van de Ven wrote: On Sat, 12 Jan 2008 17:40:30 +0300 Ivan Kokshaysky [EMAIL PROTECTED] wrote: + if (reg 256) + return pci_conf1_read(seg,bus,devfn,reg,len,value); + btw this is my main objection to your patch; it intertwines the conf1 and mmconfig code even more. When (and I'm saying when not if) systems arrive that only have MMCONFIG for some of the devices, we'll have to detangle this again, and I'm really not looking forward to that. conf1 has been a hardcoded dependencies of mmconfig for years. Ivan's patch does not make it worse (in fact it considerably simplifies that code, making it easier to untangle later). IMHO, either your patch or Ivan's can be a good base, but: 1) For your remark above to be given any consideration, your patch should be modified to remove the hardcoded conf1 from the *current* mmconfig code, otherwise we end up with 3 set of ops (mmconfig + conf1+ a possible third set of operations) intertwined in a confusing manner. And removing that dependency is not a straightforward operation unless you also do 2): 2) the pci_enable_ext_config() function and dev-ext_cfg_space field, sysfs interface should be removed from the patch. There has never been a problem reporting crashes or any undefined behaviour while trying to access ext-conf-space, all the problems where *using mmconfig to access legacy-conf-space*. The if (dev-cfg_space_ext 0) checks can instead be replaced by if (reg = 256). Otherwise when using per-device explicit enabling, just *checking* whether ext-conf-space is available by calling pci_enable_ext_config(), will make some of the old problems of *loosing legacy conf-space* come back: you would have introduced a new user-space and kernel API while only solving half the problems, not a good deal. if you do 1) and 2), then you really support the good properties you claimed: - You can use mmconfig for ext-space and something else for legacy-space. - You can use mmconfig for everything (for instance if conf1 is not implemented). Of course it is as straightforward to modify Ivan's patch to also have the same properties. Loic Loic -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 13:23:35 -0500 Loic Prylli [EMAIL PROTECTED] wrote: Matthew pointed a patch that basically does what you suggested; only one comment on your mail left after that: 2) the pci_enable_ext_config() function and dev-ext_cfg_space field, sysfs interface should be removed from the patch. There has never been a problem reporting crashes or any undefined behaviour while trying to access ext-conf-space, all the problems where *using mmconfig to access legacy-conf-space*. This entirely misses the point of why I made the patch. The point is NOT that devices are buggy. The point is that right now, 99.99% of the machines out there do NOT need extended config space (no matter how it gets accessed), yet at the same time they suffered from it's issues for... what 2 years now? The point of my patch was to make people who don't need extended config space, not have to deal with it anymore. Note: There is not a 100% overlap between need and will not be used in the patches that use legacy for 256. In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an lspci -vx from automated scripts). Is that a problem? We've had 2 years of mess, with one not-enough patch after another. There still are problems TODAY (eg im 2.6.24-rc7). The patch that falls back to an alternative method for below 256 is no doubt a step in the right direction. (although I'm not all that happy about mixing access types, it's not provably incorrect) Is it enough? I'm not sure. Only time can tell I suppose, but the risk side is that if it is not enough, users who don't need the extended config space for functionality will suffer the bugs AGAIN. So in short, my approach was NOT about fix PCI, it is about fix the user experience. It's a stopgap for sure, until the underlying mechanism gets reliable. It's been 2 years. maybe this next step is it, maybe it isn't. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, Jan 13, 2008 at 10:41:24AM -0800, Arjan van de Ven wrote: Note: There is not a 100% overlap between need and will not be used in the patches that use legacy for 256. In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an lspci -vx from automated scripts). I believe you to be mistaken in this belief. If you take Ivan's patch, conf1 is used for all accesses below 256 bytes. lspci -x only dumps config space up to 64 bytes; lspci - is needed to show extended pci config space. -- Intel are signing my paycheques ... these opinions are still mine Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/13/2008 1:41 PM, Arjan van de Ven wrote: On Sun, 13 Jan 2008 13:23:35 -0500 Loic Prylli [EMAIL PROTECTED] wrote: Matthew pointed a patch that basically does what you suggested; only one comment on your mail left after that: 2) the pci_enable_ext_config() function and dev-ext_cfg_space field, sysfs interface should be removed from the patch. There has never been a problem reporting crashes or any undefined behaviour while trying to access ext-conf-space, all the problems where *using mmconfig to access legacy-conf-space*. This entirely misses the point of why I made the patch. The point is NOT that devices are buggy. The point is that right now, 99.99% of the machines out there do NOT need extended config space (no matter how it gets accessed), The point of my patch was to make people who don't need extended config space, not have to deal with it anymore. I think I got your point the first time, and I agree it is sound. But in my subjective and biased opinion, I just think ext-conf-space is already useful and widespread enough (being used is not the same as being strictly required for basic operation) for your proposed tradeoff to not be optimal (protecting against future/non-proven hardware bugs, i.e. bringing non-proven benefits, at the expense of making life harder for ext-conf-space users while bringing additional extra API/code). To take an example from the linux tree: the driver/pci/pcie/aer code uses ext-conf-space for every pcie-root (currently several distributions enable it by default), does it mean opt-in would be automatically activated for most pcie hierarchies (defeating most of the benefits of being opt-in), or we just disable that code by default? Does lspci -v will automatically opt-in all pcie (right now by default it tries to list the extended-capabilities for pcie and pcix), or do we now require manual explicit sysfs operations to get the whole thing? Is is an additional flag to lspci (if so will that flag also apply to pcix, possibly causing a crash for lspci -v -opt-in-all-potential-ext-devices on some machines). Note: There is not a 100% overlap between need and will not be used in the patches that use legacy for 256. In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an lspci -vx from automated scripts). To go one step your direction, I have already argued in a couple of emails that I would prefer to not implement ext-conf-space access for any PCI-X devices (removing PCI-X2 from pci_ext_cfg_size), because there we are trying to support devices that we don't really know exists or will ever exists. And protecting against unproven bugs makes more sense when it only removes unproven benefits. Is that a problem? We've had 2 years of mess, with one not-enough patch after another. There still are problems TODAY (eg im 2.6.24-rc7). The patch that falls back to an alternative method for below 256 is no doubt a step in the right direction. (although I'm not all that happy about mixing access types, it's not provably incorrect) Is it enough? I'm not sure. FWIW, I have in my tree a patch almost identical to Ivan's dated December 2005. Because of the constant activity on the mmconfig front (that I thought would make it obsolete), I never took the effort of suggesting it before one month ago (I am not a regular user of linux-kernel). I admit nobody else should view it that way, but for me rather than the last attempt at fixing mmconfig, it's a patch first used two years ago that would have arguably prevented all problems that have been reported since then. Besides, recent mails show that hypothetically, we could even not change anything to the existing conf-space code, since the only known bug remaining is the one associated with bar probing and could be adressed by: http://www.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24-rc6/2.6.24-rc6-mm1/broken-out/pci-disable-decoding-during-sizing-of-bars.patch [ Thanks to Robert hancok and Grant Grundler for explaining to me the history of bar-probing last month ] Even if that bar-probing patch was applied (maybe it needs to be more combat-proven), by default, it still seems better to not use mmconfig for legacy-conf-space access, but going two extra precaution steps beyond what seems necessary might be excessive. Only time can tell I suppose, but the risk side is that if it is not enough, users who don't need the extended config space for functionality will suffer the bugs AGAIN. You can indeed never exclude 100% that possibility, but if they see a problem again, it is likely to be a new category of hardware/BIOS bugs never seen before. Loic -- 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
Re: [Patch v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On 1/13/2008 3:43 PM, Matthew Wilcox wrote: On Sun, Jan 13, 2008 at 10:41:24AM -0800, Arjan van de Ven wrote: Note: There is not a 100% overlap between need and will not be used in the patches that use legacy for 256. In the other patches posted, extended config space will be used in cases where it won't be with my patch. (Most obvious one is an lspci -vx from automated scripts). I believe you to be mistaken in this belief. If you take Ivan's patch, conf1 is used for all accesses below 256 bytes. lspci -x only dumps config space up to 64 bytes; lspci - is needed to show extended pci config space. I agree with Arjan about that not a 100% overlap. It is about the extra ext-conf-space access done while probing in drivers/pci/probe.c: dev-cfg_size = pci_cfg_space_size(dev); (and lspci -v will also query/show the list of extended-caps for pci-x/pcie-x devices that have some, provided the kernel can access ext-conf-space). With Ivan's patch, that line would still cause one extended-conf-space access at offset 256 for pcie/pci-x2 devices (to check the ability to query ext-space). Arjan opt-in patch would prevent that extra access. IMHO that access is OK and harmless in all cases, we are already protected by MCFG/e820 checks, but I agree one can express a different opinion based on trying to prevent never-seen/potential hardware/BIOS bugs. FWIW it is also there that I was suggested to exclude PCI-X2 devices (when restricted to pcie, that access while probing cannot even cause the harmless master-abort/0x), but there is a small trade-off. Loic -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
Arjan van de Ven wrote: The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. again sadly you're wrong. As someone gently pointed out to me, you are in a position to know this, so I probably am wrong. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 16:28:08 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Arjan van de Ven wrote: The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. again sadly you're wrong. As someone gently pointed out to me, you are in a position to know this, so I probably am wrong. I suspect Arjan is wrong. It might be some Intel agenda but I still see fairly new driver reference code that is hardcoding port accesses even when designed for Redmond products. Alan -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Mon, 14 Jan 2008 00:54:34 + Alan Cox [EMAIL PROTECTED] wrote: On Sun, 13 Jan 2008 16:28:08 -0500 Tony Camuso [EMAIL PROTECTED] wrote: Arjan van de Ven wrote: The PCI spec provides for conf1 as an architected solution. It's not going away, and especially not in x86 land where Port IO is built-in to the CPU. I suspect Arjan is wrong. It might be some Intel agenda but I still see fairly new driver reference code that is hardcoding port accesses even when designed for Redmond products. I find it hard to believe that even they have their drivers do PCI config access via ports directly from the drivers, and especially in driver reference code... -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
To all ... Well, here is what I perceive we've got so far. . Some PCI Northbridges do not work with MMCONFIG. . Some PCI BARs can overlap the MMCONFIG area during bus sizing. It is hoped that new BIOSes will locate MMCONFIG in an area safely out of the way of bus sizing code, but there can be no guarantees. . conf1 is going away in newer x86 implementations in the not too distant future. . The PCI express spec requires platforms to provide access to the extended config area, and there are express devices today using that area for AER. . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. . We have a flurry of patches all claiming to solve all or some of these problems. Arjan, I realize it may not be possible for you to answer this question, but I feel compelled to ask it anyway. Is it possible that future x86 architectures will be implementing a SAL-like interface to abstract PCI config access altogether? Or can we condense these patches down to a set that does the following? . If the system is capable of conf1, then PCI config access at offsets 256 should be confined to conf1. This solution is most effective for existing and legacy systems. . If the system does not support MMCONFIG, of if MMCONFIG is not working, then accesses to offsets 256 return -1 and an error status. . For systems, where the conf1 mechanism is NOT available, then MMCONFIG should be the PCI access mechanism for all offsets. For such systems, we must assume that the BIOS has become smart enough to locate MMCONFIG in a region safe from encroachment by bus sizing code. -- 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 v2] Make PCI extended config space (MMCONFIG) a driver opt-in
On Sun, 13 Jan 2008 22:29:23 -0500 Tony Camuso [EMAIL PROTECTED] wrote: . There is no need to provide different PCI config access mechanisms at device granularity, since the PCI config access mechanism between the CPU and the Northbridge is opaque to the devices. PCI config mechanisms only need to differ at the Northbridge level. This ignores the lets make it not matter for the 99% of the users case. . If the system is capable of conf1, then PCI config access at offsets 256 should be confined to conf1. This solution is most effective for existing and legacy systems. not conf1 but what the platform thinks is the best method for 256. We have this nice abstraction for the platform to select the best method... we should use it. And still, it's another attempt to get this fixed (well.. it's been 2 years in the coming so far, maybe this will be the last one, maybe it will not be... we'll see I suppose, but it sucks to be a user who doesn't need any of the functionality that the extended config space provides in theory but gets to suffer more of the issues) I'm all in favor of making this more reliable, but really.. we've thought it was fixed time and time again over the last two years. Please consider limiting the scope of the damage as well. -- If you want to reach me at my work email, use [EMAIL PROTECTED] For development, discussion and tips for power savings, visit http://www.lesswatts.org -- 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/
