| 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
| ||
|
Message-ID: <20150730035543.GE15229@google.com>
Date: Wed, 29 Jul 2015 20:55:43 -0700
From: Steve Rutherford <srutherford@...gle.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
alex.williamson@...hat.com, yang.z.zhang@...el.com,
srutherford@...el.com
Subject: Re: [PATCH 2/2] KVM: x86: store IOAPIC-handled vectors in each VCPU
On Wed, Jul 29, 2015 at 03:37:35PM +0200, Paolo Bonzini wrote:
> We can reuse the algorithm that computes the EOI exit bitmap to figure
> out which vectors are handled by the IOAPIC. The only difference
> between the two is for edge-triggered interrupts other than IRQ8
> that have no notifiers active; however, the IOAPIC does not have to
> do anything special for these interrupts anyway.
>
> This again limits the interactions between the IOAPIC and the LAPIC,
> making it easier to move the former to userspace.
>
> Inspired by a patch from Steve Rutherford.
>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 3 ++-
> arch/x86/kvm/ioapic.c | 18 ++----------------
> arch/x86/kvm/ioapic.h | 8 --------
> arch/x86/kvm/lapic.c | 10 ++++++++--
> arch/x86/kvm/svm.c | 2 +-
> arch/x86/kvm/vmx.c | 3 ++-
> arch/x86/kvm/x86.c | 8 +++-----
> 7 files changed, 18 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2f9e504f9f0c..d0e991ef6ef0 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -383,6 +383,7 @@ struct kvm_vcpu_arch {
> u64 efer;
> u64 apic_base;
> struct kvm_lapic *apic; /* kernel irqchip context */
> + u64 eoi_exit_bitmap[4];
> unsigned long apic_attention;
> int32_t apic_arb_prio;
> int mp_state;
> @@ -808,7 +809,7 @@ struct kvm_x86_ops {
> int (*vm_has_apicv)(struct kvm *kvm);
> void (*hwapic_irr_update)(struct kvm_vcpu *vcpu, int max_irr);
> void (*hwapic_isr_update)(struct kvm *kvm, int isr);
> - void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
> + void (*load_eoi_exitmap)(struct kvm_vcpu *vcpu);
> void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
> void (*set_apic_access_page_addr)(struct kvm_vcpu *vcpu, hpa_t hpa);
> void (*deliver_posted_interrupt)(struct kvm_vcpu *vcpu, int vector);
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index eaf4ec38d980..2dcda0f188ba 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -233,19 +233,6 @@ static void kvm_ioapic_inject_all(struct kvm_ioapic *ioapic, unsigned long irr)
> }
>
>
> -static void update_handled_vectors(struct kvm_ioapic *ioapic)
> -{
> - DECLARE_BITMAP(handled_vectors, 256);
> - int i;
> -
> - memset(handled_vectors, 0, sizeof(handled_vectors));
> - for (i = 0; i < IOAPIC_NUM_PINS; ++i)
> - __set_bit(ioapic->redirtbl[i].fields.vector, handled_vectors);
> - memcpy(ioapic->handled_vectors, handled_vectors,
> - sizeof(handled_vectors));
> - smp_wmb();
> -}
> -
> void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
> {
> struct kvm_ioapic *ioapic = vcpu->kvm->arch.vioapic;
> @@ -310,7 +297,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
> e->bits |= (u32) val;
> e->fields.remote_irr = 0;
> }
> - update_handled_vectors(ioapic);
> mask_after = e->fields.mask;
> if (mask_before != mask_after)
> kvm_fire_mask_notifiers(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index, mask_after);
> @@ -594,7 +580,6 @@ static void kvm_ioapic_reset(struct kvm_ioapic *ioapic)
> ioapic->id = 0;
> memset(ioapic->irq_eoi, 0x00, IOAPIC_NUM_PINS);
> rtc_irq_eoi_tracking_reset(ioapic);
> - update_handled_vectors(ioapic);
> }
>
> static const struct kvm_io_device_ops ioapic_mmio_ops = {
> @@ -623,8 +608,10 @@ int kvm_ioapic_init(struct kvm *kvm)
> if (ret < 0) {
> kvm->arch.vioapic = NULL;
> kfree(ioapic);
> + return ret;
> }
>
> + kvm_vcpu_request_scan_ioapic(kvm);
> return ret;
> }
>
> @@ -661,7 +648,6 @@ int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state)
> memcpy(ioapic, state, sizeof(struct kvm_ioapic_state));
> ioapic->irr = 0;
> ioapic->irr_delivered = 0;
> - update_handled_vectors(ioapic);
> kvm_vcpu_request_scan_ioapic(kvm);
> kvm_ioapic_inject_all(ioapic, state->irr);
> spin_unlock(&ioapic->lock);
> diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
> index 3dbd0e2aac4e..bf36d66a1951 100644
> --- a/arch/x86/kvm/ioapic.h
> +++ b/arch/x86/kvm/ioapic.h
> @@ -73,7 +73,6 @@ struct kvm_ioapic {
> struct kvm *kvm;
> void (*ack_notifier)(void *opaque, int irq);
> spinlock_t lock;
> - DECLARE_BITMAP(handled_vectors, 256);
> struct rtc_status rtc_status;
> struct delayed_work eoi_inject;
> u32 irq_eoi[IOAPIC_NUM_PINS];
> @@ -98,13 +97,6 @@ static inline struct kvm_ioapic *ioapic_irqchip(struct kvm *kvm)
> return kvm->arch.vioapic;
> }
>
> -static inline bool kvm_ioapic_handles_vector(struct kvm *kvm, int vector)
> -{
> - struct kvm_ioapic *ioapic = kvm->arch.vioapic;
> - smp_rmb();
> - return test_bit(vector, ioapic->handled_vectors);
> -}
> -
> void kvm_rtc_eoi_tracking_restore_one(struct kvm_vcpu *vcpu);
> bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
> int short_hand, unsigned int dest, int dest_mode);
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 9be64c77d6db..c158f4667aa3 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -869,14 +869,20 @@ int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2)
> return vcpu1->arch.apic_arb_prio - vcpu2->arch.apic_arb_prio;
> }
>
> +static bool kvm_ioapic_handles_vector(struct kvm_lapic *apic, int vector)
> +{
> + return test_bit(vector, (ulong *)apic->vcpu->arch.eoi_exit_bitmap);
> +}
> +
> static void kvm_ioapic_send_eoi(struct kvm_lapic *apic, int vector)
> {
> - if (kvm_ioapic_handles_vector(apic->vcpu->kvm, vector)) {
> + if (kvm_ioapic_handles_vector(apic, vector)) {
> int trigger_mode;
> if (apic_test_vector(vector, apic->regs + APIC_TMR))
> trigger_mode = IOAPIC_LEVEL_TRIG;
> else
> trigger_mode = IOAPIC_EDGE_TRIG;
> +
Nit: Not sure if you meant to add this.
> kvm_ioapic_update_eoi(apic->vcpu, vector, trigger_mode);
> }
> }
> @@ -1922,7 +1928,7 @@ static void apic_sync_pv_eoi_to_guest(struct kvm_vcpu *vcpu,
> /* Cache not set: could be safe but we don't bother. */
> apic->highest_isr_cache == -1 ||
> /* Need EOI to update ioapic. */
> - kvm_ioapic_handles_vector(vcpu->kvm, apic->highest_isr_cache)) {
> + kvm_ioapic_handles_vector(apic, apic->highest_isr_cache)) {
> /*
> * PV EOI was disabled by apic_sync_pv_eoi_from_guest
> * so we need not do anything here.
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index d1a114d8d22b..676c9eb10174 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3765,7 +3765,7 @@ static int svm_vm_has_apicv(struct kvm *kvm)
> return 0;
> }
>
> -static void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
> +static void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu)
> {
> return;
> }
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 4014a82ae846..7323ea8fba88 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8111,8 +8111,9 @@ static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
> }
> }
>
> -static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
> +static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu)
> {
> + u64 *eoi_exit_bitmap = vcpu->arch.eoi_exit_bitmap;
> if (!vmx_vm_has_apicv(vcpu->kvm))
> return;
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 48dc954542db..4ad7334aa604 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6148,15 +6148,13 @@ static void process_smi(struct kvm_vcpu *vcpu)
>
> static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
> {
> - u64 eoi_exit_bitmap[4];
> -
> if (!kvm_apic_hw_enabled(vcpu->arch.apic))
> return;
>
> - memset(eoi_exit_bitmap, 0, 32);
> + memset(vcpu->arch.eoi_exit_bitmap, 0, 256 / 8);
>
> - kvm_ioapic_scan_entry(vcpu, eoi_exit_bitmap);
> - kvm_x86_ops->load_eoi_exitmap(vcpu, eoi_exit_bitmap);
> + kvm_ioapic_scan_entry(vcpu, vcpu->arch.eoi_exit_bitmap);
Nit: Does scan entry still need to carry around the pointer to the bitmap?
> + kvm_x86_ops->load_eoi_exitmap(vcpu);
> }
>
> static void kvm_vcpu_flush_tlb(struct kvm_vcpu *vcpu)
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks good. Just picked a few nits.
--
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