* Re: [PATCH net-next v10 2/2] net: mana: force full-page RX buffers via ethtool private flag
From: sashiko-bot @ 2026-06-03 20:28 UTC (permalink / raw)
To: Dipayaan Roy; +Cc: linux-hyperv, bpf
In-Reply-To: <20260602202801.1873742-3-dipayanroy@linux.microsoft.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Forcing `apc->port_is_up = true` on the `mana_attach` error path causes a double-destroy of the hardware vport when the interface is later brought down.
- [Medium] Latent bug: Changes to future private flags will be silently ignored if MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF is not simultaneously toggled.
--
commit d7983f6668f209a0b7bfb427c64d5517b326c7b1
Author: Dipayaan Roy <dipayanroy@linux.microsoft.com>
net: mana: force full-page RX buffers via ethtool private flag
Adds an ethtool private flag full-page-rx to bypass page_pool fragment
allocation and force one RX buffer per page. Includes logic in
mana_set_priv_flags to dynamically reconfigure the port on flag change.
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
[ ... ]
> +static int mana_set_priv_flags(struct net_device *ndev, u32 priv_flags)
> +{
> + struct mana_port_context *apc = netdev_priv(ndev);
> + u32 changed = apc->priv_flags ^ priv_flags;
> + u32 old_priv_flags = apc->priv_flags;
> + bool schedule_port_reset = false;
> + int err = 0;
> +
> + if (!changed)
> + return 0;
> +
> + /* Reject unknown bits */
> + if (priv_flags & ~GENMASK(MANA_PRIV_FLAG_MAX - 1, 0))
> + return -EINVAL;
> +
> + if (changed & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF)) {
> + apc->priv_flags = priv_flags;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: Medium]
When additional private flags are added in the future, will changes to those
new flags be silently ignored if MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF isn't
also toggled?
Since this assignment is nested inside the FULL_PAGE_RXBUF check, changing
only a future new flag will bypass this assignment and the function will
return success while discarding the new flag state.
Should this assignment be moved outside the flag-specific conditional block?
> +
> + if (!apc->port_is_up) {
[ ... ]
> + err = mana_attach(ndev);
> + if (err) {
> + netdev_err(ndev, "mana_attach failed: %d\n", err);
> + apc->priv_flags = old_priv_flags;
> +
> + /* Restore 'port_is_up' so the reset work handler
> + * can properly detach/attach. Without this,
> + * the handler sees port_is_up=false and skips
> + * queue allocation, leaving the port dead.
> + */
> + apc->port_is_up = true;
^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does forcing port_is_up to true here cause a double-destroy of the hardware
vport when the interface is later brought down?
If mana_attach fails, it cleans up any partial state by destroying the vport.
Leaving the port artificially marked as up means a subsequent interface down
event or reset worker will see apc->port_is_up as true and call
mana_detach with from_close set to true.
This bypasses the netif_device_present check and triggers a second call to
mana_destroy_vport, which could drop apc->vport_use_count below zero and
send a duplicate deregister message to the firmware for the already-freed
hw_vport_handle.
> + schedule_port_reset = true;
> + }
> + }
> +
> +out:
> + mana_pre_dealloc_rxbufs(apc);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260602202801.1873742-1-dipayanroy@linux.microsoft.com?part=2
^ permalink raw reply
* Re: [PATCH net v3] net: mana: Optimize irq affinity for low vcpu configs
From: Jacob Keller @ 2026-06-03 21:49 UTC (permalink / raw)
To: Shradha Gupta, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Simon Horman,
Erni Sri Satya Vennela, Dipayaan Roy, Shiraz Saleem,
Michael Kelley, Long Li, Yury Norov
Cc: linux-hyperv, linux-kernel, netdev, Paul Rosswurm, Shradha Gupta,
Saurabh Singh Sengar, stable
In-Reply-To: <20260601102749.1768304-1-shradhagupta@linux.microsoft.com>
On 6/1/2026 3:27 AM, Shradha Gupta wrote:
> In mana driver, the number of IRQs allocated is capped by the
> min(num_cpu + 1, queue count). In cases, where the IRQ count is greater
> than the vcpu count, we want to utilize all the vCPUs, irrespective of
> their NUMA/core bindings.
>
> This is important, especially in the envs where number of vCPUs are so
> few that the softIRQ handling overhead on two IRQs on the same vCPU is
> much more than their overheads if they were spread across sibling vCPUs.
>
> This behaviour is more evident with dynamic IRQ allocation. Since MANA
> IRQs are assigned at a later stage compared to static allocation, other
> device IRQs may already be affinitized to the vCPUs. As a result, IRQ
> weights become imbalanced, causing multiple MANA IRQs to land on the
> same vCPU, while some vCPUs have none.
>
> In such cases when many parallel TCP connections are tested, the
> throughput drops significantly.
>
> Test envs:
> =======================================================
> Case 1: without this patch
> =======================================================
> 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
>
> TYPE effective vCPU aff
> =======================================================
> IRQ0: HWC 0
> IRQ1: mana_q1 0
> IRQ2: mana_q2 2
> IRQ3: mana_q3 0
> IRQ4: mana_q4 3
>
> %soft on each vCPU(mpstat -P ALL 1) on receiver
> vCPU 0 1 2 3
> =======================================================
> pass 1: 38.85 0.03 24.89 24.65
> pass 2: 39.15 0.03 24.57 25.28
> pass 3: 40.36 0.03 23.20 23.17
>
> =======================================================
> Case 2: with this patch
> =======================================================
> 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
>
> TYPE effective vCPU aff
> =======================================================
> IRQ0: HWC 0
> IRQ1: mana_q1 0
> IRQ2: mana_q2 1
> IRQ3: mana_q3 2
> IRQ4: mana_q4 3
>
> %soft on each vCPU(mpstat -P ALL 1) on receiver
> vCPU 0 1 2 3
> =======================================================
> pass 1: 15.42 15.85 14.99 14.51
> pass 2: 15.53 15.94 15.81 15.93
> pass 3: 16.41 16.35 16.40 16.36
>
> =======================================================
> Throughput Impact(in Gbps, same env)
> =======================================================
> TCP conn with patch w/o patch
> 20480 15.65 7.73
> 10240 15.63 8.93
> 8192 15.64 9.69
> 6144 15.64 13.16
> 4096 15.69 15.75
> 2048 15.69 15.83
> 1024 15.71 15.28
>
> Fixes: 755391121038 ("net: mana: Allocate MSI-X vectors dynamically")
> Cc: stable@vger.kernel.org
> Co-developed-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> ---
> Changes in v3
> * Optimize the comments in mana_gd_setup_dyn_irqs()
> * add more details in the dev_dbg for extra IRQs
> ---
> Changes in v2
> * Removed the unused skip_first_cpu variable
> * fixed exit condition in irq_setup_linear() with len == 0
> * changed return type of irq_setup_linear() as it will always be 0
> * removed the unnecessary rcu_read_lock() in irq_setup_linear()
> * added appropriate comments to indicate expected behaviour when
> IRQs are more than or equal to num_online_cpus()
> ---
> .../net/ethernet/microsoft/mana/gdma_main.c | 60 ++++++++++++++++---
> 1 file changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 712a0881d720..00a28b3ca0a6 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -197,6 +197,8 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> } else {
> /* If dynamic allocation is enabled we have already allocated
> * hwc msi
> + * Also, we make sure in this case the following is always true
> + * (num_msix_usable - 1 HWC) <= num_online_cpus()
> */
> gc->num_msix_usable = min(resp.max_msix, num_online_cpus() + 1);
> }
> @@ -1717,11 +1719,24 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> return 0;
> }
>
> +/* should be called with cpus_read_lock() held */
> +static void irq_setup_linear(unsigned int *irqs, unsigned int len)
> +{
> + int cpu;
> +
> + for_each_online_cpu(cpu) {
> + if (len == 0)
> + break;
> +
> + irq_set_affinity_and_hint(*irqs++, cpumask_of(cpu));
> + len--;
> + }
> +}
I would find all of this a bit easier to follow if irq_setup_linear()
and irq_setup() had a mana prefix so it was more obvious these are
specific to the driver. Of course irq_setup is pre-existing, and its not
my driver so do as you will :)
> +
> static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> {
> struct gdma_context *gc = pci_get_drvdata(pdev);
> struct gdma_irq_context *gic;
> - bool skip_first_cpu = false;
> int *irqs, irq, err, i;
>
> irqs = kmalloc_objs(int, nvec);
> @@ -1729,6 +1744,8 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> return -ENOMEM;
>
> /*
> + * In this function, num_msix_usable = HWC IRQ + Queue IRQ.
> + * nvec is only Queue IRQ (HWC already setup).
> * While processing the next pci irq vector, we start with index 1,
> * as IRQ vector at index 0 is already processed for HWC.
> * However, the population of irqs array starts with index 0, to be
> @@ -1767,13 +1784,42 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> * first CPU sibling group since they are already affinitized to HWC IRQ
> */
> cpus_read_lock();
> - if (gc->num_msix_usable <= num_online_cpus())
> - skip_first_cpu = true;
> + if (gc->num_msix_usable <= num_online_cpus()) {
> + err = irq_setup(irqs, nvec, gc->numa_node, true);
> + if (err) {
> + cpus_read_unlock();
> + goto free_irq;
> + }
> + } else {
> + /*
> + * When num_msix_usable are more than num_online_cpus, our
> + * queue IRQs should be equal to num of online vCPUs.
> + * We try to make sure queue IRQs spread across all vCPUs.
> + * In such a case NUMA or CPU core affinity does not matter.
> + * Note: in this case the total mana IRQ should always be
> + * num_online_cpus + 1. The first HWC IRQ is already handled
> + * in HWC setup calls
> + * However, if CPUs went offline since num_msix_usable was
> + * computed, queue IRQs will be more than num_online_cpus().
> + * In such cases remaining extra IRQs will retain their default
> + * affinity.
> + */
> + int first_unassigned = num_online_cpus();
> + if (nvec > first_unassigned) {
> + char buf[32];
> +
> + if (first_unassigned == nvec - 1)
> + snprintf(buf, sizeof(buf), "%d",
> + first_unassigned);
> + else
> + snprintf(buf, sizeof(buf), "%d-%d",
> + first_unassigned, nvec - 1);
> +
> + dev_dbg(&pdev->dev,
> + "MANA IRQ indices #%s will retain the default CPU affinity\n", buf);
> + }
>
> - err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
> - if (err) {
> - cpus_read_unlock();
> - goto free_irq;
> + irq_setup_linear(irqs, nvec);
irq_setup() doesn't have a driver prefix, but is actually a static
function in gdma_main.c, so its implementation is specific to this
driver despite its name.
So if I understand this change correctly, if the number of usable MSI-X
vectors is smaller than the number of CPUs, you contineu to use the
current irq_setup logic.. otherwise you switch to the simpler "linear"
logic.
I guess this means the logic and heuristic used in irq_setup() breaks
down when the number of vectors is large and number of vCPU is small?
Makes sense.
> }
>
> cpus_read_unlock();
>
> base-commit: 8415598365503ced2e3d019491b0a2756c85c494
^ permalink raw reply
* [PATCH v2 0/4] Convert remaining buses to generic driver_override handling
From: Runyu Xiao @ 2026-06-04 3:52 UTC (permalink / raw)
To: gregkh, rafael
Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
jianhao.xu, runyu.xiao
In-Reply-To: <20260602160829.560904-1-runyu.xiao@seu.edu.cn>
This series converts four remaining buses from bus-private
driver_override handling to the generic driver-core infrastructure:
- AMBA
- RPMSG
- VMBUS
- CDX
These buses still keep private driver_override storage and read it
directly from their match paths. However, bus match() callbacks can be
reached from __driver_attach() without the device lock held, so those
raw reads can race with updates that replace and free the override
string.
The driver core already provides generic driver_override storage and
matching helpers with the required internal locking. Other buses have
already been converted to that model. This series switches the
remaining users above to the same infrastructure by:
- removing bus-private driver_override storage
- dropping bus-local driver_override sysfs handling
- enabling struct bus_type.driver_override
- using device_match_driver_override() in match paths
Bus-specific behavior is preserved where needed:
- VMBUS keeps its dummy-id fallback for override-based binding
- CDX keeps its override_only matching semantics
- RPMSG converts its in-kernel override registration path to
device_set_driver_override() and drops the old transport-local
frees of bus-private override storage
Before preparing this v2 series, I rechecked the affected source paths
against v7.1-rc6. I also reran the existing report-specific no-device
KCSAN stand-ins on a local v7.1-rc6 guest for all four buses. Those
reruns again produced target-stack reports for the corresponding
driver_override update/match paths.
That runtime validation is still stand-in based rather than direct
hardware execution, but it reuses the real driver_set_override() helper
from the running v7.1-rc6 guest kernel and preserves the relevant
patch-local reader/writer contracts and caller chains.
Since v1:
- reworked the series around the generic driver_override
infrastructure instead of trying to serialize bus match() with
device_lock(dev)
- split the changes by bus
- preserved VMBUS dummy-id fallback behavior explicitly
- preserved CDX override_only matching semantics explicitly
- converted the RPMSG in-kernel override registration path to the
core helper
- reran the four report-specific no-device KCSAN stand-ins on a
local v7.1-rc6 guest and refreshed the validation basis
- refreshed the commit messages accordingly
Runyu Xiao (4):
amba: use generic driver_override infrastructure
rpmsg: core: use generic driver_override infrastructure
vmbus: use generic driver_override infrastructure
cdx: use generic driver_override infrastructure
drivers/amba/bus.c | 35 +++++--------------------------
drivers/cdx/cdx.c | 40 +++++-------------------------------
drivers/hv/vmbus_drv.c | 36 +++++---------------------------
drivers/rpmsg/qcom_glink_native.c | 2 --
drivers/rpmsg/rpmsg_core.c | 41 ++++++-------------------------------
drivers/rpmsg/virtio_rpmsg_bus.c | 1 -
include/linux/amba/bus.h | 5 -----
include/linux/cdx/cdx_bus.h | 1 -
include/linux/hyperv.h | 6 ------
include/linux/rpmsg.h | 4 ----
10 files changed, 21 insertions(+), 150 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v2 1/4] amba: use generic driver_override infrastructure
From: Runyu Xiao @ 2026-06-04 3:52 UTC (permalink / raw)
To: gregkh, rafael
Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
jianhao.xu, runyu.xiao, stable
In-Reply-To: <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
AMBA devices still keep driver_override in bus-private storage.
The sysfs write side updates that string through driver_set_override(),
which replaces the pointer and frees the old value. However,
driver_match_device() can call amba_match() from __driver_attach()
without holding the device lock, and amba_match() still dereferences
that private pointer directly.
That means a bind/unbind or reprobe path can race with a concurrent
driver_override update and make amba_match() compare against freed
memory.
Fix this by switching AMBA to the driver-core driver_override
infrastructure. This lets the core own the sysfs attribute and storage,
and uses device_match_driver_override() for the locked read in the match
path.
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: 3cf385713460 ("ARM: 8256/1: driver coamba: add device binding path 'driver_override'")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/amba/bus.c | 35 +++++------------------------------
include/linux/amba/bus.h | 5 -----
2 files changed, 5 insertions(+), 35 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 6d479caf89cb..df8333f90906 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -82,33 +82,6 @@ static void amba_put_disable_pclk(struct amba_device *pcdev)
}
-static ssize_t driver_override_show(struct device *_dev,
- struct device_attribute *attr, char *buf)
-{
- struct amba_device *dev = to_amba_device(_dev);
- ssize_t len;
-
- device_lock(_dev);
- len = sprintf(buf, "%s\n", dev->driver_override);
- device_unlock(_dev);
- return len;
-}
-
-static ssize_t driver_override_store(struct device *_dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct amba_device *dev = to_amba_device(_dev);
- int ret;
-
- ret = driver_set_override(_dev, &dev->driver_override, buf, count);
- if (ret)
- return ret;
-
- return count;
-}
-static DEVICE_ATTR_RW(driver_override);
-
#define amba_attr_func(name,fmt,arg...) \
static ssize_t name##_show(struct device *_dev, \
struct device_attribute *attr, char *buf) \
@@ -126,7 +99,6 @@ amba_attr_func(resource, "\t%016llx\t%016llx\t%016lx\n",
static struct attribute *amba_dev_attrs[] = {
&dev_attr_id.attr,
&dev_attr_resource.attr,
- &dev_attr_driver_override.attr,
NULL,
};
ATTRIBUTE_GROUPS(amba_dev);
@@ -209,6 +181,7 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
{
struct amba_device *pcdev = to_amba_device(dev);
const struct amba_driver *pcdrv = to_amba_driver(drv);
+ int ret;
mutex_lock(&pcdev->periphid_lock);
if (!pcdev->periphid) {
@@ -230,8 +203,9 @@ static int amba_match(struct device *dev, const struct device_driver *drv)
mutex_unlock(&pcdev->periphid_lock);
/* When driver_override is set, only bind to the matching driver */
- if (pcdev->driver_override)
- return !strcmp(pcdev->driver_override, drv->name);
+ ret = device_match_driver_override(dev, drv);
+ if (ret >= 0)
+ return ret;
return amba_lookup(pcdrv->id_table, pcdev) != NULL;
}
@@ -435,6 +409,7 @@ static const struct dev_pm_ops amba_pm = {
*/
const struct bus_type amba_bustype = {
.name = "amba",
+ .driver_override = true,
.dev_groups = amba_dev_groups,
.match = amba_match,
.uevent = amba_uevent,
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 9946276aff73..6c54d5c0d21f 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -71,11 +71,6 @@ struct amba_device {
unsigned int cid;
struct amba_cs_uci_id uci;
unsigned int irq[AMBA_NR_IRQS];
- /*
- * Driver name to force a match. Do not set directly, because core
- * frees it. Use driver_set_override() to set or clear it.
- */
- const char *driver_override;
};
struct amba_driver {
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/4] rpmsg: core: use generic driver_override infrastructure
From: Runyu Xiao @ 2026-06-04 3:52 UTC (permalink / raw)
To: gregkh, rafael
Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
jianhao.xu, runyu.xiao, stable
In-Reply-To: <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
RPMSG still keeps driver_override in bus-private storage.
That private pointer can be updated from the sysfs driver_override
attribute, and also from rpmsg_register_device_override(). Both paths
replace the pointer and can free the old value.
However, driver_match_device() can call rpmsg_dev_match() from
__driver_attach() without holding the device lock, and rpmsg_dev_match()
still dereferences that private pointer directly.
This leaves the match path racing with concurrent driver_override
updates, with the usual risk of comparing against freed memory.
Switch rpmsg to the driver-core driver_override infrastructure. This
removes the private storage, uses device_match_driver_override() for the
locked read in rpmsg_dev_match(), and converts
rpmsg_register_device_override() to device_set_driver_override() so the
in-kernel override path uses the same core-managed storage. With that
storage now owned by struct device, drop the remaining rpmsg transport
release-path frees of rpdev->driver_override as well.
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: 39e47767ec9b ("rpmsg: Add driver_override device attribute for rpmsg_device")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/rpmsg/qcom_glink_native.c | 2 --
drivers/rpmsg/rpmsg_core.c | 41 ++++++--------------------------------
drivers/rpmsg/virtio_rpmsg_bus.c | 1 -
include/linux/rpmsg.h | 4 ----
4 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index e7f7831d37f8..11d3007db5cd 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -358,33 +358,6 @@ rpmsg_show_attr(src, src, "0x%x\n");
rpmsg_show_attr(dst, dst, "0x%x\n");
rpmsg_show_attr(announce, announce ? "true" : "false", "%s\n");
-static ssize_t driver_override_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct rpmsg_device *rpdev = to_rpmsg_device(dev);
- int ret;
-
- ret = driver_set_override(dev, &rpdev->driver_override, buf, count);
- if (ret)
- return ret;
-
- return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct rpmsg_device *rpdev = to_rpmsg_device(dev);
- ssize_t len;
-
- device_lock(dev);
- len = sysfs_emit(buf, "%s\n", rpdev->driver_override);
- device_unlock(dev);
- return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
static ssize_t modalias_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -405,7 +378,6 @@ static struct attribute *rpmsg_dev_attrs[] = {
&dev_attr_dst.attr,
&dev_attr_src.attr,
&dev_attr_announce.attr,
- &dev_attr_driver_override.attr,
NULL,
};
ATTRIBUTE_GROUPS(rpmsg_dev);
@@ -424,9 +396,11 @@ static int rpmsg_dev_match(struct device *dev, const struct device_driver *drv)
const struct rpmsg_driver *rpdrv = to_rpmsg_driver(drv);
const struct rpmsg_device_id *ids = rpdrv->id_table;
unsigned int i;
+ int ret;
- if (rpdev->driver_override)
- return !strcmp(rpdev->driver_override, drv->name);
+ ret = device_match_driver_override(dev, drv);
+ if (ret >= 0)
+ return ret;
if (ids)
for (i = 0; ids[i].name[0]; i++)
@@ -533,6 +507,7 @@ static void rpmsg_dev_remove(struct device *dev)
static const struct bus_type rpmsg_bus = {
.name = "rpmsg",
+ .driver_override = true,
.match = rpmsg_dev_match,
.dev_groups = rpmsg_dev_groups,
.uevent = rpmsg_uevent,
@@ -560,9 +535,7 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
device_initialize(dev);
if (driver_override) {
- ret = driver_set_override(dev, &rpdev->driver_override,
- driver_override,
- strlen(driver_override));
+ ret = device_set_driver_override(dev, driver_override);
if (ret) {
dev_err(dev, "device_set_override failed: %d\n", ret);
put_device(dev);
@@ -573,8 +546,6 @@ int rpmsg_register_device_override(struct rpmsg_device *rpdev,
ret = device_add(dev);
if (ret) {
dev_err(dev, "device_add failed: %d\n", ret);
- kfree(rpdev->driver_override);
- rpdev->driver_override = NULL;
put_device(dev);
}
diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 401a4ece0c97..d9d4468e4cbd 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -1626,7 +1626,6 @@ static void qcom_glink_rpdev_release(struct device *dev)
{
struct rpmsg_device *rpdev = to_rpmsg_device(dev);
- kfree(rpdev->driver_override);
kfree(rpdev);
}
@@ -1862,7 +1861,6 @@ static void qcom_glink_device_release(struct device *dev)
/* Release qcom_glink_alloc_channel() reference */
kref_put(&channel->refcount, qcom_glink_channel_release);
- kfree(rpdev->driver_override);
kfree(rpdev);
}
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 5ae15111fb4f..1b8bb05924af 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -374,7 +374,6 @@ static void virtio_rpmsg_release_device(struct device *dev)
struct rpmsg_device *rpdev = to_rpmsg_device(dev);
struct virtio_rpmsg_channel *vch = to_virtio_rpmsg_channel(rpdev);
- kfree(rpdev->driver_override);
kfree(vch);
}
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 83266ce14642..2e40eb54155e 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -41,9 +41,6 @@ struct rpmsg_channel_info {
* rpmsg_device - device that belong to the rpmsg bus
* @dev: the device struct
* @id: device id (used to match between rpmsg drivers and devices)
- * @driver_override: driver name to force a match; do not set directly,
- * because core frees it; use driver_set_override() to
- * set or clear it.
* @src: local address
* @dst: destination address
* @ept: the rpmsg endpoint of this channel
@@ -53,7 +50,6 @@ struct rpmsg_channel_info {
struct rpmsg_device {
struct device dev;
struct rpmsg_device_id id;
- const char *driver_override;
u32 src;
u32 dst;
struct rpmsg_endpoint *ept;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 4/4] cdx: use generic driver_override infrastructure
From: Runyu Xiao @ 2026-06-04 3:52 UTC (permalink / raw)
To: gregkh, rafael
Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
jianhao.xu, runyu.xiao, stable
In-Reply-To: <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
CDX devices still keep driver_override in bus-private storage.
The sysfs write side updates that string through driver_set_override(),
which replaces the pointer and frees the old value. However,
driver_match_device() can call cdx_bus_match() from __driver_attach()
without holding the device lock, and cdx_bus_match() still dereferences
that private pointer directly.
That means the CDX match path can race with a concurrent
driver_override update and compare against freed memory.
Switch CDX to the driver-core driver_override infrastructure. This
removes the private driver_override storage, lets the core provide the
sysfs attribute, and uses device_match_driver_override() for the locked
read in cdx_bus_match().
Preserve the existing CDX override_only semantics: entries marked
override_only still require a matching driver_override, but ordinary ID
matches continue to work unchanged.
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: 48a6c7bced2a ("cdx: add device attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/cdx/cdx.c | 40 +++++--------------------------------
include/linux/cdx/cdx_bus.h | 1 -
2 files changed, 5 insertions(+), 36 deletions(-)
diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 9196dc50a48d..d3d230247262 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -156,8 +156,6 @@ static int cdx_unregister_device(struct device *dev,
} else {
cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
debugfs_remove_recursive(cdx_dev->debugfs_dir);
- kfree(cdx_dev->driver_override);
- cdx_dev->driver_override = NULL;
}
/*
@@ -268,6 +266,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
const struct cdx_driver *cdx_drv = to_cdx_driver(drv);
const struct cdx_device_id *found_id = NULL;
const struct cdx_device_id *ids;
+ int ret;
if (cdx_dev->is_bus)
return false;
@@ -275,7 +274,8 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
ids = cdx_drv->match_id_table;
/* When driver_override is set, only bind to the matching driver */
- if (cdx_dev->driver_override && strcmp(cdx_dev->driver_override, drv->name))
+ ret = device_match_driver_override(dev, drv);
+ if (ret == 0)
return false;
found_id = cdx_match_id(ids, cdx_dev);
@@ -289,7 +289,7 @@ static int cdx_bus_match(struct device *dev, const struct device_driver *drv)
*/
if (!found_id->override_only)
return true;
- if (cdx_dev->driver_override)
+ if (ret > 0)
return true;
ids = found_id + 1;
@@ -453,36 +453,6 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(modalias);
-static ssize_t driver_override_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct cdx_device *cdx_dev = to_cdx_device(dev);
- int ret;
-
- if (WARN_ON(dev->bus != &cdx_bus_type))
- return -EINVAL;
-
- ret = driver_set_override(dev, &cdx_dev->driver_override, buf, count);
- if (ret)
- return ret;
-
- return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct cdx_device *cdx_dev = to_cdx_device(dev);
- ssize_t len;
-
- device_lock(dev);
- len = sysfs_emit(buf, "%s\n", cdx_dev->driver_override);
- device_unlock(dev);
- return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
@@ -552,7 +522,6 @@ static struct attribute *cdx_dev_attrs[] = {
&dev_attr_class.attr,
&dev_attr_revision.attr,
&dev_attr_modalias.attr,
- &dev_attr_driver_override.attr,
NULL,
};
@@ -646,6 +615,7 @@ ATTRIBUTE_GROUPS(cdx_bus);
const struct bus_type cdx_bus_type = {
.name = "cdx",
+ .driver_override = true,
.match = cdx_bus_match,
.probe = cdx_probe,
.remove = cdx_remove,
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index b1ba97f6c9ad..f1a107b232da 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -165,7 +165,6 @@ struct cdx_device {
bool enabled;
u32 msi_dev_id;
u32 num_msi;
- const char *driver_override;
struct mutex irqchip_lock;
bool msi_write_pending;
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/4] vmbus: use generic driver_override infrastructure
From: Runyu Xiao @ 2026-06-04 3:52 UTC (permalink / raw)
To: gregkh, rafael
Cc: dakr, driver-core, linux, andersson, mathieu.poirier, kys,
haiyangz, wei.liu, decui, longli, nipun.gupta, nikhil.agarwal,
linux-remoteproc, linux-arm-msm, linux-hyperv, linux-kernel,
jianhao.xu, runyu.xiao, stable
In-Reply-To: <20260604035239.1711889-1-runyu.xiao@seu.edu.cn>
VMBUS devices still keep driver_override in bus-private storage.
The sysfs write side updates that string through driver_set_override(),
which replaces the pointer and frees the old value. However,
driver_match_device() can call into hv_vmbus_get_id() from
__driver_attach() without holding the device lock, and hv_vmbus_get_id()
still dereferences that private pointer directly.
That means a bind/reprobe path can race with a concurrent
driver_override update and make the match logic inspect freed memory.
Switch vmbus to the driver-core driver_override infrastructure. This
removes the private driver_override storage and uses
device_match_driver_override() for the locked read in the match path.
Keep the existing vmbus semantics intact: if driver_override matches but
no dynamic or static device ID matches, continue to return the dummy
vmbus_device_null ID so override-only binding still works as before.
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
Fixes: d765edbb301c ("vmbus: add driver_override support")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
drivers/hv/vmbus_drv.c | 36 +++++-------------------------------
include/linux/hyperv.h | 6 ------
2 files changed, 5 insertions(+), 37 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index d28ff45d4cfd..a81e2b097636 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -538,34 +538,6 @@ static ssize_t device_show(struct device *dev,
}
static DEVICE_ATTR_RO(device);
-static ssize_t driver_override_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct hv_device *hv_dev = device_to_hv_device(dev);
- int ret;
-
- ret = driver_set_override(dev, &hv_dev->driver_override, buf, count);
- if (ret)
- return ret;
-
- return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct hv_device *hv_dev = device_to_hv_device(dev);
- ssize_t len;
-
- device_lock(dev);
- len = sysfs_emit(buf, "%s\n", hv_dev->driver_override);
- device_unlock(dev);
-
- return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
/* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_id.attr,
@@ -596,7 +568,6 @@ static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_channel_vp_mapping.attr,
&dev_attr_vendor.attr,
&dev_attr_device.attr,
- &dev_attr_driver_override.attr,
NULL,
};
@@ -708,9 +679,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
{
const guid_t *guid = &dev->dev_type;
const struct hv_vmbus_device_id *id;
+ int ret;
/* When driver_override is set, only bind to the matching driver */
- if (dev->driver_override && strcmp(dev->driver_override, drv->name))
+ ret = device_match_driver_override(&dev->device, &drv->driver);
+ if (ret == 0)
return NULL;
/* Look at the dynamic ids first, before the static ones */
@@ -719,7 +692,7 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
id = hv_vmbus_dev_match(drv->id_table, guid);
/* driver_override will always match, send a dummy id */
- if (!id && dev->driver_override)
+ if (!id && ret > 0)
id = &vmbus_device_null;
return id;
@@ -1021,6 +994,7 @@ static const struct dev_pm_ops vmbus_pm = {
/* The one and only one */
static const struct bus_type hv_bus = {
.name = "vmbus",
+ .driver_override = true,
.match = vmbus_match,
.shutdown = vmbus_shutdown,
.remove = vmbus_remove,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 964f1be8150c..f9ede569602d 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1272,12 +1272,6 @@ struct hv_device {
u16 device_id;
struct device device;
- /*
- * Driver name to force a match. Do not set directly, because core
- * frees it. Use driver_set_override() to set or clear it.
- */
- const char *driver_override;
-
struct vmbus_channel *channel;
struct kset *channels_kset;
struct device_dma_parameters dma_parms;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 4/4] cdx: use generic driver_override infrastructure
From: sashiko-bot @ 2026-06-04 4:08 UTC (permalink / raw)
To: Runyu Xiao; +Cc: linux-hyperv
In-Reply-To: <20260604035239.1711889-5-runyu.xiao@seu.edu.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Stale kernel-doc entry for removed `@driver_override` member in `struct cdx_device`.
--
commit c9a2a632f33a0544d4ef0145d2020070218d6838
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>
cdx: use generic driver_override infrastructure
This commit switches the CDX subsystem to use the generic driver-core
driver_override infrastructure. It removes the private driver_override
storage and uses device_match_driver_override() to prevent races during
bus matching.
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
[ ... ]
> diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
> index b1ba97f6c9ad6..f1a107b232daa 100644
> --- a/include/linux/cdx/cdx_bus.h
> +++ b/include/linux/cdx/cdx_bus.h
> @@ -165,7 +165,6 @@ struct cdx_device {
> bool enabled;
> u32 msi_dev_id;
> u32 num_msi;
> - const char *driver_override;
[Severity: Low]
Does this leave a stale kernel-doc entry for @driver_override in the
struct cdx_device documentation block?
The struct documentation still references the removed field:
* @driver_override: driver name to force a match; do not set directly,
* because core frees it; use driver_set_override() to
* set or clear it.
This might trigger an excess kerneldoc parameter warning during the kernel
build (with W=1).
> struct mutex irqchip_lock;
> bool msi_write_pending;
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604035239.1711889-1-runyu.xiao@seu.edu.cn?part=4
^ permalink raw reply
* Re: [PATCH v2 3/4] vmbus: use generic driver_override infrastructure
From: sashiko-bot @ 2026-06-04 4:13 UTC (permalink / raw)
To: Runyu Xiao; +Cc: linux-hyperv
In-Reply-To: <20260604035239.1711889-4-runyu.xiao@seu.edu.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] A time-of-check to time-of-use (TOCTOU) race condition in `vmbus_probe()` allows `drv->probe` to be called with a NULL `dev_id`, leading to a NULL pointer dereference in drivers like `storvsc_probe()`.
--
commit 76d8c5c515ad429741f331c31ec380bf773956cb
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>
vmbus: use generic driver_override infrastructure
This commit switches the vmbus subsystem to use the generic driver core
driver_override infrastructure rather than bus-private storage. It removes
the private driver_override storage and updates the match path to use
device_match_driver_override() for locked reads, preventing the match
logic from inspecting freed memory during concurrent updates.
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index d28ff45d4cfdd..a81e2b0976367 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
[ ... ]
> @@ -708,9 +679,11 @@ static const struct hv_vmbus_device_id *hv_vmbus_get_id(const struct hv_driver *
> {
> const guid_t *guid = &dev->dev_type;
> const struct hv_vmbus_device_id *id;
> + int ret;
>
> /* When driver_override is set, only bind to the matching driver */
> - if (dev->driver_override && strcmp(dev->driver_override, drv->name))
> + ret = device_match_driver_override(&dev->device, &drv->driver);
> + if (ret == 0)
> return NULL;
[Severity: High]
This is a pre-existing issue, but does this behavior still leave a
time-of-check to time-of-use race condition open when called later
during probing?
The driver core __driver_attach() calls driver_match_device() locklessly,
which delegates to vmbus_match() and eventually hv_vmbus_get_id(). If a
matching driver name is written to the driver_override sysfs attribute (or
a matching dynamic ID is present), it returns a match.
Before __driver_attach() acquires the device lock to call
driver_probe_device(), a concurrent sysfs write can clear the
driver_override attribute or remove the dynamic ID.
Inside vmbus_probe(), which executes with the device lock held,
hv_vmbus_get_id() is called again. Because the state was changed
concurrently, it can evaluate to NULL:
drivers/hv/vmbus_drv.c:vmbus_probe() {
...
dev_id = hv_vmbus_get_id(drv, dev);
if (drv->probe) {
ret = drv->probe(dev, dev_id);
...
}
Since vmbus_probe() lacks a NULL check for dev_id, it directly calls
drv->probe(dev, dev_id) with dev_id equal to NULL.
Sub-drivers like storvsc_probe() then unconditionally dereference dev_id
(for example, checking dev_id->driver_data), which results in a kernel panic.
Could a NULL check for dev_id be added in vmbus_probe() to fully prevent
this race?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260604035239.1711889-1-runyu.xiao@seu.edu.cn?part=3
^ permalink raw reply
* Re: [PATCH net v3] net: mana: Optimize irq affinity for low vcpu configs
From: Shradha Gupta @ 2026-06-04 4:39 UTC (permalink / raw)
To: Jacob Keller
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Erni Sri Satya Vennela,
Dipayaan Roy, Shiraz Saleem, Michael Kelley, Long Li, Yury Norov,
linux-hyperv, linux-kernel, netdev, Paul Rosswurm, Shradha Gupta,
Saurabh Singh Sengar, stable
In-Reply-To: <10988db9-8a8d-4ad0-917e-317dd4b20253@intel.com>
On Wed, Jun 03, 2026 at 02:49:24PM -0700, Jacob Keller wrote:
> On 6/1/2026 3:27 AM, Shradha Gupta wrote:
> > In mana driver, the number of IRQs allocated is capped by the
> > min(num_cpu + 1, queue count). In cases, where the IRQ count is greater
> > than the vcpu count, we want to utilize all the vCPUs, irrespective of
> > their NUMA/core bindings.
> >
> > This is important, especially in the envs where number of vCPUs are so
> > few that the softIRQ handling overhead on two IRQs on the same vCPU is
> > much more than their overheads if they were spread across sibling vCPUs.
> >
> > This behaviour is more evident with dynamic IRQ allocation. Since MANA
> > IRQs are assigned at a later stage compared to static allocation, other
> > device IRQs may already be affinitized to the vCPUs. As a result, IRQ
> > weights become imbalanced, causing multiple MANA IRQs to land on the
> > same vCPU, while some vCPUs have none.
> >
> > In such cases when many parallel TCP connections are tested, the
> > throughput drops significantly.
> >
> > Test envs:
> > =======================================================
> > Case 1: without this patch
> > =======================================================
> > 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
> >
> > TYPE effective vCPU aff
> > =======================================================
> > IRQ0: HWC 0
> > IRQ1: mana_q1 0
> > IRQ2: mana_q2 2
> > IRQ3: mana_q3 0
> > IRQ4: mana_q4 3
> >
> > %soft on each vCPU(mpstat -P ALL 1) on receiver
> > vCPU 0 1 2 3
> > =======================================================
> > pass 1: 38.85 0.03 24.89 24.65
> > pass 2: 39.15 0.03 24.57 25.28
> > pass 3: 40.36 0.03 23.20 23.17
> >
> > =======================================================
> > Case 2: with this patch
> > =======================================================
> > 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
> >
> > TYPE effective vCPU aff
> > =======================================================
> > IRQ0: HWC 0
> > IRQ1: mana_q1 0
> > IRQ2: mana_q2 1
> > IRQ3: mana_q3 2
> > IRQ4: mana_q4 3
> >
> > %soft on each vCPU(mpstat -P ALL 1) on receiver
> > vCPU 0 1 2 3
> > =======================================================
> > pass 1: 15.42 15.85 14.99 14.51
> > pass 2: 15.53 15.94 15.81 15.93
> > pass 3: 16.41 16.35 16.40 16.36
> >
> > =======================================================
> > Throughput Impact(in Gbps, same env)
> > =======================================================
> > TCP conn with patch w/o patch
> > 20480 15.65 7.73
> > 10240 15.63 8.93
> > 8192 15.64 9.69
> > 6144 15.64 13.16
> > 4096 15.69 15.75
> > 2048 15.69 15.83
> > 1024 15.71 15.28
> >
> > Fixes: 755391121038 ("net: mana: Allocate MSI-X vectors dynamically")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> > Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> > Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Reviewed-by: Simon Horman <horms@kernel.org>
> > ---
> > Changes in v3
> > * Optimize the comments in mana_gd_setup_dyn_irqs()
> > * add more details in the dev_dbg for extra IRQs
> > ---
> > Changes in v2
> > * Removed the unused skip_first_cpu variable
> > * fixed exit condition in irq_setup_linear() with len == 0
> > * changed return type of irq_setup_linear() as it will always be 0
> > * removed the unnecessary rcu_read_lock() in irq_setup_linear()
> > * added appropriate comments to indicate expected behaviour when
> > IRQs are more than or equal to num_online_cpus()
> > ---
> > .../net/ethernet/microsoft/mana/gdma_main.c | 60 ++++++++++++++++---
> > 1 file changed, 53 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > index 712a0881d720..00a28b3ca0a6 100644
> > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > @@ -197,6 +197,8 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> > } else {
> > /* If dynamic allocation is enabled we have already allocated
> > * hwc msi
> > + * Also, we make sure in this case the following is always true
> > + * (num_msix_usable - 1 HWC) <= num_online_cpus()
> > */
> > gc->num_msix_usable = min(resp.max_msix, num_online_cpus() + 1);
> > }
> > @@ -1717,11 +1719,24 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> > return 0;
> > }
> >
> > +/* should be called with cpus_read_lock() held */
> > +static void irq_setup_linear(unsigned int *irqs, unsigned int len)
> > +{
> > + int cpu;
> > +
> > + for_each_online_cpu(cpu) {
> > + if (len == 0)
> > + break;
> > +
> > + irq_set_affinity_and_hint(*irqs++, cpumask_of(cpu));
> > + len--;
> > + }
> > +}
>
> I would find all of this a bit easier to follow if irq_setup_linear()
> and irq_setup() had a mana prefix so it was more obvious these are
> specific to the driver. Of course irq_setup is pre-existing, and its not
> my driver so do as you will :)
>
> > +
> > static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> > {
> > struct gdma_context *gc = pci_get_drvdata(pdev);
> > struct gdma_irq_context *gic;
> > - bool skip_first_cpu = false;
> > int *irqs, irq, err, i;
> >
> > irqs = kmalloc_objs(int, nvec);
> > @@ -1729,6 +1744,8 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> > return -ENOMEM;
> >
> > /*
> > + * In this function, num_msix_usable = HWC IRQ + Queue IRQ.
> > + * nvec is only Queue IRQ (HWC already setup).
> > * While processing the next pci irq vector, we start with index 1,
> > * as IRQ vector at index 0 is already processed for HWC.
> > * However, the population of irqs array starts with index 0, to be
> > @@ -1767,13 +1784,42 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> > * first CPU sibling group since they are already affinitized to HWC IRQ
> > */
> > cpus_read_lock();
> > - if (gc->num_msix_usable <= num_online_cpus())
> > - skip_first_cpu = true;
> > + if (gc->num_msix_usable <= num_online_cpus()) {
> > + err = irq_setup(irqs, nvec, gc->numa_node, true);
> > + if (err) {
> > + cpus_read_unlock();
> > + goto free_irq;
> > + }
> > + } else {
> > + /*
> > + * When num_msix_usable are more than num_online_cpus, our
> > + * queue IRQs should be equal to num of online vCPUs.
> > + * We try to make sure queue IRQs spread across all vCPUs.
> > + * In such a case NUMA or CPU core affinity does not matter.
> > + * Note: in this case the total mana IRQ should always be
> > + * num_online_cpus + 1. The first HWC IRQ is already handled
> > + * in HWC setup calls
> > + * However, if CPUs went offline since num_msix_usable was
> > + * computed, queue IRQs will be more than num_online_cpus().
> > + * In such cases remaining extra IRQs will retain their default
> > + * affinity.
> > + */
> > + int first_unassigned = num_online_cpus();
> > + if (nvec > first_unassigned) {
> > + char buf[32];
> > +
> > + if (first_unassigned == nvec - 1)
> > + snprintf(buf, sizeof(buf), "%d",
> > + first_unassigned);
> > + else
> > + snprintf(buf, sizeof(buf), "%d-%d",
> > + first_unassigned, nvec - 1);
> > +
> > + dev_dbg(&pdev->dev,
> > + "MANA IRQ indices #%s will retain the default CPU affinity\n", buf);
> > + }
> >
> > - err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
> > - if (err) {
> > - cpus_read_unlock();
> > - goto free_irq;
> > + irq_setup_linear(irqs, nvec);
>
> irq_setup() doesn't have a driver prefix, but is actually a static
> function in gdma_main.c, so its implementation is specific to this
> driver despite its name.
>
> So if I understand this change correctly, if the number of usable MSI-X
> vectors is smaller than the number of CPUs, you contineu to use the
> current irq_setup logic.. otherwise you switch to the simpler "linear"
> logic.
>
> I guess this means the logic and heuristic used in irq_setup() breaks
> down when the number of vectors is large and number of vCPU is small?
>
> Makes sense.
>
Hi Jacob,
Yes, that's the right understanding.
Regarding the function names, let me take that up in a seperate patch to
add prefixes to all such functions.
Thanks.
> > }
> >
> > cpus_read_unlock();
> >
> > base-commit: 8415598365503ced2e3d019491b0a2756c85c494
^ permalink raw reply
* [PATCH net 0/2] net: mana: fix error-path issues in queue setup
From: Aditya Garg @ 2026-06-04 8:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, horms, shradhagupta, dipayanroy, ernis,
kees, shacharr, stephen, gargaditya, gargaditya, ssengar,
linux-hyperv, netdev, linux-kernel
Two error-path fixes in MANA queue setup, both surfaced during Sashiko
AI review of a recently upstreamed patch series.
Patch 1 initializes queue->id to INVALID_QUEUE_ID in
mana_gd_create_mana_wq_cq() so that a CQ creation failure before the
firmware id is assigned does not NULL gc->cq_table[0] and silently
break whichever real CQ owns that slot. This mirrors the existing
pattern in mana_gd_create_eq().
Patch 2 guards mana_destroy_txq()'s call to mana_destroy_wq_obj() with
an INVALID_MANA_HANDLE check, mirroring mana_destroy_rxq(). Without
it, TX setup failures lead to a firmware-rejected destroy of (u64)-1
and a spurious error in dmesg.
Aditya Garg (2):
net: mana: initialize gdma queue id to INVALID_QUEUE_ID
net: mana: guard TX wq object destroy with INVALID_MANA_HANDLE check
drivers/net/ethernet/microsoft/mana/gdma_main.c | 2 ++
drivers/net/ethernet/microsoft/mana/mana_en.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply
* [PATCH net 2/2] net: mana: guard TX wq object destroy with INVALID_MANA_HANDLE check
From: Aditya Garg @ 2026-06-04 8:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, horms, shradhagupta, dipayanroy, ernis,
kees, shacharr, stephen, gargaditya, gargaditya, ssengar,
linux-hyperv, netdev, linux-kernel
In-Reply-To: <20260604080137.1995269-1-gargaditya@linux.microsoft.com>
mana_create_txq() has several error paths (after mana_alloc_queues() or
mana_create_wq_obj() failure) where tx_qp[i]->tx_object stays as the
INVALID_MANA_HANDLE sentinel set at allocation. mana_destroy_txq() then
unconditionally calls mana_destroy_wq_obj() with (u64)-1, which firmware
rejects and logs an error.
Mirror the RX-side pattern in mana_destroy_rxq() and skip the destroy
when the handle is still INVALID_MANA_HANDLE.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
Reviewed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index db14357d3732..4a2cd800111e 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2351,7 +2351,8 @@ static void mana_destroy_txq(struct mana_port_context *apc)
netif_napi_del_locked(napi);
apc->tx_qp[i]->txq.napi_initialized = false;
}
- mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i]->tx_object);
+ if (apc->tx_qp[i]->tx_object != INVALID_MANA_HANDLE)
+ mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i]->tx_object);
mana_deinit_cq(apc, &apc->tx_qp[i]->tx_cq);
--
2.43.0
^ permalink raw reply related
* [PATCH net 1/2] net: mana: initialize gdma queue id to INVALID_QUEUE_ID
From: Aditya Garg @ 2026-06-04 8:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, horms, shradhagupta, dipayanroy, ernis,
kees, shacharr, stephen, gargaditya, gargaditya, ssengar,
linux-hyperv, netdev, linux-kernel
In-Reply-To: <20260604080137.1995269-1-gargaditya@linux.microsoft.com>
mana_gd_create_mana_wq_cq() leaves queue->id as 0 (from kzalloc_obj())
until mana_create_wq_obj() assigns the firmware-returned id. If creation
fails before that, cleanup calls mana_gd_destroy_cq() with id 0, NULLing
gc->cq_table[0] and silently breaking whichever real CQ owns that slot.
Initialize queue->id to INVALID_QUEUE_ID right after allocation, matching
mana_gd_create_eq(). The existing (id >= max_num_cqs) guard then
short-circuits cleanly.
Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
Reviewed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 712a0881d720..9d145517c6dc 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1200,6 +1200,8 @@ int mana_gd_create_mana_wq_cq(struct gdma_dev *gd,
if (!queue)
return -ENOMEM;
+ queue->id = INVALID_QUEUE_ID;
+
gmi = &queue->mem_info;
err = mana_gd_alloc_memory(gc, spec->queue_size, gmi);
if (err) {
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] clocksource/hyperv: Implement read_snapshot() for TSC page clocksource
From: David Woodhouse @ 2026-06-04 9:35 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paolo Bonzini, Vitaly Kuznetsov, Michael Kelley, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Daniel Lezcano, David Woodhouse, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, kvm,
linux-kernel, linux-hyperv, netdev
In-Reply-To: <20260604095755.64849-1-dwmw2@infradead.org>
From: David Woodhouse <dwmw@amazon.co.uk>
Implement the read_snapshot() callback for the Hyper-V TSC page
clocksource. This returns the derived 10MHz reference time (for
timekeeping) while also providing the raw TSC value that was used
to compute it.
When the TSC page is valid, hv_read_tsc_page_tsc() atomically
captures both values from a single RDTSC inside the sequence-counter
protected read. When the TSC page is invalid (sequence == 0), the
hw_csid and hw_cycles are set to zero indicating no value is available.
This enables ktime_get_snapshot_id() to provide the raw TSC to consumers
like KVM's master clock when running nested guests under Hyper-V.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/clocksource/hyperv_timer.c | 37 ++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index e9f5034a1bc8..df567795d175 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -444,6 +444,22 @@ static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
return read_hv_clock_tsc();
}
+static u64 notrace read_hv_clock_tsc_cs_snapshot(struct clocksource *arg,
+ struct clocksource_hw_snapshot *chs)
+{
+ u64 time;
+
+ if (hv_read_tsc_page_tsc(tsc_page, &chs->hw_cycles, &time)) {
+ chs->hw_csid = CSID_X86_TSC;
+ } else {
+ chs->hw_cycles = 0;
+ chs->hw_csid = CSID_GENERIC;
+ time = read_hv_clock_msr();
+ }
+
+ return time;
+}
+
static u64 noinstr read_hv_sched_clock_tsc(void)
{
return (read_hv_clock_tsc() - hv_sched_clock_offset) *
@@ -492,18 +508,19 @@ static int hv_cs_enable(struct clocksource *cs)
#endif
static struct clocksource hyperv_cs_tsc = {
- .name = "hyperv_clocksource_tsc_page",
- .rating = 500,
- .read = read_hv_clock_tsc_cs,
- .mask = CLOCKSOURCE_MASK(64),
- .flags = CLOCK_SOURCE_IS_CONTINUOUS,
- .suspend= suspend_hv_clock_tsc,
- .resume = resume_hv_clock_tsc,
+ .name = "hyperv_clocksource_tsc_page",
+ .rating = 500,
+ .read = read_hv_clock_tsc_cs,
+ .read_snapshot = read_hv_clock_tsc_cs_snapshot,
+ .mask = CLOCKSOURCE_MASK(64),
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+ .suspend = suspend_hv_clock_tsc,
+ .resume = resume_hv_clock_tsc,
#ifdef HAVE_VDSO_CLOCKMODE_HVCLOCK
- .enable = hv_cs_enable,
- .vdso_clock_mode = VDSO_CLOCKMODE_HVCLOCK,
+ .enable = hv_cs_enable,
+ .vdso_clock_mode = VDSO_CLOCKMODE_HVCLOCK,
#else
- .vdso_clock_mode = VDSO_CLOCKMODE_NONE,
+ .vdso_clock_mode = VDSO_CLOCKMODE_NONE,
#endif
};
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/3] x86/kvmclock: Implement read_snapshot() for kvmclock clocksource
From: David Woodhouse @ 2026-06-04 9:35 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paolo Bonzini, Vitaly Kuznetsov, Michael Kelley, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Daniel Lezcano, David Woodhouse, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, kvm,
linux-kernel, linux-hyperv, netdev
In-Reply-To: <20260604095755.64849-1-dwmw2@infradead.org>
From: David Woodhouse <dwmw@amazon.co.uk>
Implement the read_snapshot() callback for the kvmclock clocksource.
This returns the kvmclock nanosecond value (for timekeeping) while
also providing the raw TSC value that was used to compute it.
The TSC is read inside the pvclock seqlock-protected region, ensuring
the raw TSC and derived kvmclock value are atomically paired.
This enables ktime_get_snapshot_id() to provide the raw TSC to consumers
like the vmclock PTP driver, which currently has to do a separate call
to get_cycles() to obtain a value at *approximately* the same time, to
feed through the vmclock calculation.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
---
arch/x86/kernel/kvmclock.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index b5991d53fc0e..cb3d0ca1fa22 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -87,6 +87,27 @@ static u64 kvm_clock_get_cycles(struct clocksource *cs)
return kvm_clock_read();
}
+static u64 kvm_clock_get_cycles_snapshot(struct clocksource *cs,
+ struct clocksource_hw_snapshot *chs)
+{
+ struct pvclock_vcpu_time_info *src;
+ unsigned version;
+ u64 ret, tsc;
+
+ preempt_disable_notrace();
+ src = this_cpu_pvti();
+ do {
+ version = pvclock_read_begin(src);
+ tsc = rdtsc_ordered();
+ ret = __pvclock_read_cycles(src, tsc);
+ } while (pvclock_read_retry(src, version));
+ preempt_enable_notrace();
+
+ chs->hw_cycles = tsc;
+ chs->hw_csid = CSID_X86_TSC;
+ return ret;
+}
+
static noinstr u64 kvm_sched_clock_read(void)
{
return pvclock_clocksource_read_nowd(this_cpu_pvti()) - kvm_sched_clock_offset;
@@ -156,13 +177,14 @@ static int kvm_cs_enable(struct clocksource *cs)
}
static struct clocksource kvm_clock = {
- .name = "kvm-clock",
- .read = kvm_clock_get_cycles,
- .rating = 400,
- .mask = CLOCKSOURCE_MASK(64),
- .flags = CLOCK_SOURCE_IS_CONTINUOUS,
- .id = CSID_X86_KVM_CLK,
- .enable = kvm_cs_enable,
+ .name = "kvm-clock",
+ .read = kvm_clock_get_cycles,
+ .read_snapshot = kvm_clock_get_cycles_snapshot,
+ .rating = 400,
+ .mask = CLOCKSOURCE_MASK(64),
+ .flags = CLOCK_SOURCE_IS_CONTINUOUS,
+ .id = CSID_X86_KVM_CLK,
+ .enable = kvm_cs_enable,
};
static void kvm_register_clock(char *txt)
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/3] timekeeping: Implement and use read_snapshot() functionality
From: David Woodhouse @ 2026-06-04 9:35 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paolo Bonzini, Vitaly Kuznetsov, Michael Kelley, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Daniel Lezcano, David Woodhouse, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, kvm,
linux-kernel, linux-hyperv, netdev
Commit ca1ec8bfac8c ("timekeeping: Add clocksource read_snapshot()
method and hw_cycles to snapshot") provided a way for clocksources like
kvmclock and Hyper-V to expose the underlying hardware counter value
from which their clock reading is calculated.
This is useful for consumers like the KVM masterclock (when hosting
nested guests), and vmclock which needs the hardware counter value to
calculate time according to the information it's given.
Now that Thomas has beaten my initial proof of concept¹ into shape and
merged it (thanks) here are the two providers and the trivial consumer
case in the vmclock driver, reworked accordingly.
I will rework the other four KVM master clock patches into my kvmclock
series² which I'll rebase on top of tip/timers/ptp in due course.
¹ https://lore.kernel.org/all/20260526230635.136914-1-dwmw2@infradead.org/#r
² https://lore.kernel.org/all/20260509224824.3264567-1-dwmw2@infradead.org/
David Woodhouse (3):
clocksource/hyperv: Implement read_snapshot() for TSC page clocksource
x86/kvmclock: Implement read_snapshot() for kvmclock clocksource
ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing
arch/x86/kernel/kvmclock.c | 36 +++++++++++++++++++++++++++++-------
drivers/clocksource/hyperv_timer.c | 37 +++++++++++++++++++++++++++----------
drivers/ptp/ptp_vmclock.c | 4 ++++
3 files changed, 60 insertions(+), 17 deletions(-)
base-commit: ca1ec8bfac8c95d0fed9e3611ea21400d1f37262
^ permalink raw reply
* [PATCH v2 3/3] ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing
From: David Woodhouse @ 2026-06-04 9:35 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paolo Bonzini, Vitaly Kuznetsov, Michael Kelley, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Daniel Lezcano, David Woodhouse, Richard Cochran, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, kvm,
linux-kernel, linux-hyperv, netdev
In-Reply-To: <20260604095755.64849-1-dwmw2@infradead.org>
From: David Woodhouse <dwmw@amazon.co.uk>
When the system clocksource is kvmclock or Hyper-V (not the TSC
directly), vmclock_get_crosststamp() previously fell through to a
separate get_cycles() call, losing the atomic pairing between the
system time snapshot and the TSC reading.
Now that ktime_get_snapshot_id() populates hw_cycles with the
underlying TSC value for derived clocksources, use it when available.
This gives a perfect (system_time, tsc) pairing for the device time
calculation.
The SUPPORT_KVMCLOCK wrapper is still needed to convert the TSC into
kvmclock nanoseconds for system_counter->cycles, because otherwise
get_device_system_crosststamp() can't interpret the result against
the system clock.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
---
drivers/ptp/ptp_vmclock.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index d6a5a533164a..eebdcd5ebc08 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -140,6 +140,10 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
if (sts->pre_sts.cs_id == st->cs_id) {
cycle = sts->pre_sts.cycles;
sts->post_sts = sts->pre_sts;
+ } else if (sts->pre_sts.hw_csid == st->cs_id &&
+ sts->pre_sts.hw_cycles) {
+ cycle = sts->pre_sts.hw_cycles;
+ sts->post_sts = sts->pre_sts;
} else {
cycle = get_cycles();
ptp_read_system_postts(sts);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH net v3] net: mana: Optimize irq affinity for low vcpu configs
From: Paolo Abeni @ 2026-06-04 10:45 UTC (permalink / raw)
To: Shradha Gupta, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Konstantin Taranov, Simon Horman,
Erni Sri Satya Vennela, Dipayaan Roy, Shiraz Saleem,
Michael Kelley, Long Li, Yury Norov
Cc: linux-hyperv, linux-kernel, netdev, Paul Rosswurm, Shradha Gupta,
Saurabh Singh Sengar, stable
In-Reply-To: <20260601102749.1768304-1-shradhagupta@linux.microsoft.com>
On 6/1/26 12:27 PM, Shradha Gupta wrote:
> In mana driver, the number of IRQs allocated is capped by the
> min(num_cpu + 1, queue count). In cases, where the IRQ count is greater
> than the vcpu count, we want to utilize all the vCPUs, irrespective of
> their NUMA/core bindings.
>
> This is important, especially in the envs where number of vCPUs are so
> few that the softIRQ handling overhead on two IRQs on the same vCPU is
> much more than their overheads if they were spread across sibling vCPUs.
>
> This behaviour is more evident with dynamic IRQ allocation. Since MANA
> IRQs are assigned at a later stage compared to static allocation, other
> device IRQs may already be affinitized to the vCPUs. As a result, IRQ
> weights become imbalanced, causing multiple MANA IRQs to land on the
> same vCPU, while some vCPUs have none.
>
> In such cases when many parallel TCP connections are tested, the
> throughput drops significantly.
>
> Test envs:
> =======================================================
> Case 1: without this patch
> =======================================================
> 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
>
> TYPE effective vCPU aff
> =======================================================
> IRQ0: HWC 0
> IRQ1: mana_q1 0
> IRQ2: mana_q2 2
> IRQ3: mana_q3 0
> IRQ4: mana_q4 3
>
> %soft on each vCPU(mpstat -P ALL 1) on receiver
> vCPU 0 1 2 3
> =======================================================
> pass 1: 38.85 0.03 24.89 24.65
> pass 2: 39.15 0.03 24.57 25.28
> pass 3: 40.36 0.03 23.20 23.17
>
> =======================================================
> Case 2: with this patch
> =======================================================
> 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
>
> TYPE effective vCPU aff
> =======================================================
> IRQ0: HWC 0
> IRQ1: mana_q1 0
> IRQ2: mana_q2 1
> IRQ3: mana_q3 2
> IRQ4: mana_q4 3
>
> %soft on each vCPU(mpstat -P ALL 1) on receiver
> vCPU 0 1 2 3
> =======================================================
> pass 1: 15.42 15.85 14.99 14.51
> pass 2: 15.53 15.94 15.81 15.93
> pass 3: 16.41 16.35 16.40 16.36
>
> =======================================================
> Throughput Impact(in Gbps, same env)
> =======================================================
> TCP conn with patch w/o patch
> 20480 15.65 7.73
> 10240 15.63 8.93
> 8192 15.64 9.69
> 6144 15.64 13.16
> 4096 15.69 15.75
> 2048 15.69 15.83
> 1024 15.71 15.28
>
> Fixes: 755391121038 ("net: mana: Allocate MSI-X vectors dynamically")
> Cc: stable@vger.kernel.org
> Co-developed-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
Why do you consider this patch a fix? To me is a configuration
improvement and should land on net-next.
> @@ -1717,11 +1719,24 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> return 0;
> }
>
> +/* should be called with cpus_read_lock() held */
Minor nit: s/should/must/ or just drop the comment, as
`for_each_online_cpu()` usage implies that.
> +static void irq_setup_linear(unsigned int *irqs, unsigned int len)
> +{
> + int cpu;
> +
> + for_each_online_cpu(cpu) {
> + if (len == 0)
> + break;
> +
> + irq_set_affinity_and_hint(*irqs++, cpumask_of(cpu));
> + len--;
> + }
As this is another heuristic regarding irq spreading, why don't you
implement that inside irq_setup()?
> @@ -1767,13 +1784,42 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> * first CPU sibling group since they are already affinitized to HWC IRQ
> */
> cpus_read_lock();
> - if (gc->num_msix_usable <= num_online_cpus())
> - skip_first_cpu = true;
> + if (gc->num_msix_usable <= num_online_cpus()) {
> + err = irq_setup(irqs, nvec, gc->numa_node, true);
> + if (err) {
> + cpus_read_unlock();
> + goto free_irq;
> + }
> + } else {
> + /*
> + * When num_msix_usable are more than num_online_cpus, our
> + * queue IRQs should be equal to num of online vCPUs.
> + * We try to make sure queue IRQs spread across all vCPUs.
> + * In such a case NUMA or CPU core affinity does not matter.
> + * Note: in this case the total mana IRQ should always be
> + * num_online_cpus + 1. The first HWC IRQ is already handled
> + * in HWC setup calls
> + * However, if CPUs went offline since num_msix_usable was
> + * computed, queue IRQs will be more than num_online_cpus().
> + * In such cases remaining extra IRQs will retain their default
> + * affinity.
> + */
> + int first_unassigned = num_online_cpus();
> + if (nvec > first_unassigned) {
An empty line is needed between the variable declaration and the code.
/P
^ permalink raw reply
* Re: [RFC PATCH 00/20] mshv: enable kexec with Hyper-V donated pages and partitions
From: Mike Rapoport @ 2026-06-04 12:17 UTC (permalink / raw)
To: Jork Loeser
Cc: linux-hyperv, linux-mm, kexec, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Pasha Tatashin, Pratyush Yadav,
Alexander Graf, Jason Miu, Andrew Morton, David Hildenbrand,
Muchun Song, Oscar Salvador, Baoquan He, Catalin Marinas,
Will Deacon, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Kees Cook, Ran Xiaokai,
Justinien Bouron, Sourabh Jain, Pingfan Liu, Rafael J. Wysocki,
Mario Limonciello, linux-arm-kernel, x86, linux-kernel,
Michael Kelley
In-Reply-To: <3197c9c9-9e4f-c592-bb7-ac422f89115@linux.microsoft.com>
On Wed, Jun 03, 2026 at 10:25:58AM -0700, Jork Loeser wrote:
>
>
> On Wed, 3 Jun 2026, Mike Rapoport wrote:
>
> > On Mon, Jun 01, 2026 at 01:09:41PM -0700, Jork Loeser wrote:
> > > On Sun, 31 May 2026, Mike Rapoport wrote:
> > >
> > > > > Patch 19: Export kexec_in_progress for modules
> > > >
> > > > Isn't there another way to differentiate kexec reboot?
> >
> > There's that "kexec reboot" string passed as the cmd to the reboot
> > notifier.
> > Maybe we can make it somehow more well defined API and use it?
>
> A string? Dear my - the compiler won't flag it on an API change then, not
> ideal clearly. What's wrong with exporting kexec_in_progress()?
The policy in general is avoid exports unless strictly necessary.
A string can be declared as const char *KEXEC_REBOOT = "kexec reboot" and
used in both kexec and mshv. Not ideal, but still better.
No strong feelings from my side, just EXPORT_SYMBOL there felt a bit off.
> Best,
> Jork
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
From: sashiko-bot @ 2026-06-04 16:39 UTC (permalink / raw)
To: Anton Leontev; +Cc: linux-hyperv
In-Reply-To: <20260603163851.18058-1-leontyevantony@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch replaces `phys_to_virt()` with `kmap_local_page()` in `netvsc_copy_to_send_buf()` to support HIGHMEM SKB fragments. However, `kmap_local_page()` maps exactly one page (e.g., 4096 bytes). SKB fragments (such as those from TCP sends using `sk_page_frag()` or NFS) can be backed by compound pages where the fragment size (`len`) exceeds a single page, or where `offset + len` crosses a page boundary. The `pb` array populated by `init_page_array()` contains exactly one entry per fragment, meaning `pb[i].len` can exceed `PAGE_SIZE`. When `memcpy(dest, src + offset, len)` is called, it will read past the 4KB window mapped by `kmap_local_page()`. On 32-bit HIGHMEM systems, this accesses adjacent unmapped or unrelated fixmap slots, causing a fatal page fault or memory leakage.
--
commit dfe36312b193784583b28a894f895984d7d0ebba
Author: Anton Leontev <leontyevantony@gmail.com>
hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
This patch replaces phys_to_virt() with kmap_local_page() to support HIGHMEM
SKB fragments that are beyond the LOWMEM boundary on 32-bit systems.
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 59e95341f9b1e..2038d9f5c9f9a 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
[ ... ]
> @@ -965,11 +966,14 @@ static void netvsc_copy_to_send_buf(struct netvsc_device *net_device,
> }
>
> for (i = 0; i < page_count; i++) {
> - char *src = phys_to_virt(pb[i].pfn << HV_HYP_PAGE_SHIFT);
> - u32 offset = pb[i].offset;
> + phys_addr_t paddr = pb[i].pfn << HV_HYP_PAGE_SHIFT;
> + struct page *page = phys_to_page(paddr);
> + u32 offset = offset_in_page(paddr) + pb[i].offset;
> u32 len = pb[i].len;
> + char *src = kmap_local_page(page);
>
> memcpy(dest, (src + offset), len);
[Severity: Critical]
Will this memcpy() read beyond the 1-page mapping established by
kmap_local_page()?
If the fragment size (len) plus offset exceeds PAGE_SIZE, it seems memcpy()
will read past the locally mapped window. SKB fragments can be backed by
compound pages where the fragment size exceeds a single page, and
init_page_array() assigns the full fragment size to pb[i].len:
drivers/net/hyperv/netvsc_drv.c:init_page_array() {
...
cur_pb->offset = offset_in_hvpage(offset);
cur_pb->len = skb_frag_size(frag);
cur_pb->pfn = pfn + (offset >> HV_HYP_PAGE_SHIFT);
...
}
Other parts of the subsystem also document that a pb entry can cross page
boundaries:
drivers/net/hyperv/netvsc.c:netvsc_build_mpb_array() {
...
/* Each entry in the input "pb" array must describe a contiguous range
* of guest physical memory so that the pfns are sequential if the range
* crosses a page boundary. */
...
for (j = 0; j < HVPFN_UP(offset + len); j++)
mpb_entry->pfnArray[j] = pb[i].pfn + j;
...
}
Could this regression cause a fatal page fault on 32-bit HIGHMEM guests when
an SKB fragment crosses a page boundary, or leak unrelated fixmap memory if
the adjacent slot happens to be mapped?
> + kunmap_local(src);
> dest += len;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603163851.18058-1-leontyevantony@gmail.com?part=1
^ permalink raw reply
* [PATCH net v3] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
From: Anton Leontev @ 2026-06-04 16:59 UTC (permalink / raw)
To: netdev
Cc: linux-hyperv, haiyangz, kys, wei.liu, decui, longli, kuba, pabeni,
edumazet, davem, stable, linux-kernel, Anton Leontev
netvsc_copy_to_send_buf() copies page buffer entries into the VMBus
send buffer using phys_to_virt() on the entry PFN. Entries for the
RNDIS header and the skb linear data come from kmalloc'd memory and
are always in the kernel direct map, but entries for skb fragments
reference page cache or user pages, which on 32-bit x86 with
CONFIG_HIGHMEM=y can live above the LOWMEM boundary. For such a page
phys_to_virt() returns an address outside the direct map and the
subsequent memcpy() faults on the transmit softirq path, which is
fatal.
Map the pages with kmap_local_page() instead, handling two properties
of the page buffer entries:
- pb[i].pfn is a Hyper-V PFN at HV_HYP_PAGE_SIZE (4K) granularity,
not a native PFN. Reconstruct the physical address first and derive
the native page from it, so the mapping stays correct where
PAGE_SIZE > HV_HYP_PAGE_SIZE (e.g. arm64 with 64K pages).
- Since commit 41a6328b2c55 ("hv_netvsc: Preserve contiguous PFN
grouping in the page buffer array"), an entry describes a full
physically contiguous fragment and pb[i].len can exceed PAGE_SIZE,
while kmap_local_page() maps a single page. Copy page by page,
splitting at native page boundaries.
The copy path only handles packets smaller than the send section size
(6144 bytes by default); larger packets take the cp_partial path where
only the RNDIS header is copied. So entries here are bounded by the
section size and a copy is split at most once on 4K-page systems. On
!CONFIG_HIGHMEM configs kmap_local_page() folds to page_address() and
no mapping work is added.
Fixes: c25aaf814a63 ("hyperv: Enable sendbuf mechanism on the send path")
Cc: stable@vger.kernel.org
Signed-off-by: Anton Leontev <leontyevantony@gmail.com>
---
v3:
- Copy page by page: since 41a6328b2c55 a pb entry describes a full
contiguous fragment and pb[i].len can exceed PAGE_SIZE, while
kmap_local_page() maps a single page. Split copies at native page
boundaries.
v2:
- Derive the native page and in-page offset from the physical
address instead of passing the Hyper-V 4K PFN to pfn_to_page(),
correct where PAGE_SIZE > 4K (e.g. arm64 64K pages).
I do not have a 32-bit HIGHMEM Hyper-V setup to exercise this path;
testing help from the Hyper-V folks would be much appreciated.
drivers/net/hyperv/netvsc.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 59e95341f9b1..4d319c50955e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -12,6 +12,7 @@
#include <linux/sched.h>
#include <linux/wait.h>
#include <linux/mm.h>
+#include <linux/highmem.h>
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/slab.h>
@@ -965,12 +966,22 @@ static void netvsc_copy_to_send_buf(struct netvsc_device *net_device,
}
for (i = 0; i < page_count; i++) {
- char *src = phys_to_virt(pb[i].pfn << HV_HYP_PAGE_SHIFT);
- u32 offset = pb[i].offset;
+ phys_addr_t paddr = (pb[i].pfn << HV_HYP_PAGE_SHIFT) +
+ pb[i].offset;
u32 len = pb[i].len;
- memcpy(dest, (src + offset), len);
- dest += len;
+ while (len) {
+ struct page *page = phys_to_page(paddr);
+ u32 off = offset_in_page(paddr);
+ u32 chunk = min_t(u32, len, PAGE_SIZE - off);
+ char *src = kmap_local_page(page);
+
+ memcpy(dest, src + off, chunk);
+ kunmap_local(src);
+ dest += chunk;
+ paddr += chunk;
+ len -= chunk;
+ }
}
if (padding)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
From: Anton Leontev @ 2026-06-04 16:59 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hyperv
In-Reply-To: <20260604163900.A09211F00893@smtp.kernel.org>
You are right, thanks. Since 41a6328b2c55 ("hv_netvsc: Preserve
contiguous PFN grouping in the page buffer array") a pb entry
describes a full physically contiguous fragment and pb[i].len can
exceed PAGE_SIZE, while kmap_local_page() maps a single page. v3
copies page by page, splitting at native page boundaries, and was
posted as a new thread per netdev process.
чт, 4 июн. 2026 г. в 19:39, <sashiko-bot@kernel.org>:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] The patch replaces `phys_to_virt()` with `kmap_local_page()` in `netvsc_copy_to_send_buf()` to support HIGHMEM SKB fragments. However, `kmap_local_page()` maps exactly one page (e.g., 4096 bytes). SKB fragments (such as those from TCP sends using `sk_page_frag()` or NFS) can be backed by compound pages where the fragment size (`len`) exceeds a single page, or where `offset + len` crosses a page boundary. The `pb` array populated by `init_page_array()` contains exactly one entry per fragment, meaning `pb[i].len` can exceed `PAGE_SIZE`. When `memcpy(dest, src + offset, len)` is called, it will read past the 4KB window mapped by `kmap_local_page()`. On 32-bit HIGHMEM systems, this accesses adjacent unmapped or unrelated fixmap slots, causing a fatal page fault or memory leakage.
> --
>
> commit dfe36312b193784583b28a894f895984d7d0ebba
> Author: Anton Leontev <leontyevantony@gmail.com>
>
> hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
>
> This patch replaces phys_to_virt() with kmap_local_page() to support HIGHMEM
> SKB fragments that are beyond the LOWMEM boundary on 32-bit systems.
>
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> > index 59e95341f9b1e..2038d9f5c9f9a 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> [ ... ]
> > @@ -965,11 +966,14 @@ static void netvsc_copy_to_send_buf(struct netvsc_device *net_device,
> > }
> >
> > for (i = 0; i < page_count; i++) {
> > - char *src = phys_to_virt(pb[i].pfn << HV_HYP_PAGE_SHIFT);
> > - u32 offset = pb[i].offset;
> > + phys_addr_t paddr = pb[i].pfn << HV_HYP_PAGE_SHIFT;
> > + struct page *page = phys_to_page(paddr);
> > + u32 offset = offset_in_page(paddr) + pb[i].offset;
> > u32 len = pb[i].len;
> > + char *src = kmap_local_page(page);
> >
> > memcpy(dest, (src + offset), len);
>
> [Severity: Critical]
> Will this memcpy() read beyond the 1-page mapping established by
> kmap_local_page()?
>
> If the fragment size (len) plus offset exceeds PAGE_SIZE, it seems memcpy()
> will read past the locally mapped window. SKB fragments can be backed by
> compound pages where the fragment size exceeds a single page, and
> init_page_array() assigns the full fragment size to pb[i].len:
>
> drivers/net/hyperv/netvsc_drv.c:init_page_array() {
> ...
> cur_pb->offset = offset_in_hvpage(offset);
> cur_pb->len = skb_frag_size(frag);
> cur_pb->pfn = pfn + (offset >> HV_HYP_PAGE_SHIFT);
> ...
> }
>
> Other parts of the subsystem also document that a pb entry can cross page
> boundaries:
>
> drivers/net/hyperv/netvsc.c:netvsc_build_mpb_array() {
> ...
> /* Each entry in the input "pb" array must describe a contiguous range
> * of guest physical memory so that the pfns are sequential if the range
> * crosses a page boundary. */
> ...
> for (j = 0; j < HVPFN_UP(offset + len); j++)
> mpb_entry->pfnArray[j] = pb[i].pfn + j;
> ...
> }
>
> Could this regression cause a fatal page fault on 32-bit HIGHMEM guests when
> an SKB fragment crosses a page boundary, or leak unrelated fixmap memory if
> the adjacent slot happens to be mapped?
>
> > + kunmap_local(src);
> > dest += len;
> > }
> >
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260603163851.18058-1-leontyevantony@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH net v2] hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
From: sashiko-bot @ 2026-06-04 17:25 UTC (permalink / raw)
To: Anton Leontev; +Cc: linux-hyperv
In-Reply-To: <20260603172543.19230-1-leontyevantony@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] `memcpy()` overruns the single-page mapping created by `kmap_local_page()` when an SKB fragment spans multiple pages.
Pre-existing issues:
- [High] Concurrent writers to the per-channel `tx_stats.syncp` will corrupt the statistics seqcount, deadlocking readers on 32-bit systems.
--
commit f885c2ff3318238c254b9deff5078d8c10f79e15
Author: Anton Leontev <leontyevantony@gmail.com>
hv_netvsc: use kmap_local_page in netvsc_copy_to_send_buf
This commit replaces phys_to_virt() with kmap_local_page() to safely access
highmem SKB fragments on 32-bit systems, preventing page faults.
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 59e95341f9b1e..2038d9f5c9f9a 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
[ ... ]
> @@ -965,11 +966,14 @@ static void netvsc_copy_to_send_buf(struct netvsc_device *net_device,
> }
>
> for (i = 0; i < page_count; i++) {
> - char *src = phys_to_virt(pb[i].pfn << HV_HYP_PAGE_SHIFT);
> - u32 offset = pb[i].offset;
> + phys_addr_t paddr = pb[i].pfn << HV_HYP_PAGE_SHIFT;
> + struct page *page = phys_to_page(paddr);
> + u32 offset = offset_in_page(paddr) + pb[i].offset;
> u32 len = pb[i].len;
> + char *src = kmap_local_page(page);
>
> memcpy(dest, (src + offset), len);
[Severity: Critical]
Does this memcpy() risk overrunning the single-page mapping?
kmap_local_page() creates a temporary thread-local virtual mapping of exactly
one native page (4096 bytes on x86). However, the length of the fragment (len)
can exceed a single page.
For example, init_page_array() maps the entire linear skb->data into a single
hv_page_buffer entry, and skb_headlen() can exceed 4096 bytes with jumbo
frames or GRO/TSO packets.
If pb[i].len + offset > PAGE_SIZE, this will read beyond the mapped 4096-byte
window and access unmapped fixmap slots, potentially triggering a page fault
on the transmit softirq path.
> + kunmap_local(src);
> dest += len;
> }
>
[Severity: High]
This is a pre-existing issue, but can concurrent writers to the per-channel
tx_stats.syncp corrupt the statistics seqcount?
Looking at netvsc_ndoxdp_xmit() in drivers/net/hyperv/netvsc_bpf.c, it selects
a transmit queue using q_idx = smp_processor_id() % queues and updates the stats:
drivers/net/hyperv/netvsc_bpf.c:netvsc_ndoxdp_xmit() {
...
tx_stats = &nvsc_dev->chan_table[q_idx].tx_stats;
u64_stats_update_begin(&tx_stats->syncp);
tx_stats->xdp_xmit += count;
u64_stats_update_end(&tx_stats->syncp);
...
}
If num_online_cpus() > real_num_tx_queues, multiple CPUs can simultaneously
execute XDP transmit for the same queue.
Additionally, the NAPI TX completion path independently updates the same
syncp on the CPU bound to the VMBus interrupt:
drivers/net/hyperv/netvsc.c:netvsc_send_tx_complete() {
...
tx_stats = &net_device->chan_table[q_idx].tx_stats;
u64_stats_update_begin(&tx_stats->syncp);
tx_stats->packets += packet->total_packets;
tx_stats->bytes += packet->total_bytes;
u64_stats_update_end(&tx_stats->syncp);
...
}
Since u64_stats_update_begin() does not use locks, concurrent updates could
leave the seqcount odd forever. This would cause readers like ndo_get_stats64()
to spin infinitely in u64_stats_fetch_retry(), deadlocking the CPU on 32-bit
systems.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603172543.19230-1-leontyevantony@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH net-next v10 2/2] net: mana: force full-page RX buffers via ethtool private flag
From: Jacob Keller @ 2026-06-04 18:40 UTC (permalink / raw)
To: Dipayaan Roy, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
edumazet, kuba, pabeni, leon, longli, kotaranov, horms,
shradhagupta, ssengar, ernis, shirazsaleem, linux-hyperv, netdev,
linux-kernel, linux-rdma, stephen, dipayanroy, leitao, kees,
john.fastabend, hawk, bpf, daniel, ast, sdf, yury.norov,
pavan.chebbi
In-Reply-To: <20260602202801.1873742-3-dipayanroy@linux.microsoft.com>
On 6/2/2026 1:24 PM, Dipayaan Roy wrote:
> On some ARM64 platforms with 4K PAGE_SIZE, page_pool fragment
> allocation in the RX refill path can cause 15-20% throughput
> regression under high connection counts (>16 TCP streams).
>
> Add an ethtool private flag "full-page-rx" that allows the user to
> force one RX buffer per page, bypassing the page_pool fragment path.
> This restores line-rate (180+ Gbps) performance on affected platforms.
>
> Usage:
> ethtool --set-priv-flags eth0 full-page-rx on
>
> There is no behavioral change by default. The flag must be explicitly
> enabled by the user or udev rule.
>
> The existing single-buffer-per-page logic for XDP and jumbo frames is
> consolidated into a new helper mana_use_single_rxbuf_per_page() which
> is now the single decision point for both the automatic and
> user-controlled paths.
>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> ---
I had one or two minor nits, but nothing that I think really deserves a
v11. The only real comment is a future "gotcha" that could happen if you
ever added a second private flag, which seems unlikely and maybe not
worth dealing with until it matters.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> drivers/net/ethernet/microsoft/mana/mana_en.c | 22 +++-
> .../ethernet/microsoft/mana/mana_ethtool.c | 103 ++++++++++++++++++
> include/net/mana/mana.h | 8 ++
> 3 files changed, 131 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index db14357d3732..447cecfd3f67 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -744,6 +744,25 @@ static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)
> return va;
> }
>
> +static bool
> +mana_use_single_rxbuf_per_page(struct mana_port_context *apc, u32 mtu)
> +{
> + /* On some platforms with 4K PAGE_SIZE, page_pool fragment allocation
> + * in the RX refill path (~2kB buffer) can cause significant throughput
> + * regression under high connection counts. Allow user to force one RX
> + * buffer per page via ethtool private flag to bypass the fragment
> + * path.
> + */
> + if (apc->priv_flags & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF))
> + return true;
> +
> + /* For xdp and jumbo frames make sure only one packet fits per page. */
> + if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 || mana_xdp_get(apc))
> + return true;
Technically you could combine all three into one if, but I agree that
clarity and space for the comment about why the private flag exists
makes sense.
> +
> + return false;
> +}
> +
> /* Get RX buffer's data size, alloc size, XDP headroom based on MTU */
> static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
> int mtu, u32 *datasize, u32 *alloc_size,
> @@ -754,8 +773,7 @@ static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
> /* Calculate datasize first (consistent across all cases) */
> *datasize = mtu + ETH_HLEN;
>
> - /* For xdp and jumbo frames make sure only one packet fits per page */
> - if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 || mana_xdp_get(apc)) {
> + if (mana_use_single_rxbuf_per_page(apc, mtu)) {
> if (mana_xdp_get(apc)) {
> *headroom = XDP_PACKET_HEADROOM;
> *alloc_size = PAGE_SIZE;
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> index 7e79681634db..f22bbb325948 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> @@ -133,6 +133,10 @@ static const struct mana_stats_desc mana_phy_stats[] = {
> { "hc_tc7_tx_pause_phy", offsetof(struct mana_ethtool_phy_stats, tx_pause_tc7_phy) },
> };
>
> +static const char mana_priv_flags[MANA_PRIV_FLAG_MAX][ETH_GSTRING_LEN] = {
> + [MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF] = "full-page-rx"
> +};
> +
> static int mana_get_sset_count(struct net_device *ndev, int stringset)
> {
> struct mana_port_context *apc = netdev_priv(ndev);
> @@ -144,6 +148,10 @@ static int mana_get_sset_count(struct net_device *ndev, int stringset)
> ARRAY_SIZE(mana_phy_stats) +
> ARRAY_SIZE(mana_hc_stats) +
> num_queues * (MANA_STATS_RX_COUNT + MANA_STATS_TX_COUNT);
> +
> + case ETH_SS_PRIV_FLAGS:
> + return MANA_PRIV_FLAG_MAX;
> +
> default:
> return -EINVAL;
> }
> @@ -192,6 +200,14 @@ static void mana_get_strings_stats(struct mana_port_context *apc, u8 **data)
> }
> }
>
> +static void mana_get_strings_priv_flags(u8 **data)
> +{
> + int i;
> +
> + for (i = 0; i < MANA_PRIV_FLAG_MAX; i++)
> + ethtool_puts(data, mana_priv_flags[i]);
> +}
> +
> static void mana_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
> {
> struct mana_port_context *apc = netdev_priv(ndev);
> @@ -200,6 +216,9 @@ static void mana_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
> case ETH_SS_STATS:
> mana_get_strings_stats(apc, &data);
> break;
> + case ETH_SS_PRIV_FLAGS:
> + mana_get_strings_priv_flags(&data);
> + break;
> default:
> break;
> }
> @@ -590,6 +609,88 @@ static int mana_get_link_ksettings(struct net_device *ndev,
> return 0;
> }
>
> +static u32 mana_get_priv_flags(struct net_device *ndev)
> +{
> + struct mana_port_context *apc = netdev_priv(ndev);
> +
> + return apc->priv_flags;
> +}
> +
> +static int mana_set_priv_flags(struct net_device *ndev, u32 priv_flags)
> +{
> + struct mana_port_context *apc = netdev_priv(ndev);
> + u32 changed = apc->priv_flags ^ priv_flags;
> + u32 old_priv_flags = apc->priv_flags;
> + bool schedule_port_reset = false;
> + int err = 0;
> +
> + if (!changed)
> + return 0;
> +
> + /* Reject unknown bits */
> + if (priv_flags & ~GENMASK(MANA_PRIV_FLAG_MAX - 1, 0))
> + return -EINVAL;
Good. Explicit rejection ensures that there's no risk of bad value. I
think this is only required for the legacy ioctl interface, and won't be
able to have a bit set that isn't in your accepted list. However the
legacy ioctl interface looks like it doesn't do that double checking, so
its good to have this.
> +
> + if (changed & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF)) {
> + apc->priv_flags = priv_flags;
> +
In the (unlikely) event that you need another private flag in the
future, this bit seems like it shouldn't be inside the if block here. It
seems like you'd want to either do this at the end or up front. Of
course it doesn't matter as long as this is the only private flag you have.
> + if (!apc->port_is_up) {
> + /* Port is down, flag updated to apply on next up
> + * so just return.
> + */
> + return 0;
> + }
> +
> + /* Pre-allocate buffers to prevent failure in mana_attach
> + * later
> + */
> + err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> + if (err) {
> + netdev_err(ndev,
> + "Insufficient memory for new allocations\n");
> + apc->priv_flags = old_priv_flags;
> + return err;
> + }
> +
> + err = mana_detach(ndev, false);
> + if (err) {
> + netdev_err(ndev, "mana_detach failed: %d\n", err);
> + apc->priv_flags = old_priv_flags;
> +
> + /* Port is in an inconsistent state. Restore
> + * 'port_is_up' so that queue reset work handler
> + * can properly detach and re-attach.
> + */
> + apc->port_is_up = true;
> + schedule_port_reset = true;
> + goto out;
> + }
> +
> + err = mana_attach(ndev);
> + if (err) {
> + netdev_err(ndev, "mana_attach failed: %d\n", err);
> + apc->priv_flags = old_priv_flags;
> +
> + /* Restore 'port_is_up' so the reset work handler
> + * can properly detach/attach. Without this,
> + * the handler sees port_is_up=false and skips
> + * queue allocation, leaving the port dead.
> + */
> + apc->port_is_up = true;
> + schedule_port_reset = true;
> + }
I might have made this bit a separate function, but that comes from
history of working with older drivers which accumulated a larger number
of private flags. Given that we frown on adding new ones except in more
rare cases these days, this is probably fine.
> + }
> +
> +out:
> + mana_pre_dealloc_rxbufs(apc);
> +
> + if (schedule_port_reset)
> + queue_work(apc->ac->per_port_queue_reset_wq,
> + &apc->queue_reset_work);
> +
> + return err;
> +}
> +
> const struct ethtool_ops mana_ethtool_ops = {
> .supported_coalesce_params = ETHTOOL_COALESCE_RX_CQE_FRAMES,
> .get_ethtool_stats = mana_get_ethtool_stats,
> @@ -608,4 +709,6 @@ const struct ethtool_ops mana_ethtool_ops = {
> .set_ringparam = mana_set_ringparam,
> .get_link_ksettings = mana_get_link_ksettings,
> .get_link = ethtool_op_get_link,
> + .get_priv_flags = mana_get_priv_flags,
> + .set_priv_flags = mana_set_priv_flags,
> };
> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index d9c27310fd04..26fd5e041a47 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h
> @@ -30,6 +30,12 @@ enum TRI_STATE {
> TRI_STATE_TRUE = 1
> };
>
> +/* MANA ethtool private flag bit positions */
> +enum mana_priv_flag_bits {
> + MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF = 0,
> + MANA_PRIV_FLAG_MAX,
For cases like this, I find it helpful to add a comment indicating this
must be the last entry. (and in that case, drop the trailing comma).
> +};
> +
> /* Number of entries for hardware indirection table must be in power of 2 */
> #define MANA_INDIRECT_TABLE_MAX_SIZE 512
> #define MANA_INDIRECT_TABLE_DEF_SIZE 64
> @@ -531,6 +537,8 @@ struct mana_port_context {
> u32 rxbpre_headroom;
> u32 rxbpre_frag_count;
>
> + u32 priv_flags;
> +
> struct bpf_prog *bpf_prog;
>
> /* Create num_queues EQs, SQs, SQ-CQs, RQs and RQ-CQs, respectively. */
^ permalink raw reply
* [PATCH net-next v2] net: mana: Add Interrupt Moderation support
From: Haiyang Zhang @ 2026-06-04 23:41 UTC (permalink / raw)
To: linux-hyperv, netdev, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Simon Horman,
Shradha Gupta, Erni Sri Satya Vennela, Dipayaan Roy, Aditya Garg,
Kees Cook, Breno Leitao, linux-kernel, linux-rdma
Cc: paulros
From: Haiyang Zhang <haiyangz@microsoft.com>
Add Static and Dynamic Interrupt Moderation (DIM) support for
Rx and Tx.
Update queue creation procedure with new data struct with the related
settings.
Add functions to collect stat for DIM, and workers to update DIM data
and settings.
Update ethtool handler to get/set the moderation settings from a user.
By default, adaptive-rx/tx (DIM) are enabled.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
v2:
Updated with comments from Jedrzej.
---
drivers/net/ethernet/microsoft/Kconfig | 1 +
.../net/ethernet/microsoft/mana/gdma_main.c | 29 ++++
drivers/net/ethernet/microsoft/mana/mana_en.c | 105 +++++++++++++-
.../ethernet/microsoft/mana/mana_ethtool.c | 129 +++++++++++++++++-
include/net/mana/gdma.h | 24 +++-
include/net/mana/mana.h | 45 ++++++
6 files changed, 325 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/Kconfig b/drivers/net/ethernet/microsoft/Kconfig
index 3f36ee6a8ece..e9be18c92ca5 100644
--- a/drivers/net/ethernet/microsoft/Kconfig
+++ b/drivers/net/ethernet/microsoft/Kconfig
@@ -21,6 +21,7 @@ config MICROSOFT_MANA
depends on X86_64 || (ARM64 && !CPU_BIG_ENDIAN)
depends on PCI_HYPERV
select AUXILIARY_BUS
+ select DIMLIB
select PAGE_POOL
select NET_SHAPER
help
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 712a0881d720..9d7d39f7786a 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
/* Copyright (c) 2021, Microsoft Corporation. */
+#include <linux/bitfield.h>
#include <linux/debugfs.h>
#include <linux/module.h>
#include <linux/pci.h>
@@ -405,6 +406,7 @@ static int mana_gd_disable_queue(struct gdma_queue *queue)
#define DOORBELL_OFFSET_RQ 0x400
#define DOORBELL_OFFSET_CQ 0x800
#define DOORBELL_OFFSET_EQ 0xFF8
+#define DOORBELL_OFFSET_DIM 0x820
static void mana_gd_ring_doorbell(struct gdma_context *gc, u32 db_index,
enum gdma_queue_type q_type, u32 qid,
@@ -445,6 +447,16 @@ static void mana_gd_ring_doorbell(struct gdma_context *gc, u32 db_index,
addr += DOORBELL_OFFSET_SQ;
break;
+ case GDMA_DIM:
+ e.dim.id = qid;
+ e.dim.mod_usec = FIELD_GET(MANA_INTR_MODR_USEC_MAX, tail_ptr);
+ e.dim.mod_usec_vld = !!(tail_ptr & MANA_INTR_MODR_USEC_VLD);
+ e.dim.mod_comps = FIELD_GET(MANA_INTR_MODR_COMP_MASK, tail_ptr);
+ e.dim.mod_comps_vld = num_req;
+
+ addr += DOORBELL_OFFSET_DIM;
+ break;
+
default:
WARN_ON(1);
return;
@@ -479,6 +491,23 @@ void mana_gd_ring_cq(struct gdma_queue *cq, u8 arm_bit)
}
EXPORT_SYMBOL_NS(mana_gd_ring_cq, "NET_MANA");
+void mana_gd_ring_dim(struct gdma_queue *cq, u32 mod_usec, bool mod_usec_vld,
+ u32 mod_comps, bool mod_comps_vld)
+{
+ struct gdma_context *gc = cq->gdma_dev->gdma_context;
+ u32 dim_val;
+
+ /* Convert the DIM values to doorbell parameters */
+ dim_val = FIELD_PREP(MANA_INTR_MODR_USEC_MAX, mod_usec) |
+ FIELD_PREP(MANA_INTR_MODR_COMP_MASK, mod_comps);
+ if (mod_usec_vld)
+ dim_val |= MANA_INTR_MODR_USEC_VLD;
+
+ mana_gd_ring_doorbell(gc, cq->gdma_dev->doorbell, GDMA_DIM, cq->id,
+ dim_val, mod_comps_vld);
+}
+EXPORT_SYMBOL_NS(mana_gd_ring_dim, "NET_MANA");
+
#define MANA_SERVICE_PERIOD 10
static void mana_serv_rescan(struct pci_dev *pdev)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index db14357d3732..b1e0c444f414 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -1551,6 +1551,9 @@ int mana_create_wq_obj(struct mana_port_context *apc,
mana_gd_init_req_hdr(&req.hdr, MANA_CREATE_WQ_OBJ,
sizeof(req), sizeof(resp));
+
+ req.hdr.req.msg_version = GDMA_MESSAGE_V3;
+ req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
req.vport = vport;
req.wq_type = wq_type;
req.wq_gdma_region = wq_spec->gdma_region;
@@ -1559,6 +1562,9 @@ int mana_create_wq_obj(struct mana_port_context *apc,
req.cq_size = cq_spec->queue_size;
req.cq_moderation_ctx_id = cq_spec->modr_ctx_id;
req.cq_parent_qid = cq_spec->attached_eq;
+ req.req_cq_moderation = cq_spec->req_cq_moderation;
+ req.cq_moderation_comp = cq_spec->cq_moderation_comp;
+ req.cq_moderation_usec = cq_spec->cq_moderation_usec;
err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
sizeof(resp));
@@ -2256,6 +2262,70 @@ static void mana_poll_rx_cq(struct mana_cq *cq)
xdp_do_flush();
}
+static void mana_rx_dim_work(struct work_struct *work)
+{
+ struct dim *dim = container_of(work, struct dim, work);
+ struct dim_cq_moder cur_moder;
+ struct mana_cq *cq;
+
+ cur_moder = net_dim_get_rx_moderation(dim->mode, dim->profile_ix);
+ cq = container_of(dim, struct mana_cq, dim);
+
+ cur_moder.usec = min_t(u16, cur_moder.usec, MANA_INTR_MODR_USEC_MAX);
+ cur_moder.pkts = min_t(u16, cur_moder.pkts, MANA_INTR_MODR_COMP_MAX);
+
+ mana_gd_ring_dim(cq->gdma_cq, cur_moder.usec, true,
+ cur_moder.pkts, true);
+
+ dim->state = DIM_START_MEASURE;
+}
+
+static void mana_tx_dim_work(struct work_struct *work)
+{
+ struct dim *dim = container_of(work, struct dim, work);
+ struct dim_cq_moder cur_moder;
+ struct mana_cq *cq;
+
+ cur_moder = net_dim_get_tx_moderation(dim->mode, dim->profile_ix);
+ cq = container_of(dim, struct mana_cq, dim);
+
+ cur_moder.usec = min_t(u16, cur_moder.usec, MANA_INTR_MODR_USEC_MAX);
+ cur_moder.pkts = min_t(u16, cur_moder.pkts, MANA_INTR_MODR_COMP_MAX);
+
+ mana_gd_ring_dim(cq->gdma_cq, cur_moder.usec, true,
+ cur_moder.pkts, true);
+
+ dim->state = DIM_START_MEASURE;
+}
+
+static void mana_update_rx_dim(struct mana_cq *cq)
+{
+ struct mana_port_context *apc = netdev_priv(cq->rxq->ndev);
+ struct mana_rxq *rxq = cq->rxq;
+ struct dim_sample dim_sample = {};
+
+ if (!apc->rx_dim_enabled)
+ return;
+
+ dim_update_sample(READ_ONCE(cq->dim_event_ctr), rxq->stats.packets,
+ rxq->stats.bytes, &dim_sample);
+ net_dim(&cq->dim, &dim_sample);
+}
+
+static void mana_update_tx_dim(struct mana_cq *cq)
+{
+ struct mana_port_context *apc = netdev_priv(cq->txq->ndev);
+ struct mana_txq *txq = cq->txq;
+ struct dim_sample dim_sample = {};
+
+ if (!apc->tx_dim_enabled)
+ return;
+
+ dim_update_sample(READ_ONCE(cq->dim_event_ctr), txq->stats.packets,
+ txq->stats.bytes, &dim_sample);
+ net_dim(&cq->dim, &dim_sample);
+}
+
static int mana_cq_handler(void *context, struct gdma_queue *gdma_queue)
{
struct mana_cq *cq = context;
@@ -2274,7 +2344,13 @@ static int mana_cq_handler(void *context, struct gdma_queue *gdma_queue)
if (w < cq->budget) {
mana_gd_ring_cq(gdma_queue, SET_ARM_BIT);
cq->work_done_since_doorbell = 0;
- napi_complete_done(&cq->napi, w);
+
+ if (napi_complete_done(&cq->napi, w)) {
+ if (cq->type == MANA_CQ_TYPE_RX)
+ mana_update_rx_dim(cq);
+ else
+ mana_update_tx_dim(cq);
+ }
} else if (cq->work_done_since_doorbell >=
(cq->gdma_cq->queue_size / COMP_ENTRY_SIZE) * 4) {
/* MANA hardware requires at least one doorbell ring every 8
@@ -2306,6 +2382,7 @@ static void mana_schedule_napi(void *context, struct gdma_queue *gdma_queue)
{
struct mana_cq *cq = context;
+ WRITE_ONCE(cq->dim_event_ctr, cq->dim_event_ctr + 1);
napi_schedule_irqoff(&cq->napi);
}
@@ -2348,6 +2425,7 @@ static void mana_destroy_txq(struct mana_port_context *apc)
if (apc->tx_qp[i]->txq.napi_initialized) {
napi_synchronize(napi);
napi_disable_locked(napi);
+ cancel_work_sync(&apc->tx_qp[i]->tx_cq.dim.work);
netif_napi_del_locked(napi);
apc->tx_qp[i]->txq.napi_initialized = false;
}
@@ -2478,6 +2556,10 @@ static int mana_create_txq(struct mana_port_context *apc,
cq_spec.queue_size = cq->gdma_cq->queue_size;
cq_spec.modr_ctx_id = 0;
cq_spec.attached_eq = cq->gdma_cq->cq.parent->id;
+ cq_spec.req_cq_moderation = apc->tx_dim_enabled ||
+ (apc->intr_modr_tx_usec && apc->intr_modr_tx_comp);
+ cq_spec.cq_moderation_usec = apc->intr_modr_tx_usec;
+ cq_spec.cq_moderation_comp = apc->intr_modr_tx_comp;
err = mana_create_wq_obj(apc, apc->port_handle, GDMA_SQ,
&wq_spec, &cq_spec,
@@ -2512,6 +2594,9 @@ static int mana_create_txq(struct mana_port_context *apc,
napi_enable_locked(&cq->napi);
txq->napi_initialized = true;
+ INIT_WORK(&cq->dim.work, mana_tx_dim_work);
+ cq->dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
+
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
}
@@ -2546,6 +2631,7 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
napi_synchronize(napi);
napi_disable_locked(napi);
+ cancel_work_sync(&rxq->rx_cq.dim.work);
netif_napi_del_locked(napi);
}
@@ -2783,6 +2869,10 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
cq_spec.queue_size = cq->gdma_cq->queue_size;
cq_spec.modr_ctx_id = 0;
cq_spec.attached_eq = cq->gdma_cq->cq.parent->id;
+ cq_spec.req_cq_moderation = apc->rx_dim_enabled ||
+ (apc->intr_modr_rx_usec && apc->intr_modr_rx_comp);
+ cq_spec.cq_moderation_usec = apc->intr_modr_rx_usec;
+ cq_spec.cq_moderation_comp = apc->intr_modr_rx_comp;
err = mana_create_wq_obj(apc, apc->port_handle, GDMA_RQ,
&wq_spec, &cq_spec, &rxq->rxobj);
@@ -2818,6 +2908,9 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
napi_enable_locked(&cq->napi);
+ INIT_WORK(&cq->dim.work, mana_rx_dim_work);
+ cq->dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
+
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
out:
if (!err)
@@ -3450,6 +3543,16 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
apc->port_idx = port_idx;
apc->cqe_coalescing_enable = 0;
+ /* Initialize interrupt moderation settings if supported by HW */
+ if (gc->pf_cap_flags1 & GDMA_PF_CAP_FLAG_1_DYN_INTERRUPT_MODERATION) {
+ apc->intr_modr_rx_usec = MANA_INTR_MODR_USEC_DEF;
+ apc->intr_modr_rx_comp = MANA_INTR_MODR_COMP_DEF;
+ apc->intr_modr_tx_usec = MANA_INTR_MODR_USEC_DEF;
+ apc->intr_modr_tx_comp = MANA_INTR_MODR_COMP_DEF;
+ apc->rx_dim_enabled = MANA_ADAPTIVE_RX_DEF;
+ apc->tx_dim_enabled = MANA_ADAPTIVE_TX_DEF;
+ }
+
mutex_init(&apc->vport_mutex);
apc->vport_use_count = 0;
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index 04350973e19e..9347901dadd9 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -419,6 +419,15 @@ static int mana_get_coalesce(struct net_device *ndev,
!kernel_coal->rx_cqe_nsecs)
kernel_coal->rx_cqe_nsecs = MANA_RX_CQE_NSEC_DEF;
+ ec->rx_coalesce_usecs = apc->intr_modr_rx_usec;
+ ec->rx_max_coalesced_frames = apc->intr_modr_rx_comp;
+
+ ec->tx_coalesce_usecs = apc->intr_modr_tx_usec;
+ ec->tx_max_coalesced_frames = apc->intr_modr_tx_comp;
+
+ ec->use_adaptive_rx_coalesce = apc->rx_dim_enabled;
+ ec->use_adaptive_tx_coalesce = apc->tx_dim_enabled;
+
return 0;
}
@@ -428,9 +437,34 @@ static int mana_set_coalesce(struct net_device *ndev,
struct netlink_ext_ack *extack)
{
struct mana_port_context *apc = netdev_priv(ndev);
- u8 saved_cqe_coalescing_enable;
+ struct {
+ u16 intr_modr_rx_usec;
+ u16 intr_modr_rx_comp;
+ u16 intr_modr_tx_usec;
+ u16 intr_modr_tx_comp;
+ u8 cqe_coalescing_enable;
+ bool rx_dim_enabled;
+ bool tx_dim_enabled;
+ } saved;
+ bool modr_changed = false;
+ bool dim_changed = false;
+ struct gdma_context *gc;
int err;
+ gc = apc->ac->gdma_dev->gdma_context;
+
+ /* Both static and dynamic interrupt moderation (DIM) rely on the
+ * same HW capability advertised by the PF.
+ */
+ if ((ec->use_adaptive_rx_coalesce || ec->use_adaptive_tx_coalesce ||
+ ec->rx_coalesce_usecs || ec->tx_coalesce_usecs ||
+ ec->rx_max_coalesced_frames || ec->tx_max_coalesced_frames) &&
+ !(gc->pf_cap_flags1 & GDMA_PF_CAP_FLAG_1_DYN_INTERRUPT_MODERATION)) {
+ NL_SET_ERR_MSG(extack,
+ "Interrupt Moderation is not supported by HW");
+ return -EOPNOTSUPP;
+ }
+
if (kernel_coal->rx_cqe_frames != 1 &&
kernel_coal->rx_cqe_frames != MANA_RXCOMP_OOB_NUM_PPI) {
NL_SET_ERR_MSG_FMT(extack,
@@ -440,17 +474,94 @@ static int mana_set_coalesce(struct net_device *ndev,
return -EINVAL;
}
- saved_cqe_coalescing_enable = apc->cqe_coalescing_enable;
+ if (ec->rx_coalesce_usecs > MANA_INTR_MODR_USEC_MAX ||
+ ec->tx_coalesce_usecs > MANA_INTR_MODR_USEC_MAX) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "coalesce usecs must be <= %lu",
+ MANA_INTR_MODR_USEC_MAX);
+ return -EINVAL;
+ }
+
+ if (ec->rx_max_coalesced_frames > MANA_INTR_MODR_COMP_MAX ||
+ ec->tx_max_coalesced_frames > MANA_INTR_MODR_COMP_MAX) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "coalesce frames must be <= %lu",
+ MANA_INTR_MODR_COMP_MAX);
+ return -EINVAL;
+ }
+
+ if (ec->rx_coalesce_usecs != apc->intr_modr_rx_usec ||
+ ec->rx_max_coalesced_frames != apc->intr_modr_rx_comp ||
+ ec->tx_coalesce_usecs != apc->intr_modr_tx_usec ||
+ ec->tx_max_coalesced_frames != apc->intr_modr_tx_comp)
+ modr_changed = true;
+
+ saved.intr_modr_rx_usec = apc->intr_modr_rx_usec;
+ saved.intr_modr_rx_comp = apc->intr_modr_rx_comp;
+ saved.intr_modr_tx_usec = apc->intr_modr_tx_usec;
+ saved.intr_modr_tx_comp = apc->intr_modr_tx_comp;
+
+ apc->intr_modr_rx_usec = ec->rx_coalesce_usecs;
+ apc->intr_modr_rx_comp = ec->rx_max_coalesced_frames;
+ apc->intr_modr_tx_usec = ec->tx_coalesce_usecs;
+ apc->intr_modr_tx_comp = ec->tx_max_coalesced_frames;
+
+ if (!!ec->use_adaptive_rx_coalesce != apc->rx_dim_enabled ||
+ !!ec->use_adaptive_tx_coalesce != apc->tx_dim_enabled)
+ dim_changed = true;
+
+ saved.rx_dim_enabled = apc->rx_dim_enabled;
+ saved.tx_dim_enabled = apc->tx_dim_enabled;
+ apc->rx_dim_enabled = !!ec->use_adaptive_rx_coalesce;
+ apc->tx_dim_enabled = !!ec->use_adaptive_tx_coalesce;
+
+ saved.cqe_coalescing_enable = apc->cqe_coalescing_enable;
apc->cqe_coalescing_enable =
kernel_coal->rx_cqe_frames == MANA_RXCOMP_OOB_NUM_PPI;
if (!apc->port_is_up)
return 0;
- err = mana_config_rss(apc, TRI_STATE_TRUE, false, false);
- if (err)
- apc->cqe_coalescing_enable = saved_cqe_coalescing_enable;
+ if (apc->cqe_coalescing_enable != saved.cqe_coalescing_enable &&
+ !modr_changed && !dim_changed) {
+ /* If only CQE coalescing setting is changed, we can just update
+ * RSS configuration.
+ */
+ err = mana_config_rss(apc, TRI_STATE_TRUE, false, false);
+ if (err) {
+ netdev_err(ndev, "Change CQE coalescing failed: %d\n",
+ err);
+ apc->cqe_coalescing_enable =
+ saved.cqe_coalescing_enable;
+ return err;
+ }
+ return 0;
+ }
+
+ if (modr_changed || dim_changed) {
+ err = mana_detach(ndev, false);
+ if (err) {
+ netdev_err(ndev, "mana_detach failed: %d\n", err);
+ goto restore_modr;
+ }
+
+ err = mana_attach(ndev);
+ if (err) {
+ netdev_err(ndev, "mana_attach failed: %d\n", err);
+ goto restore_modr;
+ }
+ }
+
+ return 0;
+restore_modr:
+ apc->cqe_coalescing_enable = saved.cqe_coalescing_enable;
+ apc->intr_modr_rx_usec = saved.intr_modr_rx_usec;
+ apc->intr_modr_rx_comp = saved.intr_modr_rx_comp;
+ apc->intr_modr_tx_usec = saved.intr_modr_tx_usec;
+ apc->intr_modr_tx_comp = saved.intr_modr_tx_comp;
+ apc->rx_dim_enabled = saved.rx_dim_enabled;
+ apc->tx_dim_enabled = saved.tx_dim_enabled;
return err;
}
@@ -574,7 +685,13 @@ static int mana_get_link_ksettings(struct net_device *ndev,
}
const struct ethtool_ops mana_ethtool_ops = {
- .supported_coalesce_params = ETHTOOL_COALESCE_RX_CQE_FRAMES,
+ .supported_coalesce_params = ETHTOOL_COALESCE_RX_CQE_FRAMES |
+ ETHTOOL_COALESCE_RX_USECS |
+ ETHTOOL_COALESCE_RX_MAX_FRAMES |
+ ETHTOOL_COALESCE_TX_USECS |
+ ETHTOOL_COALESCE_TX_MAX_FRAMES |
+ ETHTOOL_COALESCE_USE_ADAPTIVE_RX |
+ ETHTOOL_COALESCE_USE_ADAPTIVE_TX,
.get_ethtool_stats = mana_get_ethtool_stats,
.get_sset_count = mana_get_sset_count,
.get_strings = mana_get_strings,
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 70d62bc32837..86d58b0150c7 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -47,6 +47,7 @@ enum gdma_queue_type {
GDMA_RQ,
GDMA_CQ,
GDMA_EQ,
+ GDMA_DIM,
};
enum gdma_work_request_flags {
@@ -126,6 +127,17 @@ union gdma_doorbell_entry {
u64 tail_ptr : 31;
u64 arm : 1;
} eq;
+
+ struct {
+ u64 id : 24;
+ u64 reserved : 8;
+ u64 mod_usec : 10;
+ u64 reserve1 : 5;
+ u64 mod_usec_vld : 1;
+ u64 mod_comps : 8;
+ u64 reserve2 : 7;
+ u64 mod_comps_vld: 1;
+ } dim;
}; /* HW DATA */
struct gdma_msg_hdr {
@@ -484,6 +496,9 @@ void mana_gd_ring_cq(struct gdma_queue *cq, u8 arm_bit);
int mana_schedule_serv_work(struct gdma_context *gc, enum gdma_eqe_type type);
+void mana_gd_ring_dim(struct gdma_queue *cq, u32 mod_usec, bool mod_usec_vld,
+ u32 mod_comps, bool mod_comps_vld);
+
struct gdma_wqe {
u32 reserved :24;
u32 last_vbytes :8;
@@ -629,6 +644,9 @@ enum {
/* Driver supports self recovery on Hardware Channel timeouts */
#define GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY BIT(25)
+/* Driver supports dynamic interrupt moderation - DIM */
+#define GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION BIT(28)
+
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
@@ -643,7 +661,8 @@ enum {
GDMA_DRV_CAP_FLAG_1_SKB_LINEARIZE | \
GDMA_DRV_CAP_FLAG_1_PROBE_RECOVERY | \
GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY | \
- GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY)
+ GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY | \
+ GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION)
#define GDMA_DRV_CAP_FLAGS2 0
@@ -679,6 +698,9 @@ struct gdma_verify_ver_req {
u8 os_ver_str4[128];
}; /* HW DATA */
+/* HW supports dynamic interrupt moderation - DIM */
+#define GDMA_PF_CAP_FLAG_1_DYN_INTERRUPT_MODERATION BIT(15)
+
struct gdma_verify_ver_resp {
struct gdma_resp_hdr hdr;
u64 gdma_protocol_ver;
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index d9c27310fd04..582e59f078e7 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -4,6 +4,7 @@
#ifndef _MANA_H
#define _MANA_H
+#include <linux/dim.h>
#include <net/xdp.h>
#include <net/net_shaper.h>
@@ -64,6 +65,19 @@ enum TRI_STATE {
/* Maximum number of packets per coalesced CQE */
#define MANA_RXCOMP_OOB_NUM_PPI 4
+/* Default/max interrupt moderation settings */
+#define MANA_INTR_MODR_USEC_DEF 0
+#define MANA_INTR_MODR_COMP_DEF 0
+
+#define MANA_ADAPTIVE_RX_DEF true
+#define MANA_ADAPTIVE_TX_DEF true
+
+/* DIM doorbell value field layout */
+#define MANA_INTR_MODR_USEC_MAX GENMASK(9, 0)
+#define MANA_INTR_MODR_USEC_VLD BIT(15)
+#define MANA_INTR_MODR_COMP_MAX GENMASK(7, 0)
+#define MANA_INTR_MODR_COMP_MASK GENMASK(23, 16)
+
/* Update this count whenever the respective structures are changed */
#define MANA_STATS_RX_COUNT (6 + MANA_RXCOMP_OOB_NUM_PPI - 1)
#define MANA_STATS_TX_COUNT 11
@@ -297,6 +311,10 @@ struct mana_cq {
int work_done;
int work_done_since_doorbell;
int budget;
+
+ /* DIM - Dynamic Interrupt Moderation */
+ struct dim dim;
+ u16 dim_event_ctr;
};
struct mana_recv_buf_oob {
@@ -562,6 +580,15 @@ struct mana_port_context {
u8 cqe_coalescing_enable;
u32 cqe_coalescing_timeout_ns;
+ /* Interrupt moderation settings */
+ u16 intr_modr_rx_usec;
+ u16 intr_modr_rx_comp;
+ u16 intr_modr_tx_usec;
+ u16 intr_modr_tx_comp;
+
+ bool rx_dim_enabled;
+ bool tx_dim_enabled;
+
struct mana_ethtool_stats eth_stats;
struct mana_ethtool_phy_stats phy_stats;
@@ -622,6 +649,9 @@ struct mana_obj_spec {
u32 queue_size;
u32 attached_eq;
u32 modr_ctx_id;
+ u8 req_cq_moderation;
+ u16 cq_moderation_comp;
+ u16 cq_moderation_usec;
};
enum mana_command_code {
@@ -753,6 +783,15 @@ struct mana_create_wqobj_req {
u32 cq_size;
u32 cq_moderation_ctx_id;
u32 cq_parent_qid;
+
+ /* V2 */
+ u8 allow_rqwqe_chain;
+
+ /* V3 */
+ u8 req_cq_moderation;
+ u16 cq_moderation_comp;
+ u16 cq_moderation_usec;
+ u8 reserved2[2];
}; /* HW DATA */
struct mana_create_wqobj_resp {
@@ -760,6 +799,12 @@ struct mana_create_wqobj_resp {
u32 wq_id;
u32 cq_id;
mana_handle_t wq_obj;
+
+ /* V2 */
+ u16 cq_moderation_comp;
+ u16 cq_moderation_usec;
+ u8 cq_moderation_enabled;
+ u8 reserved1[3];
}; /* HW DATA */
/* Destroy WQ Object */
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox