On 28.02.2022 12:20, Roger Pau Monné wrote:
> On Thu, Feb 24, 2022 at 03:16:08PM +0100, Jan Beulich wrote:
>> On 18.02.2022 18:29, Jane Malalane wrote:
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -3333,15 +3333,15 @@ static void vmx_install_vlapic_mapping(struct vcpu
>>> *v)
>>>
>>> void vmx_vlapic_msr_changed(struct vcpu *v)
>>> {
>>> - int virtualize_x2apic_mode;
>>> + bool virtualize_x2apic_mode;
>>> struct vlapic *vlapic = vcpu_vlapic(v);
>>> unsigned int msr;
>>>
>>> virtualize_x2apic_mode = ( (cpu_has_vmx_apic_reg_virt ||
>>> cpu_has_vmx_virtual_intr_delivery) &&
>>> - cpu_has_vmx_virtualize_x2apic_mode );
>>> + v->domain->arch.hvm.assisted_x2apic );
>>
>> Following from my comment on patch 1, I'd expect this to become a simple
>> assignment of v->domain->arch.hvm.assisted_x2apic (at which point the
>> local variable could go away), just like ...
>
> I think we want to keep assisted_x{2}apic mapped to
> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES and
> SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE respectively, so that in the
> future we could add further controls for
> SECONDARY_EXEC_APIC_REGISTER_VIRT and interrupt delivery.
If we want to be able to control more (most?) VMX sub-features, it
would seem to me as if this would better be modeled accordingly
right away. At that point there would likely need to be VMX and SVM
specific controls rather than general HVM ones. Plus then it might
make sense to match bit assignments in our interface with that in
the VT-x spec.
Jan
