Re: [PATCH 0/5] Introduce a quirk to control memslot zap behavior

[Yan Zhao]

On Fri, Jun 14, 2024 at 04:01:07AM +0800, Edgecombe, Rick P wrote:
> On Thu, 2024-06-13 at 14:06 +0800, Yan Zhao wrote:
> >       a) Add a condition for TDX VM type in kvm_arch_flush_shadow_memslot()
> >          besides the testing of kvm_check_has_quirk(). It is similar to
> >          "all new VM types have the quirk disabled". e.g.
> > 
> >          static inline bool kvm_memslot_flush_zap_all(struct kvm
> > *kvm)                    
> >         
> > {                                                                             
> >    
> >               return kvm->arch.vm_type != KVM_X86_TDX_VM
> > &&                               
> >                      kvm_check_has_quirk(kvm,
> > KVM_X86_QUIRK_SLOT_ZAP_ALL);                
> >          }
> >          
> >       b) Init the disabled_quirks based on VM type in kernel, extend
> >          disabled_quirk querying/setting interface to enforce the quirk to
> >          be disabled for TDX.
> 
> I'd prefer to go with option (a) here. Because we don't have any behavior
> defined yet for KVM_X86_TDX_VM, we don't really need to "disable a quirk" of it.
> 
> Instead we could just define KVM_X86_QUIRK_SLOT_ZAP_ALL to be about the behavior
> of the existing vm_types. It would be a few lines of documentation to save
> implementing and maintaining a whole interface with special logic for TDX. So to
> me it doesn't seem worth it, unless there is some other user for a new more
> complex quirk interface.
What about introducing a forced disabled_quirk field?

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 8152b5259435..32859952fa75 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1425,6 +1425,7 @@ struct kvm_arch {
        u32 bsp_vcpu_id;

        u64 disabled_quirks;
+       u64 force_disabled_quirks;

        enum kvm_irqchip_mode irqchip_mode;
        u8 nr_reserved_ioapic_pins;
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index e4bb1db45476..629a95cbe568 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -565,6 +565,7 @@ int tdx_vm_init(struct kvm *kvm)
 {
        kvm->arch.has_private_mem = true;

+       kvm->arch.force_disabled_quirks |= KVM_X86_QUIRK_SLOT_ZAP_ALL;
        /*
         * Because guest TD is protected, VMM can't parse the instruction in TD.
         * Instead, guest uses MMIO hypercall.  For unmodified device driver,
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 229497fd3266..53ef06a06517 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -291,7 +291,8 @@ static inline void kvm_register_write(struct kvm_vcpu *vcpu,

 static inline bool kvm_check_has_quirk(struct kvm *kvm, u64 quirk)
 {
-       return !(kvm->arch.disabled_quirks & quirk);
+       return !(kvm->arch.disabled_quirks & quirk) &&
+              !(kvm->arch.force_disabled_quirks & quirk);
 }