Re: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Mon, 2007-09-10 at 23:15 -0700, Greg KH wrote: > On Tue, Sep 11, 2007 at 09:29:43AM +0800, Shaohua Li wrote: > > On Sun, 2007-09-09 at 09:43 -0700, Greg KH wrote: > > > On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: > > > > From: Matthew Wilcox <[EMAIL PROTECTED]> > > > > Date: Thu, 6 Sep 2007 05:57:31 -0600 > > > > > > > > > I'm not sure your analysis is correct. Here's what my draft copy of > > > > > the pcie 2.0 spec says: > > > > > > > > > > Enble No Snoop ? If this bit is Set, the Function is permitted to > > > > > Set the No Snoop bit in the Requester Attributes of transactions it > > > > > initiates that do not require hardware enforced cache coherency (see > > > > > Section 2.2.6.5). Note that setting this bit to 1b should not cause > > > > > a Function to Set the No Snoop attribute on all transactions that it > > > > > initiates. Even when this bit is Set, a Function is only permitted > > > > > to Set the No Snoop attribute on a transaction when it can guarantee > > > > > that the address of the transaction is not stored in any cache in > > > > > the system. This bit permitted to be hardwired to 0b if a Function > > > > > would never Set the No Snoop attribute in transactions it initiates. > > > > > Default value of this bit is 1b. > > > > > > > > > > That implies that devices are only allowed to set it when it's safe to > > > > > do so ... and we don't need to turn it off. > > > > > > > > This is my understanding of this area of PCI-E as well, and I > > > > also agree that therefore we should not turn this bit off. > > > > > > I agree. But Shaohua, do you see any problems that this patch fixes? > > No, I didn't see any breakage, just worry about it's a potential issue. > > Hm, well, if you don't mind, I'd like to leave it as is for now, as no > one is reporting any problems with this, and there seems to be some > disagreement as to if it is really needed or not. > > Is that ok? Ok. Regards, Shaohua - 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Tue, Sep 11, 2007 at 09:29:43AM +0800, Shaohua Li wrote: > On Sun, 2007-09-09 at 09:43 -0700, Greg KH wrote: > > On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: > > > From: Matthew Wilcox <[EMAIL PROTECTED]> > > > Date: Thu, 6 Sep 2007 05:57:31 -0600 > > > > > > > I'm not sure your analysis is correct. Here's what my draft copy of > > > > the pcie 2.0 spec says: > > > > > > > > Enble No Snoop ? If this bit is Set, the Function is permitted to > > > > Set the No Snoop bit in the Requester Attributes of transactions it > > > > initiates that do not require hardware enforced cache coherency (see > > > > Section 2.2.6.5). Note that setting this bit to 1b should not cause > > > > a Function to Set the No Snoop attribute on all transactions that it > > > > initiates. Even when this bit is Set, a Function is only permitted > > > > to Set the No Snoop attribute on a transaction when it can guarantee > > > > that the address of the transaction is not stored in any cache in > > > > the system. This bit permitted to be hardwired to 0b if a Function > > > > would never Set the No Snoop attribute in transactions it initiates. > > > > Default value of this bit is 1b. > > > > > > > > That implies that devices are only allowed to set it when it's safe to > > > > do so ... and we don't need to turn it off. > > > > > > This is my understanding of this area of PCI-E as well, and I > > > also agree that therefore we should not turn this bit off. > > > > I agree. But Shaohua, do you see any problems that this patch fixes? > No, I didn't see any breakage, just worry about it's a potential issue. Hm, well, if you don't mind, I'd like to leave it as is for now, as no one is reporting any problems with this, and there seems to be some disagreement as to if it is really needed or not. Is that ok? 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Tue, Sep 11, 2007 at 09:29:43AM +0800, Shaohua Li wrote: On Sun, 2007-09-09 at 09:43 -0700, Greg KH wrote: On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: From: Matthew Wilcox [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 05:57:31 -0600 I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop ? If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. This is my understanding of this area of PCI-E as well, and I also agree that therefore we should not turn this bit off. I agree. But Shaohua, do you see any problems that this patch fixes? No, I didn't see any breakage, just worry about it's a potential issue. Hm, well, if you don't mind, I'd like to leave it as is for now, as no one is reporting any problems with this, and there seems to be some disagreement as to if it is really needed or not. Is that ok? 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Mon, 2007-09-10 at 23:15 -0700, Greg KH wrote: On Tue, Sep 11, 2007 at 09:29:43AM +0800, Shaohua Li wrote: On Sun, 2007-09-09 at 09:43 -0700, Greg KH wrote: On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: From: Matthew Wilcox [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 05:57:31 -0600 I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop ? If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. This is my understanding of this area of PCI-E as well, and I also agree that therefore we should not turn this bit off. I agree. But Shaohua, do you see any problems that this patch fixes? No, I didn't see any breakage, just worry about it's a potential issue. Hm, well, if you don't mind, I'd like to leave it as is for now, as no one is reporting any problems with this, and there seems to be some disagreement as to if it is really needed or not. Is that ok? Ok. Regards, Shaohua - 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Sun, 2007-09-09 at 09:43 -0700, Greg KH wrote: > On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: > > From: Matthew Wilcox <[EMAIL PROTECTED]> > > Date: Thu, 6 Sep 2007 05:57:31 -0600 > > > > > I'm not sure your analysis is correct. Here's what my draft copy of > > > the pcie 2.0 spec says: > > > > > > Enble No Snoop ? If this bit is Set, the Function is permitted to > > > Set the No Snoop bit in the Requester Attributes of transactions it > > > initiates that do not require hardware enforced cache coherency (see > > > Section 2.2.6.5). Note that setting this bit to 1b should not cause > > > a Function to Set the No Snoop attribute on all transactions that it > > > initiates. Even when this bit is Set, a Function is only permitted > > > to Set the No Snoop attribute on a transaction when it can guarantee > > > that the address of the transaction is not stored in any cache in > > > the system. This bit permitted to be hardwired to 0b if a Function > > > would never Set the No Snoop attribute in transactions it initiates. > > > Default value of this bit is 1b. > > > > > > That implies that devices are only allowed to set it when it's safe to > > > do so ... and we don't need to turn it off. > > > > This is my understanding of this area of PCI-E as well, and I > > also agree that therefore we should not turn this bit off. > > I agree. But Shaohua, do you see any problems that this patch fixes? No, I didn't see any breakage, just worry about it's a potential issue. - 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Sun, 2007-09-09 at 09:43 -0700, Greg KH wrote: On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: From: Matthew Wilcox [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 05:57:31 -0600 I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop ? If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. This is my understanding of this area of PCI-E as well, and I also agree that therefore we should not turn this bit off. I agree. But Shaohua, do you see any problems that this patch fixes? No, I didn't see any breakage, just worry about it's a potential issue. - 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: > From: Matthew Wilcox <[EMAIL PROTECTED]> > Date: Thu, 6 Sep 2007 05:57:31 -0600 > > > I'm not sure your analysis is correct. Here's what my draft copy of > > the pcie 2.0 spec says: > > > > Enble No Snoop ? If this bit is Set, the Function is permitted to > > Set the No Snoop bit in the Requester Attributes of transactions it > > initiates that do not require hardware enforced cache coherency (see > > Section 2.2.6.5). Note that setting this bit to 1b should not cause > > a Function to Set the No Snoop attribute on all transactions that it > > initiates. Even when this bit is Set, a Function is only permitted > > to Set the No Snoop attribute on a transaction when it can guarantee > > that the address of the transaction is not stored in any cache in > > the system. This bit permitted to be hardwired to 0b if a Function > > would never Set the No Snoop attribute in transactions it initiates. > > Default value of this bit is 1b. > > > > That implies that devices are only allowed to set it when it's safe to > > do so ... and we don't need to turn it off. > > This is my understanding of this area of PCI-E as well, and I > also agree that therefore we should not turn this bit off. I agree. But Shaohua, do you see any problems that this patch fixes? 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Thu, Sep 06, 2007 at 05:40:38AM -0700, David Miller wrote: From: Matthew Wilcox [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 05:57:31 -0600 I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop ? If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. This is my understanding of this area of PCI-E as well, and I also agree that therefore we should not turn this bit off. I agree. But Shaohua, do you see any problems that this patch fixes? 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Thu, 2007-09-06 at 05:57 -0600, Matthew Wilcox wrote: > On Thu, Sep 06, 2007 at 01:35:08PM +0800, Shaohua Li wrote: > > PCIE 'Enable No Snoop' bit is set by default per PCIE spec, but OS > > assumes PCI DMA is snooped, which is legacy PCI device does. Enabling no > > snoop might cause potential DMA issues. This patch disables it, if a > > driver can work with no snoop, we can provide a helper to enable it. > > I'm not sure your analysis is correct. Here's what my draft copy of > the pcie 2.0 spec says: > > Enble No Snoop If this bit is Set, the Function is permitted to > Set the No Snoop bit in the Requester Attributes of transactions it > initiates that do not require hardware enforced cache coherency (see > Section 2.2.6.5). Note that setting this bit to 1b should not cause > a Function to Set the No Snoop attribute on all transactions that it > initiates. Even when this bit is Set, a Function is only permitted > to Set the No Snoop attribute on a transaction when it can guarantee > that the address of the transaction is not stored in any cache in > the system. This bit permitted to be hardwired to 0b if a Function > would never Set the No Snoop attribute in transactions it initiates. > Default value of this bit is 1b. > > That implies that devices are only allowed to set it when it's safe to > do so ... and we don't need to turn it off. Yes, I saw this in spec too. Devices don't know how OS handle DMA, so it's possible the bit is wrongly set. My point is setting the bit can be done by driver based. If a driver thinks it's really safe to do so, it can set the bit. Thanks, Shaohua - 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
From: Matthew Wilcox <[EMAIL PROTECTED]> Date: Thu, 6 Sep 2007 05:57:31 -0600 > I'm not sure your analysis is correct. Here's what my draft copy of > the pcie 2.0 spec says: > > Enble No Snoop If this bit is Set, the Function is permitted to > Set the No Snoop bit in the Requester Attributes of transactions it > initiates that do not require hardware enforced cache coherency (see > Section 2.2.6.5). Note that setting this bit to 1b should not cause > a Function to Set the No Snoop attribute on all transactions that it > initiates. Even when this bit is Set, a Function is only permitted > to Set the No Snoop attribute on a transaction when it can guarantee > that the address of the transaction is not stored in any cache in > the system. This bit permitted to be hardwired to 0b if a Function > would never Set the No Snoop attribute in transactions it initiates. > Default value of this bit is 1b. > > That implies that devices are only allowed to set it when it's safe to > do so ... and we don't need to turn it off. This is my understanding of this area of PCI-E as well, and I also agree that therefore we should not turn this bit off. - 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Thu, Sep 06, 2007 at 01:35:08PM +0800, Shaohua Li wrote: > PCIE 'Enable No Snoop' bit is set by default per PCIE spec, but OS > assumes PCI DMA is snooped, which is legacy PCI device does. Enabling no > snoop might cause potential DMA issues. This patch disables it, if a > driver can work with no snoop, we can provide a helper to enable it. I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop ?? If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. -- 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Thu, Sep 06, 2007 at 01:35:08PM +0800, Shaohua Li wrote: PCIE 'Enable No Snoop' bit is set by default per PCIE spec, but OS assumes PCI DMA is snooped, which is legacy PCI device does. Enabling no snoop might cause potential DMA issues. This patch disables it, if a driver can work with no snoop, we can provide a helper to enable it. I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop ?? If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. -- 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: [RFC] disable PCIE 'Enable No Snoop' bit by default
On Thu, 2007-09-06 at 05:57 -0600, Matthew Wilcox wrote: On Thu, Sep 06, 2007 at 01:35:08PM +0800, Shaohua Li wrote: PCIE 'Enable No Snoop' bit is set by default per PCIE spec, but OS assumes PCI DMA is snooped, which is legacy PCI device does. Enabling no snoop might cause potential DMA issues. This patch disables it, if a driver can work with no snoop, we can provide a helper to enable it. I'm not sure your analysis is correct. Here's what my draft copy of the pcie 2.0 spec says: Enble No Snoop If this bit is Set, the Function is permitted to Set the No Snoop bit in the Requester Attributes of transactions it initiates that do not require hardware enforced cache coherency (see Section 2.2.6.5). Note that setting this bit to 1b should not cause a Function to Set the No Snoop attribute on all transactions that it initiates. Even when this bit is Set, a Function is only permitted to Set the No Snoop attribute on a transaction when it can guarantee that the address of the transaction is not stored in any cache in the system. This bit permitted to be hardwired to 0b if a Function would never Set the No Snoop attribute in transactions it initiates. Default value of this bit is 1b. That implies that devices are only allowed to set it when it's safe to do so ... and we don't need to turn it off. Yes, I saw this in spec too. Devices don't know how OS handle DMA, so it's possible the bit is wrongly set. My point is setting the bit can be done by driver based. If a driver thinks it's really safe to do so, it can set the bit. Thanks, Shaohua - 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/
[RFC] disable PCIE 'Enable No Snoop' bit by default
PCIE 'Enable No Snoop' bit is set by default per PCIE spec, but OS
assumes PCI DMA is snooped, which is legacy PCI device does. Enabling no
snoop might cause potential DMA issues. This patch disables it, if a
driver can work with no snoop, we can provide a helper to enable it.
I didn't see any real breaks, but did see some devices with the bit enabled
Signed-off-by: Shaohua Li <[EMAIL PROTECTED]>
Index: linux/drivers/pci/probe.c
===
--- linux.orig/drivers/pci/probe.c 2007-09-06 13:18:07.0 +0800
+++ linux/drivers/pci/probe.c 2007-09-06 13:21:30.0 +0800
@@ -694,6 +694,19 @@ static void pci_read_irq(struct pci_dev
#define LEGACY_IO_RESOURCE (IORESOURCE_IO | IORESOURCE_PCI_FIXED)
+static void pcie_setup_device(struct pci_dev *dev)
+{
+ u16 reg16;
+ int pos;
+
+ pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
+ if (pos) {
+ pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, );
+ reg16 &= ~PCI_EXP_DEVCTL_NOSNOOP_EN;
+ pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, reg16);
+ }
+}
+
/**
* pci_setup_device - fill in class and map information of a device
* @dev: the device structure to fill
@@ -795,6 +808,7 @@ static int pci_setup_device(struct pci_d
dev->class = PCI_CLASS_NOT_DEFINED;
}
+ pcie_setup_device(dev);
/* We found a fine healthy device, go go go... */
return 0;
}
-
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/
