On Wed, 2022-05-18 at 15:39 +0000, Sean Christopherson wrote:
On Wed, May 18, 2022, Maxim Levitsky wrote:
On Wed, 2022-05-18 at 16:28 +0800, Chao Gao wrote:
struct kvm_arch { @@ -1258,6 +1260,7 @@ struct kvm_arch { hpa_t hv_root_tdp; spinlock_t hv_root_tdp_lock; #endif
- bool apic_id_changed;
What's the value of this boolean? No one reads it.
I use it in later patches to kill the guest during nested VM entry if it attempts to use nested AVIC after any vCPU changed APIC ID.
Then the flag should be introduced in the later patch, because (a) it's dead code if that patch is never merged and (b) it's impossible to review this patch for correctness without seeing the usage, e.g. setting apic_id_changed isn't guarded with a lock and so the usage may or may not be susceptible to races.
I can't disagree with you on this, this was just somewhat a hack I wasn't sure (and not yet 100% sure I will move forward with) so I cut this corner.
Thanks for the review!
Best regards, Maxim Levitsky
- apic->vcpu->kvm->arch.apic_id_changed = true;
+}