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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 05/22/2014 08:48 PM, Alexander Graf wrote:
> 
> 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 <aik@ozlabs.ru>
>>> [...]
>>>
>>>>>>>> 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.


Guest switches to DDW always if the feature is there. SPAPR DDW API even
allows deleting the default one if the platform supports "reset" (which I
do not implement for now).



-- 
Alexey