| Message ID | 20240124204909.105952-2-oliver.upton@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Improvements to GICv3 LPI injection | expand |
Hi Oliver,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Oliver-Upton/KVM-arm64-vgic-Store-LPIs-in-an-xarray/20240125-045255
base: 6613476e225e090cc9aad49be7fa504e290dd33d
patch link: https://lore.kernel.org/r/20240124204909.105952-2-oliver.upton%40linux.dev
patch subject: [PATCH 01/15] KVM: arm64: vgic: Store LPIs in an xarray
config: arm64-randconfig-r081-20240129 (https://download.01.org/0day-ci/archive/20240204/202402041412.mZlOxFFw-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 4a39d08908942b2d415db405844cbe4af73e75d4)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202402041412.mZlOxFFw-lkp@intel.com/
New smatch warnings:
arch/arm64/kvm/vgic/vgic-its.c:128 vgic_add_lpi() warn: inconsistent returns '&dist->lpi_list_lock'.
arch/arm64/kvm/vgic/vgic-its.c:128 vgic_add_lpi() warn: inconsistent returns 'flags'.
Old smatch warnings:
arch/arm64/kvm/vgic/vgic-its.c:324 update_lpi_config() warn: inconsistent returns '&irq->irq_lock'.
arch/arm64/kvm/vgic/vgic-its.c:324 update_lpi_config() warn: inconsistent returns 'flags'.
vim +128 arch/arm64/kvm/vgic/vgic-its.c
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 39 static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid,
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 40 struct kvm_vcpu *vcpu)
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 41 {
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 42 struct vgic_dist *dist = &kvm->arch.vgic;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 43 struct vgic_irq *irq = vgic_get_irq(kvm, NULL, intid), *oldirq;
388d4359680b56 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2018-05-11 44 unsigned long flags;
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 45 int ret;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 46
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 47 /* In this case there is no put, since we keep the reference. */
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 48 if (irq)
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 49 return irq;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 50
3ef231670b9e90 arch/arm64/kvm/vgic/vgic-its.c Jia He 2021-09-07 51 irq = kzalloc(sizeof(struct vgic_irq), GFP_KERNEL_ACCOUNT);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 52 if (!irq)
99e5e886a0a59d virt/kvm/arm/vgic/vgic-its.c Christoffer Dall 2016-08-01 53 return ERR_PTR(-ENOMEM);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 54
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 55 ret = xa_reserve_irq(&dist->lpi_xa, intid, GFP_KERNEL_ACCOUNT);
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 56 if (ret) {
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 57 kfree(irq);
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 58 return ERR_PTR(ret);
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 59 }
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 60
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 61 INIT_LIST_HEAD(&irq->lpi_list);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 62 INIT_LIST_HEAD(&irq->ap_list);
8fa3adb8c6beee virt/kvm/arm/vgic/vgic-its.c Julien Thierry 2019-01-07 63 raw_spin_lock_init(&irq->irq_lock);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 64
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 65 irq->config = VGIC_CONFIG_EDGE;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 66 kref_init(&irq->refcount);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 67 irq->intid = intid;
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 68 irq->target_vcpu = vcpu;
8df3c8f33f46ad virt/kvm/arm/vgic/vgic-its.c Christoffer Dall 2018-07-16 69 irq->group = 1;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 70
fc3bc475231e12 virt/kvm/arm/vgic/vgic-its.c Julien Thierry 2019-01-07 71 raw_spin_lock_irqsave(&dist->lpi_list_lock, flags);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 72
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 73 /*
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 74 * There could be a race with another vgic_add_lpi(), so we need to
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 75 * check that we don't add a second list entry with the same LPI.
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 76 */
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 77 list_for_each_entry(oldirq, &dist->lpi_list_head, lpi_list) {
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 78 if (oldirq->intid != intid)
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 79 continue;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 80
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 81 /* Someone was faster with adding this LPI, lets use that. */
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 82 kfree(irq);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 83 irq = oldirq;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 84
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 85 /*
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 86 * This increases the refcount, the caller is expected to
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 87 * call vgic_put_irq() on the returned pointer once it's
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 88 * finished with the IRQ.
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 89 */
d97594e6bc1b4a virt/kvm/arm/vgic/vgic-its.c Marc Zyngier 2016-07-17 90 vgic_get_irq_kref(irq);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 91
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 92 goto out_unlock;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 93 }
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 94
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 95 ret = xa_err(xa_store(&dist->lpi_xa, intid, irq, 0));
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 96 if (ret) {
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 97 xa_release(&dist->lpi_xa, intid);
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 98 kfree(irq);
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 99 return ERR_PTR(ret);
should be goto out_unlock or something
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 100 }
3e55a25b7db23f arch/arm64/kvm/vgic/vgic-its.c Oliver Upton 2024-01-24 101
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 102 list_add_tail(&irq->lpi_list, &dist->lpi_list_head);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 103 dist->lpi_list_count++;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 104
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 105 out_unlock:
fc3bc475231e12 virt/kvm/arm/vgic/vgic-its.c Julien Thierry 2019-01-07 106 raw_spin_unlock_irqrestore(&dist->lpi_list_lock, flags);
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 107
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 108 /*
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 109 * We "cache" the configuration table entries in our struct vgic_irq's.
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 110 * However we only have those structs for mapped IRQs, so we read in
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 111 * the respective config data from memory here upon mapping the LPI.
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 112 *
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 113 * Should any of these fail, behave as if we couldn't create the LPI
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 114 * by dropping the refcount and returning the error.
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 115 */
6ce18e3a5f3308 virt/kvm/arm/vgic/vgic-its.c Marc Zyngier 2017-10-27 116 ret = update_lpi_config(kvm, irq, NULL, false);
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 117 if (ret) {
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 118 vgic_put_irq(kvm, irq);
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 119 return ERR_PTR(ret);
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 120 }
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 121
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 122 ret = vgic_v3_lpi_sync_pending_status(kvm, irq);
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 123 if (ret) {
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 124 vgic_put_irq(kvm, irq);
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 125 return ERR_PTR(ret);
57bdb436ce869a virt/kvm/arm/vgic/vgic-its.c Zenghui Yu 2020-04-14 126 }
06bd5359549d7a virt/kvm/arm/vgic/vgic-its.c Eric Auger 2017-05-04 127
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 @128 return irq;
df9f58fbea9bc6 virt/kvm/arm/vgic/vgic-its.c Andre Przywara 2016-07-15 129 }
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c index e949e1d0fd9f..411719053107 100644 --- a/arch/arm64/kvm/vgic/vgic-init.c +++ b/arch/arm64/kvm/vgic/vgic-init.c @@ -56,6 +56,7 @@ void kvm_vgic_early_init(struct kvm *kvm) INIT_LIST_HEAD(&dist->lpi_list_head); INIT_LIST_HEAD(&dist->lpi_translation_cache); raw_spin_lock_init(&dist->lpi_list_lock); + xa_init_flags(&dist->lpi_xa, XA_FLAGS_LOCK_IRQ); } /* CREATION */ @@ -366,6 +367,8 @@ static void kvm_vgic_dist_destroy(struct kvm *kvm) if (vgic_supports_direct_msis(kvm)) vgic_v4_teardown(kvm); + + xa_destroy(&dist->lpi_xa); } static void __kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu) diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c index e2764d0ffa9f..f152d670113f 100644 --- a/arch/arm64/kvm/vgic/vgic-its.c +++ b/arch/arm64/kvm/vgic/vgic-its.c @@ -52,6 +52,12 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid, if (!irq) return ERR_PTR(-ENOMEM); + ret = xa_reserve_irq(&dist->lpi_xa, intid, GFP_KERNEL_ACCOUNT); + if (ret) { + kfree(irq); + return ERR_PTR(ret); + } + INIT_LIST_HEAD(&irq->lpi_list); INIT_LIST_HEAD(&irq->ap_list); raw_spin_lock_init(&irq->irq_lock); @@ -86,6 +92,13 @@ static struct vgic_irq *vgic_add_lpi(struct kvm *kvm, u32 intid, goto out_unlock; } + ret = xa_err(xa_store(&dist->lpi_xa, intid, irq, 0)); + if (ret) { + xa_release(&dist->lpi_xa, intid); + kfree(irq); + return ERR_PTR(ret); + } + list_add_tail(&irq->lpi_list, &dist->lpi_list_head); dist->lpi_list_count++; diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c index db2a95762b1b..c126014f8395 100644 --- a/arch/arm64/kvm/vgic/vgic.c +++ b/arch/arm64/kvm/vgic/vgic.c @@ -131,6 +131,7 @@ void __vgic_put_lpi_locked(struct kvm *kvm, struct vgic_irq *irq) return; list_del(&irq->lpi_list); + xa_erase(&dist->lpi_xa, irq->intid); dist->lpi_list_count--; kfree(irq); diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h index 8cc38e836f54..795b35656b54 100644 --- a/include/kvm/arm_vgic.h +++ b/include/kvm/arm_vgic.h @@ -13,6 +13,7 @@ #include <linux/spinlock.h> #include <linux/static_key.h> #include <linux/types.h> +#include <linux/xarray.h> #include <kvm/iodev.h> #include <linux/list.h> #include <linux/jump_label.h> @@ -275,6 +276,7 @@ struct vgic_dist { /* Protects the lpi_list and the count value below. */ raw_spinlock_t lpi_list_lock; + struct xarray lpi_xa; struct list_head lpi_list_head; int lpi_list_count;
Using a linked-list for LPIs is less than ideal as it of course requires iterative searches to find a particular entry. An xarray is a better data structure for this use case, as it provides faster searches and can still handle a potentially sparse range of INTID allocations. Start by storing LPIs in an xarray, punting usage of the xarray to a subsequent change. Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- arch/arm64/kvm/vgic/vgic-init.c | 3 +++ arch/arm64/kvm/vgic/vgic-its.c | 13 +++++++++++++ arch/arm64/kvm/vgic/vgic.c | 1 + include/kvm/arm_vgic.h | 2 ++ 4 files changed, 19 insertions(+)