linux-kernel - RE: [PATCH v13 06/12] KVM: x86: Add Intel PT virtualization work mode

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: <82D7661F83C1A047AF7DC287873BF1E172BD64F1@SHSMSX101.ccr.corp.intel.com>
Date:   Thu, 25 Oct 2018 00:35:11 +0000
From:   "Kang, Luwei" <luwei.kang@...el.com>
To:     Jim Mattson <jmattson@...gle.com>
CC:     kvm list <kvm@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Paolo Bonzini" <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        "songliubraving@...com" <songliubraving@...com>,
        Peter Zijlstra <peterz@...radead.org>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>,
        "kstewart@...uxfoundation.org" <kstewart@...uxfoundation.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        "mattst88@...il.com" <mattst88@...il.com>,
        Janakarajan Natarajan <Janakarajan.Natarajan@....com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        "Josh Poimboeuf" <jpoimboe@...hat.com>,
        Marc Orr <marcorr@...gle.com>, Uros Bizjak <ubizjak@...il.com>,
        "Christopherson, Sean J" <sean.j.christopherson@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Chao Peng <chao.p.peng@...ux.intel.com>
Subject: RE: [PATCH v13 06/12] KVM: x86: Add Intel PT virtualization work
 mode

> > From: Chao Peng <chao.p.peng@...ux.intel.com>
> >
> > Intel Processor Trace virtualization can be work in one of 2 possible
> > modes:
> >
> > a. System-Wide mode (default):
> >    When the host configures Intel PT to collect trace packets
> >    of the entire system, it can leave the relevant VMX controls
> >    clear to allow VMX-specific packets to provide information
> >    across VMX transitions.
> >    KVM guest will not aware this feature in this mode and both
> >    host and KVM guest trace will output to host buffer.
> >
> > b. Host-Guest mode:
> >    Host can configure trace-packet generation while in
> >    VMX non-root operation for guests and root operation
> >    for native executing normally.
> >    Intel PT will be exposed to KVM guest in this mode, and
> >    the trace output to respective buffer of host and guest.
> >    In this mode, tht status of PT will be saved and disabled
> >    before VM-entry and restored after VM-exit if trace
> >    a virtual machine.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@...ux.intel.com>
> > Signed-off-by: Luwei Kang <luwei.kang@...el.com>
> > ---
> 
> > +#define SECONDARY_EXEC_PT_USE_GPA              0x01000000
> > +#define VM_EXIT_CLEAR_IA32_RTIT_CTL            0x02000000
> > +#define VM_ENTRY_LOAD_IA32_RTIT_CTL            0x00040000
> 
> Where are all of these bits documented? I'm looking at the latest SDM, volume 3 (325384-067US), and none of these bits aredocumented
> there.

This part is in the " Intel® Architecture Instruction Set Extensions and Future Features Programming Reference"
https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf

> 
> > +       GUEST_IA32_RTIT_CTL             = 0x00002814,
> > +       GUEST_IA32_RTIT_CTL_HIGH        = 0x00002815,
> 
> Where is this VMCS field documented?
> 
> > +/* Default is SYSTEM mode. */
> > +static int __read_mostly pt_mode = PT_MODE_SYSTEM;
> > +module_param(pt_mode, int, S_IRUGO);
> 
> As a module parameter, this doesn't allow much flexibility. Is it possible to make this decision per-VM, using a VM capability that can be set
> by userspace? (In that case, it may make sense to have a module parameter which allows/disallows the per-VM capability.)

It is a good idea from my point of view, I think it need more discussion and can be implement in next phase if have strong requirement.

> 
> 
> > +static inline bool cpu_has_vmx_intel_pt(void) {
> > +       u64 vmx_msr;
> > +
> > +       rdmsrl(MSR_IA32_VMX_MISC, vmx_msr);
> > +       return !!(vmx_msr & MSR_IA32_VMX_MISC_INTEL_PT); }
> 
> Instead of the rdmsr here, wouldn't it be better to cache the IA32_VMX_MISC MSR in vmcs_config?
> Nit: throughout this change, the '!!' isn't necessary when casting an integer type to bool.

MSR_IA32_VMX_MISC is not read frequency and just read once in this patch set during initialization.

Thanks,
Luwei Kang