Re: [PATCH] vmx, apicv: fix save/restore issue with apicv

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/10/14 07:38, Zhang, Yang Z wrote:
> Jan Beulich wrote on 2014-10-14:
>>>>> On 14.10.14 at 15:10, <yang.z.zhang@intel.com> wrote:
>>> Jan Beulich wrote on 2014-10-14:
>>>>>>> On 14.10.14 at 07:43, <yang.z.zhang@intel.com> wrote:
>>>>> Jan Beulich wrote on 2014-10-13:
>>>>>>>>> On 11.10.14 at 09:54, <yang.z.zhang@intel.com> wrote:
>>>>>>> @@ -1597,6 +1599,29 @@ static void vmx_process_isr(int isr,
>>>>>>> struct vcpu
>>>>>> *v)
>>>>>>>          status |= isr << VMX_GUEST_INTR_STATUS_SVI_OFFSET;
>>>>>>>          __vmwrite(GUEST_INTR_STATUS, status);
>>>>>>>      }
>>>>>>> +
>>>>>>> +    eoi_exit_bitmap = (unsigned int
>>>>>>> + *)v->arch.hvm_vmx.eoi_exit_bitmap;
>>>>>> No casts like this please. This is a bitmap and would hence
>>>>>> preferably be treated that way consistently. The code here isn't
>>>> performance critical.
>>>>> Yes, it's performance critical from the live migration's point
>>>>> and that's why I use the cast here and
>>>> Compared to everything else involved in migration, a few extra
>>>> cycles here
>>> Xen is far behind KVM on live migration. If we want to remain the
>>> competitive, I'd suggest doing a cleanup on current migration logic
>>> to make it more effective.
>> But that surely needs to be done elsewhere, where more than a couple
>> of nanoseconds are to be gained.
> If you think it is ok, I will use vector based operations:
>
> +    for ( vector = 0x10; vector < NR_VECTORS; vector++ )
> +        if (vlapic_test_vector(vector, &s->regs->data[APIC_IRR]) ||
> +                vlapic_test_vector(vector, &s->regs->data[APIC_ISR]))
> +            set_bit(vector,  v->arch.hvm_vmx.eoi_exit_bitmap);
>
> Andrew, is it ok for you? 

That covers the appropriate vector range, (and getting a working fix is
far more important than arguing its efficiency at this point in the
release).

This looks fine to me.