On Mon, Jan 7, 2019 at 1:09 AM Wei Wang <wei.w.wang@xxxxxxxxx> wrote: > > On 01/03/2019 11:25 PM, Jim Mattson wrote: > > On Wed, Jan 2, 2019 at 11:55 PM Wei Wang <wei.w.wang@xxxxxxxxx> wrote: > > > >> Right, thanks. Probably better to change it to below: > >> > >> msr_info->data = 0; > >> data = native_read_msr(MSR_IA32_PERF_CAPABILITIES); > >> if (vcpu->kvm->arch.lbr_in_guest) > >> msr_info->data |= (data & X86_PERF_CAP_MASK_LBR_FMT); > >> > > This still breaks backwards compatibility. Returning 0 and raising #GP > > are not the same. > > I'm not sure about raising GP# in this case. > > This PERF_CAP msr contains more things than the lbr format. > For example, a guest with lbr=false option could read it to get PEBS_FMT, > which is PERF_CAP[11:8]. We should offer those bits in this case. > > When lbr=false, the lbr feature is not usable by the guest, > so I think whatever value (0 or other value) of the LBR_FMT bits that > we give to the guest might not be important. The issue is compatibility. Prior to your change, reading this MSR from a VM would raise #GP. After your change, it won't. That means that if you have a VM migrating between hosts with kernel versions before and after this change, the results will be inconsistent. In the forward migration path, RDMSR will first raise #GP, and later will return 0. In the backward migration path, RDMSR will first return 0, and later it will raise #GP. To avoid these inconsistencies, the new MSR behavior should be explicitly enabled by an opt-in ioctl from userspace (not necessarily the same ioctl that enables LBR).
