Re: [Qemu-devel] [RFC PATCH 07/10] spapr_pci: Allow PCI host bridge DMA window to be configured

[Thomas Huth <thuth@redhat.com>]

On 17/09/15 15:09, David Gibson wrote:
> At present the PCI host bridge (PHB) for the pseries machine type has a
> fixed DMA window from 0..1GB (in PCI address space) which is mapped to real
> memory via the PAPR paravirtualized IOMMU.
> 
> For better support of VFIO devices, we're going to want to allow for
> different configurations of the DMA window.
> 
> Eventually we'll want to allow the guest itself to reconfigure the window
> via the PAPR dynamic DMA window interface, but as a preliminary this patch
> allows the user to reconfigure the window with new properties on the PHB
> device.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_pci.c          | 7 +++++--
>  include/hw/pci-host/spapr.h | 3 +--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index b088491..622c4ac 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1394,7 +1394,7 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
>      sPAPRTCETable *tcet;
>      uint32_t nb_table;
>  
> -    nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
> +    nb_table = sphb->dma_win_size >> SPAPR_TCE_PAGE_SHIFT;
>      tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
>                                 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
>      if (!tcet) {
> @@ -1404,7 +1404,7 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
>      }
>  
>      /* Register default 32bit DMA window */
> -    memory_region_add_subregion(&sphb->iommu_root, 0,
> +    memory_region_add_subregion(&sphb->iommu_root, sphb->dma_win_addr,
>                                  spapr_tce_get_iommu(tcet));
>  }
>  
> @@ -1437,6 +1437,9 @@ static Property spapr_phb_properties[] = {
>                         SPAPR_PCI_IO_WIN_SIZE),
>      DEFINE_PROP_BOOL("dynamic-reconfiguration", sPAPRPHBState, dr_enabled,
>                       true),
> +    /* Default DMA window is 0..1GB */
> +    DEFINE_PROP_UINT64("dma_win_addr", sPAPRPHBState, dma_win_addr, 0),
> +    DEFINE_PROP_UINT64("dma_win_size", sPAPRPHBState, dma_win_size, 0x40000000),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 5322b56..7de5e02 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -78,6 +78,7 @@ struct sPAPRPHBState {
>      MemoryRegion memwindow, iowindow, msiwindow;
>  
>      uint32_t dma_liobn;
> +    hwaddr dma_win_addr, dma_win_size;

Maybe use dma_addr_t for dma_win_addr? And dma_win_size isn't an
address, so I'd maybe use uint64_t here instead.

>      AddressSpace iommu_as;
>      MemoryRegion iommu_root;
>  
> @@ -115,8 +116,6 @@ struct sPAPRPHBVFIOState {
>  
>  #define SPAPR_PCI_MSI_WINDOW         0x40000000000ULL
>  
> -#define SPAPR_PCI_DMA32_SIZE         0x40000000
> -
>  static inline qemu_irq spapr_phb_lsi_qirq(struct sPAPRPHBState *phb, int pin)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());

Anyway, patch looks fine to me, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>