On 28.02.2022 16:48, Roger Pau Monné wrote:
> On Mon, Feb 28, 2022 at 02:14:26PM +0100, Jan Beulich wrote:
>> 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.
>
> I would have to check the AMD interface for hardware APIC
> virtualization support, I'm not sure how different the control values
> are there.
>
>> Plus then it might
>> make sense to match bit assignments in our interface with that in
>> the VT-x spec.
>
> That could work for things in secondary_exec_control, but posted
> interrupts are controlled in pin based exec control, so we would need
> to expose two different fields? Not sure it's worth the extra effort
> to match bit positions with the spec (or maybe I'm not understanding
> this correctly).
>
> Are you suggesting a (VMX) generic interface where the hypervisor
> exposes the raw vmx_secondary_exec_control and possibly
> vmx_pin_based_exec_control and let the toolstack play with it, setting
> in the VMCS what it gets back from the toolstack?
Not necessarily all of them, but on a case by case basis. But _where_
a control bit would appear (if supported) would be well known up front.
This wouldn't be very different from VMX'es "supports the 1-setting of"
information provided via MSRs. The hypervisor would indicate which of
the bits can be controlled on a per-guest basis.
Jan
