Il 27/08/2014 16:05, Wei Wang ha scritto:
> kvm_ioapic_scan_entry() needs to update tmr. The previous lapic tmr value
> (old_tmr) needs to sync with ioapic to get an accurate updated tmr
> value before the updating work.
>
> Tested-by: Rongrong Liu <rongrongx.liu@xxxxxxxxx>
> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
> Signed-off-by: Wei Wang <wei.w.wang@xxxxxxxxx>
This is also a very terse commit message. As mentioned in the review
of the other patch, I'm not sure this change is correct, but in any case
here is how a better commit message could have looked like:
According to the Intel manuals, TMR is only modified upon "acceptance
of an interrupt into the IRR". Currently, this is not what KVM does;
any IOAPIC scan will modify the TMR.
The TMR is used to track whether an EOI message needs to be sent to
the IOAPIC. In KVM, this means that we need to add the vector to the
EOI exit bitmap, and in fact the next patch will use the TMR exactly
for this purpose. However, if we change the TMR value for an active
interrupt we risk missing an EOI, similar to the scenario fixed by
commit 0f6c0a740b7d (KVM: x86: always exit on EOIs for interrupts listed
in the IOAPIC redir table, 2014-07-30).
This patch ensures that the TMR does not change between acceptance of
an interrupt into the IRR and the corresponding EOI cycle; to do this,
we mix values from the old TMR (where ISR|IRR=1) and from the IOAPIC's
redirection table (where ISR|IRR=0 in the LAPIC).
We still deviate from the spec by setting a value for the TMR even
when the corresponding bit in IRR|ISR is 0, but that's mostly invisible
to guests.
Paolo
> ---
> arch/x86/kvm/lapic.c | 19 +++++++++++++++++--
> arch/x86/kvm/x86.c | 2 +-
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 08e8a89..8c1162d 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -518,10 +518,25 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
> void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
> {
> struct kvm_lapic *apic = vcpu->arch.apic;
> + u32 irr;
> + u32 isr;
> + u32 old_tmr, new_tmr;
> int i;
>
> - for (i = 0; i < 8; i++)
> - apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);
> + /*
> + * The updated tmr value comes from level-triggerd interrupts that
> + * have already been delieverd to lapic and new coming ones which
> + * are pending in ioapic. According to the x86 spec, tmr is valid
> + * when irr or isr is set.
> + */
> + for (i = 0; i < 8; i++) {
> + irr = kvm_apic_get_reg(apic, APIC_IRR + 0x10 * i);
> + isr = kvm_apic_get_reg(apic, APIC_ISR + 0x10 * i);
> + old_tmr = kvm_apic_get_reg(apic, APIC_TMR + 0x10 * i);
> + new_tmr = (~(irr | isr) & tmr[i])
> + | ((irr | isr) & old_tmr);
> + apic_set_reg(apic, APIC_TMR + 0x10 * i, new_tmr);
> + }
> }
>
> static void apic_update_ppr(struct kvm_lapic *apic)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5f5edb6..d401684 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5991,8 +5991,8 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
> memset(tmr, 0, 32);
>
> kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
> - kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap);
> kvm_apic_update_tmr(vcpu, tmr);
> + kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap);
> }
>
> /*
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
