On 28/01/2015 17:02, Wincy Van wrote:
> +static int vmx_deliver_nested_posted_interrupt(struct kvm_vcpu *vcpu,
> + int vector)
> +{
> + if (is_guest_mode(vcpu) &&
> + vector == to_vmx(vcpu)->nested.posted_intr_nv &&
> + vcpu->mode == IN_GUEST_MODE) {
> + /* the PIR and ON have been set by L1. */
What happens if there is a L2->L0->L2 exit on the target VCPU, and the
guest exits before apic->send_IPI_mask sends the IPI?
The L1 hypervisor might "know" somehow that there cannot be a concurrent
L2->L1->L2 exit, and not do the equivalent of KVM's
kvm_make_request(KVM_REQ_EVENT, vcpu);
after it sets ON.
So I think you have to do something like
static bool vmx_is_nested_posted_interrupt(struct kvm_vcpu *vcpu,
int vector)
{
return (is_guest_mode(vcpu) &&
vector == to_vmx(vcpu)->nested.posted_intr_nv);
}
and in vmx_deliver_posted_interrupt:
r = 0;
if (!vmx_is_nested_posted_interrupt(vcpu, vector)) {
if (pi_test_and_set_pir(vector, &vmx->pi_desc))
return;
r = pi_test_and_set_on(&vmx->pi_desc);
}
kvm_make_request(KVM_REQ_EVENT, vcpu);
#ifdef CONFIG_SMP
if (!r && (vcpu->mode == IN_GUEST_MODE))
apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
POSTED_INTR_VECTOR);
else
#endif
kvm_vcpu_kick(vcpu);
What do you think?
Paolo
> + apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
> + POSTED_INTR_VECTOR);
> + return 0;
> + }
> + return -1;
> +}
On Mon, Feb 2, 2015 at 7:03 PM, Paolo Bonzini <[email protected]> wrote:
>
>
> On 28/01/2015 17:02, Wincy Van wrote:
>> +static int vmx_deliver_nested_posted_interrupt(struct kvm_vcpu *vcpu,
>> + int vector)
>> +{
>> + if (is_guest_mode(vcpu) &&
>> + vector == to_vmx(vcpu)->nested.posted_intr_nv &&
>> + vcpu->mode == IN_GUEST_MODE) {
>> + /* the PIR and ON have been set by L1. */
>
> What happens if there is a L2->L0->L2 exit on the target VCPU, and the
> guest exits before apic->send_IPI_mask sends the IPI?
>
> The L1 hypervisor might "know" somehow that there cannot be a concurrent
> L2->L1->L2 exit, and not do the equivalent of KVM's
>
In non-nested case, if a posted intr was not accomplished by hardware,
we will sync the pir to irr and set rvi to accomplish it.
Current implementation may lead some of the nested posted intrs delay
for a short time(wait for a nested-vmexit).
> kvm_make_request(KVM_REQ_EVENT, vcpu);
>
> after it sets ON.
>
> So I think you have to do something like
>
> static bool vmx_is_nested_posted_interrupt(struct kvm_vcpu *vcpu,
> int vector)
> {
> return (is_guest_mode(vcpu) &&
> vector == to_vmx(vcpu)->nested.posted_intr_nv);
> }
>
> and in vmx_deliver_posted_interrupt:
>
> r = 0;
> if (!vmx_is_nested_posted_interrupt(vcpu, vector)) {
> if (pi_test_and_set_pir(vector, &vmx->pi_desc))
> return;
>
> r = pi_test_and_set_on(&vmx->pi_desc);
> }
> kvm_make_request(KVM_REQ_EVENT, vcpu);
> #ifdef CONFIG_SMP
> if (!r && (vcpu->mode == IN_GUEST_MODE))
> apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
> POSTED_INTR_VECTOR);
> else
> #endif
> kvm_vcpu_kick(vcpu);
>
>
> What do you think?
>
I think that there would be a way to avoid that delay, but may hurt performance:
When doing nested posted intr, we can set a request bit:
if (is_guest_mode(vcpu) &&
vector == to_vmx(vcpu)->nested.posted_intr_nv &&
vcpu->mode == IN_GUEST_MODE) {
/* the PIR and ON have been set by L1. */
apic->send_IPI_mask(get_cpu_mask(vcpu->cpu),
POSTED_INTR_VECTOR);
+ kvm_make_request(KVM_REQ_ACCOMP_POSTED_INTR, vcpu);
return 0;
}
If a posted intr was not accomplished by hardware, we can check that
bit before checking KVM_REQ_EVENT, and if that bit is set, we can do:
static void vmx_accomp_nested_posted_intr(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
if (is_guest_mode(vcpu) &&
vmx->nested.posted_intr_nv != -1 &&
pi_test_on(vmx->nested.pi_desc))
kvm_apic_set_irr(vcpu,
vmx->nested.posted_intr_nv);
}
Then we will get an nested-vmexit in vmx_check_nested_events, that
posted intr will be handled by L1 immediately.
This mechanism will also emulate the hardware's behavior: If a posted
intr was not accomplished by hardware, we will get an interrupt with
POSTED_INTR_NV.
Would this be better?
Thanks,
Wincy
On 02/02/2015 16:33, Wincy Van wrote:
> static void vmx_accomp_nested_posted_intr(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> if (is_guest_mode(vcpu) &&
> vmx->nested.posted_intr_nv != -1 &&
> pi_test_on(vmx->nested.pi_desc))
> kvm_apic_set_irr(vcpu,
> vmx->nested.posted_intr_nv);
> }
>
> Then we will get an nested-vmexit in vmx_check_nested_events, that
> posted intr will be handled by L1 immediately.
> This mechanism will also emulate the hardware's behavior: If a posted
> intr was not accomplished by hardware, we will get an interrupt with
> POSTED_INTR_NV.
Yes.
> Would this be better?
I think you do not even need a new bit. You can use KVM_REQ_EVENT and
(to complete my suggestion, which was not enough) do the above in
vmx_check_nested_events.
Paolo
Paolo Bonzini wrote on 2015-02-03:
>
>
> On 02/02/2015 16:33, Wincy Van wrote:
>> static void vmx_accomp_nested_posted_intr(struct kvm_vcpu *vcpu) {
>> struct vcpu_vmx *vmx = to_vmx(vcpu);
>>
>> if (is_guest_mode(vcpu) &&
>> vmx->nested.posted_intr_nv != -1 &&
>> pi_test_on(vmx->nested.pi_desc))
>> kvm_apic_set_irr(vcpu,
>> vmx->nested.posted_intr_nv); }
>> Then we will get an nested-vmexit in vmx_check_nested_events, that
>> posted intr will be handled by L1 immediately.
>> This mechanism will also emulate the hardware's behavior: If a
>> posted intr was not accomplished by hardware, we will get an
Actually, we cannot say "not accomplished by hardware". It more like we don't do the job well. See my below answer.
>> interrupt with POSTED_INTR_NV.
>
> Yes.
This is not enough. From L1's point, L2 is in vmx non-root mode. So we should emulate the posted interrupt in L0 correctly, say:
1. clear ON bit
2. ack interrupt
3, syn pir to virr
4. update RVI.
Then let the hardware(virtual interrupt delivery) to accomplish interrupt injection.
Force a vmexit more like a trick. It's better to follow the hardware's behavior unless we cannot do it.
>
>> Would this be better?
>
> I think you do not even need a new bit. You can use KVM_REQ_EVENT and
> (to complete my suggestion, which was not enough) do the above in
> vmx_check_nested_events.
>
> Paolo
Best regards,
Yang
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Tue, Feb 3, 2015 at 9:21 AM, Zhang, Yang Z <[email protected]> wrote:
> Paolo Bonzini wrote on 2015-02-03:
>>
>>
>> On 02/02/2015 16:33, Wincy Van wrote:
>>> static void vmx_accomp_nested_posted_intr(struct kvm_vcpu *vcpu) {
>>> struct vcpu_vmx *vmx = to_vmx(vcpu);
>>>
>>> if (is_guest_mode(vcpu) &&
>>> vmx->nested.posted_intr_nv != -1 &&
>>> pi_test_on(vmx->nested.pi_desc))
>>> kvm_apic_set_irr(vcpu,
>>> vmx->nested.posted_intr_nv); }
>>> Then we will get an nested-vmexit in vmx_check_nested_events, that
>>> posted intr will be handled by L1 immediately.
>>> This mechanism will also emulate the hardware's behavior: If a
>>> posted intr was not accomplished by hardware, we will get an
>
> Actually, we cannot say "not accomplished by hardware". It more like we don't do the job well. See my below answer.
>
Yes, exactly.
>>> interrupt with POSTED_INTR_NV.
>>
>> Yes.
>
> This is not enough. From L1's point, L2 is in vmx non-root mode. So we should emulate the posted interrupt in L0 correctly, say:
> 1. clear ON bit
> 2. ack interrupt
> 3, syn pir to virr
> 4. update RVI.
> Then let the hardware(virtual interrupt delivery) to accomplish interrupt injection.
>
> Force a vmexit more like a trick. It's better to follow the hardware's behavior unless we cannot do it.
>
Yes, I will try again to do this.
Thanks,
Wincy