On 24/02/2022 17:04, Jan Beulich wrote:
> On 24.02.2022 17:59, Jane Malalane wrote:
>> On 24/02/2022 14:16, Jan Beulich wrote:
>>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments
>>> unless you have verified the sender and know the content is safe.
>>>
>>> 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 ...
>>>
>>>> - if ( !cpu_has_vmx_virtualize_apic_accesses &&
>>>> + if ( !v->domain->arch.hvm.assisted_xapic &&
>>>> !virtualize_x2apic_mode )
>>>> return;
>>>
>>> ... here.
>> Previosuly we discussed setting v->domain->arch.hvm.assisted_xapic equal
>> to only cpu_has_vmx_virtualize_x2apic_mode, that's why I have those
>> additional checks as at least apic_reg_virt or virtual_intr_delivery are
>> needed for the subsequent parts of this function. I might be
>> misunderstanding your question.
>
> My expectation would have been that assisted_x2apic_available is assigned
> what is (in context above) assigned to virtualize_x2apic_mode (in patch 1).
> Anything deviating from this needs, I think, explaining there.
Oh, sorry, I kept thinking you meant cpu_has_... instead of the local
variable and it was just all confusing me. Would it help if I didn't use
a local variable at all, or changed the name then. This is really just a
check that is done before executing the code below in the function.
Thanks,
Jane.
