qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: P J P <ppandit@redhat.com>, QEMU Developers <qemu-devel@nongnu.org>
Cc: Donghai Zdh <donghai.zdh@alibaba-inc.com>,
	Prasad J Pandit <pjp@fedoraproject.org>,
	Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [PATCH for v2.4.1] exec: fix a glitch in checking dma r/w access
Date: Mon, 25 Jan 2016 15:37:59 +0100	[thread overview]
Message-ID: <56A63347.2030103@redhat.com> (raw)
In-Reply-To: <1453732190-13416-1-git-send-email-ppandit@redhat.com>



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(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
>  static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>  {
>      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=false).  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;
> 

  reply	other threads:[~2016-01-25 14:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-25 14:29 [Qemu-devel] [PATCH for v2.4.1] exec: fix a glitch in checking dma r/w access P J P
2016-01-25 14:37 ` Paolo Bonzini [this message]
2016-01-25 18:19   ` P J P
2016-01-25 22:15     ` Paolo Bonzini
2016-01-27  9:38       ` P J P

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A63347.2030103@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=donghai.zdh@alibaba-inc.com \
    --cc=peter.maydell@linaro.org \
    --cc=pjp@fedoraproject.org \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).