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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2015-09-23:
>>>> 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?

There is no need to send the PI since the CPU needs to handle the softirq 
immediately if there is softirq pending. And we can let the next vmenty to 
inject pending interrupt .

> Couldn't the entire check, including the IPI send, then go away?

Yes, there is no correctness issue if the entire check is removed. But to avoid 
the needless PI, it's better to have the check there. Also, I don't think the 
IPI send can be avoided here. 

> 
> 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

You are right. I forget to enable the interrupt.

> 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).

If we put the check ahead of emulation and realmode check, it may call 
vmx_do_vmentry twice: one to pick up the interrupt and one to do 
emulation/realmode. To avoid it, I choice to put it behind them.

> 
> Jan


Best regards,
Yang



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