From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver Date: Fri, 13 Jul 2007 15:05:08 +0200 Message-ID: <20070713130508.GQ5328@kernel.dk> References: <20070704132212.726923000@pademelon.sonytel.be> <20070704132346.591348000@pademelon.sonytel.be> <1184331741.3402.13.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from brick.kernel.dk ([80.160.20.94]:23834 "EHLO kernel.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758874AbXGMNFY (ORCPT ); Fri, 13 Jul 2007 09:05:24 -0400 Content-Disposition: inline In-Reply-To: <1184331741.3402.13.camel@localhost.localdomain> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: James Bottomley Cc: Geert Uytterhoeven , Paul Mackerras , Alessandro Rubini , linuxppc-dev@ozlabs.org, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, Geoff Levand On Fri, Jul 13 2007, James Bottomley wrote: > On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote: > > + 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. Well, even worse is that fact that it's using KM_USER0 from interrupt context. -- Jens Axboe