On 17/12/2019 10:07, Thiago Jung Bauermann wrote:
>
> Alexey Kardashevskiy <aik@xxxxxxxxx> writes:
>
>> By default a pseries guest supports a H_PUT_TCE hypercall which maps
>> a single IOMMU page in a DMA window. Additionally the hypervisor may
>> support H_PUT_TCE_INDIRECT/H_STUFF_TCE which update multiple TCEs at once;
>> this is advertised via the device tree /rtas/ibm,hypertas-functions
>> property which Linux converts to FW_FEATURE_MULTITCE.
>>
>> FW_FEATURE_MULTITCE is checked when dma_iommu_ops is used; however
>> the code managing the huge DMA window (DDW) ignores it and calls
>> H_PUT_TCE_INDIRECT even if it is explicitly disabled via
>> the "multitce=off" kernel command line parameter.
>>
>> This adds FW_FEATURE_MULTITCE checking to the DDW code path.
>>
>> This changes tce_build_pSeriesLP to take liobn and page size as
>> the huge window does not have iommu_table descriptor which usually
>> the place to store these numbers.
>>
>> Fixes: 4e8b0cf46b25 ("powerpc/pseries: Add support for dynamic dma windows")
>> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
>
> Reviewed-by: Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxx>
> Tested-by: Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxx>
>
> Some minor nits below. Feel free to ignore.
>
>> @@ -146,25 +146,25 @@ static int tce_build_pSeriesLP(struct iommu_table *tbl, long tcenum,
>> int ret = 0;
>> long tcenum_start = tcenum, npages_start = npages;
>>
>> - rpn = __pa(uaddr) >> TCE_SHIFT;
>> + rpn = __pa(uaddr) >> tceshift;
>> proto_tce = TCE_PCI_READ;
>> if (direction != DMA_TO_DEVICE)
>> proto_tce |= TCE_PCI_WRITE;
>>
>> while (npages--) {
>> - tce = proto_tce | (rpn & TCE_RPN_MASK) << TCE_RPN_SHIFT;
>> - rc = plpar_tce_put((u64)tbl->it_index, (u64)tcenum << 12, tce);
>> + tce = proto_tce | (rpn & TCE_RPN_MASK) << tceshift;
>> + rc = plpar_tce_put((u64)liobn, (u64)tcenum << tceshift, tce);
>
> Is it necessary to cast to u64 here? plpar_tce_put() takes unsigned long
> for both arguments.
Looked as an unrelated change. Small but still unrelated.
>
>> @@ -261,16 +263,16 @@ static int tce_buildmulti_pSeriesLP(struct iommu_table *tbl, long tcenum,
>> return ret;
>> }
>>
>> -static void tce_free_pSeriesLP(struct iommu_table *tbl, long tcenum, long npages)
>> +static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long npages)
>> {
>> u64 rc;
>>
>> while (npages--) {
>> - rc = plpar_tce_put((u64)tbl->it_index, (u64)tcenum << 12, 0);
>> + rc = plpar_tce_put((u64)liobn, (u64)tcenum << 12, 0);
>
> Same comment regarding cast to u64.
>
>> @@ -400,6 +402,20 @@ static int tce_setrange_multi_pSeriesLP(unsigned long start_pfn,
>> u64 rc = 0;
>> long l, limit;
>>
>> + if (!firmware_has_feature(FW_FEATURE_MULTITCE)) {
>> + unsigned long tceshift = be32_to_cpu(maprange->tce_shift);
>> + unsigned long dmastart = (start_pfn << PAGE_SHIFT) +
>> + be64_to_cpu(maprange->dma_base);
>> + unsigned long tcenum = dmastart >> tceshift;
>> + unsigned long npages = num_pfn << PAGE_SHIFT >>
>> + be32_to_cpu(maprange->tce_shift);
>
> Could use the tceshift variable here.
True, overlooked.
Thanks for the reviews!
>
>> + void *uaddr = __va(start_pfn << PAGE_SHIFT);
>> +
>> + return tce_build_pSeriesLP(be32_to_cpu(maprange->liobn),
>> + tcenum, tceshift, npages, (unsigned long) uaddr,
>> + DMA_BIDIRECTIONAL, 0);
>> + }
>> +
>> local_irq_disable(); /* to protect tcep and the page behind it */
>> tcep = __this_cpu_read(tce_page);
>
>
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
>
--
Alexey
