From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SSsGY-0006Vl-UK for qemu-devel@nongnu.org; Fri, 11 May 2012 12:00:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SSsGP-0004RH-Gl for qemu-devel@nongnu.org; Fri, 11 May 2012 12:00:26 -0400 Received: from cantor2.suse.de ([195.135.220.15]:45749 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SSsGP-0004Qs-7L for qemu-devel@nongnu.org; Fri, 11 May 2012 12:00:17 -0400 Mime-Version: 1.0 (Apple Message framework v1257) Content-Type: text/plain; charset=us-ascii From: Alexander Graf In-Reply-To: Date: Fri, 11 May 2012 18:00:08 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <3D426BAD-FACE-41EE-B3CA-98286ADAE926@suse.de> References: <1336383010-28692-1-git-send-email-agraf@suse.de> Subject: Re: [Qemu-devel] [PATCH] linux-user: Fix stale tbs after mmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Riku Voipio , qemu-devel Developers On 11.05.2012, at 17:46, Peter Maydell wrote: > On 7 May 2012 12:38, Alexander Graf wrote: >>=20 >> On 07.05.2012, at 13:32, Alexander Graf wrote: >>=20 >>>=20 >>> On 07.05.2012, at 12:37, Peter Maydell wrote: >>>=20 >>>> On 7 May 2012 10:30, Alexander Graf wrote: >>>>> @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, = abi_ulong len, int prot, >>>>> page_dump(stdout); >>>>> printf("\n"); >>>>> #endif >>>>> + tb_invalidate_phys_page_range(start, start + len, 0); >>>>> mmap_unlock(); >>>>> return start; >>>>=20 >>>> The comment at the top of tb_invalidate_phys_page_range() says >>>> "start and end must refer to the same physical page" -- is it >>>> out of date or does that not apply to user-mode? >>>>=20 >>>> Do you need to also invalidate the range on munmap() and >>>> mprotect-to-not-executable in order to correctly fault on >>>> the case of: >>>> map something >>>> execute it >>>> unmap it >>>> try to execute it again >>>>=20 >>>> ? (haven't tested that case but it seems like it might be an issue) >>>=20 >>> Yeah, the issue does exist: >>=20 >> And the below patch on top of my revised patch fixes it. >=20 > I think these two patches look correct (and as you pointed out > on irc I was wrong about mprotect, which effectively already > handles flushing the tb if needed). If you can roll them together > into a single patch with a commit message and signed-off-by > you can add my Reviewed-by: tag to it. Well, if we invalidate on unmap, do we need to invalidate on mmap() = still? Or is only invalidating on unmap enough? Maybe when you use fixed = addresses... hrm Ah whatever, let's just flush everywhere now and then optimize later. Alex