SATA open bugs

Hi Tejun, Alan, Jeff,

This is open bug list for SATA subsystem. It has more items than other subsystems, but mainly because this is such a hot technology now and so much work is being done in this area. Needless to say It is very well maintained and exemplary for some other areas that are not that lucky...
Andrew and myself did some initial cleaning and updates but we really need your participation and help with the rest.
Some of open bugs are not actually on this list, only the ones that has been quiet for a while or never been looked at.
Older bugs are probably not really bugs anymore, since SATA has changed so much but you can probably clean up this list with good guesses about things being fixed or fixed already, or simply too cold to keep open.

Thanks,
--Natalie

http://bugzilla.kernel.org/show_bug.cgi?id=7693
http://bugzilla.kernel.org/show_bug.cgi?id=8738 adaptec and sata bad interaction
http://bugzilla.kernel.org/show_bug.cgi?id=8563 - waiting on alan
http://bugzilla.kernel.org/show_bug.cgi?id=8441 - patch review
http://bugzilla.kernel.org/show_bug.cgi?id=8649
http://bugzilla.kernel.org/show_bug.cgi?id=8649
http://bugzilla.kernel.org/show_bug.cgi?id=8424 - patch review
http://bugzilla.kernel.org/show_bug.cgi?id=8410 - patch status from tejun
http://bugzilla.kernel.org/show_bug.cgi?id=8331 - waiting on reporter
http://bugzilla.kernel.org/show_bug.cgi?id=8281
http://bugzilla.kernel.org/show_bug.cgi?id=8223 small fixup, low priority tejun is out of time
http://bugzilla.kernel.org/show_bug.cgi?id=7997
http://bugzilla.kernel.org/show_bug.cgi?id=7869
http://bugzilla.kernel.org/show_bug.cgi?id=7811 - regression
http://bugzilla.kernel.org/show_bug.cgi?id=7805 tejun is pinging pktcdvd maintainer (?)
http://bugzilla.kernel.org/show_bug.cgi?id=7599
http://bugzilla.kernel.org/show_bug.cgi?id=7547 tejun is asking " and Jens
http://bugzilla.kernel.org/show_bug.cgi?id=7545 cold
http://bugzilla.kernel.org/show_bug.cgi?id=7507
http://bugzilla.kernel.org/show_bug.cgi?id=7501
http://bugzilla.kernel.org/show_bug.cgi?id=7500
http://bugzilla.kernel.org/show_bug.cgi?id=7489 status?
http://bugzilla.kernel.org/show_bug.cgi?id=7411 status?
http://bugzilla.kernel.org/show_bug.cgi?id=7241 waiting on reporter
http://bugzilla.kernel.org/show_bug.cgi?id=7119 cold fixed?
http://bugzilla.kernel.org/show_bug.cgi?id=7029 reporter is trying to get the hw
http://bugzilla.kernel.org/show_bug.cgi?id=6891 - old sata report
http://bugzilla.kernel.org/show_bug.cgi?id=6890 - old kernel, needs retesting
http://bugzilla.kernel.org/show_bug.cgi?id=6665 - old kernel
http://bugzilla.kernel.org/show_bug.cgi?id=6594 - cold
http://bugzilla.kernel.org/show_bug.cgi?id=6592 - pinged
http://bugzilla.kernel.org/show_bug.cgi?id=6587
http://bugzilla.kernel.org/show_bug.cgi?id=6516 - waiting on reporter
http://bugzilla.kernel.org/show_bug.cgi?id=6253 - cold? waiting on reporter
http://bugzilla.kernel.org/show_bug.cgi?id=6207 - cold?
http://bugzilla.kernel.org/show_bug.cgi?id=5863 - cold
http://bugzilla.kernel.org/show_bug.cgi?id=5798 - cold
http://bugzilla.kernel.org/show_bug.cgi?id=4968 - cold
http://bugzilla.kernel.org/show_bug.cgi?id=4238 pinged
http://bugzilla.kernel.org/show_bug.cgi?id=3993 pinged
http://bugzilla.kernel.org/show_bug.cgi?id=3725 pinged
http://bugzilla.kernel.org/show_bug.cgi?id=3352 cold? waiting on reporter
http://bugzilla.kernel.org/show_bug.cgi?id=2893 pinged

...

...

Err.. this would be far more useful if the posting also included one-liner descriptions of the bugs.

Clicking on one link at a time over and over just doesn't encourage the same level of participation.

Cheers

Hello, Natalie.

This bug isn't related to libata in anyway. It's "Can't use radeonfb with dvi port on a Samsung SyncMaster 204B (1600x1200)".

This is SATA support in SAS controller (SCSI) and probably solved by now.

This is being worked on. It requires some redesign in IRQ masking and polling code.

This one is chrp specific. The native mode fix didn't work. I'll follow up later.

This one is on Alan.

This is done. Closing.

Will check with SIMG and see whether we need to worry about flash.

On my todo list. I think I'll defer this till we do EH per-queue.

Closed.

Seems to be yet another IRQ routing problem on via chipset. I don't think this has much to do with libata drivers.

Not a libata bug. This is caused by pktcdvd holding onto the device thus preventing it from being revalidated. Didn't get pong for my ping yet. Jens, any ideas?

Closed.

This one is pktcdvd too. Jens?

Marked as duplicate of bug 8421.

Asked for testing on newer kernels.

Working on it now.

Patch proposed.

Closed.

Asked Alan to close it.

Ditto.

Pinged.

Pinged.

Closed as INSUFFICIENT_DATA.

Eeeek. Yet another via IRQ problem. Pinged.

Marked as duplicate of bug 8421.

Should probably be closed. It's too old now. Jeff?

Closed.

Pinged.

Definitely not a libata bug. Pinged.

Rejected as INSUFFICIENT_DATA.

Ditto.

Closed.

Closed.

INSUFFICIENT_DATA

FIXED

I think it's about time to close this one. It's way too cold. Jeff?

Closed.

Way too cold. Somebody please close this.

Jeff, please close this as INSUFFICIENT_DATA.

Thanks.

tejun

Ask Peter, I haven't worked on pktcdvd in ~5 years or so :-). CC'ed.

Jens Axboe

Following up in private with original porter now.

I really think this is a combination of a chip bug and firmware configuration being futzed. We might have to leave IRQ steering at 14/15 even in PCI mode, because the ISA bridge isn't so smart as the docs imply it is.

Matt Sealey <[email protected]>
Genesi, Manager, Developer Relations

I think not - something horrible is happening in dma_alloc_coherent when called from dmam_* with a non PCI device

Seems to be some kind of AMD64 specific DMA mapping bug ?

I think it's dev->dma_mask == NULL. Clearly you're passing a non DMA able device to dma_alloc_coherent(). Which seems like a caller bug.

-Andi

Ok - other archs seem to just return NULL in this case. If its your view that dma_alloc_coherent(dev) should explode not return NULL then we can wrap dma_alloc_coherent but I'm not sure the caller library has any business knowing whether a specific class of device is DMA capable on a specific platform ?

Alan

Ok then the initialization would fail.

I could do that, but clearly there must be some other bug in the process that tries to do DMA on such random devices in the first place.

Someone must ask that caller library to DMA to/from that device in the first place. Whoever it is it is wrong.

Or perhaps you got the wrong device here? For ISA devices we traditionally used NULL. Or if you set up your own ISA devices (which I can't see a reason for but there might be one I'm missing) at least give them a dma mask. Then it should probably work on x86-64.

-Andi

No I disagree.

The libata code currently (and this seems to work for all but x86-64) does the following if it is setting up a potentially DMA capable device

- Allocate a dma_coherent buffer
- If it is refused then turn off DMA and use PIO

It has no idea whether a pcmcia, isa, platform or even PCI device happens to be DMA capable, and there are platforms with PCI but very limited DMA for example in the embedded space. In fact it has no idea this level whether it is working a PCI, ISA, PCMCIA, SBUS or some other bus device. It's supposed to be generic code.

Obviously doing anything other than dma_alloc_coherent if the allocation fails is stupid but trying to allocate dma memory to find out if the device can be used with DMA on a given platform is quite logical and sensible in some cases.

Thus I think dma_alloc_coherent() for an ISA, PCMCIA or other class (platform particularly) shouldn't explode on AMD64 but simply return NULL. Its a sane request to make when you don't in your library know what dev is.

Alan

Where does the device come from? What device is it? Is that known already?

But at least on x86-64 the device is likely DMA capable. At least PCMCIA usually is. Near all devices are DMA capable, except perhaps SMBus and some real oddballs. So the device should not have a NULL dma_mask in the first place. Or it is the wrong device. e.g. we have platform devices for things like timers etc. which are really not DMA capable because it doesn't make much sense to DMA your timer; but there should be really no reason to use such a device in a libata driver.

If it's some kind of PCMCIA device like implied by pata_pcmcia then it's likely DMA capable.

So if I return NULL then either a really DMA capable device will not work. Or you got a real oddball device that you shouldn't have gotten in the first place.

Perhaps the PCMCIA code sets up some of its devices wrong?

-Andi

At the higher level someone passed us a device and some mappings and function methods and said "this is an IDE controller"

The core code has no idea or interest in where it came from.

PCMCIA usally isn't.

What about other platforms ? I don't want libata to have

#ifdef CONFIG_X86_64
	/* this platform is different)
	if (hack-around-in-dev-struct())
		return NULL;

#endif

All I want is

This is a device, the caller has said it may be DMA capable dma_alloc_coherent
NULL. ok in this situation it isn't

I don't want to know about arch internals. I don't want to know about platform specific DMA rules. I just want to be able to ask for DMA and get told yes or no. Oops is not a useful error return.

Ok you're stabbing in the dark. I guess more debugging is needed.

Hmm?

I'm not blaming libata here, just whoever gave that bogus device to it.

-Andi

Huh ?

PCMCIA is not DMA capable, and indeed passing a pcmcia device to AMD64 boxes dma_alloc_ functions causes an Oops.

For the simple example the PCMCIA layer.

I'll submit a patch to check this in my next batch.

But as James pointed out you'll likely need similar patches on other architectures.

-Andi

BTW unless I'm misreading the i386 code it'll not fail here, but allocate memory. Surely that will cause failures later if you rely on it failing? If you don't rely on it then changing x86-64 will also not help you.

-Andi

Eww that'll do strange things.

Ok so we do in fact need some kind of proper way to ask if a device is DMA capable ?

Alan

In theory I could change i386 too to return NULL in this case (dma_mask == NULL). But I wonder how how many existing PCMCIA drivers I would break this way?

Probably needs more testing at least.

My current patch is in ftp://ftp.firstfloor.org/pub/ak/x86_64/late-merge/patches/dma-alloc-mask

Right now it seems to be (dev->dma_mask != NULL)

-Andi

I'll follow that path for now then

Not in non platform code, please ... somewhere on the Janitor's list is moving the dma_mask from the bus specific devices into the generic device ... when that happens this quantity will become u64 and they won't know what to do with the NULL check.

Even today, it's wrong for this to be NULL. It's a bug somewhere in the pcmcia layer. What should happen is that there should be a u64 dma_mask somewhere in struct pcmcia_device where this should be pointing.

James

Ok filed for "kernel summit"

Surely we don't need to wait until then? This is the correct fix, isn't it? (Obviously I'll split it into a generic and a pcmcia specific piece if it looks OK to everyone).

It sets the PCMCIA dma_mask up correctly and introduces a DMA_MASK_NONE (I prefer that to DMA_0BIT_MASK but I can add that too if people want) and gives Alan his is_device_dma_capable() API.

James

index a996071..b3837d3 100644
@@ -23,6 +23,7 @@
 #include <linux/crc32.h>
 #include <linux/firmware.h>
 #include <linux/kref.h>
+#include <linux/dma-mapping.h>
 
 #define IN_CARD_SERVICES
 #include <pcmcia/cs_types.h>

@@ -670,6 +671,9 @@ struct pcmcia_device * pcmcia_device_add(struct pcmcia_socket *s, unsigned int f
 	p_dev->dev.bus = &pcmcia_bus_type;
 	p_dev->dev.parent = s->dev.parent;
 	p_dev->dev.release = pcmcia_release_dev;
+	/* by default don't allow DMA */
+	p_dev->dma_mask = DMA_MASK_NONE;
+	p_dev->dev.dma_mask = &p_dev->dma_mask;
 	bus_id_len = sprintf (p_dev->dev.bus_id, "%d.%d", p_dev->socket->sock, p_dev->device_no);
 
 	p_dev->devname = kmalloc(6 + bus_id_len + 1, GFP_KERNEL);

index 2dc21cb..0ebfafb 100644
@@ -24,6 +24,8 @@ enum dma_data_direction {
 #define DMA_28BIT_MASK	0x000000000fffffffULL
 #define DMA_24BIT_MASK	0x0000000000ffffffULL
 
+#define DMA_MASK_NONE	0x0ULL
+
 static inline int valid_dma_direction(int dma_direction)
 {
 	return ((dma_direction == DMA_BIDIRECTIONAL) ||

@@ -31,6 +33,11 @@ static inline int valid_dma_direction(int dma_direction)
 		(dma_direction == DMA_FROM_DEVICE));
 }
 
+static inline int is_device_dma_capable(struct device *dev)
+{
+	return dev->dma_mask != NULL && *dev->dma_mask != DMA_MASK_NONE;
+}
+
 #ifdef CONFIG_HAS_DMA
 #include <asm/dma-mapping.h>
 #else

index 90ef552..f047a1f 100644
@@ -184,6 +184,7 @@ struct pcmcia_device {
 
 	char *			prod_id[4];
 
+	u64			dma_mask;
 	struct device		dev;
 
 #ifdef CONFIG_PCMCIA_IOCTL

Patch looks good to me.

-Andi

No one else has commented ... shall I just submit the following two for the merge window?

James

Some devices are incapable of DMA and need to be recognised as such. Introduce a NONE dma mask to facilitate this plus an inline function:
is_device_dma_capable() to check this.

index 2dc21cb..0ebfafb 100644
@@ -24,6 +24,8 @@ enum dma_data_direction {
 #define DMA_28BIT_MASK	0x000000000fffffffULL
 #define DMA_24BIT_MASK	0x0000000000ffffffULL
 
+#define DMA_MASK_NONE	0x0ULL
+
 static inline int valid_dma_direction(int dma_direction)
 {
 	return ((dma_direction == DMA_BIDIRECTIONAL) ||

@@ -31,6 +33,11 @@ static inline int valid_dma_direction(int dma_direction)
 		(dma_direction == DMA_FROM_DEVICE));
 }
 
+static inline int is_device_dma_capable(struct device *dev)
+{
+	return dev->dma_mask != NULL && *dev->dma_mask != DMA_MASK_NONE;
+}
+
 #ifdef CONFIG_HAS_DMA
 #include <asm/dma-mapping.h>
 #else

index 90ef552..f047a1f 100644

Most non cardbus devices can't do dma, so flag them as such in the device creation routine.

index a996071..b3837d3 100644
@@ -23,6 +23,7 @@
 #include <linux/crc32.h>
 #include <linux/firmware.h>
 #include <linux/kref.h>
+#include <linux/dma-mapping.h>
 
 #define IN_CARD_SERVICES
 #include <pcmcia/cs_types.h>

@@ -670,6 +671,9 @@ struct pcmcia_device * pcmcia_device_add(struct pcmcia_socket *s, unsigned int f
 	p_dev->dev.bus = &pcmcia_bus_type;
 	p_dev->dev.parent = s->dev.parent;
 	p_dev->dev.release = pcmcia_release_dev;
+	/* by default don't allow DMA */
+	p_dev->dma_mask = DMA_MASK_NONE;
+	p_dev->dev.dma_mask = &p_dev->dma_mask;
 	bus_id_len = sprintf (p_dev->dev.bus_id, "%d.%d", p_dev->socket->sock, p_dev->device_no);
 
 	p_dev->devname = kmalloc(6 + bus_id_len + 1, GFP_KERNEL);

index 90ef552..f047a1f 100644
@@ -184,6 +184,7 @@ struct pcmcia_device {
 
 	char *			prod_id[4];
 
+	u64			dma_mask;
 	struct device		dev;
 
 #ifdef CONFIG_PCMCIA_IOCTL

I'm with Andi here ... you're fortunate that parisc has no working IDE/SATA interface (or rather we have so few running boxes with PCMCIA), but if it did this call sequence would explode on that platform for a different reason.

You cannot allocate a dma_coherent buffer without passing in the correct underlying device ... on parisc we'd explode trying to find the iommu to map through (some of our hw has more than one).

PCMCIA should be correctly plumbed into the generic model. It's really just a bridge ... how that bridge is wired critically affects how coherent memory is allocated.

James

We do pass the correct underlying device. Then dma_alloc_coherent oopses on x86_64 if you pass some valid devices (eg pcmcia).

If the device you're passing has a NULL dma_mask pointer that means the platform hasn't set it up correctly for dma ... and that's the underlying problem (although it's not necessarily a libata problem): you can't call dma_ operations on a device that hasn't been set up in the platform for it.

James

Well it may not do DMA.

Right.

Ok so what is the right way given a random *correctly* set up device that may or may not be DMA capable to determine this. Its an arbitary device pointer for an arbitary device class on an arbitary platform which may or may not support DMA for this device class ?

Can I called dma_alloc_coherent and get a NULL back or do we need a device_can_dma() check in each platform ?

Alan

Unfortunately, I don't think we've considered that possibility in the integration of the dma API and generic devices.

Fundamentally it's a different question. The dma_alloc is just saying can you set up a coherent memory area that this device can reach. Often the platform cannot necessarily discover this. I'd say the best signal is that the driver has called dma_set_mask(dev, DMA_MASK_NONE) (which isn't defined yet). So the platform has a signal that the device isn't DMA capable.

James

This one's not a bug. the aic94xx didn't attach to sata devices before post 2.6.22 by design. The 2.6.23 rc's should work.

James