All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: liu ping fan <qemulist@gmail.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Anthony Liguori <anthony@codemonkey.ws>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock
Date: Mon, 24 Sep 2012 09:44:59 +0200	[thread overview]
Message-ID: <50600F7B.5080106@redhat.com> (raw)
In-Reply-To: <CAJnKYQmsiXPw=BknEZZb2+eEyKoKW5dakwv5TUVGR3mHLc-+VA@mail.gmail.com>

On 09/24/2012 08:33 AM, liu ping fan wrote:
> On Wed, Sep 19, 2012 at 5:50 PM, Avi Kivity <avi@redhat.com> wrote:
> > On 09/19/2012 12:34 PM, Jan Kiszka wrote:
> >>
> >> What about the following:
> >>
> >> What we really need to support in practice is MMIO access triggers RAM
> >> access of device model. Scenarios where a device access triggers another
> >> MMIO access could likely just be rejected without causing troubles.
> >>
> >> So, when we dispatch a request to a device, we mark that the current
> >> thread is in a MMIO dispatch and reject any follow-up c_p_m_rw that does
> >> _not_ target RAM, ie. is another, nested MMIO request - independent of
> >> its destination. How much of the known issues would this solve? And what
> >> would remain open?
> >
> > Various iommu-like devices re-dispatch I/O, like changing endianness or
> > bitband.  I don't know whether it targets I/O rather than RAM.
> >
> Have not found the exact code. But I think the call chain may look
> like this: dev mmio-handler --> c_p_m_rw() --> iommu mmio-handler -->
> c_p_m_rw()
> And I think you worry about the case for "c_p_m_rw() --> iommu
> mmio-handler". Right? How about introduce an member can_nest for
> MemoryRegionOps of iommu's mr?
>

I would rather push the iommu logic into the memory API:

  memory_region_init_iommu(MemoryRegion *mr, const char *name,
                           MemoryRegion *target, MemoryRegionIOMMUOps *ops,
                           unsigned size)

  struct MemoryRegionIOMMUOps {
      target_physical_addr_t (*translate)(target_physical_addr_t addr,
bool write);
      void (*fault)(target_physical_addr_t addr);
  };

I'll look at a proposal for this.  It's a generalized case of
memory_region_init_alias().

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

  reply	other threads:[~2012-09-24  7:45 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-19  3:02 [Qemu-devel] [big lock] Discussion about the convention of device's DMA each other after breaking down biglock liu ping fan
2012-09-19  8:06 ` Avi Kivity
2012-09-19  9:00   ` liu ping fan
2012-09-19  9:07     ` Avi Kivity
2012-09-19  9:11       ` liu ping fan
2012-09-19  9:14         ` Paolo Bonzini
2012-09-19  9:19           ` liu ping fan
2012-09-19  9:23             ` Avi Kivity
2012-09-19  9:27               ` Jan Kiszka
2012-09-19  9:28                 ` Jan Kiszka
2012-09-20  7:51               ` liu ping fan
2012-09-20  7:54                 ` Paolo Bonzini
2012-09-20  8:09                   ` liu ping fan
2012-09-20  8:27                     ` Paolo Bonzini
2012-09-20  9:07                 ` Avi Kivity
2012-09-21  7:27                   ` liu ping fan
2012-09-21  8:21                     ` Paolo Bonzini
2012-09-19  9:21           ` Avi Kivity
2012-09-19  9:51             ` Paolo Bonzini
2012-09-19 10:06               ` Avi Kivity
2012-09-19 10:19                 ` Paolo Bonzini
2012-09-19 10:27                   ` Avi Kivity
2012-09-19  9:34     ` Jan Kiszka
2012-09-19  9:50       ` Avi Kivity
2012-09-19 10:18         ` Jan Kiszka
2012-09-24  6:33         ` liu ping fan
2012-09-24  7:44           ` Avi Kivity [this message]
2012-09-24  8:32             ` liu ping fan
2012-09-24  9:42               ` Avi Kivity
2012-09-27  3:13                 ` liu ping fan
2012-09-27  9:16                   ` Avi Kivity
2012-09-27  9:29                     ` Paolo Bonzini
2012-09-27  9:34                       ` Avi Kivity
2012-09-27  9:36                         ` Paolo Bonzini
2012-09-27 10:08                           ` Avi Kivity
2012-09-27 10:22                             ` Paolo Bonzini
2012-09-27 10:48                               ` Avi Kivity
2012-09-29  9:20                     ` liu ping fan
2012-09-30  8:13                       ` Avi Kivity
2012-09-30  8:48                         ` liu ping fan
2012-09-30 11:18                           ` Avi Kivity
2012-09-30 11:04                         ` Blue Swirl
2012-09-30 11:17                           ` Avi Kivity
2012-09-30 11:48                             ` Blue Swirl
2012-09-20  8:11       ` liu ping fan

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=50600F7B.5080106@redhat.com \
    --to=avi@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=jan.kiszka@siemens.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.