On Mon, 2024-03-25 at 12:05 -0700, Isaku Yamahata wrote: > Right, the guest has to accept it on VE. If the unmap was intentional by guest, > that's fine. The unmap is unintentional (with vMTRR), the guest doesn't expect > VE with the GPA. > > > > But, I guess we should punt to userspace is the guest tries to use > > MTRRs, not that userspace can handle it happening in a TD... But it > > seems cleaner and safer then skipping zapping some pages inside the > > zapping code. > > > > I'm still not sure if I understand the intention and constraints fully. > > So please correct. This (the skipping the zapping for some operations) > > is a theoretical correctness issue right? It doesn't resolve a TD > > crash? > > For lapic, it's safe guard. Because TDX KVM disables APICv with > APICV_INHIBIT_REASON_TDX, apicv won't call kvm_zap_gfn_range(). Ah, I see it: https://lore.kernel.org/lkml/38e2f8a77e89301534d82325946eb74db3e47815.1708933498.git.isaku.yamahata@xxxxxxxxx/ Then it seems a warning would be more appropriate if we are worried there might be a way to still call it. If we are confident it can't, then we can just ignore this case. > > For MTRR, the purpose is to make the guest boot (without the guest kernel > command line like clearcpuid=mtrr) . > If we can assume the guest won't touch MTRR registers somehow, KVM can return an > error to TDG.VP.VMCALL<RDMSR, WRMSR>(MTRR registers). So it doesn't call > kvm_zap_gfn_range(). Or we can use KVM_EXIT_X86_{RDMSR, WRMSR} as you suggested. My understanding is that Sean prefers to exit to userspace when KVM can't handle something, versus making up behavior that keeps known guests alive. So I would think we should change this patch to only be about not using the zapping roots optimization. Then a separate patch should exit to userspace on attempt to use MTRRs. And we ignore the APIC one. This is trying to guess what maintainers would want here. I'm less sure what Paolo prefers.
