| Message ID | 1407477745-15743-1-git-send-email-yang.z.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2014/8/8 14:02, Yang Zhang wrote: > From: Yang Zhang <yang.z.zhang@Intel.com> > > Guest may mask the IOAPIC entry before issue EOI. In such case, > EOI will not be intercepted by hypervisor due to the corrensponding > bit in eoi exit bitmap is not setting. > > The solution is to check ISR + TMR to construct the EOI exit bitmap. > > This patch is a better fixing for the issue that commit "0f6c0a740b" > tries to solve. > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > --- > arch/x86/kvm/lapic.c | 22 ++++++++++++++++++++++ > arch/x86/kvm/lapic.h | 2 ++ > arch/x86/kvm/x86.c | 9 +++++++++ > virt/kvm/ioapic.c | 7 ++++--- > 4 files changed, 37 insertions(+), 3 deletions(-) > > hi, alex > > This patch is not tested since i don't have the environment to do it. Can you > help to test it? thanks. > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 08e8a89..d2f9a6e 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -71,6 +71,11 @@ > #define VEC_POS(v) ((v) & (32 - 1)) > #define REG_POS(v) (((v) >> 5) << 4) > > +static inline u32 apic_read_reg(struct kvm_lapic *apic, int reg_off) > +{ > + return *((u32 *) (apic->regs + reg_off)); > +} > + I think we already define the same in the arch/x86/kvm/lapic.h file, static inline u32 kvm_apic_get_reg(struct kvm_lapic *apic, int reg_off) { return *((u32 *) (apic->regs + reg_off)); } Thanks Tiejun > static inline void apic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val) > { > *((u32 *) (apic->regs + reg_off)) = val; > @@ -515,6 +520,23 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu) > __clear_bit(KVM_APIC_PV_EOI_PENDING, &vcpu->arch.apic_attention); > } > > +void kvm_apic_zap_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, > + u32 *tmr) > +{ > + u32 i, reg_off, intr_in_service[8]; > + struct kvm_lapic *apic = vcpu->arch.apic; > + > + for (i = 0; i < 8; i++) { > + reg_off = 0x10 * i; > + intr_in_service[i] = apic_read_reg(apic, APIC_ISR + reg_off) & > + apic_read_reg(apic, APIC_TMR + reg_off); > + if (intr_in_service[i]) { > + *((u32 *)eoi_exit_bitmap + i) |= intr_in_service[i]; > + tmr[i] |= intr_in_service[i]; > + } > + } > +} > + > void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr) > { > struct kvm_lapic *apic = vcpu->arch.apic; > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 6a11845..4ee3d70 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -53,6 +53,8 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value); > u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu); > void kvm_apic_set_version(struct kvm_vcpu *vcpu); > > +void kvm_apic_zap_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, > + u32 *tmr); > void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr); > void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir); > int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 204422d..755b556 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6005,6 +6005,15 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) > memset(tmr, 0, 32); > > kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr); > + /* > + * Guest may mask the IOAPIC entry before issue EOI. In such case, > + * EOI will not be intercepted by hypervisor due to the corrensponding > + * bit in eoi exit bitmap is not setting. > + * > + * The solution is to check ISR + TMR to construct the EOI exit bitmap. > + */ > + kvm_apic_zap_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr); > + > kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap); > kvm_apic_update_tmr(vcpu, tmr); > } > diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c > index e8ce34c..2458a1d 100644 > --- a/virt/kvm/ioapic.c > +++ b/virt/kvm/ioapic.c > @@ -254,9 +254,10 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, > spin_lock(&ioapic->lock); > for (index = 0; index < IOAPIC_NUM_PINS; index++) { > e = &ioapic->redirtbl[index]; > - if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG || > - kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index) || > - index == RTC_GSI) { > + if (!e->fields.mask && > + (e->fields.trig_mode == IOAPIC_LEVEL_TRIG || > + kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, > + index) || index == RTC_GSI)) { > if (kvm_apic_match_dest(vcpu, NULL, 0, > e->fields.dest_id, e->fields.dest_mode)) { > __set_bit(e->fields.vector, > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2014-08-08 at 14:02 +0800, Yang Zhang wrote: > From: Yang Zhang <yang.z.zhang@Intel.com> > > Guest may mask the IOAPIC entry before issue EOI. In such case, > EOI will not be intercepted by hypervisor due to the corrensponding > bit in eoi exit bitmap is not setting. > > The solution is to check ISR + TMR to construct the EOI exit bitmap. > > This patch is a better fixing for the issue that commit "0f6c0a740b" > tries to solve. > > Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com> > --- > arch/x86/kvm/lapic.c | 22 ++++++++++++++++++++++ > arch/x86/kvm/lapic.h | 2 ++ > arch/x86/kvm/x86.c | 9 +++++++++ > virt/kvm/ioapic.c | 7 ++++--- > 4 files changed, 37 insertions(+), 3 deletions(-) > > hi, alex > > This patch is not tested since i don't have the environment to do it. Can you > help to test it? thanks. Yes, this also fixes the problem. > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 08e8a89..d2f9a6e 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c ... > @@ -515,6 +520,23 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu) > __clear_bit(KVM_APIC_PV_EOI_PENDING, &vcpu->arch.apic_attention); > } > > +void kvm_apic_zap_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, > + u32 *tmr) > +{ > + u32 i, reg_off, intr_in_service[8]; There's no reason for intr_in_service to be an array here. Thanks, Alex > + struct kvm_lapic *apic = vcpu->arch.apic; > + > + for (i = 0; i < 8; i++) { > + reg_off = 0x10 * i; > + intr_in_service[i] = apic_read_reg(apic, APIC_ISR + reg_off) & > + apic_read_reg(apic, APIC_TMR + reg_off); > + if (intr_in_service[i]) { > + *((u32 *)eoi_exit_bitmap + i) |= intr_in_service[i]; > + tmr[i] |= intr_in_service[i]; > + } > + } > +} > + -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 08e8a89..d2f9a6e 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -71,6 +71,11 @@ #define VEC_POS(v) ((v) & (32 - 1)) #define REG_POS(v) (((v) >> 5) << 4) +static inline u32 apic_read_reg(struct kvm_lapic *apic, int reg_off) +{ + return *((u32 *) (apic->regs + reg_off)); +} + static inline void apic_set_reg(struct kvm_lapic *apic, int reg_off, u32 val) { *((u32 *) (apic->regs + reg_off)) = val; @@ -515,6 +520,23 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu) __clear_bit(KVM_APIC_PV_EOI_PENDING, &vcpu->arch.apic_attention); } +void kvm_apic_zap_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, + u32 *tmr) +{ + u32 i, reg_off, intr_in_service[8]; + struct kvm_lapic *apic = vcpu->arch.apic; + + for (i = 0; i < 8; i++) { + reg_off = 0x10 * i; + intr_in_service[i] = apic_read_reg(apic, APIC_ISR + reg_off) & + apic_read_reg(apic, APIC_TMR + reg_off); + if (intr_in_service[i]) { + *((u32 *)eoi_exit_bitmap + i) |= intr_in_service[i]; + tmr[i] |= intr_in_service[i]; + } + } +} + void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr) { struct kvm_lapic *apic = vcpu->arch.apic; diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 6a11845..4ee3d70 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -53,6 +53,8 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value); u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu); void kvm_apic_set_version(struct kvm_vcpu *vcpu); +void kvm_apic_zap_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, + u32 *tmr); void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr); void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir); int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 204422d..755b556 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6005,6 +6005,15 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu) memset(tmr, 0, 32); kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr); + /* + * Guest may mask the IOAPIC entry before issue EOI. In such case, + * EOI will not be intercepted by hypervisor due to the corrensponding + * bit in eoi exit bitmap is not setting. + * + * The solution is to check ISR + TMR to construct the EOI exit bitmap. + */ + kvm_apic_zap_eoi_exitmap(vcpu, eoi_exit_bitmap, tmr); + kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap); kvm_apic_update_tmr(vcpu, tmr); } diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index e8ce34c..2458a1d 100644 --- a/virt/kvm/ioapic.c +++ b/virt/kvm/ioapic.c @@ -254,9 +254,10 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap, spin_lock(&ioapic->lock); for (index = 0; index < IOAPIC_NUM_PINS; index++) { e = &ioapic->redirtbl[index]; - if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG || - kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index) || - index == RTC_GSI) { + if (!e->fields.mask && + (e->fields.trig_mode == IOAPIC_LEVEL_TRIG || + kvm_irq_has_notifier(ioapic->kvm, KVM_IRQCHIP_IOAPIC, + index) || index == RTC_GSI)) { if (kvm_apic_match_dest(vcpu, NULL, 0, e->fields.dest_id, e->fields.dest_mode)) { __set_bit(e->fields.vector,