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 <a...@ozlabs.ru> > > > >Reviewed-by: David Gibson <da...@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.
[snip]
> >>+static void spapr_tce_table_do_enable(sPAPRTCETable *tcet);
> >>+
> >> static int spapr_tce_table_post_load(void *opaque, int version_id)
> >> {
> >> sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque);
> >>@@ -98,22 +107,42 @@ static int spapr_tce_table_post_load(void *opaque, int
> >>version_id)
> >> spapr_vio_set_bypass(tcet->vdev, tcet->bypass);
> >> }
> >>
> >>+ if (!tcet->migtable) {
> >
> >What's the case where migtable will be NULL? IIUC an old->new
> >migration will result in the data saved for "table" being loaded into
> >"migtable".
> >
> >So "migtable" should only be NULL, when tce->enabled is also false?
>
>
> Seems to be true and this is just extra precaution. Remove?
Yes, remove as a cleanup, but that can be done later, I won't hold up
the main patch series for this.
--
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
pgprVK_J_i2Xp.pgp
Description: PGP signature
