The per-CPU tlbstate contains sensitive information which should be really
only accessible in core code. It is exported to modules because some inline
functions which are required by KVM need access to it.
The following series creates regular exported functions for the few things
which are needed by KVM and hides the struct definition and some low level
helpers from modules.
The series is also available from git:
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel x86/tlb
Thanks,
tglx
8<-----------------
arch/x86/events/core.c | 11
arch/x86/include/asm/mmu_context.h | 88 -------
arch/x86/include/asm/paravirt.h | 12
arch/x86/include/asm/pgtable_32.h | 2
arch/x86/include/asm/tlbflush.h | 454 ++++---------------------------------
arch/x86/kernel/alternative.c | 55 ++++
arch/x86/kernel/cpu/common.c | 25 +-
arch/x86/kernel/cpu/mtrr/generic.c | 4
arch/x86/kernel/paravirt.c | 21 -
arch/x86/kernel/process.c | 11
arch/x86/mm/init.c | 14 +
arch/x86/mm/init_64.c | 2
arch/x86/mm/ioremap.c | 2
arch/x86/mm/kmmio.c | 2
arch/x86/mm/mem_encrypt.c | 2
arch/x86/mm/pat/set_memory.c | 2
arch/x86/mm/pgtable.c | 8
arch/x86/mm/pgtable_32.c | 2
arch/x86/mm/tlb.c | 367 +++++++++++++++++++++++++++++
arch/x86/platform/uv/tlb_uv.c | 2
drivers/xen/privcmd.c | 1
21 files changed, 556 insertions(+), 531 deletions(-)
Just looking over some exports at the end of the series (and thus
ignoring bisection issues):
- Is there any good reason to keep __flush_tlb_all inline vs moving it
out of line and kill the flush_tlb_local and flush_tlb_global exports.
Also there is just a single modular users in SVM, I wonder if there is
any way to get rid of that one as well.
Also I think cpu_tlbstate itself could be marked static in tlb.c with
a few more changes, I wonder if would be worth it?
On Sun, Apr 19, 2020 at 10:31:37PM +0200, Thomas Gleixner wrote:
> The per-CPU tlbstate contains sensitive information which should be really
> only accessible in core code. It is exported to modules because some inline
> functions which are required by KVM need access to it.
>
> The following series creates regular exported functions for the few things
> which are needed by KVM and hides the struct definition and some low level
> helpers from modules.
>
> The series is also available from git:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel x86/tlb
>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
On 4/19/20 10:31 PM, Thomas Gleixner wrote:
> The per-CPU tlbstate contains sensitive information which should be really
> only accessible in core code. It is exported to modules because some inline
> functions which are required by KVM need access to it.
>
> The following series creates regular exported functions for the few things
> which are needed by KVM and hides the struct definition and some low level
> helpers from modules.
>
> The series is also available from git:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel x86/tlb
>
> Thanks,
>
> tglx
>
Reviewed-by: Alexandre Chartre <[email protected]>
For all patches.
alex.
On 4/20/20 11:20 AM, Christoph Hellwig wrote:
> Just looking over some exports at the end of the series (and thus
> ignoring bisection issues):
>
> - Is there any good reason to keep __flush_tlb_all inline vs moving it
> out of line and kill the flush_tlb_local and flush_tlb_global exports.
> Also there is just a single modular users in SVM, I wonder if there is
> any way to get rid of that one as well.
>
> Also I think cpu_tlbstate itself could be marked static in tlb.c with
> a few more changes, I wonder if would be worth it?
>
For Address Space Isolation (ASI), I was planning on storing the ASI session
into cpu_tlbstate (https://lkml.org/lkml/2020/2/26/699) as the ASI session
then provides the TLB flushing information based on the ASI used. In that case,
I would need cpu_tlbstate to be non-static. Otherwise I can have my own percpu
asi_session structures, but using cpu_tlbstate seemed more appropriate to me.
This is opened for discussion; for now, I am waiting for more changes that tglx
is making, before rebasing ASI.
alex.
Christoph Hellwig <[email protected]> writes:
> Just looking over some exports at the end of the series (and thus
> ignoring bisection issues):
>
> - Is there any good reason to keep __flush_tlb_all inline vs moving it
> out of line and kill the flush_tlb_local and flush_tlb_global exports.
> Also there is just a single modular users in SVM, I wonder if there is
> any way to get rid of that one as well.
I'll have a look again.
> Also I think cpu_tlbstate itself could be marked static in tlb.c with
> a few more changes, I wonder if would be worth it?
Unfortunately it can't. We need it in the low level entry code due to
PTI and solving that would be a messy surgery.
Thanks,
tglx
Alexandre Chartre <[email protected]> writes:
> On 4/20/20 11:20 AM, Christoph Hellwig wrote:
>> Just looking over some exports at the end of the series (and thus
>> ignoring bisection issues):
>>
>> - Is there any good reason to keep __flush_tlb_all inline vs moving it
>> out of line and kill the flush_tlb_local and flush_tlb_global exports.
>> Also there is just a single modular users in SVM, I wonder if there is
>> any way to get rid of that one as well.
>>
>> Also I think cpu_tlbstate itself could be marked static in tlb.c with
>> a few more changes, I wonder if would be worth it?
>>
> For Address Space Isolation (ASI), I was planning on storing the ASI session
> into cpu_tlbstate (https://lkml.org/lkml/2020/2/26/699) as the ASI session
> then provides the TLB flushing information based on the ASI used. In that case,
> I would need cpu_tlbstate to be non-static. Otherwise I can have my own percpu
> asi_session structures, but using cpu_tlbstate seemed more appropriate to me.
> This is opened for discussion; for now, I am waiting for more changes that tglx
> is making, before rebasing ASI.
Even in that case we could restrict the availability to arch/x86/mm/
which would still make it available for ASI.
Thanks,
tglx
On Mon, Apr 20, 2020 at 07:27:06PM +0200, Thomas Gleixner wrote:
> Christoph Hellwig <[email protected]> writes:
> > Just looking over some exports at the end of the series (and thus
> > ignoring bisection issues):
> >
> > - Is there any good reason to keep __flush_tlb_all inline vs moving it
> > out of line and kill the flush_tlb_local and flush_tlb_global exports.
> > Also there is just a single modular users in SVM, I wonder if there is
> > any way to get rid of that one as well.
>
> I'll have a look again.
Regarding the SVM case, the only usage is for a TLB errata. At a glance,
svm_init_erratum_383() and is_erratum_383() don't use any KVM hooks, i.e. I
don't see anything that would prevent moving those to .../kernel/cpu/amd.c.
Sean Christopherson <[email protected]> writes:
> On Mon, Apr 20, 2020 at 07:27:06PM +0200, Thomas Gleixner wrote:
>> Christoph Hellwig <[email protected]> writes:
>> > Just looking over some exports at the end of the series (and thus
>> > ignoring bisection issues):
>> >
>> > - Is there any good reason to keep __flush_tlb_all inline vs moving it
>> > out of line and kill the flush_tlb_local and flush_tlb_global exports.
>> > Also there is just a single modular users in SVM, I wonder if there is
>> > any way to get rid of that one as well.
>>
>> I'll have a look again.
>
> Regarding the SVM case, the only usage is for a TLB errata. At a glance,
> svm_init_erratum_383() and is_erratum_383() don't use any KVM hooks, i.e. I
> don't see anything that would prevent moving those to .../kernel/cpu/amd.c.
Right, but that would trade one export vs. two SVM errata specific
exports. Not really a win.
Thanks,
tglx
On Sun, Apr 19, 2020 at 1:36 PM Thomas Gleixner <[email protected]> wrote:
>
> The per-CPU tlbstate contains sensitive information which should be really
> only accessible in core code. It is exported to modules because some inline
> functions which are required by KVM need access to it.
>
> The following series creates regular exported functions for the few things
> which are needed by KVM and hides the struct definition and some low level
> helpers from modules.
>
> The series is also available from git:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel x86/tlb
>
> Thanks,
The whole series is Acked-by: Andy Lutomirski <[email protected]>
On Tue, Apr 21, 2020 at 11:09:07AM +0200, Thomas Gleixner wrote:
> Sean Christopherson <[email protected]> writes:
>
> > On Mon, Apr 20, 2020 at 07:27:06PM +0200, Thomas Gleixner wrote:
> >> Christoph Hellwig <[email protected]> writes:
> >> > Just looking over some exports at the end of the series (and thus
> >> > ignoring bisection issues):
> >> >
> >> > - Is there any good reason to keep __flush_tlb_all inline vs moving it
> >> > out of line and kill the flush_tlb_local and flush_tlb_global exports.
> >> > Also there is just a single modular users in SVM, I wonder if there is
> >> > any way to get rid of that one as well.
> >>
> >> I'll have a look again.
> >
> > Regarding the SVM case, the only usage is for a TLB errata. At a glance,
> > svm_init_erratum_383() and is_erratum_383() don't use any KVM hooks, i.e. I
> > don't see anything that would prevent moving those to .../kernel/cpu/amd.c.
>
> Right, but that would trade one export vs. two SVM errata specific
> exports. Not really a win.
True. I was thinking the svm_init_erratum_383() call could be moved to
init_amd(), assuming the MSR's magic bit 47 is "safe" outside of SVM
enabled, but that's probably not worth risking the potential for breakage
unless you really want to hide __flush_tlb_all().