lkml.org 
[lkml]   [2021]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform_has()
    On Wed, Sep 15, 2021 at 07:18:34PM +0200, Christophe Leroy wrote:
    > Could you please provide more explicit explanation why inlining such an
    > helper is considered as bad practice and messy ?

    Tom already told you to look at the previous threads. Let's read them
    together. This one, for example:

    https://lore.kernel.org/lkml/YSScWvpXeVXw%2Fed5@infradead.org/

    | > To take it out of line, I'm leaning towards the latter, creating a new
    | > file that is built based on the ARCH_HAS_PROTECTED_GUEST setting.
    |
    | Yes. In general everytime architectures have to provide the prototype
    | and not just the implementation of something we end up with a giant mess
    | sooner or later. In a few cases that is still warranted due to
    | performance concerns, but i don't think that is the case here.

    So I think what Christoph means here is that you want to have the
    generic prototype defined in a header and arches get to implement it
    exactly to the letter so that there's no mess.

    As to what mess exactly, I'd let him explain that.

    > Because as demonstrated in my previous response some days ago, taking that
    > outline ends up with an unneccessary ugly generated code and we don't
    > benefit front GCC's capability to fold in and opt out unreachable code.

    And this is real fast path where a couple of instructions matter or what?

    set_memory_encrypted/_decrypted doesn't look like one to me.

    > I can't see your point here. Inlining the function wouldn't add any
    > ifdeffery as far as I can see.

    If the function is touching defines etc, they all need to be visible.
    If that function needs to call other functions - which is the case on
    x86, perhaps not so much on power - then you need to either ifdef around
    them or provide stubs with ifdeffery in the headers. And you need to
    make them global functions instead of keeping them static to the same
    compilation unit, etc, etc.

    With a separate compilation unit, you don't need any of that and it is
    all kept in that single file.

    --
    Regards/Gruss,
    Boris.

    https://people.kernel.org/tglx/notes-about-netiquette

    \
     
     \ /
      Last update: 2021-09-15 20:48    [W:5.205 / U:0.828 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site