lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <A9667DDFB95DB7438FA9D7D576C3D87E0AD5A4E1@SHSMSX104.ccr.corp.intel.com>
Date:	Thu, 30 Jul 2015 23:26:28 +0000
From:	"Zhang, Yang Z" <yang.z.zhang@...el.com>
To:	Paolo Bonzini <pbonzini@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>
CC:	"alex.williamson@...hat.com" <alex.williamson@...hat.com>,
	"srutherford@...el.com" <srutherford@...el.com>
Subject: RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

Paolo Bonzini wrote on 2015-07-29:
> Do not compute TMR in advance.  Instead, set the TMR just before the
> interrupt is accepted into the IRR.  This limits the coupling between
> IOAPIC and LAPIC.
> 

Uh.., it back to original way which is wrong. You cannot modify the apic page(here is the TMR reg) directly when the corresponding VMCS may be used at same time.


> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>  arch/x86/kvm/ioapic.c |  9 ++-------
>  arch/x86/kvm/ioapic.h |  3 +--
>  arch/x86/kvm/lapic.c  | 19 ++++++++++---------
>  arch/x86/kvm/lapic.h  |  1 -
>  arch/x86/kvm/x86.c    |  5 +----
>  5 files changed, 14 insertions(+), 23 deletions(-)
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 856f79105bb5..eaf4ec38d980 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -246,8 +246,7 @@ static void update_handled_vectors(struct kvm_ioapic
> *ioapic)
>  	smp_wmb();
>  }
> -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> -			u32 *tmr)
> +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
>  {
>  	struct kvm_ioapic *ioapic = vcpu->kvm->arch.vioapic;
>  	union kvm_ioapic_redirect_entry *e;
> @@ -260,13 +259,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu,
> u64 *eoi_exit_bitmap,
>  		    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)) {
> +				e->fields.dest_id, e->fields.dest_mode))
>  				__set_bit(e->fields.vector,
>  					(unsigned long *)eoi_exit_bitmap);
> -				if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG)
> -					__set_bit(e->fields.vector, -						(unsigned long *)tmr); -			}
>  		}
>  	}
>  	spin_unlock(&ioapic->lock);
> diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
> index ca0b0b4e6256..3dbd0e2aac4e 100644
> --- a/arch/x86/kvm/ioapic.h
> +++ b/arch/x86/kvm/ioapic.h
> @@ -120,7 +120,6 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
> kvm_lapic *src,
>  		struct kvm_lapic_irq *irq, unsigned long *dest_map);
>  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
> -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
> -			u32 *tmr);
> +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
> 
>  #endif
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 2a5ca97c263b..9be64c77d6db 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -551,15 +551,6 @@ static void pv_eoi_clr_pending(struct kvm_vcpu
> *vcpu)
>  	__clear_bit(KVM_APIC_PV_EOI_PENDING, &vcpu->arch.apic_attention);
>  }
> -void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
> -{
> -	struct kvm_lapic *apic = vcpu->arch.apic;
> -	int i;
> -
> -	for (i = 0; i < 8; i++)
> -		apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);
> -}
> -
>  static void apic_update_ppr(struct kvm_lapic *apic)
>  {
>  	u32 tpr, isrv, ppr, old_ppr;
> @@ -781,6 +772,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int
> delivery_mode,
>  	case APIC_DM_LOWEST:
>  		vcpu->arch.apic_arb_prio++;
>  	case APIC_DM_FIXED:
> +		if (unlikely(trig_mode && !level))
> +			break;
> +
>  		/* FIXME add logic for vcpu on reset */
>  		if (unlikely(!apic_enabled(apic)))
>  			break;
> @@ -790,6 +784,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
> int delivery_mode,
>  		if (dest_map)
>  			__set_bit(vcpu->vcpu_id, dest_map);
> +		if (apic_test_vector(vector, apic->regs + APIC_TMR) != !!trig_mode) {
> +			if (trig_mode)
> +				apic_set_vector(vector, apic->regs + APIC_TMR);
> +			else
> +				apic_clear_vector(vector, apic->regs + APIC_TMR);
> +		}
> +
>  		if (kvm_x86_ops->deliver_posted_interrupt)
>  			kvm_x86_ops->deliver_posted_interrupt(vcpu, vector);
>  		else {
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 764037991d26..eb46d6bcaa75 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -57,7 +57,6 @@ 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_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
>  void __kvm_apic_update_irr(u32 *pir, void *regs);
>  void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
>  int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 23e47a0b054b..48dc954542db 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6149,17 +6149,14 @@ static void process_smi(struct kvm_vcpu *vcpu)
>  static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
>  {
>  	u64 eoi_exit_bitmap[4];
> -	u32 tmr[8];
> 
>  	if (!kvm_apic_hw_enabled(vcpu->arch.apic))
>  		return;
>  
>  	memset(eoi_exit_bitmap, 0, 32);
> -	memset(tmr, 0, 32);
> 
> -	kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap, tmr);
> +	kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap);
>  	kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap);
>  -	kvm_apic_update_tmr(vcpu, tmr); }
>  
>  static void kvm_vcpu_flush_tlb(struct kvm_vcpu *vcpu)


Best regards,
Yang


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ