Re: [PATCH 3/5] x86/perf: expose LBR format in PERF_CAPABILITIES

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, May 23, 2022 at 10:12:55AM +0200, Jan Beulich wrote:
> On 20.05.2022 16:58, Andrew Cooper wrote:
> > On 20/05/2022 15:19, Jan Beulich wrote:
> >> On 20.05.2022 16:10, Andrew Cooper wrote:
> >>> On 20/05/2022 14:37, Roger Pau Monne wrote:
> >>>> --- a/xen/include/public/arch-x86/cpufeatureset.h
> >>>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> >>>> @@ -135,7 +135,7 @@ XEN_CPUFEATURE(SSSE3,         1*32+ 9) /*A  
> >>>> Supplemental Streaming SIMD Extensio
> >>>>  XEN_CPUFEATURE(FMA,           1*32+12) /*A  Fused Multiply Add */
> >>>>  XEN_CPUFEATURE(CX16,          1*32+13) /*A  CMPXCHG16B */
> >>>>  XEN_CPUFEATURE(XTPR,          1*32+14) /*   Send Task Priority Messages 
> >>>> */
> >>>> -XEN_CPUFEATURE(PDCM,          1*32+15) /*   Perf/Debug Capability MSR */
> >>>> +XEN_CPUFEATURE(PDCM,          1*32+15) /*S  Perf/Debug Capability MSR */
> >>> This is the bit which requires more toolstack logic to safely enable. 
> >>> Using 's' for off-by-default is fine if we want to get the series in now.
> >>>
> >>> But before we expose the MSR generally, we need to:
> >>>
> >>> 1) Put the configuration in msr_policy so the toolstack can reason about 
> >>> it
> >>> 2) Reject migration attempts to destinations where the LBR format changes
> >> Since this could be quite restrictive, and since people needing to know
> >> they need to hide this feature for migration to work, I guess this would
> >> further want qualifying by "did the guest actually use LBRs so far"?
> > 
> > In practice, it's every major generation ("tock" on Intel's old model),
> > so isn't actually limiting the kinds of heterogeneous setups used in
> > production.  (Migration gets steadily less stable the further apart the
> > two CPUs are.)
> > 
> > As to dynamic, no - that would be a security bug in a cloud scenario,
> > because there must not be anything the guest can do to interfere with
> > the manageability.
> > 
> > Use of LBR is rare, as demonstrated by the fact that noone has
> > complained about the fact that migrating such a VM will malfunction.
> > 
> > As we now have a way of reporting "no model-specific LBR",
> 
> Which only rather new guest kernels will know to look for. Hence ...
> 
> > I'm tempted
> > to suggest that VMs get no LBR by default, and someone wanting LBR has
> > to opt in, which is also an explicit agreement to the migration limitation.
> 
> ... while in principle I agree with this, I see a practical issue.

I think it should be fine to expose no model-specific LBR support in
PERF_CAPABILITIES, but we shouldn't change the behavior of
DEBUGCTLMSR.LBR exposed to guests if the underlying platform has
model-specific LBRs and those are known to Xen.

That way old guest kernels that ignore PERF_CAPABILITIES.LBR_FORMAT
will continue to work, while newish kernels that check the format will
avoid using LBRs.

In case we introduce a guest config option to enable LBR, should we
then expose the native LBR format in PERF_CAPABILITIES?  Or would it
be better to just keep the current model and not expose
PERF_CAPABILITIES at all (and don't report PDCM in CPUID in that
case).

Thanks, Roger.