linux-kernel - Re: [PATCH v5 01/26] KVM: x86: hyper-v: Expose access to debug MSRs in the partition privilege flags

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]

Date:   Thu, 18 Aug 2022 17:59:22 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>,
        Nathan Chancellor <nathan@...nel.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 01/26] KVM: x86: hyper-v: Expose access to debug MSRs
 in the partition privilege flags

Sean Christopherson <seanjc@...gle.com> writes:

> On Thu, Aug 18, 2022, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@...gle.com> writes:
>> 
>> > On Tue, Aug 02, 2022, Vitaly Kuznetsov wrote:
>> >> For some features, Hyper-V spec defines two separate CPUID bits: one
>> >> listing whether the feature is supported or not and another one showing
>> >> whether guest partition was granted access to the feature ("partition
>> >> privilege mask"). 'Debug MSRs available' is one of such features. Add
>> >> the missing 'access' bit.
>> >> 
>> >> Note: hv_check_msr_access() deliberately keeps checking
>> >> HV_FEATURE_DEBUG_MSRS_AVAILABLE bit instead of the new HV_ACCESS_DEBUG_MSRS
>> >> to not break existing VMMs (QEMU) which only expose one bit. Normally, VMMs
>> >> should set either both these bits or none.
>> >
>> > This is not the right approach long term.  If KVM absolutely cannot unconditionally
>> > switch to checking HV_ACCESS_DEBUG_MSRS because it would break QEMU users, then we
>> > should add a quirk, but sweeping the whole thing under the rug is wrong.
>> >
>> 
>> First, this patch is kind of unrelated to the series so in case it's the
>> only thing which blocks it from being merged -- let's just pull it out
>> and discuss separately.
>
> Regarding the series, are there any true dependencies between the eVMCS patches
> (1 - 11) and the VMCS sanitization rework (12 - 26)?  I.e. can the VMCS rework
> be queued ahead of the eVMCS v1 support?

My memory is a bit blurry already but I think PATCH11 ("KVM: VMX: Get
rid of eVMCS specific VMX controls sanitization") needs to go before
PATCH24 ("KVM: nVMX: Use sanitized allowed-1 bits for VMX control MSRs")
to have "bug compatibility" and resolve Jim's concern: guest visible
VMX feature MSR values are not supposed to change. Currently, we filter
out unsupported features from eVMCS for KVM itself but not for L1 as we
expose raw host MSR values there. This is likely broken if L1 decides to
*use* these features for real but that's another story.

-- 
Vitaly