lkml.org 
[lkml]   [2017]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v8 06/08] iommu/ipmmu-vmsa: Use fwspec iommu_priv on ARM64
    Hi Robin,

    On Wed, May 17, 2017 at 11:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
    > Hi Magnus,
    >
    > On 17/05/17 11:07, Magnus Damm wrote:
    >> From: Magnus Damm <damm+renesas@opensource.se>
    >>
    >> Convert from archdata to iommu_priv via iommu_fwspec on ARM64 but
    >> let 32-bit ARM keep on using archdata for now.
    >>
    >> Once the 32-bit ARM code and the IPMMU driver is able to move over
    >> to CONFIG_IOMMU_DMA=y then coverting to fwspec via ->of_xlate() will
    >> be easy.
    >>
    >> For now fwspec ids and num_ids are not used to allow code sharing between
    >> 32-bit and 64-bit ARM code inside the driver.
    >
    > That's not what I meant at all - this just looks like a crazy
    > unmaintainable mess that's making things that much harder to unpick in
    > future.
    >
    > Leaving the existing code fossilised and building up half of a second
    > separate driver around it wrapped in ifdefs is not only hideous, it's
    > more work in the end than simply pulling things into the right shape to
    > begin with. What I meant was to start with the below (compile-tested
    > only), and add the of_xlate support on top. AFAICS the arm/arm64
    > differences overall should only come down to a bit of special-casing in
    > add_device(), and (optionally) whether you outright reject
    > IOMMU_DOMAIN_DMA or not.
    >
    > Sorry, but I just can't agree with the two-drivers-in-one approach.

    Thanks for checking the code and sorry to disappoint you by not using
    ->ids[] and ->num_ids right away. The two-drivers-on-one approach
    comes from wanting to use the same IOMMU driver on both 32-bit and
    64-bit ARM architectures but the former is lacking IOMMU_DMA support
    upstream.

    Obviously using ->ids[] and ->num_ids is the right forward, and in my
    mind it is only a question of time and merge order. I'm more than
    happy to make use of your patch but I'm wondering if it will work on
    32-bit ARM and R-Car Gen2 that currently does not use ->of_xlate(). I
    can see that you're using iommu_fwspec_init() so it might work right
    out of the box. Thanks for your help cooking up the patch by the way.

    From my side I was hoping on doing one larger feature jump for 32-bit
    ARM by moving over to IOMMU_DMA=y together with ->of_xlate()and fwspec
    at the same time. Doing minor increments of the legacy code that is
    still using special mapping code instead of OMMU_DMA=y in case of
    32-bit ARM just feels like a lot of potential breakage and little gain
    to me.

    What's the plan for supporting IOMMU_DMA=y on 32-bit ARM? Anything I
    can do to help? Or do you prefer minor increments on 32-bit ARM over
    larger feature jumps?

    Let me know what you think. My plan is to revisit this topic early next week.

    Cheers,

    / magnus

    \
     
     \ /
      Last update: 2017-05-18 07:24    [W:5.279 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site