Re: [Qemu-ppc] [PATCH 8/9] spapr_iommu: Introduce page_shift in sPAPRTCETable

[Alexander Graf]

On 22.05.14 12:46, Alexey Kardashevskiy wrote:
> On 05/22/2014 08:45 PM, Alexander Graf wrote:
> > On 22.05.14 12:24, Alexey Kardashevskiy wrote:
> > > On 05/22/2014 08:09 PM, Alexander Graf wrote:
> > > > On 22.05.14 01:45, Alexey Kardashevskiy wrote:
> > > > > On 05/22/2014 08:11 AM, Alexander Graf wrote:
> > > > > > On 21.05.14 16:21, Alexey Kardashevskiy wrote:
> > > > > > > At the moment only 4K pages are supported by sPAPRTCETable. Since sPAPR
> > > > > > > spec allows other page sizes and we are going to implement them, we need
> > > > > > > page size to be configrable.
> > > > > > >
> > > > > > > This adds @page_shift into sPAPRTCETable and replaces
> > > > > > > SPAPR_TCE_PAGE_SHIFT
> > > > > > > with it whereever it is possible.
> > > > > > >
> > > > > > > This removes SPAPR_TCE_PAGE_MASK as it is no longer used.
> > > > > > >
> > > > > > > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > [...]
> >
> > > > > > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > > > > > index fdd4c07..c9850d4 100644
> > > > > > > --- a/hw/ppc/spapr_pci.c
> > > > > > > +++ b/hw/ppc/spapr_pci.c
> > > > > > > @@ -656,6 +656,7 @@ static void spapr_phb_finish_realize(sPAPRPHBState
> > > > > > > *sphb, Error **errp)
> > > > > > >          sPAPRTCETable *tcet;
> > > > > > >            tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
> > > > > > > +                               SPAPR_TCE_PAGE_SHIFT,
> > > > > > >                                     0x40000000 >> SPAPR_TCE_PAGE_SHIFT);
> > > > > > >          if (!tcet) {
> > > > > > >              error_setg(errp, "Unable to create TCE table for %s",
> > > > > > > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > > > > > > index b84e481..d7e9e6a 100644
> > > > > > > --- a/hw/ppc/spapr_vio.c
> > > > > > > +++ b/hw/ppc/spapr_vio.c
> > > > > > > @@ -457,6 +457,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
> > > > > > >          if (pc->rtce_window_size) {
> > > > > > >              uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
> > > > > > >              dev->tcet = spapr_tce_new_table(qdev, liobn,
> > > > > > > +                                        SPAPR_TCE_PAGE_SHIFT,
> > > > > > I don't quite understand who defines what the TCE page size is for a
> > > > > > given
> > > > > > device. Can you try to explain this to me?
> > > > > If it is default window (for PCI) or window for VIO - it is 4K. If it is a
> > > > > dynamic DMA window - page size is a parameter of RTAS call which creates
> > > > > the window.
> > > > Could we change that default size for non-dynamic windows somehow? 4k is
> > > > really fine grained.
> > > No, this is hardcoded in a million places and old distros. Normally it is
> > > maximum 1GB or 2MB table, not too bad. And with DDW support, we can make
> > > the default one really small (64MB?) and lose even less.
> > >
> > > SPAPR:
> > > R1–7.3.31–4. For the Dynamic DMA Windows (DDW) option: The platform must
> > > provide a default DMA window
> > > for each PE, and all of the following must be true:
> > > a. The window is defined by the “ibm,dma-window” property in the OF device
> > > tree.
> > > b. The window is defined with 4 KB I/O pages.
> > > c. The window is located entirely below 4 GB.
> > Cool, would be interesting to see how that affects performance. I like most
> > of this patch set btw, I think it should be good to go in in the next round.
>
> 10Gbit ethernet is ok but 40Gbit seems to suffer from this limitation.

Oh, I was implying that it'd be interesting to see how we fare when we 
do a really small default DMA window to force the guest to use DDW and 
preferably even with very large pages.


Alex