Hello Sean Christopherson,
Commit f3cebc75e742 ("KVM: SVM: Set target pCPU during IRTE update if
target vCPU is running") from Aug 8, 2023 (linux-next), leads to the
following Smatch static checker warning:
arch/x86/kvm/svm/avic.c:841 svm_ir_list_add()
error: we previously assumed 'pi->ir_data' could be null (see line 804)
arch/x86/kvm/svm/avic.c
792 static int svm_ir_list_add(struct vcpu_svm *svm, struct amd_iommu_pi_data *pi)
793 {
794 int ret = 0;
795 unsigned long flags;
796 struct amd_svm_iommu_ir *ir;
797 u64 entry;
798
799 /**
800 * In some cases, the existing irte is updated and re-set,
801 * so we need to check here if it's already been * added
802 * to the ir_list.
803 */
804 if (pi->ir_data && (pi->prev_ga_tag != 0)) {
^^^^^^^^^^^
The old code checks for NULL
805 struct kvm *kvm = svm->vcpu.kvm;
806 u32 vcpu_id = AVIC_GATAG_TO_VCPUID(pi->prev_ga_tag);
807 struct kvm_vcpu *prev_vcpu = kvm_get_vcpu_by_id(kvm, vcpu_id);
808 struct vcpu_svm *prev_svm;
809
810 if (!prev_vcpu) {
811 ret = -EINVAL;
812 goto out;
813 }
814
815 prev_svm = to_svm(prev_vcpu);
816 svm_ir_list_del(prev_svm, pi);
817 }
818
819 /**
820 * Allocating new amd_iommu_pi_data, which will get
821 * add to the per-vcpu ir_list.
822 */
823 ir = kzalloc(sizeof(struct amd_svm_iommu_ir), GFP_KERNEL_ACCOUNT);
824 if (!ir) {
825 ret = -ENOMEM;
826 goto out;
827 }
828 ir->data = pi->ir_data;
829
830 spin_lock_irqsave(&svm->ir_list_lock, flags);
831
832 /*
833 * Update the target pCPU for IOMMU doorbells if the vCPU is running.
834 * If the vCPU is NOT running, i.e. is blocking or scheduled out, KVM
835 * will update the pCPU info when the vCPU awkened and/or scheduled in.
836 * See also avic_vcpu_load().
837 */
838 entry = READ_ONCE(*(svm->avic_physical_id_cache));
839 if (entry & AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK)
840 amd_iommu_update_ga(entry & AVIC_PHYSICAL_ID_ENTRY_HOST_PHYSICAL_ID_MASK,
--> 841 true, pi->ir_data);
^^^^^^^^^^^
The patch adds an unchecked dereference. It could be a false positive if
AVIC_PHYSICAL_ID_ENTRY_IS_RUNNING_MASK implies that ->ir_data is non-NULL. In
that case could you just send an email saying "this is a false positive" and
I'll ignore this warning going forward.
842
843 list_add(&ir->node, &svm->ir_list);
844 spin_unlock_irqrestore(&svm->ir_list_lock, flags);
845 out:
846 return ret;
847 }
regards,
dan carpenter
