On Mon, Nov 07, 2022 at 05:58:04PM +0100, Jan Beulich wrote: > 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?
Using cpu_has_vmx_apic_reg_virt won't be correct, as a domain can have it disabled now after this change. When using x2APIC accesses will already be done using MSRs, so the hint is not useful in that mode. > > @@ -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. Oh, thanks. I will wait for Andrews feedback then, I think the extra blank can likely be removed at commit if we agree this is OK. > 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. Ack, thanks, I think this is the best that we can do given the status of the release, but would likely need to be quick or else it's gonna be too late. Roger.
