From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aNiHR-0003we-C3 for qemu-devel@nongnu.org; Mon, 25 Jan 2016 09:38:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aNiHN-0005kj-Bt for qemu-devel@nongnu.org; Mon, 25 Jan 2016 09:38:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58086) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aNiHN-0005kf-6R for qemu-devel@nongnu.org; Mon, 25 Jan 2016 09:38:05 -0500 References: <1453732190-13416-1-git-send-email-ppandit@redhat.com> From: Paolo Bonzini Message-ID: <56A63347.2030103@redhat.com> Date: Mon, 25 Jan 2016 15:37:59 +0100 MIME-Version: 1.0 In-Reply-To: <1453732190-13416-1-git-send-email-ppandit@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for v2.4.1] exec: fix a glitch in checking dma r/w access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: P J P , QEMU Developers Cc: Donghai Zdh , Prasad J Pandit , Peter Maydell On 25/01/2016 15:29, P J P wrote: > diff --git a/exec.c b/exec.c > index 0a4a0c5..98d97d3 100644 > --- a/exec.c > +++ b/exec.c > @@ -375,7 +375,7 @@ address_space_translate_internal(AddressSpaceDispat= ch *d, hwaddr addr, hwaddr *x > static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_w= rite) > { > if (memory_region_is_ram(mr)) { > - return !(is_write && mr->readonly); > + return (is_write && !mr->readonly); Putting the various cases in a table: Read or write? Readonly? Old New Read Yes T F Read No T F Write Yes F F Write No T T This patch changes behavior for reads (is_write=3Dfalse). For address_space_read, this makes them go through a path that is at least 100 times slower (memory_region_dispatch_read instead of just a memcpy). For address_space_map, it probably breaks everything that expects a single block of RAM to be mapped in a single step, for example virtio. So, how was this tested, and how can the bug be triggered? Paolo > } > if (memory_region_is_romd(mr)) { > return !is_write; >=20