Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver

On Fri, 13 Jul 2007, Jens Axboe wrote:

On Fri, Jul 13 2007, James Bottomley wrote:

quoted

On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote:

quoted

+                       kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+                       if (!kaddr)
+                               return -1;
+                       len = sgpnt->length;
+                       if ((req_len + len) > buflen) {
+                               active = 0;
+                               len = buflen - req_len;
+                       }
+                       memcpy(kaddr + sgpnt->offset, buf + req_len,
len);
+                       kunmap_atomic(kaddr, KM_USER0);

This isn't a SCSI objection, but this sequence appears several times in
this driver.  It's wrong for a non-PIPT architecture (and I believe the
PS3 is VIPT) because you copy into the kernel alias for the page, which
dirties the line in the cache of that alias (the user alias cache line
was already invalidated).  However, unless you flush the kernel alias to
main memory, the user could read stale data.  The way this is supposed
to be done is to do a 

flush_kernel_dcache_page(kaddr)

before doing the kunmap.

Otherwise it looks OK from the SCSI point of view.

kmap() just returns page_address() on ppc64, as there's no highmem.
kunmap() is a no-op.

So technically I could just use page_address() directly, but Christoph wanted
me to keep the kmap()/kunmap() sequence because it's considered a good
practice.

Well, even worse is that fact that it's using KM_USER0 from interrupt
context.

So should I replace it by e.g. KM_IRQ0?
I'm not so familiar with these parts, and I couldn't find what these values
really mean.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
 
Phone:    +32 (0)2 700 8453	
Fax:      +32 (0)2 700 8622	
E-mail:   Geert.Uytterhoeven@sonycom.com	
Internet: http://www.sony-europe.com/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619