linux-kernel - Re: [PATCH 03/22] kvm: mmu: Init / Uninit the TDP MMU

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]

Message-ID: <CANgfPd-ErY5BgPooX0URYY1SFpLADJ9UfHKYQJsuE4JNh6dKRQ@mail.gmail.com>
Date:   Wed, 30 Sep 2020 11:36:58 -0700
From:   Ben Gardon <bgardon@...gle.com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     LKML <linux-kernel@...r.kernel.org>, kvm <kvm@...r.kernel.org>,
        Cannon Matthews <cannonmatthews@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Peter Xu <peterx@...hat.com>, Peter Shier <pshier@...gle.com>,
        Peter Feiner <pfeiner@...gle.com>,
        Junaid Shahid <junaids@...gle.com>,
        Jim Mattson <jmattson@...gle.com>,
        Yulei Zhang <yulei.kernel@...il.com>,
        Wanpeng Li <kernellwp@...il.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Xiao Guangrong <xiaoguangrong.eric@...il.com>
Subject: Re: [PATCH 03/22] kvm: mmu: Init / Uninit the TDP MMU

On Tue, Sep 29, 2020 at 10:35 PM Sean Christopherson
<sean.j.christopherson@...el.com> wrote:
>
> Nit on all the shortlogs, can you use "KVM: x86/mmu" instead of "kvm: mmu"?

Will do.

>
> On Fri, Sep 25, 2020 at 02:22:43PM -0700, Ben Gardon wrote:
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > new file mode 100644
> > index 0000000000000..8241e18c111e6
> > --- /dev/null
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -0,0 +1,34 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#include "tdp_mmu.h"
> > +
> > +static bool __read_mostly tdp_mmu_enabled = true;
> > +module_param_named(tdp_mmu, tdp_mmu_enabled, bool, 0644);
>
> Do y'all actually toggle tdp_mmu_enabled while VMs are running?  I can see
> having a per-VM capability, or a read-only module param, but a writable
> module param is... interesting.

We don't use this much, but it is useful when running tests to be able
to go back and forth between running with and without the TDP MMU. I
should have added a note that the module parameter is mostly for
development purposes.

>
> > +static bool is_tdp_mmu_enabled(void)
> > +{
> > +     if (!READ_ONCE(tdp_mmu_enabled))
> > +             return false;
> > +
> > +     if (WARN_ONCE(!tdp_enabled,
> > +                   "Creating a VM with TDP MMU enabled requires TDP."))
>
> This should be enforced, i.e. clear tdp_mmu_enabled if !tdp_enabled.  As is,
> it's a user triggerable WARN, which is not good, e.g. with PANIC_ON_WARN.

That's a good point.

>
> > +             return false;
> > +
> > +     return true;
> > +}