Re: [Xen-devel] [PATCH] Remove a set operation for VCPU_KICK_SOFTIRQ when post interrupt to vm.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.09.15 at 11:15, <yang.z.zhang@xxxxxxxxx> wrote:
> Jan Beulich wrote on 2015-09-23:
>>>>> On 23.09.15 at 05:50, <yang.z.zhang@xxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>>> @@ -1678,8 +1678,9 @@ static void __vmx_deliver_posted_interrupt(struct
>> vcpu *v)
>>>      {
>>>          unsigned int cpu = v->processor;
>>> -        if ( !test_and_set_bit(VCPU_KICK_SOFTIRQ,
>>> &softirq_pending(cpu)) -             && (cpu != smp_processor_id()) ) +
>>>        if ( !test_bit(VCPU_KICK_SOFTIRQ, &softirq_pending(cpu)) +      
>>>       && pi_test_on(&v->arch.hvm_vmx.pi_desc) +             && (cpu !=
>>> smp_processor_id()))
>>>              send_IPI_mask(cpumask_of(cpu), posted_intr_vector);
>>>      }
>>>  }
>> 
>> So this still removes the setting of the softirq - how can that be
>> correct (namely in the cpu == smp_processor_id() case)? Did you
>> perhaps mean
> 
> Why it will cause problem? The pending interrupt is covered by the check 
> before vmentry: if the outstanding bit is setting, it will redo the vmentry. 
> So even there is no softirq, the pending interrupt still can be injected to 
> guest in time.

Then what's the point of checking whether that softirq is pending?
Couldn't the entire check, including the IPI send, then go away?

Apart from that I just noticed that your jump to .Lvmx_do_vmentry
is wrong: At that label interrupts have to be enabled. And I guess
the check would also better be moved ahead of the emulation and
realmode check (in which case you could as well branch to
.Lvmx_process_softirqs and avoid said interrupt enabling problem).

Jan


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