Re: [Xen-devel] [PATCH v2] x86/vlapic: Don't reset APIC ID when handling INIT signal

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.04.17 at 09:41, <chao.gao@xxxxxxxxx> wrote:
> On Wed, Apr 19, 2017 at 08:17:14AM -0600, Jan Beulich wrote:
>>>>> On 19.04.17 at 08:40, <chao.gao@xxxxxxxxx> wrote:
>>> @@ -1257,7 +1257,12 @@ void vlapic_reset(struct vlapic *vlapic)
>>>      }
>>>      vlapic_set_reg(vlapic, APIC_ICR,     0);
>>>      vlapic_set_reg(vlapic, APIC_ICR2,    0);
>>> -    vlapic_set_reg(vlapic, APIC_LDR,     0);
>>> +    /*
>>> +     * LDR is read-only in x2APIC mode. Preserve its value when handling
>>> +     * INIT signal in x2APIC mode.
>>> +     */
>>> +    if ( !vlapic_x2apic_mode(vlapic) )
>>
>>In order for this to work you need to ...
>>
>>> +        vlapic_set_reg(vlapic, APIC_LDR, 0);
>>>      vlapic_set_reg(vlapic, APIC_TASKPRI, 0);
>>>      vlapic_set_reg(vlapic, APIC_TMICT,   0);
>>>      vlapic_set_reg(vlapic, APIC_TMCCT,   0);
>>> @@ -1275,6 +1280,22 @@ void vlapic_reset(struct vlapic *vlapic)
>>>      destroy_periodic_time(&vlapic->pt);
>>>  }
>>>  
>>> +/* Reset the VLAPIC back to its power-on/reset state. */
>>> +void vlapic_reset(struct vlapic *vlapic)
>>> +{
>>> +    const struct vcpu *v = vlapic_vcpu(vlapic);
>>> +
>>> +    if ( !has_vlapic(v->domain) )
>>> +        return;
>>> +
>>> +    vlapic_set_reg(vlapic, APIC_ID, (v->vcpu_id * 2) << 24);
>>> +    vlapic_set_reg(vlapic, APIC_LDR, 0);
>>> +    vlapic_do_init(vlapic);
>>> +
>>> +    vlapic->hw.apic_base_msr &= ~(uint64_t)MSR_IA32_APICBASE_EXTD;
>>
>>... do this before calling vlapic_do_init() afaict (and I see no strong
>>reason why you would need a cast here)). Likely it is a good idea
>>then to also ...
> 
> Got it and will fix. The APIC_LDR is also cleared in vlapic_reset.
> Do you mean it will do a cast automatically? I don't know that before.
> I was afraid that the higher qword of ~MSR_IA32_APICBASE_EXTD will is 
> zero.

Depends on whether the value is signed (in which case it will be
sign extended) or unsigned (resulting in zero-extension). The
constant in this case is signed, so the high bits wouldn't be unduly
chopped off.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel