From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from sg-1-23.ptr.blmpb.com (sg-1-23.ptr.blmpb.com [118.26.132.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5C4DA2874FF for ; Wed, 4 Feb 2026 01:59:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=118.26.132.23 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770170401; cv=none; b=oqDzrvehUagYUIPtQdI3LGetWV6eUz4lTR3yADd5N1h422liwg03ZDlLQ9rjRB6x8BNFblcvO8rGMrfcxRjc/8/7HUh9dRYJ0gQ4G5sfj4Kr99ho5GRndpfNBSOc+t713olM1RxLm54QFR9NPGXdgmLUMhl0Jzs+u8VwlR+oEZ4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770170401; c=relaxed/simple; bh=c0LSkUxanVT2XXAb9ocPR30mzigoIbt2KLY5WYBZn74=; h=In-Reply-To:Content-Type:Date:Message-Id:Mime-Version:References: From:Subject:To:Cc; b=Xr0Mx6vIgaZ5wuM6QDXb3j7TiFUpWcQj3rOftO+fiXuHhUdSmuoYhllYP2+OaiCyt0X7qFl3sq1G7Pzs1odhvRlNNcym/0ItIYYhy0/jtfHkHF/KhlUaF28V6Oc7ZP7NQ0WoLyTHd8TSaZcLngN1lKHOHFLHCzg6q+89MSoIK3g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=riscv-computing.com; spf=pass smtp.mailfrom=riscv-computing.com; dkim=pass (2048-bit key) header.d=riscv-computing.com header.i=@riscv-computing.com header.b=IptItgTk; arc=none smtp.client-ip=118.26.132.23 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=riscv-computing.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=riscv-computing.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=riscv-computing.com header.i=@riscv-computing.com header.b="IptItgTk" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; s=feishu2506131828; d=riscv-computing.com; t=1770170388; h=from:subject:mime-version:from:date:message-id:subject:to:cc: reply-to:content-type:mime-version:in-reply-to:message-id; bh=3FQkGOV+NC5ox+buWoMpx+RVNOee1LQbTe8Icg3qZGQ=; b=IptItgTkCbbttbiLPvv6+8WmBOYzuI0lTB2qKS5ZIL/NIhurOuMJXEHIje0LPLPE/7Vz/J XbFJFr/TAQAxffwx2s17q1UyACcd8DDDFzrk26u7HJCKi4gA5LsKqPrHARAkldJb66549q mZySXB4HtoCZCOOS+lNXPWEMikX+YRmwx7gzHAjSGn3/Y0SmFexEFlCxk93s8LMU19Qzwn nsP6P71H37Ig8AOA4fMSoUm7MNRLV7cRgwE3yuReCD3JpIoEAYAKKokDVERZJhhPiKcAqV EcphXIECdsHDqO9AXKZK97bGPEceHBSDb065LwJySs9us+49f6u0X/vIy/bGxA== X-Lms-Return-Path: In-Reply-To: <87ecn23q6e.ffs@tglx> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Original-From: Yingjun Ni Date: Wed, 4 Feb 2026 09:59:43 +0800 User-Agent: Mozilla Thunderbird Message-Id: <39cbfcdf-db96-4e2a-bcca-ef10298492fd@riscv-computing.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260203080256.9401-2-yingjun.ni@riscv-computing.com> <87ecn23q6e.ffs@tglx> From: "Yingjun Ni" Subject: Re: [PATCH] irqchip/riscv-imsic: Fix irq migration failure issue when cpu hotplug. Received: from [192.168.102.222] ([14.127.203.227]) by smtp.feishu.cn with ESMTPS; Wed, 04 Feb 2026 09:59:45 +0800 To: "Thomas Gleixner" , , , , , Cc: , > On Tue, Feb 03 2026 at 16:02, Yingjun Ni wrote: >> Add a null pointer check for irq_write_msi_msg to fix NULL pointer >> dereference issue when migrating irq. >> >> Modify the return value of imsic_irq_set_affinity to let the subdomain >> PCI-MSIX migrate the irq to a new cpu when cpu hotplug. >> >> Don't set vec->move_next in imsic_vector_move_update when the cpu is >> offline, because it will never be cleared. > You completely fail to explain the actual problem and the root > cause. See > > https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#change= log > >> drivers/irqchip/irq-riscv-imsic-platform.c | 8 ++++++-- >> drivers/irqchip/irq-riscv-imsic-state.c | 5 +++++ >> 2 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/irqchip/irq-riscv-imsic-platform.c b/drivers/irqchi= p/irq-riscv-imsic-platform.c >> index 643c8e459611..131e4f2b5431 100644 >> --- a/drivers/irqchip/irq-riscv-imsic-platform.c >> +++ b/drivers/irqchip/irq-riscv-imsic-platform.c >> @@ -93,9 +93,13 @@ static void imsic_irq_compose_msg(struct irq_data *d,= struct msi_msg *msg) >> static void imsic_msi_update_msg(struct irq_data *d, struct imsic_vect= or *vec) >> { >> struct msi_msg msg =3D { }; >> + struct irq_chip *irq_chip =3D irq_data_get_irq_chip(d); >> + >> + if (!irq_chip->irq_write_msi_msg) >> + return; > I have no idea how this ever worked. The irq_data pointer belongs to the > IMSIC base domain, which definitely does not have a irq_write_msi_msg() > callback and never can have one. > > The write message callback is always implemented by the top most domain, > in this case the PCI/MSI[x] per device domain. > > So this code is simply broken and your NULL pointer check just makes it > differently broken. Sorry, my mistake, the NULL pointer issue has been fixed by commit=20 (c475c0b71314 irqchip/riscv-imsic: Remove redundant irq_data lookups) >> imsic_irq_compose_vector_msg(vec, &msg); >> - irq_data_get_irq_chip(d)->irq_write_msi_msg(d, &msg); >> + irq_chip->irq_write_msi_msg(d, &msg); >> } >> =20 >> static int imsic_irq_set_affinity(struct irq_data *d, const struct cpu= mask *mask_val, >> @@ -173,7 +177,7 @@ static int imsic_irq_set_affinity(struct irq_data *d= , const struct cpumask *mask >> /* Move state of the old vector to the new vector */ >> imsic_vector_move(old_vec, new_vec); >> =20 >> - return IRQ_SET_MASK_OK_DONE; >> + return IRQ_SET_MASK_OK; > Have you actually looked at the consequences of this change? > >> } >> =20 >> static void imsic_irq_force_complete_move(struct irq_data *d) >> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/i= rq-riscv-imsic-state.c >> index b6cebfee9461..cd1bf9516878 100644 >> --- a/drivers/irqchip/irq-riscv-imsic-state.c >> +++ b/drivers/irqchip/irq-riscv-imsic-state.c >> @@ -362,6 +362,10 @@ static bool imsic_vector_move_update(struct imsic_l= ocal_priv *lpriv, >> /* Update enable and move details */ >> enabled =3D READ_ONCE(vec->enable); >> WRITE_ONCE(vec->enable, new_enable); >> + >> + if (!cpu_online(vec->cpu) && is_old_vec) >> + goto out; > This is definitely not correct as this should still cleanup software > state, no? if vec->move_next is not cleared when the cpu is offline, the following=20 issue will occur. cat /proc/interrupts =C2=A0 =C2=A0 CPU0=C2=A0 CPU1=C2=A0 CPU2 CPU3 23:=C2=A0 =C2=A00=C2=A0 =C2=A0 =C2=A0 =C2=A00=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A00=C2=A0 =C2=A0 =C2=A0 =C2=A066=C2=A0 =C2=A0PCI-MSIX-0000:00:01.0 eth0= -rx-0 echo 0 > /sys/bus/cpu/devices/cpu3/online cat /proc/interrupts =C2=A0 =C2=A0 CPU0=C2=A0 CPU1=C2=A0 CPU2 23:=C2=A0 =C2=A00=C2=A0 =C2=A0 =C2=A0 =C2=A00=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A066=C2=A0 =C2=A0PCI-MSIX-0000:00:01.0 eth0-rx-0 echo 0 > /sys/bus/cpu/devices/cpu2/online [=C2=A0 =C2=A035.697380] IRQ23: set affinity failed(-16). [=C2=A0 =C2=A035.698381] CPU2: off > Thanks, > > tglx