lkml.org 
[lkml]   [2015]   [Apr]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH kernel v9 15/32] powerpc/powernv/ioda/ioda2: Rework TCE invalidation in tce_build()/tce_free()
    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 <aik@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 <david@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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2015-04-30 07:21    [W:3.680 / U:0.336 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site