On 04.11.2022 17:18, Roger Pau Monne wrote: > --- a/xen/arch/x86/hvm/viridian/viridian.c > +++ b/xen/arch/x86/hvm/viridian/viridian.c > @@ -197,7 +197,7 @@ void cpuid_viridian_leaves(const struct vcpu *v, uint32_t > leaf, > res->a = CPUID4A_RELAX_TIMER_INT; > if ( viridian_feature_mask(d) & HVMPV_hcall_remote_tlb_flush ) > res->a |= CPUID4A_HCALL_REMOTE_TLB_FLUSH; > - if ( !cpu_has_vmx_apic_reg_virt ) > + if ( !has_assisted_xapic(d) ) > res->a |= CPUID4A_MSR_BASED_APIC;
Isn't this too restrictive when considering x2APIC? IOW is there anything wrong with leaving this as is? > @@ -3432,6 +3436,10 @@ void vmx_vlapic_msr_changed(struct vcpu *v) > vmx_set_msr_intercept(v, MSR_X2APIC_PPR, VMX_MSR_R); > vmx_set_msr_intercept(v, MSR_X2APIC_TMICT, VMX_MSR_R); > vmx_set_msr_intercept(v, MSR_X2APIC_TMCCT, VMX_MSR_R); > + > + v->arch.hvm.vmx.secondary_exec_control |= > + SECONDARY_EXEC_APIC_REGISTER_VIRT; > + > } Nit: stray trailing blank line inside the block. Everything else looks plausible to me, but from prior discussion I wonder whether the result isn't still going to be too coarse grained for Andrew's taste. Jan
