On Thu, Mar 26, 2026 at 2:57 PM Jim Mattson <[email protected]> wrote: > > On Thu, Mar 26, 2026 at 2:26 PM Yosry Ahmed <[email protected]> wrote: > > > > On Thu, Mar 26, 2026 at 2:19 PM Jim Mattson <[email protected]> wrote: > > > > > > On Tue, Feb 17, 2026 at 3:27 PM Sean Christopherson <[email protected]> > > > wrote: > > > > > > > Side topic, either as a prep patch (to duplicate code) or as a > > > > follow-up patch > > > > (to move the PAT handling in x86.c to vmx.c), the "common" handling of > > > > PAT should > > > > be fully forked between VMX and SVM. As of this patch, it's not just > > > > misleading, > > > > it's actively dangerous since calling kvm_get_msr_common() for SVM > > > > would get the > > > > wrong value. > > > > > > Though I included this change in v5 and v6, TIL that TDX calls > > > kvm_[gs]et_msr_common(MSR_IA32_CR_PAT), so the common handling is not > > > fully forked. > > > > Do you plan to drop this patch or add PAT handling in > > tdx_{get/set}_msr()? If you'll drop it, maybe add a warning if > > kvm_[gs]et_msr_common(MSR_IA32_CR_PAT) is called from SVM? > > I plan to leave the MSR_IA32_CR_PAT code in kvm_[gs]et_msr_common(). > Replicating the code in tdx.c seems like the wrong direction to go. > > There's no precedent that I can see for checking the vendor module in > common code (though we do have kvm_x86_ops.name). I could add a > warning if invoked with (vcpu->arch.efer & EFER_SVME). WDYT?
Yeah this should work. I assume Sean can decide to drop it when applying if he dislikes it, but given that he said the code is misleading and it would be dangerous if we end up calling from SVM -- I personally think the warning will act as documentation for the former and safeguard for the latter.
