linux-kernel - Re: [PATCH v3 4/8] powerpc/pseries/svm: Add a powerpc version of cc_platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 15 Sep 2021 20:47:37 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Michael Ellerman <mpe@...erman.id.au>,
        Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        linux-efi@...r.kernel.org, Brijesh Singh <brijesh.singh@....com>,
        kvm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        platform-driver-x86@...r.kernel.org,
        Paul Mackerras <paulus@...ba.org>, linux-s390@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>, x86@...nel.org,
        amd-gfx@...ts.freedesktop.org,
        Christoph Hellwig <hch@...radead.org>,
        linux-graphics-maintainer@...are.com,
        Tom Lendacky <thomas.lendacky@....com>,
        Tianyu Lan <Tianyu.Lan@...rosoft.com>,
        kexec@...ts.infradead.org, linux-kernel@...r.kernel.org,
        iommu@...ts.linux-foundation.org, linux-fsdevel@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [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