Re: [PATCH v4 18/30] KVM: x86: Move "struct kvm_x86_msr_filter" definition to msrs.c

[Huang, Kai]

On Tue, 2026-06-16 at 09:19 -0700, Sean Christopherson wrote:
> On Tue, Jun 16, 2026, Kai Huang wrote:
> > On Tue, 2026-06-16 at 15:43 +0800, Binbin Wu wrote:
> > > > However, does moving the structure definition to "msrs.h" fix the problem?
> > > 
> > > Yes, it does.
> > > 
> > > Similar issue for struct kvm_x86_pmu_event_filter also could be resolved
> > > by moving the structure definition to "pmu.h"
> > 
> > Thanks for confirming.  I think we should do this.
> 
> I really don't want to go that route, especially since there's absolutely no
> reason to use srcu_dereference_check() during destruction.  KVM isn't actually
> checking anything, and the (S)RCU pointers _must_ be protected during destruction,
> otherwise use-after-free is all but guaranteed.

Yeah agreed.
> 
> Unfortunately, every RCU macro I can find does typeof(*p) somewhere in its flow.
> But I would still strongly prefer to open code a __force to strip the __rcu than
> expose the structures outside of msrs.c and pmu.c.
> 
> This makes gcc8 and gcc9 happy on my end.  I'll squash the changes into their
> respective patches, and update the changelogs.

Fine to me (seems we are having more and more functional changes in this series
but this particular change may not be the case).

FWIW, I also tested CLANG that your diff builds successfully.  I only tried
clang version 20.1.8 (Fedora 20.1.8-4.fc42) since I don't have other versions.

> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 95b3bc7b449e..b3c180e16e29 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -9975,13 +9975,13 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
>         if (kvm->arch.created_mediated_pmu)
>                 perf_release_mediated_pmu();
>         kvm_destroy_vcpus(kvm);
> -       kvm_free_msr_filter(srcu_dereference_check(kvm->arch.msr_filter, &kvm->srcu, 1));
> +       kvm_free_msr_filter((void * __force)kvm->arch.msr_filter);
>  #ifdef CONFIG_KVM_IOAPIC
>         kvm_pic_destroy(kvm);
>         kvm_ioapic_destroy(kvm);
>  #endif
>         kvfree(rcu_dereference_check(kvm->arch.apic_map, 1));

Btw, this made me look at apic_map too.  And seems we can also move "struct
kvm_apic_map" to "lapic.h".  We cannot move it to lapic.c because
kvm_sched_yield() in x86.c actually references the members of the structure.