linux-kernel - Re: [PATCH v1 2/2] perf/x86/kvm: Avoid unnecessary work in guest filtering

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] [day] [month] [year] [list]

Date:   Sat, 6 Oct 2018 18:13:57 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Andi Kleen <andi@...stfloor.org>
cc:     peterz@...radead.org, x86@...nel.org, linux-kernel@...r.kernel.org,
        eranian@...gle.com, kan.liang@...el.com, isaku.yamahata@...el.com,
        kvm@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH v1 2/2] perf/x86/kvm: Avoid unnecessary work in guest
 filtering

On Sat, 6 Oct 2018, Thomas Gleixner wrote:

> On Fri, 5 Oct 2018, Andi Kleen wrote:
> > Some time ago KVM added a workaround for PEBS events leaking
> > into guests. This uses the KVM entry/exit list to add an extra
> > disable of the PEBS_ENABLE MSR.
> > 
> > Intel also added a fix for this issue to microcode updates on
> > Haswell/Broadwell/Skylake.
> > 
> > It turns out using the MSR entry/exit list makes VM exits
> > significantly slower. The list is only needed for disabling
> > PEBS, because the GLOBAL_CTRL change gets optimized by
> > KVM into changing the VMCS.
> > 
> > This patch checks for the microcode updates that have the microcode
> 
> # grep "This patch" Documentation/process
> 
> > fix for leaking PEBS, and disables the extra entry/exit list
> > entry for PEBS_ENABLE. In addition we always clear the
> > GLOBAL_CTRL for the PEBS counter while running in the guest,
> > which is enough to make them never fire at the wrong
> > side of the host/guest transition.
> >  
> > +#define IUCODE(model, step, rev) \
> > +	{ X86_VENDOR_INTEL, 6, model, step, rev, 0, 0 }
> 
> So we are going to have this kind of defines on every usage site. Please
> put these macros into the corresponding header file.
> 
> Also this wants to be named INTEL_MIN_UCODE() so it's clear what this is
> about.

And please use designated initializers. That way the unused fields do not
need explicit zeroing and any change of the data structure either just
works or the compiler catches the breakage.

Thanks,

	tglx