qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
	chrisw@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] Re: [PATCH 2 of 5] add can_dma/post_dma for direct IO
Date: Sun, 14 Dec 2008 08:03:33 +0200	[thread overview]
Message-ID: <4944A1B5.5080300@redhat.com> (raw)
In-Reply-To: <49442410.7020608@codemonkey.ws>

Anthony Liguori wrote:
>>
>> There are N users of this code, all of which would need to cope with the
>> failure.  Or there could be one user (dma.c) which handles the failure
>> and the bouncing.
>>   
>
> N should be small long term.  It should only be for places that would 
> interact directly with CPU memory.  This would be the PCI BUS, the ISA 
> BUS, some speciality devices, and possibly virtio (although you could 
> argue it should go through the PCI BUS).

Fine, then let's rename it pci-dma.c.

>
> map() has to fail and that has nothing to do with bouncing or not 
> bouncing.  In the case of Xen, you can have a guest that has 8GB of 
> memory, and you only have 2GB of virtual address space.  If you try to 
> DMA to more than 2GB of memory, there will be a failure.  Whoever is 
> accessing memory directly in this fashion needs to cope with that.

The code already allows for failure, by partitioning the dma into 
segments.  Currently, this happens only on bounce buffer overflow, when 
the Xen code is integrated it can be expanded to accommodate this.

(There's a case for partitioning 2GB DMAs even without Xen; just to 
reduce the size of iovec allocations)

>
>> dma.c _is_ a map/unmap api, except it doesn't expose the mapped data,
>> which allows it to control scheduling as well as be easier to use.
>>   
>
> As I understand dma.c, it performs the following action: map() as much 
> as possible, call an actor on mapped memory, repeat until done, signal 
> completion.
>
> As an abstraction, it may be useful.  I would argue that it should be 
> a bit more generic though.  It should take a function pointer for map 
> and unmap too, and then you wouldn't need N versions of it for each 
> different type of API.

I don't follow.  What possible map/unmap pairs would it call, other than 
cpu_physical_memory_(map/unmap)()?

>
>> Right, but who would it notify?
>>
>> We need some place that can deal with this, and it isn't
>> _map()/_unmap(), and it isn't ide.c or scsi.c.
>>   
>
> The pattern of try to map(), do IO, unmap(), repeat only really works 
> for block IO.  It doesn't really work for network traffic.  You have 
> to map the entire packet and send it all at once.  You cannot accept a 
> partial mapping result.  The IO pattern to send an IO packet is much 
> simpler: try to map the packet, if the mapping fails, either wait 
> until more space frees up or drop the packet.  For the other uses of 
> direct memory access, like kernel loading, the same is true.

If so, the API should be extended to support more I/O patterns.

>
> What this is describing is not a DMA API.  It's a very specific IO 
> pattern.  I think that's part of what's causing confusion in this 
> series.  It's certainly not at all related to PCI DMA.

It deals with converting scatter/gather lists to iovecs, bouncing when 
this is not possible, and managing the bounce buffers.  If this is not 
dma, I'm not sure what is.  It certainly isn't part of block device 
emulation, it isn't part of the block layer (since bouncing is common to 
non-block devices).  What is it?

>
> I would argue, that you really want to add a block driver interface 
> that takes the necessary information, and implements this pattern but 
> that's not important.  Reducing code duplication is a good thing so 
> however it ends up working out is fine.

Right now the qemu block layer is totally independent of device 
emulation, and I think that's a good thing.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

  reply	other threads:[~2008-12-14  6:03 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-12 18:16 [Qemu-devel] [PATCH 0 of 5] dma api v3 Andrea Arcangeli
2008-12-12 18:16 ` [Qemu-devel] [PATCH 1 of 5] fix cpu_physical_memory len Andrea Arcangeli
2008-12-12 19:06   ` [Qemu-devel] " Anthony Liguori
2008-12-12 19:26     ` Andrea Arcangeli
2008-12-12 18:16 ` [Qemu-devel] [PATCH 2 of 5] add can_dma/post_dma for direct IO Andrea Arcangeli
2008-12-12 19:00   ` Blue Swirl
2008-12-12 19:18     ` Anthony Liguori
2008-12-12 20:05       ` Blue Swirl
2008-12-12 20:10         ` Anthony Liguori
2008-12-12 19:15   ` [Qemu-devel] " Anthony Liguori
2008-12-12 19:37     ` Andrea Arcangeli
2008-12-12 20:09       ` Anthony Liguori
2008-12-12 20:25       ` Gerd Hoffmann
2008-12-12 19:39     ` Anthony Liguori
2008-12-13  9:22       ` Avi Kivity
2008-12-13 16:45         ` Anthony Liguori
2008-12-13 19:48           ` Avi Kivity
2008-12-13 21:07             ` Anthony Liguori
2008-12-14  6:03               ` Avi Kivity [this message]
2008-12-14 19:10                 ` Anthony Liguori
2008-12-14 19:49                   ` Avi Kivity
2008-12-14 23:08                     ` Anthony Liguori
2008-12-15  0:57                       ` Paul Brook
2008-12-15  2:09                         ` Anthony Liguori
2008-12-15  6:23                           ` Avi Kivity
2008-12-15 18:35                       ` Blue Swirl
2008-12-15 22:06                         ` Anthony Liguori
2008-12-16  9:41                           ` Avi Kivity
2008-12-16 16:55                             ` Blue Swirl
2008-12-16 17:09                               ` Avi Kivity
2008-12-16 17:48                                 ` Anthony Liguori
2008-12-16 18:11                                   ` Blue Swirl
2008-12-16 15:57                           ` Blue Swirl
2008-12-16 16:29                             ` Paul Brook
2008-12-16 16:35                               ` Blue Swirl
2008-12-14 17:30       ` Jamie Lokier
2008-12-13 14:39     ` Andrea Arcangeli
2008-12-13 16:46       ` Anthony Liguori
2008-12-13 16:53         ` Andrea Arcangeli
2008-12-13 17:54           ` Andreas Färber
2008-12-13 21:11           ` Anthony Liguori
2008-12-14 16:47             ` Andrea Arcangeli
2008-12-14 17:01               ` Avi Kivity
2008-12-14 17:15                 ` Andrea Arcangeli
2008-12-14 19:59                   ` Avi Kivity
2008-12-22 16:44                     ` Ian Jackson
2008-12-22 19:44                       ` Avi Kivity
2008-12-23  0:03                         ` Thiemo Seufer
2008-12-23  1:02                           ` Andrea Arcangeli
2008-12-23 17:31                           ` Avi Kivity
2008-12-22 19:46                       ` Avi Kivity
2009-01-05 10:27                         ` Gerd Hoffmann
2008-12-13 22:47     ` Anthony Liguori
2008-12-14  6:07       ` Avi Kivity
2008-12-12 18:16 ` [Qemu-devel] [PATCH 3 of 5] rename dma.c to isa_dma.c Andrea Arcangeli
2008-12-12 18:16 ` [Qemu-devel] [PATCH 4 of 5] dma api Andrea Arcangeli
2008-12-12 18:55   ` Blue Swirl
2008-12-12 18:16 ` [Qemu-devel] [PATCH 5 of 5] bdrv_aio_readv/writev Andrea Arcangeli

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=4944A1B5.5080300@redhat.com \
    --to=avi@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=chrisw@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kvm@vger.kernel.org \
    --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).