On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
>
> On 12/09/2020 03:07, Leonardo Bras wrote:
> > Cc: linuxppc-dev@lists.ozlabs.org, linux-ker...@vger.kernel.org,
> >
> > Add a new helper _iommu_table_setparms(), and use it in
> > iommu_table_setparms() and iommu_table_setparms_lpar() to avoid duplicated
> > code.
> >
> > Also, setting tbl->it_ops was happening outsite iommu_table_setparms*(),
> > so move it to the new helper. Since we need the iommu_table_ops to be
> > declared before used, move iommu_table_lpar_multi_ops and
> > iommu_table_pseries_ops to before their respective iommu_table_setparms*().
> >
> > The tce_exchange_pseries() also had to be moved up, since it's used in
> > iommu_table_lpar_multi_ops.xchg_no_kill.
>
>
> Use forward declarations (preferred) or make a separate patch for moving
> chunks (I do not see much point).
Fixed :)
> > @@ -509,8 +559,13 @@ static void iommu_table_setparms(struct pci_controller
> > *phb,
> > const unsigned long *basep;
> > const u32 *sizep;
> > - node = phb->dn;
> > + /* Test if we are going over 2GB of DMA space */
> >
> >
> >
> > + if (phb->dma_window_base_cur + phb->dma_window_size > 0x80000000ul) {
> > + udbg_printf("PCI_DMA: Unexpected number of IOAs under this
> > PHB.\n");
> > + panic("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
> > + }
>
>
> s/0x80000000ul/2*SZ_1G/
Done!
>
> but more to the point - why this check? QEMU can create windows at 0 and
> as big as the VM requested. And I am pretty sure I can construct QEMU
> command line such as it won't have MMIO32 at all and a 4GB default DMA
> window.
>
Oh, the diff was a little strange here. I did not add this snippet, it
was already in that function, but since I created the helper, the diff
made it look like I introduced this piece of code.
Please take a look in the diff snippet bellow. (This same lines were
there.)
> > @@ -519,33 +574,25 @@ static void iommu_table_setparms(struct
> > pci_controller *phb,
> > return;
> > }
> >
> > - tbl->it_base = (unsigned long)__va(*basep);
> >
> >
> >
> > + _iommu_table_setparms(tbl, phb->bus->number, 0,
> > phb->dma_window_base_cur,
> > + phb->dma_window_size, IOMMU_PAGE_SHIFT_4K,
> > + (unsigned long)__va(*basep),
> > &iommu_table_pseries_ops);
> > if (!is_kdump_kernel())
> >
> >
> >
> > memset((void *)tbl->it_base, 0, *sizep);
> >
> > - tbl->it_busno = phb->bus->number;
> > - tbl->it_page_shift = IOMMU_PAGE_SHIFT_4K;
> > -
> > - /* Units of tce entries */
> > - tbl->it_offset = phb->dma_window_base_cur >> tbl->it_page_shift;
> > -
> > - /* Test if we are going over 2GB of DMA space */
> > - if (phb->dma_window_base_cur + phb->dma_window_size > 0x80000000ul) {
> > - udbg_printf("PCI_DMA: Unexpected number of IOAs under this
> > PHB.\n");
> > - panic("PCI_DMA: Unexpected number of IOAs under this PHB.\n");
> > - }
> > -
> > phb->dma_window_base_cur += phb->dma_window_size;
> > -
> > - /* Set the tce table size - measured in entries */
> > - tbl->it_size = phb->dma_window_size >> tbl->it_page_shift;
> > -
> > - tbl->it_index = 0;
> > - tbl->it_blocksize = 16;
> > - tbl->it_type = TCE_PCI;
> > }
> >
Thanks for reviewing, Alexey!
