On Thu, Apr 30, 2015 at 12:58:12PM +1000, Alexey Kardashevskiy wrote:
> On 04/29/2015 01:18 PM, David Gibson wrote:
> >On Sat, Apr 25, 2015 at 10:14:39PM +1000, Alexey Kardashevskiy wrote:
> >>The pnv_pci_ioda_tce_invalidate() helper invalidates TCE cache. It is
> >>supposed to be called on IODA1/2 and not called on p5ioc2. It receives
> >>start and end host addresses of TCE table.
> >>
> >>IODA2 actually needs PCI addresses to invalidate the cache. Those
> >>can be calculated from host addresses but since we are going
> >>to implement multi-level TCE tables, calculating PCI address from
> >>a host address might get either tricky or ugly as TCE table remains flat
> >>on PCI bus but not in RAM.
> >>
> >>This moves pnv_pci_ioda_tce_invalidate() from generic pnv_tce_build/
> >>pnt_tce_free and defines IODA1/2-specific callbacks which call generic
> >>ones and do PHB-model-specific TCE cache invalidation. P5IOC2 keeps
> >>using generic callbacks as before.
> >>
> >>This changes pnv_pci_ioda2_tce_invalidate() to receives TCE index and
> >>number of pages which are PCI addresses shifted by IOMMU page shift.
> >>
> >>No change in behaviour is expected.
> >>
> >>Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> >>---
> >>Changes:
> >>v9:
> >>* removed confusing comment from commit log about unintentional calling of
> >>pnv_pci_ioda_tce_invalidate()
> >>* moved mechanical changes away to "powerpc/iommu: Move tce_xxx callbacks
> >>from ppc_md to iommu_table"
> >>* fixed bug with broken invalidation in pnv_pci_ioda2_tce_invalidate -
> >>@index includes @tbl->it_offset but old code added it anyway which later
> >>broke
> >>DDW
> >>---
> >> arch/powerpc/platforms/powernv/pci-ioda.c | 86
> >> +++++++++++++++++++++----------
> >> arch/powerpc/platforms/powernv/pci.c | 17 ++----
> >> 2 files changed, 64 insertions(+), 39 deletions(-)
> >>
> >>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> >>b/arch/powerpc/platforms/powernv/pci-ioda.c
> >>index 718d5cc..f070c44 100644
> >>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
> >>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> >>@@ -1665,18 +1665,20 @@ static void pnv_ioda_setup_bus_dma(struct
> >>pnv_ioda_pe *pe,
> >> }
> >> }
> >>
> >>-static void pnv_pci_ioda1_tce_invalidate(struct pnv_ioda_pe *pe,
> >>- struct iommu_table *tbl,
> >>- __be64 *startp, __be64 *endp, bool rm)
> >>+static void pnv_pci_ioda1_tce_invalidate(struct iommu_table *tbl,
> >>+ unsigned long index, unsigned long npages, bool rm)
> >> {
> >>+ struct pnv_ioda_pe *pe = container_of(tbl->it_table_group,
> >>+ struct pnv_ioda_pe, table_group);
> >> __be64 __iomem *invalidate = rm ?
> >> (__be64 __iomem *)pe->tce_inval_reg_phys :
> >> (__be64 __iomem *)tbl->it_index;
> >> unsigned long start, end, inc;
> >> const unsigned shift = tbl->it_page_shift;
> >>
> >>- start = __pa(startp);
> >>- end = __pa(endp);
> >>+ start = __pa((__be64 *)tbl->it_base + index - tbl->it_offset);
> >>+ end = __pa((__be64 *)tbl->it_base + index - tbl->it_offset +
> >>+ npages - 1);
> >
> >This doesn't look right. The arguments to __pa don't appear to be
> >addresses (since index and if_offset are in units of (TCE) pages, not
> >bytes).
>
>
> tbl->it_base is an address and it is casted to __be64* which means:
>
> (char*)tbl->it_base + (index - tbl->it_offset)*sizeof(__be64).
>
> Which seems to be correct (I just removed extra braces compared to the old
> code), no?
Ah, yes, I'm just forgetting my C pointer arithmetic rules.
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgpR6qf3ljoUM.pgp
Description: PGP signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
