On Thu, Aug 20, 2020 at 3:35 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote: > > On Thu, Aug 20, 2020 at 3:04 PM Alexander Graf <graf@xxxxxxxxxx> wrote: > > > > > > > > On 20.08.20 02:18, Aaron Lewis wrote: > > > > > > On Wed, Aug 19, 2020 at 8:26 AM Alexander Graf <graf@xxxxxxxxxx> wrote: > > >> > > >> > > >> > > >> On 18.08.20 23:15, Aaron Lewis wrote: > > >>> > > >>> SDM volume 3: 24.6.9 "MSR-Bitmap Address" and APM volume 2: 15.11 "MS > > >>> intercepts" describe MSR permission bitmaps. Permission bitmaps are > > >>> used to control whether an execution of rdmsr or wrmsr will cause a > > >>> vm exit. For userspace tracked MSRs it is required they cause a vm > > >>> exit, so the host is able to forward the MSR to userspace. This change > > >>> adds vmx/svm support to ensure the permission bitmap is properly set to > > >>> cause a vm_exit to the host when rdmsr or wrmsr is used by one of the > > >>> userspace tracked MSRs. Also, to avoid repeatedly setting them, > > >>> kvm_make_request() is used to coalesce these into a single call. > > >>> > > >>> Signed-off-by: Aaron Lewis <aaronlewis@xxxxxxxxxx> > > >>> Reviewed-by: Oliver Upton <oupton@xxxxxxxxxx> > > >> > > >> This is incomplete, as it doesn't cover all of the x2apic registers. > > >> There are also a few MSRs that IIRC are handled differently from this > > >> logic, such as EFER. > > >> > > >> I'm really curious if this is worth the effort? I would be inclined to > > >> say that MSRs that KVM has direct access for need special handling one > > >> way or another. > > >> > > > > > > Can you please elaborate on this? It was my understanding that the > > > permission bitmap covers the x2apic registers. Also, I’m not sure how > > > > So x2apic MSR passthrough is configured specially: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/vmx/vmx.c#n3796 > > > > and I think not handled by this patch? > > By happenstance only, I think, since there is also a call there to > vmx_disable_intercept_for_msr() for the TPR when x2APIC is enabled. If we want to be more explicit about it we could add kvm_make_request(KVM_REQ_USER_MSR_UPDATE, vcpu) After the bitmap is modified, but that doesn't seem to be necessary as Jim pointed out as there are calls to vmx_disable_intercept_for_msr() there which will set the update request for us. And we only have to worry about that if the bitmap is cleared which means MSR_BITMAP_MODE_X2APIC_APICV is set, and that flag can only be set if MSR_BITMAP_MODE_X2APIC is set. So, AFAICT that is covered by my changes.
