* question regarding cacheline size @ 2006-09-07 8:31 Tejun Heo 2006-09-07 11:11 ` Matthew Wilcox 2006-09-07 11:59 ` linux-os (Dick Johnson) 0 siblings, 2 replies; 21+ messages in thread From: Tejun Heo @ 2006-09-07 8:31 UTC (permalink / raw) To: linux-pci; +Cc: Greg KH, lkml Hello, This is for PCMCIA (cardbus) version of Silicon Image 3124 SerialATA controller. When cacheline size is configured, the controller uses Read Multiple commands. • Bit [07:00]: Cache Line Size (R/W). This bit field is used to specify the system cacheline size in terms of 32-bit words. The SiI3124, when initiating a read transaction, will issue the Read Multiple PCI command if empty space in its FIFO is greater than the value programmed in this register. As the BIOS doesn't run after hotplugging cardbus card, the cache line isn't configured and the controller ends up having 0 cache line size and always using Read command. When that happens, write performance drops hard - the throughput is < 2Mbytes/s. http://thread.gmane.org/gmane.linux.ide/12908/focus=12908 So, sata_sil24 driver has to program CLS if it's not already set, but I'm not sure which number to punch in. FWIW, sil3124 doesn't seem to put restrictions on the values which can be used for CLS. There are several candidates... * L1_CACHE_BYTES / 4 : this is used by init routine in yenta_socket.c. It seems to be a sane default but I'm not sure whether L1 cache line size always coincides with the size as seen from PCI bus. * pci_cache_line_size in drivers/pci/pci.c : this is used for pci_generic_prep_mwi() and can be overridden by arch specific code. this seems more appropriate but is not exported. For all involved commands - memory read line, memory read multiple and memory write and invalidate - a value larger than actual cacheline size doesn't hurt but a smaller value may. I'm thinking of implementing a query function for pci_cache_line_size, say, int pci_cacheline_size(struct pci_dev *pdev), and use it in the device init routine. Does this sound sane? Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 8:31 question regarding cacheline size Tejun Heo @ 2006-09-07 11:11 ` Matthew Wilcox 2006-09-07 11:20 ` Tejun Heo 2006-09-07 11:59 ` linux-os (Dick Johnson) 1 sibling, 1 reply; 21+ messages in thread From: Matthew Wilcox @ 2006-09-07 11:11 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 10:31:02AM +0200, Tejun Heo wrote: > As the BIOS doesn't run after hotplugging cardbus card, the cache line > isn't configured and the controller ends up having 0 cache line size and > always using Read command. When that happens, write performance drops > hard - the throughput is < 2Mbytes/s. > > http://thread.gmane.org/gmane.linux.ide/12908/focus=12908 > > So, sata_sil24 driver has to program CLS if it's not already set, but > I'm not sure which number to punch in. FWIW, sil3124 doesn't seem to > put restrictions on the values which can be used for CLS. There are > several candidates... > > * L1_CACHE_BYTES / 4 : this is used by init routine in yenta_socket.c. > It seems to be a sane default but I'm not sure whether L1 cache line > size always coincides with the size as seen from PCI bus. > > * pci_cache_line_size in drivers/pci/pci.c : this is used for > pci_generic_prep_mwi() and can be overridden by arch specific code. > this seems more appropriate but is not exported. > > For all involved commands - memory read line, memory read multiple and > memory write and invalidate - a value larger than actual cacheline size > doesn't hurt but a smaller value may. Just call pci_set_mwi(), that'll make sure the cache line size is set correctly. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 11:11 ` Matthew Wilcox @ 2006-09-07 11:20 ` Tejun Heo 2006-09-07 12:07 ` Russell King 0 siblings, 1 reply; 21+ messages in thread From: Tejun Heo @ 2006-09-07 11:20 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-pci, Greg KH, lkml Matthew Wilcox wrote: > Just call pci_set_mwi(), that'll make sure the cache line size is set > correctly. Sounds simple enough. Just two small worries though. * It has an apparent side effect of setting PCI_COMMAND_INVALIDATE, which should be okay in sil3124's case. * The controller might have some restrictions on configurable cache line size. This is the same for MWI, so I guess this problem is just imaginary. For the time being, I'll go with pci_set_mwi() but IMHO it would be better to have a pci helper for this purpose - pci_config_cacheline_size() or something. Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 11:20 ` Tejun Heo @ 2006-09-07 12:07 ` Russell King 2006-09-07 12:23 ` Matthew Wilcox 0 siblings, 1 reply; 21+ messages in thread From: Russell King @ 2006-09-07 12:07 UTC (permalink / raw) To: Tejun Heo; +Cc: Matthew Wilcox, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 01:20:22PM +0200, Tejun Heo wrote: > Matthew Wilcox wrote: > >Just call pci_set_mwi(), that'll make sure the cache line size is set > >correctly. > > Sounds simple enough. Just two small worries though. > > * It has an apparent side effect of setting PCI_COMMAND_INVALIDATE, > which should be okay in sil3124's case. > > * The controller might have some restrictions on configurable cache line > size. This is the same for MWI, so I guess this problem is just imaginary. > > For the time being, I'll go with pci_set_mwi() but IMHO it would be > better to have a pci helper for this purpose - > pci_config_cacheline_size() or something. I've often wondered why we don't set the cache line size when we set the bus master bit - ISTR when I read the PCI spec (2.1 or 2.2) it implied that this should be set for bus master operations. I've certainly seen PCI devices which can bus master and do have the cache line size register but have the invalidate bit set forced to zero in the command register. Makes sense when you consider there are cache line considerations for "memory read multiple" and "memory read line" bus commands in addition to the "memory write and invalidate" bus command. (Consider - if you use memory read line and haven't programmed the cache line size, how does the master know how long a cache line is and hence knows when to use this command?) -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:07 ` Russell King @ 2006-09-07 12:23 ` Matthew Wilcox 2006-09-07 12:33 ` Arjan van de Ven 2006-09-07 13:02 ` Russell King 0 siblings, 2 replies; 21+ messages in thread From: Matthew Wilcox @ 2006-09-07 12:23 UTC (permalink / raw) To: Tejun Heo, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 01:07:56PM +0100, Russell King wrote: > I've often wondered why we don't set the cache line size when we set the > bus master bit - ISTR when I read the PCI spec (2.1 or 2.2) it implied > that this should be set for bus master operations. It's not required ... 3.2.1 of pci 2.3 says: While Memory Write and Invalidate is the only command that requires implementation of the Cacheline Size register, it is strongly suggested the memory read commands use it as well. A bridge that prefetches is responsible for any latent data not consumed by the master. (obviously this is talking about requirements placed on the device, not on the OS, but it'd behoove us to help the device out here). It's also useful to implement it for slave devices. PCI 2.3 has the concept of cacheline wrap transactions -- eg with a cacheline size of 0x10, it can transfer data to 0x108, then 0x10C, 0x100, 0x104, then 0x118, etc So I think we should redo the PCI subsystem to set cacheline size during the buswalk rather than waiting for drivers to ask for it to be set. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:23 ` Matthew Wilcox @ 2006-09-07 12:33 ` Arjan van de Ven 2006-09-07 12:40 ` Matthew Wilcox 2006-09-07 13:02 ` Russell King 1 sibling, 1 reply; 21+ messages in thread From: Arjan van de Ven @ 2006-09-07 12:33 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Tejun Heo, linux-pci, Greg KH, lkml > > So I think we should redo the PCI subsystem to set cacheline size during > the buswalk rather than waiting for drivers to ask for it to be set. ... while allowing for quirks for devices that go puke when this register gets written ;) (afaik there are a few) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:33 ` Arjan van de Ven @ 2006-09-07 12:40 ` Matthew Wilcox 2006-09-07 12:53 ` Tejun Heo 2006-09-07 13:01 ` Arjan van de Ven 0 siblings, 2 replies; 21+ messages in thread From: Matthew Wilcox @ 2006-09-07 12:40 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Tejun Heo, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 02:33:25PM +0200, Arjan van de Ven wrote: > > > > > So I think we should redo the PCI subsystem to set cacheline size during > > the buswalk rather than waiting for drivers to ask for it to be set. > > ... while allowing for quirks for devices that go puke when this > register gets written ;) > > (afaik there are a few) So you want: unsigned int no_cls:1; /* Device pukes on write to Cacheline Size */ in struct pci_dev? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:40 ` Matthew Wilcox @ 2006-09-07 12:53 ` Tejun Heo 2006-09-07 13:04 ` Matthew Wilcox 2006-09-07 13:10 ` Russell King 2006-09-07 13:01 ` Arjan van de Ven 1 sibling, 2 replies; 21+ messages in thread From: Tejun Heo @ 2006-09-07 12:53 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Arjan van de Ven, linux-pci, Greg KH, lkml Matthew Wilcox wrote: > On Thu, Sep 07, 2006 at 02:33:25PM +0200, Arjan van de Ven wrote: >>> So I think we should redo the PCI subsystem to set cacheline size during >>> the buswalk rather than waiting for drivers to ask for it to be set. >> ... while allowing for quirks for devices that go puke when this >> register gets written ;) >> >> (afaik there are a few) > > So you want: > > unsigned int no_cls:1; /* Device pukes on write to Cacheline Size */ > > in struct pci_dev? The spec says that devices can put additional restriction on supported cacheline size (IIRC, the example was something like power of two >= or <= certain size) and should ignore (treat as zero) if unsupported value is written. So, there might be need for more low level driver involvement which knows device restrictions, but I don't know whether such devices exist. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:53 ` Tejun Heo @ 2006-09-07 13:04 ` Matthew Wilcox 2006-09-07 13:19 ` Tejun Heo 2006-09-07 13:30 ` Jeff Garzik 2006-09-07 13:10 ` Russell King 1 sibling, 2 replies; 21+ messages in thread From: Matthew Wilcox @ 2006-09-07 13:04 UTC (permalink / raw) To: Tejun Heo; +Cc: Arjan van de Ven, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 02:53:57PM +0200, Tejun Heo wrote: > The spec says that devices can put additional restriction on supported > cacheline size (IIRC, the example was something like power of two >= or > <= certain size) and should ignore (treat as zero) if unsupported value > is written. So, there might be need for more low level driver > involvement which knows device restrictions, but I don't know whether > such devices exist. That's nothing we can do anything about. The system cacheline size is what it is. If the device doesn't support it, we can't fall back to a different size, it'll cause data corruption. So we'll just continue on, and devices which live up to the spec will act as if we hadn't programmed a cache size. For devices that don't, we'll have the quirk. Arguably devices which don't support the real system cacheline size would only get data corruption if they used MWI, so we only have to prevent them from using MWI; they could use a different cacheline size for MRM and MRL without causing data corruption. But I don't think we want to go down that route; do you? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 13:04 ` Matthew Wilcox @ 2006-09-07 13:19 ` Tejun Heo 2006-09-07 15:21 ` Grant Grundler 2006-09-07 13:30 ` Jeff Garzik 1 sibling, 1 reply; 21+ messages in thread From: Tejun Heo @ 2006-09-07 13:19 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Arjan van de Ven, linux-pci, Greg KH, lkml Matthew Wilcox wrote: > On Thu, Sep 07, 2006 at 02:53:57PM +0200, Tejun Heo wrote: >> The spec says that devices can put additional restriction on supported >> cacheline size (IIRC, the example was something like power of two >= or >> <= certain size) and should ignore (treat as zero) if unsupported value >> is written. So, there might be need for more low level driver >> involvement which knows device restrictions, but I don't know whether >> such devices exist. > > That's nothing we can do anything about. The system cacheline size is > what it is. If the device doesn't support it, we can't fall back to a > different size, it'll cause data corruption. So we'll just continue on, > and devices which live up to the spec will act as if we hadn't > programmed a cache size. For devices that don't, we'll have the quirk. For MWI, it will cause data corruption. For READ LINE and MULTIPLE, I think it would be okay. The memory is prefetchable after all. Anyways, this shouldn't be of too much problem and probably can be handled by quirks if ever needed. > Arguably devices which don't support the real system cacheline size > would only get data corruption if they used MWI, so we only have to > prevent them from using MWI; they could use a different cacheline size > for MRM and MRL without causing data corruption. But I don't think we > want to go down that route; do you? Oh yeah, that's what I was trying to say, and I don't want to go down that route. So, I guess this one is settled. Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 13:19 ` Tejun Heo @ 2006-09-07 15:21 ` Grant Grundler 2006-09-07 15:47 ` Tejun Heo 2006-09-07 16:04 ` Jeff Garzik 0 siblings, 2 replies; 21+ messages in thread From: Grant Grundler @ 2006-09-07 15:21 UTC (permalink / raw) To: Tejun Heo; +Cc: Matthew Wilcox, Arjan van de Ven, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 03:19:04PM +0200, Tejun Heo wrote: ... > For MWI, it will cause data corruption. For READ LINE and MULTIPLE, I > think it would be okay. The memory is prefetchable after all. Within the context of DMA API, memory is prefetchable by the device for "streaming" transactions but not for "coherent" memory. PCI subsystem has no way of knowing which transaction a device will use for any particular type of memory access. Only the driver can embed that knowledge. > Anyways, this shouldn't be of too much problem and probably > can be handled by quirks if ever needed. > > >Arguably devices which don't support the real system cacheline size > >would only get data corruption if they used MWI, so we only have to > >prevent them from using MWI; they could use a different cacheline size > >for MRM and MRL without causing data corruption. But I don't think we > >want to go down that route; do you? > > Oh yeah, that's what I was trying to say, and I don't want to go down > that route. So, I guess this one is settled. hrm...if the driver can put a safe value in cachelinesize register and NOT enable MWI, I can imagine a significant performance boost if the device can use MRM or MRL. But IMHO it's up to the driver writers (or other contributors) to figure that out. Current API (pci_set_mwi()) ties enabling MRM/MRL with enabling MWI and I don't see a really good reason for that. Only the converse is true - enabling MWI requires setting cachelinesize. hth, grant ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 15:21 ` Grant Grundler @ 2006-09-07 15:47 ` Tejun Heo 2006-09-07 16:00 ` Jeff Garzik 2006-09-07 16:04 ` Jeff Garzik 1 sibling, 1 reply; 21+ messages in thread From: Tejun Heo @ 2006-09-07 15:47 UTC (permalink / raw) To: Grant Grundler Cc: Matthew Wilcox, Arjan van de Ven, linux-pci, Greg KH, lkml, Jeff Garzik Grant Grundler wrote: > On Thu, Sep 07, 2006 at 03:19:04PM +0200, Tejun Heo wrote: > ... >> For MWI, it will cause data corruption. For READ LINE and MULTIPLE, I >> think it would be okay. The memory is prefetchable after all. > > Within the context of DMA API, memory is prefetchable by the device > for "streaming" transactions but not for "coherent" memory. > PCI subsystem has no way of knowing which transaction a device > will use for any particular type of memory access. Only the > driver can embed that knowledge. I think using larger cacheline value should be okay for both prefetchable and non-prefetchable memory. Using larger value tells the device to be more conservative in issuing MRL, MRW or WMI. As Russell has pointed out, cacheline-wrapping access wouldn't work but I think it's reasonable to expect for such device to be flexible about cacheline config. >> Oh yeah, that's what I was trying to say, and I don't want to go down >> that route. So, I guess this one is settled. > > hrm...if the driver can put a safe value in cachelinesize register > and NOT enable MWI, I can imagine a significant performance boost > if the device can use MRM or MRL. But IMHO it's up to the driver > writers (or other contributors) to figure that out. > > Current API (pci_set_mwi()) ties enabling MRM/MRL with enabling MWI > and I don't see a really good reason for that. Only the converse > is true - enabling MWI requires setting cachelinesize. arch/i386/pci/common.c overrides cacheline size to min 32 regardless of actual size. So, we seem to be using larger cacheline size for MWI already. Jeff pointed out that there actually are devices which limit CLS config. IMHO, making PCI configure CLS automatically and provide helpers to LLD to override it if necessary should cut it. Thanks. -- tejun ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 15:47 ` Tejun Heo @ 2006-09-07 16:00 ` Jeff Garzik 2006-09-07 17:00 ` Matthew Wilcox 0 siblings, 1 reply; 21+ messages in thread From: Jeff Garzik @ 2006-09-07 16:00 UTC (permalink / raw) To: Tejun Heo Cc: Grant Grundler, Matthew Wilcox, Arjan van de Ven, linux-pci, Greg KH, lkml Tejun Heo wrote: > arch/i386/pci/common.c overrides cacheline size to min 32 regardless of > actual size. So, we seem to be using larger cacheline size for MWI > already. It clamps the minimum size to 32, yes, but on modern machines common.c configures it to a larger size. > Jeff pointed out that there actually are devices which limit CLS config. > IMHO, making PCI configure CLS automatically and provide helpers to LLD > to override it if necessary should cut it. We still have to add a raft of quirks, if we start automatically configurating CLS... Also, many PCI devices hardcode it to zero. If we start configuring CLS automatically, I forsee a period of breakage... Jeff ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 16:00 ` Jeff Garzik @ 2006-09-07 17:00 ` Matthew Wilcox 0 siblings, 0 replies; 21+ messages in thread From: Matthew Wilcox @ 2006-09-07 17:00 UTC (permalink / raw) To: Jeff Garzik Cc: Tejun Heo, Grant Grundler, Arjan van de Ven, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 12:00:39PM -0400, Jeff Garzik wrote: > Tejun Heo wrote: > >arch/i386/pci/common.c overrides cacheline size to min 32 regardless of > >actual size. So, we seem to be using larger cacheline size for MWI > >already. > > It clamps the minimum size to 32, yes, but on modern machines common.c > configures it to a larger size. > > > >Jeff pointed out that there actually are devices which limit CLS config. > > IMHO, making PCI configure CLS automatically and provide helpers to LLD > >to override it if necessary should cut it. > > We still have to add a raft of quirks, if we start automatically > configurating CLS... Also, many PCI devices hardcode it to zero. That's not a problem. As long as they silently discard the writes to the CLS register, we'll cope. If they decide to flip out, then we have the no_cls flag. > If we start configuring CLS automatically, I forsee a period of breakage... Probably. But it's something we should have done a long time ago. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 15:21 ` Grant Grundler 2006-09-07 15:47 ` Tejun Heo @ 2006-09-07 16:04 ` Jeff Garzik 2006-09-22 23:47 ` Grant Grundler 1 sibling, 1 reply; 21+ messages in thread From: Jeff Garzik @ 2006-09-07 16:04 UTC (permalink / raw) To: Grant Grundler Cc: Tejun Heo, Matthew Wilcox, Arjan van de Ven, linux-pci, Greg KH, lkml Grant Grundler wrote: > hrm...if the driver can put a safe value in cachelinesize register > and NOT enable MWI, I can imagine a significant performance boost > if the device can use MRM or MRL. But IMHO it's up to the driver > writers (or other contributors) to figure that out. Yes. > Current API (pci_set_mwi()) ties enabling MRM/MRL with enabling MWI > and I don't see a really good reason for that. Only the converse > is true - enabling MWI requires setting cachelinesize. Correct, that's why it was done that way, when I wrote the API. Enabling MWI required making sure the BIOS configured our CLS for us, which was often not the case. No reason why we can't do a pdev->set_cls = 1; rc = pci_enable_device(pdev); or rc = pci_set_cacheline_size(pdev); Regards, Jeff ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 16:04 ` Jeff Garzik @ 2006-09-22 23:47 ` Grant Grundler 0 siblings, 0 replies; 21+ messages in thread From: Grant Grundler @ 2006-09-22 23:47 UTC (permalink / raw) To: Jeff Garzik Cc: Grant Grundler, Tejun Heo, Matthew Wilcox, Arjan van de Ven, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 12:04:07PM -0400, Jeff Garzik wrote: ... > >Current API (pci_set_mwi()) ties enabling MRM/MRL with enabling MWI > >and I don't see a really good reason for that. Only the converse > >is true - enabling MWI requires setting cachelinesize. > > Correct, that's why it was done that way, when I wrote the API. > Enabling MWI required making sure the BIOS configured our CLS for us, > which was often not the case. No reason why we can't do a > > pdev->set_cls = 1; > rc = pci_enable_device(pdev); I was thinking the pci_enable_device could safely set CLS if MWI were NOT enabled. I'm sure somethings would break that worked before but that's what quirks are for, right? Anyway, I'm convinced the arch specific PCI code should be setting CLS either at bus_fixup or via some quirks to compensate for "broken" firmware (which didn't set CLS). Maybe there is more brokn HW out there than I think...I'm easy convince that's the case. If a driver wanted to enable MWI, then pci_set_mwi() should verify (or force) CLS setttings or return an error. That part seems pretty straight forward and don't need a change in API here. thanks, grant ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 13:04 ` Matthew Wilcox 2006-09-07 13:19 ` Tejun Heo @ 2006-09-07 13:30 ` Jeff Garzik 1 sibling, 0 replies; 21+ messages in thread From: Jeff Garzik @ 2006-09-07 13:30 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Tejun Heo, Arjan van de Ven, linux-pci, Greg KH, lkml Matthew Wilcox wrote: > On Thu, Sep 07, 2006 at 02:53:57PM +0200, Tejun Heo wrote: >> The spec says that devices can put additional restriction on supported >> cacheline size (IIRC, the example was something like power of two >= or >> <= certain size) and should ignore (treat as zero) if unsupported value >> is written. So, there might be need for more low level driver >> involvement which knows device restrictions, but I don't know whether >> such devices exist. > > That's nothing we can do anything about. The system cacheline size is > what it is. If the device doesn't support it, we can't fall back to a > different size, it'll cause data corruption. So we'll just continue on, > and devices which live up to the spec will act as if we hadn't > programmed a cache size. For devices that don't, we'll have the quirk. > > Arguably devices which don't support the real system cacheline size > would only get data corruption if they used MWI, so we only have to > prevent them from using MWI; they could use a different cacheline size > for MRM and MRL without causing data corruption. But I don't think we > want to go down that route; do you? FWIW, there are definitely both ethernet and SATA PCI devices which only allow a limited set of values in the cacheline size register... and that limited set does not include some of the modern machines. Jeff ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:53 ` Tejun Heo 2006-09-07 13:04 ` Matthew Wilcox @ 2006-09-07 13:10 ` Russell King 1 sibling, 0 replies; 21+ messages in thread From: Russell King @ 2006-09-07 13:10 UTC (permalink / raw) To: Tejun Heo; +Cc: Matthew Wilcox, Arjan van de Ven, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 02:53:57PM +0200, Tejun Heo wrote: > Matthew Wilcox wrote: > >On Thu, Sep 07, 2006 at 02:33:25PM +0200, Arjan van de Ven wrote: > >>>So I think we should redo the PCI subsystem to set cacheline size during > >>>the buswalk rather than waiting for drivers to ask for it to be set. > >>... while allowing for quirks for devices that go puke when this > >>register gets written ;) > >> > >>(afaik there are a few) > > > >So you want: > > > > unsigned int no_cls:1; /* Device pukes on write to Cacheline Size */ > > > >in struct pci_dev? > > The spec says that devices can put additional restriction on supported > cacheline size (IIRC, the example was something like power of two >= or > <= certain size) and should ignore (treat as zero) if unsupported value > is written. So, there might be need for more low level driver > involvement which knows device restrictions, but I don't know whether > such devices exist. The problem is that both ends of the bus need to know the cache line size for some of these commands to work correctly. Consider, as in Matthew's case, a read command which wraps at the cache line boundary. Unless both the master and slave are programmed with the same cache line size, it's entirely possible for the wrong memory locations to be read. Eg, the device might expect 0x118, 0x11c, 0x100, 0x104, but it actually gets 0x118, 0x11c, 0x120, 0x124 because the target got programmed with 64 while the master was set to 32. This is, of course, supposing that the memory read command is used in the cache line wrap mode. -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:40 ` Matthew Wilcox 2006-09-07 12:53 ` Tejun Heo @ 2006-09-07 13:01 ` Arjan van de Ven 1 sibling, 0 replies; 21+ messages in thread From: Arjan van de Ven @ 2006-09-07 13:01 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Tejun Heo, linux-pci, Greg KH, lkml On Thu, 2006-09-07 at 06:40 -0600, Matthew Wilcox wrote: > On Thu, Sep 07, 2006 at 02:33:25PM +0200, Arjan van de Ven wrote: > > > > > > > > So I think we should redo the PCI subsystem to set cacheline size during > > > the buswalk rather than waiting for drivers to ask for it to be set. > > > > ... while allowing for quirks for devices that go puke when this > > register gets written ;) > > > > (afaik there are a few) > > So you want: > > unsigned int no_cls:1; /* Device pukes on write to Cacheline Size */ > > in struct pci_dev? something like that yes... ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 12:23 ` Matthew Wilcox 2006-09-07 12:33 ` Arjan van de Ven @ 2006-09-07 13:02 ` Russell King 1 sibling, 0 replies; 21+ messages in thread From: Russell King @ 2006-09-07 13:02 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Tejun Heo, linux-pci, Greg KH, lkml On Thu, Sep 07, 2006 at 06:23:11AM -0600, Matthew Wilcox wrote: > On Thu, Sep 07, 2006 at 01:07:56PM +0100, Russell King wrote: > > I've often wondered why we don't set the cache line size when we set the > > bus master bit - ISTR when I read the PCI spec (2.1 or 2.2) it implied > > that this should be set for bus master operations. > > It's not required ... 3.2.1 of pci 2.3 says: > > While Memory Write and Invalidate is the only command that requires > implementation of the Cacheline Size register, it is strongly suggested > the memory read commands use it as well. A bridge that prefetches is > responsible for any latent data not consumed by the master. > > (obviously this is talking about requirements placed on the device, not > on the OS, but it'd behoove us to help the device out here). > > It's also useful to implement it for slave devices. PCI 2.3 has the > concept of cacheline wrap transactions -- eg with a cacheline size of > 0x10, it can transfer data to 0x108, then 0x10C, 0x100, 0x104, then > 0x118, etc As does v2.2 and v2.1. > So I think we should redo the PCI subsystem to set cacheline size during > the buswalk rather than waiting for drivers to ask for it to be set. Agreed, and this is something I'm already doing on ARM in my pcibios_fixup_bus(). -- Russell King Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: question regarding cacheline size 2006-09-07 8:31 question regarding cacheline size Tejun Heo 2006-09-07 11:11 ` Matthew Wilcox @ 2006-09-07 11:59 ` linux-os (Dick Johnson) 1 sibling, 0 replies; 21+ messages in thread From: linux-os (Dick Johnson) @ 2006-09-07 11:59 UTC (permalink / raw) To: Tejun Heo; +Cc: linux-pci, Greg KH, lkml On Thu, 7 Sep 2006, Tejun Heo wrote: > Hello, > > This is for PCMCIA (cardbus) version of Silicon Image 3124 SerialATA > controller. When cacheline size is configured, the controller uses Read > Multiple commands. > > • Bit [07:00]: Cache Line Size (R/W). This bit field is used to specify > the system cacheline size in terms of 32-bit words. The SiI3124, when > initiating a read transaction, will issue the Read Multiple PCI command > if empty space in its FIFO is greater than the value programmed in this > register. > > As the BIOS doesn't run after hotplugging cardbus card, the cache line > isn't configured and the controller ends up having 0 cache line size and > always using Read command. When that happens, write performance drops > hard - the throughput is < 2Mbytes/s. > > http://thread.gmane.org/gmane.linux.ide/12908/focus=12908 > > So, sata_sil24 driver has to program CLS if it's not already set, but > I'm not sure which number to punch in. FWIW, sil3124 doesn't seem to > put restrictions on the values which can be used for CLS. There are > several candidates... > > * L1_CACHE_BYTES / 4 : this is used by init routine in yenta_socket.c. > It seems to be a sane default but I'm not sure whether L1 cache line > size always coincides with the size as seen from PCI bus. > > * pci_cache_line_size in drivers/pci/pci.c : this is used for > pci_generic_prep_mwi() and can be overridden by arch specific code. > this seems more appropriate but is not exported. > > For all involved commands - memory read line, memory read multiple and > memory write and invalidate - a value larger than actual cacheline size > doesn't hurt but a smaller value may. > > I'm thinking of implementing a query function for pci_cache_line_size, > say, int pci_cacheline_size(struct pci_dev *pdev), and use it in the > device init routine. Does this sound sane? > > Thanks. > > -- > tejun The cache line size specifies the system cache-line size in dword increments. For most, (ix86) this would be 8, i.e., eight 32-bit words or 32 bytes. This is from page 376, PCI System Architecture, ISBN 0-201-30974-2. It also says that a device may limit the number of cache cycles if an unsupported value is written there. In that case, the device will act as if the value 0 was written (no write-and- invalidate transactions), basically poor performance. The L1 cache size shouldn't have anything to do with this, BTW. Cheers, Dick Johnson Penguin : Linux version 2.6.16.24 on an i686 machine (5592.66 BogoMips). New book: http://www.AbominableFirebug.com/ _ \x1a\x04 **************************************************************** The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them. Thank you. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2006-09-22 23:47 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-09-07 8:31 question regarding cacheline size Tejun Heo 2006-09-07 11:11 ` Matthew Wilcox 2006-09-07 11:20 ` Tejun Heo 2006-09-07 12:07 ` Russell King 2006-09-07 12:23 ` Matthew Wilcox 2006-09-07 12:33 ` Arjan van de Ven 2006-09-07 12:40 ` Matthew Wilcox 2006-09-07 12:53 ` Tejun Heo 2006-09-07 13:04 ` Matthew Wilcox 2006-09-07 13:19 ` Tejun Heo 2006-09-07 15:21 ` Grant Grundler 2006-09-07 15:47 ` Tejun Heo 2006-09-07 16:00 ` Jeff Garzik 2006-09-07 17:00 ` Matthew Wilcox 2006-09-07 16:04 ` Jeff Garzik 2006-09-22 23:47 ` Grant Grundler 2006-09-07 13:30 ` Jeff Garzik 2006-09-07 13:10 ` Russell King 2006-09-07 13:01 ` Arjan van de Ven 2006-09-07 13:02 ` Russell King 2006-09-07 11:59 ` linux-os (Dick Johnson)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox