[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <70eb5830-b7a4-4b8d-990d-7423b23192e5@redhat.com>
Date: Tue, 8 Apr 2025 18:56:21 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>, Joerg Roedel <joro@...tes.org>,
David Woodhouse <dwmw2@...radead.org>, Lu Baolu <baolu.lu@...ux.intel.com>
Cc: kvm@...r.kernel.org, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
Maxim Levitsky <mlevitsk@...hat.com>,
Joao Martins <joao.m.martins@...cle.com>, David Matlack <dmatlack@...gle.com>
Subject: Re: [PATCH 29/67] KVM: SVM: Stop walking list of routing table
entries when updating IRTE
On 4/4/25 21:38, Sean Christopherson wrote:
> Now that KVM SVM simply uses the provided routing entry, stop walking the
> routing table to find that entry. KVM, via setup_routing_entry() and
> sanity checked by kvm_get_msi_route(), disallows having a GSI configured
> to trigger multiple MSIs.
I would squash this with the previous patch. It's not large when shown
with -b, and the coexistence of "e" and "new" after patch 28 is weird.
Paolo
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
> arch/x86/kvm/svm/avic.c | 106 ++++++++++++++++------------------------
> 1 file changed, 43 insertions(+), 63 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index eb6017b01c5f..685a7b01194b 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -852,10 +852,10 @@ int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
> unsigned int host_irq, uint32_t guest_irq,
> struct kvm_kernel_irq_routing_entry *new)
> {
> - struct kvm_kernel_irq_routing_entry *e;
> - struct kvm_irq_routing_table *irq_rt;
> bool enable_remapped_mode = true;
> - int idx, ret = 0;
> + struct vcpu_data vcpu_info;
> + struct vcpu_svm *svm = NULL;
> + int ret = 0;
>
> if (!kvm_arch_has_assigned_device(kvm) || !kvm_arch_has_irq_bypass())
> return 0;
> @@ -869,70 +869,51 @@ int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
> pr_debug("SVM: %s: host_irq=%#x, guest_irq=%#x, set=%#x\n",
> __func__, host_irq, guest_irq, !!new);
>
> - idx = srcu_read_lock(&kvm->irq_srcu);
> - irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
> + /**
> + * Here, we setup with legacy mode in the following cases:
> + * 1. When cannot target interrupt to a specific vcpu.
> + * 2. Unsetting posted interrupt.
> + * 3. APIC virtualization is disabled for the vcpu.
> + * 4. IRQ has incompatible delivery mode (SMI, INIT, etc)
> + */
> + if (new && new->type == KVM_IRQ_ROUTING_MSI &&
> + !get_pi_vcpu_info(kvm, new, &vcpu_info, &svm) &&
> + kvm_vcpu_apicv_active(&svm->vcpu)) {
> + struct amd_iommu_pi_data pi;
>
> - if (guest_irq >= irq_rt->nr_rt_entries ||
> - hlist_empty(&irq_rt->map[guest_irq])) {
> - pr_warn_once("no route for guest_irq %u/%u (broken user space?)\n",
> - guest_irq, irq_rt->nr_rt_entries);
> - goto out;
> - }
> + enable_remapped_mode = false;
>
> - hlist_for_each_entry(e, &irq_rt->map[guest_irq], link) {
> - struct vcpu_data vcpu_info;
> - struct vcpu_svm *svm = NULL;
> -
> - if (e->type != KVM_IRQ_ROUTING_MSI)
> - continue;
> -
> - WARN_ON_ONCE(new && memcmp(e, new, sizeof(*new)));
> + /*
> + * Try to enable guest_mode in IRTE. Note, the address
> + * of the vCPU's AVIC backing page is passed to the
> + * IOMMU via vcpu_info->pi_desc_addr.
> + */
> + pi.ga_tag = AVIC_GATAG(to_kvm_svm(kvm)->avic_vm_id,
> + svm->vcpu.vcpu_id);
> + pi.is_guest_mode = true;
> + pi.vcpu_data = &vcpu_info;
> + ret = irq_set_vcpu_affinity(host_irq, &pi);
>
> /**
> - * Here, we setup with legacy mode in the following cases:
> - * 1. When cannot target interrupt to a specific vcpu.
> - * 2. Unsetting posted interrupt.
> - * 3. APIC virtualization is disabled for the vcpu.
> - * 4. IRQ has incompatible delivery mode (SMI, INIT, etc)
> + * Here, we successfully setting up vcpu affinity in
> + * IOMMU guest mode. Now, we need to store the posted
> + * interrupt information in a per-vcpu ir_list so that
> + * we can reference to them directly when we update vcpu
> + * scheduling information in IOMMU irte.
> */
> - if (new && !get_pi_vcpu_info(kvm, new, &vcpu_info, &svm) &&
> - kvm_vcpu_apicv_active(&svm->vcpu)) {
> - struct amd_iommu_pi_data pi;
> -
> - enable_remapped_mode = false;
> -
> - /*
> - * Try to enable guest_mode in IRTE. Note, the address
> - * of the vCPU's AVIC backing page is passed to the
> - * IOMMU via vcpu_info->pi_desc_addr.
> - */
> - pi.ga_tag = AVIC_GATAG(to_kvm_svm(kvm)->avic_vm_id,
> - svm->vcpu.vcpu_id);
> - pi.is_guest_mode = true;
> - pi.vcpu_data = &vcpu_info;
> - ret = irq_set_vcpu_affinity(host_irq, &pi);
> -
> - /**
> - * Here, we successfully setting up vcpu affinity in
> - * IOMMU guest mode. Now, we need to store the posted
> - * interrupt information in a per-vcpu ir_list so that
> - * we can reference to them directly when we update vcpu
> - * scheduling information in IOMMU irte.
> - */
> - if (!ret && pi.is_guest_mode)
> - svm_ir_list_add(svm, irqfd, &pi);
> - }
> -
> - if (!ret && svm) {
> - trace_kvm_pi_irte_update(host_irq, svm->vcpu.vcpu_id,
> - e->gsi, vcpu_info.vector,
> - vcpu_info.pi_desc_addr, !!new);
> - }
> -
> - if (ret < 0) {
> - pr_err("%s: failed to update PI IRTE\n", __func__);
> - goto out;
> - }
> + if (!ret)
> + ret = svm_ir_list_add(svm, irqfd, &pi);
> + }
> +
> + if (!ret && svm) {
> + trace_kvm_pi_irte_update(host_irq, svm->vcpu.vcpu_id,
> + guest_irq, vcpu_info.vector,
> + vcpu_info.pi_desc_addr, !!new);
> + }
> +
> + if (ret < 0) {
> + pr_err("%s: failed to update PI IRTE\n", __func__);
> + goto out;
> }
>
> if (enable_remapped_mode)
> @@ -940,7 +921,6 @@ int avic_pi_update_irte(struct kvm_kernel_irqfd *irqfd, struct kvm *kvm,
> else
> ret = 0;
> out:
> - srcu_read_unlock(&kvm->irq_srcu, idx);
> return ret;
> }
>
Powered by blists - more mailing lists