> From: Sean Christopherson <seanjc@xxxxxxxxxx> > Sent: Tuesday, March 12, 2024 8:26 AM > > On Mon, Mar 11, 2024, Yan Zhao wrote: > > On Fri, Mar 08, 2024 at 05:09:29PM -0800, Sean Christopherson wrote: > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > > index 17a8e4fdf9c4..5dc4c24ae203 100644 > > > --- a/arch/x86/kvm/vmx/vmx.c > > > +++ b/arch/x86/kvm/vmx/vmx.c > > > @@ -7605,11 +7605,13 @@ static u8 vmx_get_mt_mask(struct kvm_vcpu > *vcpu, gfn_t gfn, bool is_mmio) > > > > > > /* > > > * Force WB and ignore guest PAT if the VM does NOT have a non- > coherent > > > - * device attached. Letting the guest control memory types on Intel > > > - * CPUs may result in unexpected behavior, and so KVM's ABI is to > trust > > > - * the guest to behave only as a last resort. > > > + * device attached and the CPU doesn't support self-snoop. Letting > the > > > + * guest control memory types on Intel CPUs without self-snoop may > > > + * result in unexpected behavior, and so KVM's (historical) ABI is to > > > + * trust the guest to behave only as a last resort. > > > */ > > > - if (!kvm_arch_has_noncoherent_dma(vcpu->kvm)) > > > + if (!static_cpu_has(X86_FEATURE_SELFSNOOP) && > > > + !kvm_arch_has_noncoherent_dma(vcpu->kvm)) > > > return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) > | VMX_EPT_IPAT_BIT; > > > > For the case of !static_cpu_has(X86_FEATURE_SELFSNOOP) && > > kvm_arch_has_noncoherent_dma(vcpu->kvm), I think we at least should > warn > > about unsafe before honoring guest memory type. > > I don't think it gains us enough to offset the potential pain such a message > would bring. Assuming the warning isn't outright ignored, the most likely > scenario > is that the warning will cause random end users to worry that the setup > they've > been running for years is broken, when in reality it's probably just fine for > their > use case. Isn't the 'worry' necessary to allow end users evaluate whether "it's probably just fine for their use case"? I saw the old comment already mentioned that doing so may lead to unexpected behaviors. But I'm not sure whether such code-level caveat has been visible enough to end users. > > I would be quite surprised if there are people running untrusted workloads > on > 10+ year old silicon *and* have passthrough devices and non-coherent > IOMMUs/DMA. this is probably true. > And anyone exposing a device directly to an untrusted workload really > should have > done their homework. or they run trusted workloads which might be tampered by virus to exceed the scope of their homework. 😊 > > And it's not like we're going to change KVM's historical behavior at this point. I agree with your point of not breaking userspace. But still think a warning might be informative to let users evaluate their setup against a newly identified "unexpected behavior" which has security implication beyond the guest, while the previous interpretation of "unexpected behavior" might be that the guest can at most shoot its own foot...
