linux-kernel - Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <47392768.3030509@shaw.ca>
Date:	Mon, 12 Nov 2007 22:26:16 -0600
From:	Robert Hancock <hancockr@...w.ca>
To:	Tejun Heo <htejun@...il.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	ide <linux-ide@...r.kernel.org>, Jeff Garzik <jeff@...zik.org>
Subject: Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB

Tejun Heo wrote:
> How about always initialize DMA mask to ATA_DMA_MASK regardless of ADMA
> mode such that PRD and PAD buffers are always accessible by register
> mode and just raising PCI dma mask and queue bounce limit if ADMA mode
> is active?

Could be done.. but, I don't want to constrain the ADMA APRD/CPB area in 
that way (there are some dual-socket Opteron boxes with this controller, 
forcing an allocation below 4GB for this could force a non-optimal node 
allocation I think..) To do this I'd have to raise the mask for the APRD 
allocation, drop it again, then raise it again in ADMA mode, which is 
kind of ugly.

Also, I'd rather not allocate the legacy PRD at all if we're in ADMA 
mode. That way, if some bug causes us to try and do legacy DMA in ADMA 
mode, we'll crash from null pointer dereference instead of potentially 
transferring incorrect data (as we had in this case) and corrupting things.

> 
>> +	/* Set appropriate DMA mask. */
>> +	pci_set_dma_mask(pdev, bounce_limit);
>> +	pci_set_consistent_dma_mask(pdev, bounce_limit);
> 
> These can fail.

Yes, it should likely do something with these return values. Though 
theoretically it shouldn't fail, since the DMA mask is either 32-bit, 
which shouldn't fail, or one that was successfully set before. Also I 
don't think the SCSI layer actually checks the slave_config return 
value.. sigh.

> 
> Also, please separate out the result TF handling to a separate patch.  I
> know it's a small change but as both introduces important behavior
> changes, I think it would be nice to have a bisection point inbetween.

Could do. That change would have to come first though, as the change to 
not allocate the PRD except when necessary would cause some cases there 
to blow up when before they might have worked in some cases.

> 
> Thanks.
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/