Re: [Qemu-devel] [RFC PATCH] spapr_vio/spapr_iommu: Move VIO bypass where it belongs

[David Gibson]

On Fri, Jan 09, 2015 at 12:02:51PM +1100, Alexey Kardashevskiy wrote:
> Instead of tweaking a TCE table device by adding there a bypass flag,
> let's add an alias to RAM and IOMMU memory region, and enable/disable
> those according to the selected bypass mode.
> This way IOMMU memory region can have size of the actual window rather
> than ram_size which is essential for upcoming DDW support.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> 
> The main reason for this patch is DDW and the fact that sPAPRTCETable
> used for DMA windows for VFIO. My latest approach removes all DMA windows
> on the guest reset (and creates a new 32bit one) which means than VFIO
> unmaps everything and this fails as normally sPAPRTCETable MemoryRegion is
> ram_size big (to support bypass) while it should be 1-2GB.

Paolo already mentioned the only significant problem I see with this,
which is migration backwards compat.  Otherwise it looks good, sorry I
messed it up in the first place :).

[snip]
> @@ -456,14 +469,25 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
>  
>      if (pc->rtce_window_size) {
>          uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
> +
> +        memory_region_init(&dev->mrroot, OBJECT(dev), "iommu-spapr-root",
> +                           ram_size);
> +        memory_region_init_alias(&dev->mrbypass, OBJECT(dev),
> +                                 "iommu-spapr-bypass", get_system_memory(),
> +                                 0, ram_size);
> +        memory_region_add_subregion_overlap(&dev->mrroot, 0, &dev->mrbypass, 
> 1);
> +        address_space_init(&dev->as, &dev->mrroot, qdev->id);
> +
>          dev->tcet = spapr_tce_new_table(qdev, liobn,
>                                          0,
>                                          SPAPR_TCE_PAGE_SHIFT,
>                                          pc->rtce_window_size >>
>                                          SPAPR_TCE_PAGE_SHIFT, false);
> -        address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet), 
> qdev->id);
> +        memory_region_add_subregion_overlap(&dev->mrroot, 0,
> +                                            spapr_tce_get_iommu(dev->tcet), 
> 2);
>      }
>  
> +

^^ Tiny nit, extraneous whitespace change.

-- 
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

pgpWlOuPKTEuA.pgp
Description: PGP signature