Re: [Qemu-devel] [PATCH qemu v7 13/14] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW)

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3487 bytes --]

On Fri, Jun 19, 2015 at 08:49:00AM +0200, Markus Armbruster wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Thu, Jun 18, 2015 at 09:35:44PM +1000, Alexey Kardashevskiy wrote:
> >> On 05/05/2015 10:49 PM, David Gibson wrote:
> >> >On Sat, Apr 25, 2015 at 10:24:43PM +1000, Alexey Kardashevskiy wrote:
> >> >>This adds support for Dynamic DMA Windows (DDW) option defined by
> >> >>the SPAPR specification which allows to have additional DMA window(s)
> >> >>
> >> >>This implements DDW for emulated and VFIO devices. As all TCE root regions
> >> >>are mapped at 0 and 64bit long (and actual tables are child regions),
> >> >>this replaces memory_region_add_subregion() with _overlap() to make
> >> >>QEMU memory API happy.
> >> >>
> >> >>This reserves RTAS token numbers for DDW calls.
> >> >>
> >> >>This implements helpers to interact with VFIO kernel interface.
> >> >>
> >> >>This changes the TCE table migration descriptor to support dynamic
> >> >>tables as from now on, PHB will create as many stub TCE table objects
> >> >>as PHB can possibly support but not all of them might be initialized at
> >> >>the time of migration because DDW might or might not be requested by
> >> >>the guest.
> >> >>
> >> >>The "ddw" property is enabled by default on a PHB but for compatibility
> >> >>the pseries-2.3 machine and older disable it.
> >> >>
> >> >>This implements DDW for VFIO. The host kernel support is required.
> >> >>This adds a "levels" property to PHB to control the number of levels
> >> >>in the actual TCE table allocated by the host kernel, 0 is the default
> >> >>value to tell QEMU to calculate the correct value. Current hardware
> >> >>supports up to 5 levels.
> >> >>
> >> >>The existing linux guests try creating one additional huge DMA window
> >> >>with 64K or 16MB pages and map the entire guest RAM to. If succeeded,
> >> >>the guest switches to dma_direct_ops and never calls TCE hypercalls
> >> >>(H_PUT_TCE,...) again. This enables VFIO devices to use the entire RAM
> >> >>and not waste time on map/unmap later.
> >> >>
> >> >>This adds 4 RTAS handlers:
> >> >>* ibm,query-pe-dma-window
> >> >>* ibm,create-pe-dma-window
> >> >>* ibm,remove-pe-dma-window
> >> >>* ibm,reset-pe-dma-window
> >> >>These are registered from type_init() callback.
> >> >>
> >> >>These RTAS handlers are implemented in a separate file to avoid polluting
> >> >>spapr_iommu.c with PCI.
> >> >>
> >> >>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> >
> >> >Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >> 
> >> I saw this and decided there are no more coments but I was wrong :)
> >
> > Right.  Note that if I add a Reviewed-by but also make comments, then
> > those comments are seeking clarification and maybe suggesting later
> > cleanups, but I think the problems are small enough that the patch is
> > still ready to go as it is.
> 
> You can help the recipient of your comments by putting your R-by behind
> the last comment.

Noted for future reference.

> Wouldn't be necessary if people never left reams of quoted material
> at the end of their replies, but that's a pipe dream :)

I do usually try to trim quoted material - looks like I forgot this
time though.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]