| 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: <20150902194246.GV10991@cbox>
Date: Wed, 2 Sep 2015 21:42:46 +0200
From: Christoffer Dall <christoffer.dall@...aro.org>
To: Eric Auger <eric.auger@...aro.org>
Cc: eric.auger@...com, linux-arm-kernel@...ts.infradead.org,
kvmarm@...ts.cs.columbia.edu, kvm@...r.kernel.org,
marc.zyngier@....com, alex.williamson@...hat.com,
feng.wu@...el.com, linux-kernel@...r.kernel.org,
patches@...aro.org, pbonzini@...hat.com
Subject: Re: [PATCH v3 07/10] KVM: arm/arm64: vgic: Allow HW interrupts for
non-shared devices
On Mon, Aug 10, 2015 at 03:21:01PM +0200, Eric Auger wrote:
> From: Marc Zyngier <marc.zyngier@....com>
>
> So far, the only use of the HW interrupt facility was the timer,
> implying that the active state is context-switched for each vcpu,
> as the device is is shared across all vcpus.
>
> This does not work for a device that has been assigned to a VM,
> as the guest is entierely in control of that device (the HW is
> not shared). In that case, it makes sense to bypass the whole
> active state switching.
>
> Also the VGIC state machine is adapted to support those assigned
> (non shared) HW IRQs:
> - nly can be sampled when it is pending
> - when queueing the IRQ (programming the LR), the pending state is
> removed as for edge sensitive IRQs
> - queued state is not modelled. Level state is not modelled
> - its injection always is valid since steming from the HW.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
> Signed-off-by: Eric Auger <eric.auger@...aro.org>
>
> ---
>
> - a mix of
> [PATCH v4 11/11] KVM: arm/arm64: vgic: Allow HW interrupts for
> non-shared devices
> [RFC v2 2/4] KVM: arm: vgic: fix state machine for forwarded IRQ
> ---
> include/kvm/arm_vgic.h | 6 +++--
> virt/kvm/arm/arch_timer.c | 3 ++-
> virt/kvm/arm/vgic.c | 58 +++++++++++++++++++++++++++++++++++------------
> 3 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index d901f1a..7ef9ce0 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -163,7 +163,8 @@ struct irq_phys_map {
> u32 virt_irq;
> u32 phys_irq;
> u32 irq;
> - bool active;
> + bool shared;
> + bool active; /* Only valid if shared */
> };
>
> struct irq_phys_map_entry {
> @@ -356,7 +357,8 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
> int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
> int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
> struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
> - int virt_irq, int irq);
> + int virt_irq, int irq,
> + bool shared);
> int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, struct irq_phys_map *map);
> bool kvm_vgic_get_phys_irq_active(struct irq_phys_map *map);
> void kvm_vgic_set_phys_irq_active(struct irq_phys_map *map, bool active);
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 76e38d2..db21d8f 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -203,7 +203,8 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
> * Tell the VGIC that the virtual interrupt is tied to a
> * physical interrupt. We do that once per VCPU.
> */
> - map = kvm_vgic_map_phys_irq(vcpu, irq->irq, host_vtimer_irq);
> + map = kvm_vgic_map_phys_irq(vcpu, irq->irq,
> + host_vtimer_irq, true);
> if (WARN_ON(IS_ERR(map)))
> return PTR_ERR(map);
>
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 9eb489a..fbd5ba5 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -400,7 +400,11 @@ void vgic_cpu_irq_clear(struct kvm_vcpu *vcpu, int irq)
>
> static bool vgic_can_sample_irq(struct kvm_vcpu *vcpu, int irq)
> {
> - return !vgic_irq_is_queued(vcpu, irq);
> + struct irq_phys_map *map = vgic_irq_map_search(vcpu, irq);
> + bool shared_hw = map && !map->shared;
why is shared true when map->shared is false?
> +
> + return !vgic_irq_is_queued(vcpu, irq) ||
> + (shared_hw && vgic_dist_irq_is_pending(vcpu, irq));
so for forwarded, non-shared, level-triggered IRQs, we always sample the
line if it's pending? Why?
> }
>
> /**
> @@ -1150,19 +1154,26 @@ static void vgic_queue_irq_to_lr(struct kvm_vcpu *vcpu, int irq,
> * active in the physical world. Otherwise the
> * physical interrupt will fire and the guest will
> * exit before processing the virtual interrupt.
> + *
> + * This is of course only valid for a shared
> + * interrupt. A non shared interrupt should already be
> + * active.
> */
> if (map) {
> - int ret;
> -
> - BUG_ON(!map->active);
> vlr.hwirq = map->phys_irq;
> vlr.state |= LR_HW;
> vlr.state &= ~LR_EOI_INT;
>
> - ret = irq_set_irqchip_state(map->irq,
> - IRQCHIP_STATE_ACTIVE,
> - true);
> - WARN_ON(ret);
> + if (map->shared) {
> + int ret;
> +
> + BUG_ON(!map->active);
> + ret = irq_set_irqchip_state(
> + map->irq,
> + IRQCHIP_STATE_ACTIVE,
> + true);
> + WARN_ON(ret);
> + }
this stuff all needs to be rebased onto my latest timer/active state
rework series.
>
> /*
> * Make sure we're not going to sample this
> @@ -1229,10 +1240,13 @@ bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
>
> static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, int irq)
> {
> + struct irq_phys_map *map = vgic_irq_map_search(vcpu, irq);
> + bool shared_hw = map && !map->shared;
same question as above?
> +
> if (!vgic_can_sample_irq(vcpu, irq))
> return true; /* level interrupt, already queued */
>
> - if (vgic_queue_irq(vcpu, 0, irq)) {
> + if (vgic_queue_irq(vcpu, 0, irq) || shared_hw) {
> if (vgic_irq_is_edge(vcpu, irq)) {
> vgic_dist_irq_clear_pending(vcpu, irq);
> vgic_cpu_irq_clear(vcpu, irq);
> @@ -1411,7 +1425,12 @@ static int vgic_sync_hwirq(struct kvm_vcpu *vcpu, struct vgic_lr vlr)
> return 0;
>
> map = vgic_irq_map_search(vcpu, vlr.irq);
> - BUG_ON(!map || !map->active);
> + BUG_ON(!map);
> +
> + if (!map->shared)
> + return 0;
> +
> + BUG_ON(map->shared && !map->active);
>
> ret = irq_get_irqchip_state(map->irq,
> IRQCHIP_STATE_ACTIVE,
> @@ -1563,6 +1582,7 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
> int edge_triggered, level_triggered;
> int enabled;
> bool ret = true, can_inject = true;
> + bool shared_hw = map && !map->shared;
same
>
> if (irq_num >= min(kvm->arch.vgic.nr_irqs, 1020))
> return -EINVAL;
> @@ -1573,7 +1593,8 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
> edge_triggered = vgic_irq_is_edge(vcpu, irq_num);
> level_triggered = !edge_triggered;
>
> - if (!vgic_validate_injection(vcpu, irq_num, level)) {
> + if (!vgic_validate_injection(vcpu, irq_num, level) &&
> + !shared_hw) {
> ret = false;
> goto out;
> }
> @@ -1742,16 +1763,21 @@ static struct list_head *vgic_get_irq_phys_map_list(struct kvm_vcpu *vcpu,
> * @vcpu: The VCPU pointer
> * @virt_irq: The virtual irq number
> * @irq: The Linux IRQ number
> + * @shared: Indicates if the interrupt has to be context-switched or
> + * if it is private to a VM
> *
> * Establish a mapping between a guest visible irq (@virt_irq) and a
> * Linux irq (@irq). On injection, @virt_irq will be associated with
> * the physical interrupt represented by @irq. This mapping can be
> * established multiple times as long as the parameters are the same.
> + * If @shared is true, the active state of the interrupt will be
> + * context-switched.
> *
> * Returns a valid pointer on success, and an error pointer otherwise
> */
> struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
> - int virt_irq, int irq)
> + int virt_irq, int irq,
> + bool shared)
> {
> struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
> struct list_head *root = vgic_get_irq_phys_map_list(vcpu, virt_irq);
> @@ -1785,7 +1811,8 @@ struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
> if (map) {
> /* Make sure this mapping matches */
> if (map->phys_irq != phys_irq ||
> - map->irq != irq)
> + map->irq != irq ||
> + map->shared != shared)
> map = ERR_PTR(-EINVAL);
>
> /* Found an existing, valid mapping */
> @@ -1796,6 +1823,7 @@ struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
> map->virt_irq = virt_irq;
> map->phys_irq = phys_irq;
> map->irq = irq;
> + map->shared = shared;
>
> list_add_tail_rcu(&entry->entry, root);
>
> @@ -1846,7 +1874,7 @@ static void vgic_free_phys_irq_map_rcu(struct rcu_head *rcu)
> */
> bool kvm_vgic_get_phys_irq_active(struct irq_phys_map *map)
> {
> - BUG_ON(!map);
> + BUG_ON(!map || !map->shared);
> return map->active;
> }
>
> @@ -1858,7 +1886,7 @@ bool kvm_vgic_get_phys_irq_active(struct irq_phys_map *map)
> */
> void kvm_vgic_set_phys_irq_active(struct irq_phys_map *map, bool active)
> {
> - BUG_ON(!map);
> + BUG_ON(!map || !map->shared);
> map->active = active;
> }
>
> --
> 1.9.1
>
--
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