All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Paul Brook <paul@codesourcery.com>
Cc: amit.shah@redhat.com, quintela@redhat.com, kraxel@redhat.com,
	qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: [Qemu-devel] Re: [PATCHv2 10/12] tap: add vhost/vhostfd options
Date: Tue, 02 Mar 2010 10:12:05 -0600	[thread overview]
Message-ID: <4B8D38D5.40507@codemonkey.ws> (raw)
In-Reply-To: <201003021553.31042.paul@codesourcery.com>

On 03/02/2010 09:53 AM, Paul Brook wrote:
>>>> The key difference is that these regions are created and destroyed
>>>> rarely and in such a way that the destruction is visible to the guest.
>>>>          
>>> So you're making ram unmap an asynchronous process, and requiring that
>>> the address space not be reused until that umap has completed?
>>>        
>> It technically already would be.  If you've got a pending DMA
>> transaction and you try to hot unplug badness will happen.  This is
>> something that is certainly exploitable.
>>      
> Hmm, I guess we probably want to make this work with all mappings then. DMA to
> a ram backed PCI BAR (e.g. video ram) is certainly feasible.
> Technically it's not the unmap that causes badness, it's freeing the
> underlying ram.
>    

Let's avoid confusing terminology.  We have RAM mappings and then we 
have PCI BARs that are mapped as IO_MEM_RAM.

PCI BARs mapped as IO_MEM_RAM are allocated by the device and live for 
the duration of the device.  If you did something that changed the BAR's 
mapping from IO_MEM_RAM to an actual IO memory type, then you'd continue 
to DMA to the allocated device memory instead of doing MMIO operations.[1]

That's completely accurate and safe.  If you did this to bare metal, I 
expect you'd get very similar results.

This is different from DMA'ing to a RAM region and then removing the RAM 
region while the IO is in flight.  In this case, the mapping disappears 
and you potentially have the guest writing to an invalid host pointer.

[1] I don't think it's useful to support DMA'ing to arbitrary IO_MEM_RAM 
areas.  Instead, I think we should always bounce to this memory.  The 
benefit is that we avoid the complications resulting from PCI hot unplug 
and reference counting.

> For these reasons I'm tempted to push the refcounting down to the ram
> allocation level. This has a couple of nice properties.
>
> Firstly we don't care about dynamic allocation any more. We just say that
> mapping changes may not effect active DMA transactions.

Only if we think it's necessary to support native DMA to arbitrary 
IO_MEM_RAM.  I contend this is never a normal or performance sensitive 
case and it's not worth supporting.

>   If virtio chooses to
> define that the vring DMA transaction starts when the device is enabled and
> ends when disabled, that's fine by me.  This probably requires revisiting the
> memory barrier issues - barriers are pointless if you don't guarantee cache
> coherence (i.e. no bounce buffers).
>
> Secondly, ram deallocation is not guest visible. The guest visible parts
> (memory unmapping) can happen immediately, and we avoid a whole set of
> unplug/replug race conditions. We may want to delay the completion of a
> monitor hotplug command until the actual deallocation occurs, but that's a
> largely separate issue.
>    

You can do the same thing and always bounce IO_MEM_RAM IO regions.  It's 
just a question of whether we think it's worth the effort to do native 
DMA to this type of memory.  I personally don't think it is at least in 
the beginning.

Regards,

Anthony Liguori

> Paul
>    

  parent reply	other threads:[~2010-03-02 16:12 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-25 18:27 [Qemu-devel] [PATCHv2 00/12] vhost-net: upstream integration Michael S. Tsirkin
2010-02-25 18:27 ` [Qemu-devel] [PATCHv2 05/12] virtio: add APIs for queue fields Michael S. Tsirkin
2010-02-25 18:49   ` Blue Swirl
2010-02-26 14:53     ` Michael S. Tsirkin
2010-02-25 19:25   ` [Qemu-devel] " Anthony Liguori
2010-02-26  8:46     ` Gleb Natapov
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 09/12] vhost: vhost net support Michael S. Tsirkin
2010-02-25 19:04   ` [Qemu-devel] " Juan Quintela
2010-02-26 14:32     ` Michael S. Tsirkin
2010-02-26 14:38       ` Anthony Liguori
2010-02-26 14:54         ` Michael S. Tsirkin
2010-02-25 19:44   ` Anthony Liguori
2010-02-26 14:49     ` Michael S. Tsirkin
2010-02-26 15:18       ` Anthony Liguori
2010-02-27 19:38         ` Michael S. Tsirkin
2010-02-28  1:59           ` Paul Brook
2010-02-28 10:15             ` Michael S. Tsirkin
2010-02-28 12:45               ` Paul Brook
2010-02-28 14:44                 ` Michael S. Tsirkin
2010-02-28 15:23                   ` Paul Brook
2010-02-28 15:37                     ` Michael S. Tsirkin
2010-02-28 16:02           ` Anthony Liguori
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 02/12] kvm: add API to set ioeventfd Michael S. Tsirkin
2010-02-25 19:19   ` [Qemu-devel] " Anthony Liguori
2010-03-02 17:41     ` Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 04/12] virtio: add notifier support Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 01/12] tap: add interface to get device fd Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 07/12] virtio: move typedef to qemu-common Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 10/12] tap: add vhost/vhostfd options Michael S. Tsirkin
2010-02-25 19:47   ` [Qemu-devel] " Anthony Liguori
2010-02-26 14:51     ` Michael S. Tsirkin
2010-02-26 15:23       ` Anthony Liguori
2010-02-27 19:44         ` Michael S. Tsirkin
2010-02-28 16:08           ` Anthony Liguori
2010-02-28 17:19             ` Michael S. Tsirkin
2010-02-28 20:57               ` Anthony Liguori
2010-02-28 21:01                 ` Michael S. Tsirkin
2010-02-28 22:38                   ` Anthony Liguori
2010-02-28 22:39                 ` Paul Brook
2010-03-01 19:27                   ` Michael S. Tsirkin
2010-03-01 21:54                     ` Anthony Liguori
2010-03-02  9:57                       ` Michael S. Tsirkin
2010-03-02 14:07                   ` Anthony Liguori
2010-03-02 14:33                     ` Paul Brook
2010-03-02 14:39                       ` Anthony Liguori
2010-03-02 14:55                         ` Paul Brook
2010-03-02 15:33                           ` Anthony Liguori
2010-03-02 15:53                             ` Paul Brook
2010-03-02 15:56                               ` Michael S. Tsirkin
2010-03-02 16:12                               ` Anthony Liguori [this message]
2010-03-02 16:21                                 ` Marcelo Tosatti
2010-03-02 16:12                 ` Marcelo Tosatti
2010-03-02 16:56                   ` Anthony Liguori
2010-03-02 17:00                     ` Michael S. Tsirkin
2010-03-02 18:00                     ` Marcelo Tosatti
2010-03-02 18:13                       ` Anthony Liguori
2010-03-02 22:41                     ` Paul Brook
2010-03-03 14:15                       ` Anthony Liguori
2010-03-03 14:43                         ` Paul Brook
2010-03-03 16:24                         ` Marcelo Tosatti
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 11/12] tap: add API to retrieve vhost net header Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 06/12] virtio: add set_status callback Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 08/12] virtio-pci: fill in notifier support Michael S. Tsirkin
2010-02-25 19:30   ` [Qemu-devel] " Anthony Liguori
2010-02-28 20:02     ` Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 03/12] notifier: event notifier implementation Michael S. Tsirkin
2010-02-25 19:22   ` [Qemu-devel] " Anthony Liguori
2010-02-28 19:59     ` Michael S. Tsirkin
2010-02-25 18:28 ` [Qemu-devel] [PATCHv2 12/12] virtio-net: vhost net support Michael S. Tsirkin
2010-02-25 19:49 ` [Qemu-devel] Re: [PATCHv2 00/12] vhost-net: upstream integration Anthony Liguori

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=4B8D38D5.40507@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=amit.shah@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.com \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.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.