On Fri, May 10, 2024 at 3:47 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > + * Since software-protected VMs don't have a notion of a shared vs. > > + * private that's separate from what KVM is tracking, the above > > + * KVM_EXIT_MEMORY_FAULT condition wouldn't occur, so avoid the > > + * special handling for that case for now. > > Very technically, it can occur if userspace _just_ modified the attributes. And > as I've said multiple times, at least for now, I want to avoid special casing > SW-protected VMs unless it is *absolutely* necessary, because their sole purpose > is to allow testing flows that are impossible to excercise without SNP/TDX hardware. Yep, it is not like they have to be optimized. > > + */ > > + if (kvm_slot_can_be_private(fault->slot) && > > + !(IS_ENABLED(CONFIG_KVM_SW_PROTECTED_VM) && > > + vcpu->kvm->arch.vm_type == KVM_X86_SW_PROTECTED_VM)) > > Heh, !(x && y) kills me, I misread this like 4 times. > > Anyways, I don't like the heuristic. It doesn't tie the restriction back to the > cause in any reasonable way. Can't this simply be? > > if (fault->is_private != kvm_mem_is_private(vcpu->kvm, fault->gfn) > return false; You beat me to it by seconds. And it can also be guarded by a check on kvm->arch.has_private_mem to avoid the attributes lookup. > Which is much, much more self-explanatory. Both more self-explanatory and more correct. Paolo
