* Re: [PATCH net-next v3 12/15] ibmveth: Expose per-queue buffer pool details via sysfs
From: Simon Horman @ 2026-07-14 13:01 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-13-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Expose per-queue buffer pool details via sysfs
This commit adds a read-only sysfs attribute buffer_pools to the VIO device
to expose per-queue buffer pool details dynamically. It outputs runtime per-queue
buffer pressure metrics like size, active, and available buffers.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 4e3f49b6346fd..ecc472ee8f71f 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -2896,6 +2896,52 @@ static const struct net_device_ops ibmveth_netdev_ops = {
[ ... ]
> +static ssize_t buffer_pools_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct net_device *netdev = dev_get_drvdata(dev);
> + struct ibmveth_adapter *adapter = netdev_priv(netdev);
> + int len = 0;
> + int i, j;
> +
> + len += scnprintf(buf + len, PAGE_SIZE - len,
> + "Queue Pool Size BuffSize Active Available\n");
> + len += scnprintf(buf + len, PAGE_SIZE - len,
> + "----- ---- ---- -------- ------ ---------\n");
[Severity: Medium]
Does this buffer_pools_show() implementation violate the sysfs "one value
per file" design rule?
The attribute appears to dump a complex, multi-line ASCII table with headers
and columns rather than exposing a single value per sysfs file.
[ ... ]
> @@ -3056,6 +3102,14 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
>
> netdev_dbg(netdev, "registered\n");
>
> + rc = sysfs_create_group(&dev->dev.kobj, &ibmveth_attr_group);
> + if (rc) {
> + netdev_err(netdev, "failed to create sysfs attributes rc=%d\n", rc);
> + unregister_netdev(netdev);
> + free_netdev(netdev);
> + return rc;
> + }
[Severity: High]
If userspace brings up the interface immediately after register_netdev()
succeeds, could adapter->work be scheduled before sysfs_create_group() fails?
If this race occurs, unregister_netdev() does not synchronously cancel the
work.
Could the global workqueue execute the pending adapter->work item on freed
memory after free_netdev() is called in this ibmveth_probe() error path?
[Severity: High]
This is a pre-existing issue in older error paths of ibmveth_probe(), but it
is duplicated here: does this error path leak the adapter->rx_buff_pool
kobjects?
Since kobject_put() is not called on the pool kobjects before free_netdev()
is executed, the kobjects remain attached to the VIO device with dangling
pointers to the freed adapter memory.
Could subsequent sysfs accesses to those pool kobjects trigger a
use-after-free?
^ permalink raw reply
* Re: [PATCH net-next v3 11/15] ibmveth: Add per-queue RX and TX statistics collection and reporting
From: Simon Horman @ 2026-07-14 12:59 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-12-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Add per-queue RX and TX statistics collection and reporting
This commit introduces per-queue statistics tracking for both receive and
transmit paths in the ibmveth driver. The counters are aggregated for legacy
interfaces and exposed individually via ethtool stringsets.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 863e5c68b42c7..4e3f49b6346fd 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -1655,6 +1655,10 @@ static int ibmveth_open(struct net_device *netdev)
> if (rc)
> goto out_cleanup_rx_interrupts;
>
> + rc = ibmveth_alloc_tx_qstats(adapter);
> + if (rc)
> + goto out_free_tx_resources;
> +
> netif_tx_start_all_queues(netdev);
>
> netdev_dbg(netdev, "open complete\n");
[ ... ]
> out_cleanup_rx_interrupts:
> ibmveth_cleanup_rx_interrupts(adapter);
> out_free_tx_resources:
> ibmveth_free_tx_resources(adapter);
[Severity: High]
Does the goto out_free_tx_resources on ibmveth_alloc_tx_qstats() failure
bypass the cleanup for RX interrupts?
Since out_free_tx_resources is physically below out_cleanup_rx_interrupts in
ibmveth_open(), failing here leaves the IRQs requested and NAPI enabled. If
an interrupt subsequently fires, could the handler access the freed RX queue
memory and cause a kernel panic?
[ ... ]
> @@ -1699,6 +1699,7 @@ static int ibmveth_close(struct net_device *netdev)
> }
> }
>
> + ibmveth_free_tx_qstats(adapter);
> ibmveth_free_tx_resources(adapter);
> ibmveth_cleanup_rx_interrupts(adapter);
> ibmveth_update_rx_no_buffer(adapter);
[ ... ]
> ibmveth_free_rx_qstats(adapter);
[Severity: High]
If ibmveth_close() is called to bring down the interface, does this
unconditionally free adapter->tx_qstats and adapter->rx_qstats without a
grace period?
Since ibmveth_get_stats64() can be called concurrently under RCU, and
ibmveth_start_xmit() can run concurrently if the datapath is not completely
synchronized, could this lead to a use-after-free or NULL pointer dereference
when these functions access the freed arrays?
[ ... ]
> @@ -1967,22 +1967,107 @@ static int ibmveth_set_features(struct net_device *dev,
[ ... ]
> +static void ibmveth_aggregate_rx_qstats(struct ibmveth_adapter *adapter)
> +{
> + u64 total_invalid = 0;
> + u64 total_large = 0;
> + int i;
> +
> + if (!adapter->rx_qstats)
> + return;
> +
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + total_invalid += adapter->rx_qstats[i].invalid_buffers;
> + total_large += adapter->rx_qstats[i].large_packets;
> + }
> +
> + adapter->rx_invalid_buffer = total_invalid;
> + adapter->rx_large_packets = total_large;
> +}
[Severity: High]
Does overwriting adapter->rx_invalid_buffer and other persistent global
counters with the dynamically allocated adapter->rx_qstats values cause a
permanent loss of interface statistics?
When the interface is brought down and back up, ibmveth_close() frees the
queue stats arrays and ibmveth_open() reallocates them as zeroed. Will this
cause all historical statistics to be wiped out on every interface reset,
breaking standard persistence behavior?
[ ... ]
> +static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> +{
[ ... ]
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + ethtool_sprintf(&p, "rx%d_packets", i);
> + ethtool_sprintf(&p, "rx%d_bytes", i);
[Severity: Low]
Are these private ethtool stringsets duplicating standard kernel statistics?
According to the subsystem guidelines, per-queue packet and byte statistics
that have a standard UAPI representation (like struct netdev_queue_stats_rx
exported via netlink) should not be duplicated as private ethtool strings.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.h b/drivers/net/ethernet/ibm/ibmveth.h
> index f7b20fd01acb5..390c660af979e 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.h
> +++ b/drivers/net/ethernet/ibm/ibmveth.h
> @@ -316,9 +316,21 @@ struct ibmveth_rx_queue_stats {
> u64 no_buffer_drops;
> };
>
> +struct ibmveth_tx_queue_stats {
> + u64 packets;
> + u64 bytes;
> + u64 large_packets;
> + u64 dropped_packets;
> + u64 send_failures;
> + u64 checksum_offload;
> +};
[Severity: Medium]
Will this structure layout prevent cache line false sharing?
The commit message mentions tracking transmit counters per TX queue to avoid
cache line contention, but struct ibmveth_tx_queue_stats appears to be
exactly 48 bytes and lacks cacheline alignment annotations like
____cacheline_aligned. When allocated as a contiguous array, will multiple
CPUs processing different queues end up writing to the same hardware
cacheline?
^ permalink raw reply
* Re: [PATCH net-next v3 10/15] ibmveth: Enable multi-queue RX receive path
From: Simon Horman @ 2026-07-14 12:55 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-11-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Enable multi-queue RX receive path
This is the first patch that sets multi_queue from H_ILLAN_ATTRIBUTES
and switches registration, buffer posting, and receive to the MQ
hcall path. It also raises num_rx_queues and enables per-queue NAPI.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index b3b3886c3eedb..863e5c68b42c7 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -826,15 +841,23 @@ static void ibmveth_replenish_buffer_pool(struct ibmveth_adapter *adapter,
> */
> static void ibmveth_update_rx_no_buffer(struct ibmveth_adapter *adapter)
> {
> - __be64 *p = adapter->buffer_list_addr[0] + 4096 - 8;
> + int i;
> +
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + __be64 *p = adapter->buffer_list_addr[i] + 4096 - 8;
> + u64 drops = be64_to_cpup(p);
>
> - adapter->rx_no_buffer = be64_to_cpup(p);
> + if (i == 0)
> + adapter->rx_no_buffer = drops;
> + }
> }
[Severity: Medium]
Will this logic silently discard the hardware drop counts for all queues
other than queue 0?
Since ethtool -S reports the global adapter->rx_no_buffer statistic, shouldn't
this loop accumulate the drops across all queues instead of only recording
them when evaluating the first queue?
[ ... ]
> @@ -842,6 +865,8 @@ static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
> int queue_index)
> {
> + struct ibmveth_rx_q *rxq = &adapter->rx_queue[queue_index];
> + unsigned long flags;
> int i;
>
> if (queue_index >= adapter->num_rx_queues)
> @@ -842,6 +865,8 @@ static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
>
> adapter->replenish_task_cycles++;
>
> + spin_lock_irqsave(&rxq->replenish_lock, flags);
> +
[Severity: Medium]
With multi-queue RX enabled, can multiple NAPI threads execute this task
concurrently for different queues?
If so, does this non-atomic increment of the global replenish_task_cycles
stat risk data corruption, torn reads on 32-bit platforms, or lost updates?
Similarly, in ibmveth_replenish_buffer_pool():
drivers/net/ethernet/ibm/ibmveth.c:ibmveth_replenish_buffer_pool() {
...
buffers_added += filled;
adapter->replenish_add_buff_success += filled;
remaining -= filled;
...
}
Even though the caller acquires the per-queue rxq->replenish_lock,
replenish_add_buff_success is a global adapter statistic. Does this update
also race with other CPUs holding the locks for their respective queues?
[ ... ]
> @@ -2268,34 +2299,40 @@ static void ibmveth_rx_csum_helper(struct sk_buff *skb,
>
> static int ibmveth_poll(struct napi_struct *napi, int budget)
> {
[ ... ]
> } else {
> struct sk_buff *skb, *new_skb;
> - int length = ibmveth_rxq_frame_length(adapter);
> - int offset = ibmveth_rxq_frame_offset(adapter);
> - int csum_good = ibmveth_rxq_csum_good(adapter);
> - int lrg_pkt = ibmveth_rxq_large_packet(adapter);
> + int length = ibmveth_rxq_frame_length(adapter, queue_index);
> + int offset = ibmveth_rxq_frame_offset(adapter, queue_index);
> + int csum_good = ibmveth_rxq_csum_good(adapter, queue_index);
> + int lrg_pkt = ibmveth_rxq_large_packet(adapter, queue_index);
[Severity: High]
This isn't a bug introduced by this patch, but does this code validate the
length and offset provided by the hardware descriptor before modifying the
SKB bounds?
Later in ibmveth_poll(), these values are used directly:
drivers/net/ethernet/ibm/ibmveth.c:ibmveth_poll() {
...
skb_reserve(skb, offset);
skb_put(skb, length);
...
}
If the hypervisor provides an untrusted or artificially large length, could
the call to skb_put() trigger an skb_over_panic() and crash the guest
kernel?
[ ... ]
Also, I noticed a potential memory leak in the error unwinding path of
ibmveth_open():
drivers/net/ethernet/ibm/ibmveth.c:ibmveth_open() {
...
if (rc) {
netdev_err(netdev, "failed to set number of rx queues\n");
goto out_unregister_queues;
}
...
out_free_buffer_pools:
ibmveth_free_buffer_pools(adapter);
out_unregister_queues:
ibmveth_dispose_subordinate_irq_mappings(adapter);
ibmveth_free_all_queues(adapter);
}
[Severity: Medium]
This is a pre-existing issue, but if netif_set_real_num_rx_queues() fails,
the code jumps to the out_unregister_queues label.
Does this bypass out_free_buffer_pools, leaving the RX buffer pools unfreed
and causing a memory leak when cleaning up after a failure?
^ permalink raw reply
* Re: [PATCH net-next v3 09/15] ibmveth: Add queue-aware RX buffer submit helper for MQ
From: Simon Horman @ 2026-07-14 12:50 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-10-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Add queue-aware RX buffer submit helper for MQ
This patch updates the ibmveth driver's buffer replenishment path to
be queue-aware for multi-queue receive support. It adds a helper to select
the correct hypervisor call and handles fallback to single-buffer mode if
batch operations are unsupported.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index fa2d4777ffc7b..b3b3886c3eedb 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -597,11 +597,73 @@ static inline void ibmveth_flush_buffer(void *addr, unsigned long length)
[ ... ]
> +static long ibmveth_add_logical_lan_buffers(struct ibmveth_adapter *adapter,
> + union ibmveth_buf_desc *descs,
> + int filled,
> + unsigned long buff_size,
> + int queue_index)
> +{
> + struct vio_dev *vdev = adapter->vdev;
> + unsigned long rc;
> +
> + if (adapter->multi_queue) {
> + unsigned long buffersznum = (buff_size << 32) | filled;
> + unsigned long ioba[IBMVETH_MAX_RX_PER_HCALL / 2] = {0};
> + int i;
> +
> + /* Pack descriptor addresses into ioba pairs.
> + * Each ioba holds two 32-bit addresses packed into 64 bits:
> + * - Even descriptors (0,2,4...) go in high 32 bits
> + * - Odd descriptors (1,3,5...) go in low 32 bits
> + */
> + for (i = 0; i < filled && i < IBMVETH_MAX_RX_PER_HCALL; i++) {
> + int pair_idx = i / 2; /* Which pair: 0-5 */
> + int is_high = (i % 2 == 0); /* High or low 32 bits */
> +
> + if (is_high)
> + ioba[pair_idx] = (unsigned long)descs[i].fields.address << 32;
> + else
> + ioba[pair_idx] |= descs[i].fields.address;
> + }
> +
> + rc = h_add_logical_lan_buffers_queue(vdev->unit_address,
> + adapter->queue_handle[queue_index],
> + buffersznum,
> + ioba[0], ioba[1], ioba[2],
> + ioba[3], ioba[4], ioba[5]);
> + adapter->hcall_stats.add_bufs_queue++;
[Severity: Medium]
Will this counter increment lose updates when multiple NAPI poll routines run
concurrently on different CPUs?
Since multi-queue enables concurrent execution, it seems global statistic
counters like hcall_stats.add_bufs_queue and replenish_add_buff_success might
need atomic or per-CPU operations to avoid read-modify-write data races.
> + } else if (filled == 1) {
> + rc = h_add_logical_lan_buffer(vdev->unit_address,
> + descs[0].desc);
> + adapter->hcall_stats.add_buf++;
> + } else {
> + rc = h_add_logical_lan_buffers(vdev->unit_address,
> + descs[0].desc, descs[1].desc,
> + descs[2].desc, descs[3].desc,
> + descs[4].desc, descs[5].desc,
> + descs[6].desc, descs[7].desc);
> + adapter->hcall_stats.add_bufs++;
> + }
> +
> + return rc;
> +}
[ ... ]
> @@ -745,24 +798,19 @@ static void ibmveth_replenish_buffer_pool(struct ibmveth_adapter *adapter,
> }
> adapter->replenish_add_buff_failure += filled;
>
> - /*
> - * If multi rx buffers hcall is no longer supported by FW
> - * e.g. in the case of Live Partition Migration
> - */
> - if (batch > 1 && lpar_rc == H_FUNCTION) {
> - /*
[ ... ]
> + if (lpar_rc == H_FUNCTION) {
> + if (adapter->multi_queue) {
> + netdev_err(adapter->netdev,
> + "Unexpected H_FUNCTION from multi-queue buffer add (queue=%d, batch=%d)\n",
> + queue_index, batch);
> + break;
> + } else if (batch > 1) {
> + netdev_warn(adapter->netdev,
> + "H_FUNCTION from legacy batch buffer add (batch=%d), falling back to single buffer mode\n",
> + batch);
> + adapter->rx_buffers_per_hcall = 1;
> + continue;
[Severity: High]
Does this continue statement cause an infinite loop?
Looking at ibmveth_replenish_buffer_pool(), the local batch variable is set
before the while loop:
batch = adapter->rx_buffers_per_hcall;
while (remaining > 0) {
...
If the fallback updates adapter->rx_buffers_per_hcall to 1 and issues a continue,
the loop will restart but the local batch variable will retain its original
value.
Would this lead to the loop repeatedly attempting the unsupported batch size
and failing with H_FUNCTION forever?
> + }
> }
> break;
> }
> @@ -784,18 +832,24 @@ static void ibmveth_update_rx_no_buffer(struct ibmveth_adapter *adapter)
> }
>
> /* replenish routine */
> -static void ibmveth_replenish_task(struct ibmveth_adapter *adapter)
> +static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
> + int queue_index)
> {
> int i;
>
> + if (queue_index >= adapter->num_rx_queues)
> + return;
> +
> adapter->replenish_task_cycles++;
[Severity: Medium]
Similar to the hcall_stats counters above, could this increment suffer from data
races when multiple queues are processed concurrently across different CPUs?
^ permalink raw reply
* Re: [PATCH net-next v3 08/15] ibmveth: Refactor open/close into MQ-ready resource pipeline
From: Simon Horman @ 2026-07-14 12:47 UTC (permalink / raw)
To: Mingming Cao
Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
linuxppc-dev, maddy, mpe, Dave Marquardt
In-Reply-To: <20260706193603.8039-9-mmc@linux.ibm.com>
On Mon, Jul 06, 2026 at 12:35:56PM -0700, Mingming Cao wrote:
...
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
...
> /**
> * ibmveth_register_logical_lan_queue - Register subordinate queue with hypervisor
> * @adapter: ibmveth adapter structure
> @@ -1466,208 +1479,108 @@ ibmveth_register_rx_queues(struct ibmveth_adapter *adapter, u64 mac_address)
> static int ibmveth_open(struct net_device *netdev)
> {
> struct ibmveth_adapter *adapter = netdev_priv(netdev);
> - u64 mac_address;
> + u64 mac_address = ether_addr_to_u64(netdev->dev_addr);
> int rxq_entries = 1;
> - unsigned long lpar_rc;
> int rc;
> - union ibmveth_buf_desc rxq_desc;
> int i;
> - struct device *dev;
>
> netdev_dbg(netdev, "open starting\n");
>
> - napi_enable(&adapter->napi[0]);
> -
> - for(i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
> + for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
> rxq_entries += adapter->rx_buff_pool[0][i].size;
>
> - rc = -ENOMEM;
> - adapter->buffer_list_addr[0] = (void *)get_zeroed_page(GFP_KERNEL);
> - if (!adapter->buffer_list_addr[0]) {
> - netdev_err(netdev, "unable to allocate list pages\n");
> + rc = ibmveth_alloc_rx_qstats(adapter);
> + if (rc)
> goto out;
> - }
>
> - adapter->filter_list_addr = (void*) get_zeroed_page(GFP_KERNEL);
> - if (!adapter->filter_list_addr) {
> - netdev_err(netdev, "unable to allocate filter pages\n");
> - goto out_free_buffer_list;
> - }
> -
> - dev = &adapter->vdev->dev;
> + rc = ibmveth_alloc_filter_list(adapter);
> + if (rc)
> + goto out_free_rx_qstats;
>
> - adapter->rx_queue[0].queue_len = sizeof(struct ibmveth_rx_q_entry) *
> - rxq_entries;
> - adapter->rx_queue[0].queue_addr =
> - dma_alloc_coherent(dev, adapter->rx_queue[0].queue_len,
> - &adapter->rx_queue[0].queue_dma, GFP_KERNEL);
> - if (!adapter->rx_queue[0].queue_addr)
> + rc = ibmveth_alloc_rx_queues(adapter, rxq_entries);
> + if (rc)
> goto out_free_filter_list;
>
> - adapter->buffer_list_dma[0] = dma_map_single(dev,
> - adapter->buffer_list_addr[0],
> - 4096, DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dev, adapter->buffer_list_dma[0])) {
> - netdev_err(netdev, "unable to map buffer list pages\n");
> + rc = ibmveth_alloc_buffer_pools(adapter);
> + if (rc)
> goto out_free_queue_mem;
> - }
>
> - adapter->filter_list_dma = dma_map_single(dev,
> - adapter->filter_list_addr, 4096, DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dev, adapter->filter_list_dma)) {
> - netdev_err(netdev, "unable to map filter list pages\n");
> - goto out_unmap_buffer_list;
> - }
> + rc = ibmveth_register_rx_queues(adapter, mac_address);
> + if (rc)
> + goto out_free_buffer_pools;
>
> - for (i = 0; i < netdev->real_num_tx_queues; i++) {
> - if (ibmveth_allocate_tx_ltb(adapter, i))
> - goto out_free_tx_ltb;
> + rc = netif_set_real_num_rx_queues(netdev, adapter->num_rx_queues);
> + if (rc) {
> + netdev_err(netdev, "failed to set number of rx queues\n");
> + goto out_unregister_queues;
> }
>
> - adapter->rx_queue[0].index = 0;
> - adapter->rx_queue[0].num_slots = rxq_entries;
> - adapter->rx_queue[0].toggle = 1;
> -
> - mac_address = ether_addr_to_u64(netdev->dev_addr);
> -
> - rxq_desc.fields.flags_len = IBMVETH_BUF_VALID |
> - adapter->rx_queue[0].queue_len;
> - rxq_desc.fields.address = adapter->rx_queue[0].queue_dma;
> -
> - netdev_dbg(netdev, "buffer list @ 0x%p\n", adapter->buffer_list_addr[0]);
> - netdev_dbg(netdev, "filter list @ 0x%p\n", adapter->filter_list_addr);
> - netdev_dbg(netdev, "receive q @ 0x%p\n", adapter->rx_queue[0].queue_addr);
> -
> - h_vio_signal(adapter->vdev->unit_address, VIO_IRQ_DISABLE);
> -
> - lpar_rc = ibmveth_register_logical_lan(adapter, rxq_desc, mac_address);
> -
> - if (lpar_rc != H_SUCCESS) {
> - netdev_err(netdev, "h_register_logical_lan failed with %ld\n",
> - lpar_rc);
> - netdev_err(netdev, "buffer TCE:0x%llx filter TCE:0x%llx rxq "
> - "desc:0x%llx MAC:0x%llx\n",
> - adapter->buffer_list_dma[0],
> - adapter->filter_list_dma,
> - rxq_desc.desc,
> - mac_address);
> - rc = -ENONET;
> - goto out_unmap_filter_list;
> - }
> + rc = ibmveth_setup_rx_interrupts(adapter);
> + if (rc)
> + goto out_unregister_queues;
>
> - for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) {
> - if (!adapter->rx_buff_pool[0][i].active)
> - continue;
> - if (ibmveth_alloc_buffer_pool(&adapter->rx_buff_pool[0][i])) {
> - netdev_err(netdev, "unable to alloc pool\n");
> - adapter->rx_buff_pool[0][i].active = 0;
> - rc = -ENOMEM;
> - goto out_free_buffer_pools;
> + if (adapter->num_rx_queues > 1) {
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + netdev_dbg(netdev, "initial replenish cycle for queue %d\n", i);
> + ibmveth_replenish_task(adapter, i);
ibmveth_replenish_task() only has one parameter
until a later patch in this series.
> }
> + } else {
> + netdev_dbg(netdev, "initial replenish cycle\n");
> + ibmveth_interrupt(adapter->queue_irq[0], &adapter->napi[0]);
> }
>
> - netdev_dbg(netdev, "registering irq 0x%x\n", netdev->irq);
> - rc = request_irq(netdev->irq, ibmveth_interrupt, 0, netdev->name,
> - netdev);
> - if (rc != 0) {
> - netdev_err(netdev, "unable to request irq 0x%x, rc %d\n",
> - netdev->irq, rc);
> - do {
> - lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
> - } while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
> -
> - goto out_free_buffer_pools;
> - }
> -
> - rc = -ENOMEM;
> -
> - netdev_dbg(netdev, "initial replenish cycle\n");
> - ibmveth_interrupt(netdev->irq, netdev);
> + rc = ibmveth_alloc_tx_resources(adapter);
> + if (rc)
> + goto out_cleanup_rx_interrupts;
>
> netif_tx_start_all_queues(netdev);
>
> netdev_dbg(netdev, "open complete\n");
> -
> return 0;
>
> +out_cleanup_rx_interrupts:
> + ibmveth_cleanup_rx_interrupts(adapter);
> +out_free_tx_resources:
> + ibmveth_free_tx_resources(adapter);
The out_free_tx_resources label is unused until a later patch of this
series, so it should be added in that patch rather than this one.
And it's not clear to me that ibmveth_free_tx_resources() should
be called when jumping to out_cleanup_rx_interrupts as
in that case ibmveth_alloc_tx_resources() hasn't run successfully.
The AI-generated review on sashiko.dev also highlights the error handling
here:
"Are the unwind labels ordered incorrectly here?
"If ibmveth_setup_rx_interrupts() fails, it jumps to
out_unregister_queues, which is placed after out_free_buffer_pools. Does
this mean we skip freeing the buffer pools and leak memory?
"Also, if ibmveth_alloc_tx_resources() fails, it internally frees
partially allocated LTBs. It then jumps to out_cleanup_rx_interrupts and
falls through to out_free_tx_resources. Because ibmveth_free_tx_ltb()
calls dma_unmap_single() unconditionally without checking or zeroing
tx_ltb_dma, will this cause a double free and an invalid DMA unmap?
"Finally, in the fall-through path from out_cleanup_rx_interrupts,
out_free_buffer_pools is executed before out_unregister_queues (which
calls ibmveth_free_all_queues() to unregister the logical LAN). Does this
free and unmap the RX buffers while the hypervisor's logical LAN is still
active, potentially allowing the hypervisor to DMA incoming packets into
freed memory?
> out_free_buffer_pools:
> - while (--i >= 0) {
> - if (adapter->rx_buff_pool[0][i].active)
> - ibmveth_free_buffer_pool(adapter,
> - &adapter->rx_buff_pool[0][i]);
> - }
> -out_unmap_filter_list:
> - dma_unmap_single(dev, adapter->filter_list_dma, 4096,
> - DMA_BIDIRECTIONAL);
> -
> -out_free_tx_ltb:
> - while (--i >= 0) {
> - ibmveth_free_tx_ltb(adapter, i);
> - }
> -
> -out_unmap_buffer_list:
> - dma_unmap_single(dev, adapter->buffer_list_dma[0], 4096,
> - DMA_BIDIRECTIONAL);
> + ibmveth_free_buffer_pools(adapter);
> +out_unregister_queues:
> + ibmveth_dispose_subordinate_irq_mappings(adapter);
> + ibmveth_free_all_queues(adapter);
> out_free_queue_mem:
> - dma_free_coherent(dev, adapter->rx_queue[0].queue_len,
> - adapter->rx_queue[0].queue_addr,
> - adapter->rx_queue[0].queue_dma);
> + ibmveth_cleanup_rx_resources(adapter);
> out_free_filter_list:
> - free_page((unsigned long)adapter->filter_list_addr);
> -out_free_buffer_list:
> - free_page((unsigned long)adapter->buffer_list_addr[0]);
> + ibmveth_free_filter_list(adapter);
> +out_free_rx_qstats:
> + ibmveth_free_rx_qstats(adapter);
> out:
> - napi_disable(&adapter->napi[0]);
> return rc;
> }
...
^ permalink raw reply
* Re: [PATCH net-next v3 07/15] ibmveth: Add RX queue register/deregister helpers for MQ
From: Simon Horman @ 2026-07-14 12:43 UTC (permalink / raw)
To: mmc
Cc: Simon Horman, netdev, bjking1, haren, ricklind, kuba, edumazet,
pabeni, linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260706193603.8039-8-mmc@linux.ibm.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
ibmveth: Add RX queue register/deregister helpers for MQ
This patch introduces new helpers to handle RX queue registration and
deregistration for MQ support. It replaces the single adapter-level
register/free paradigm with per-queue register calls to the hypervisor,
fetching queue handles and IRQs. These new paths are only fully exercised
when multi-queue mode is enabled.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 63b0184c622a4..7fc11a4e1f61d 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -401,6 +401,28 @@ ibmveth_dispose_subordinate_irq_mappings(struct ibmveth_adapter *adapter)
[ ... ]
> +static int __maybe_unused
> +ibmveth_setup_rx_interrupts(struct ibmveth_adapter *adapter)
> +{
> + struct net_device *netdev = adapter->netdev;
> + int i, rc;
> +
> + for (i = 0; i < adapter->num_rx_queues; i++) {
> + if (!adapter->queue_irq[i]) {
> + netdev_err(netdev, "queue %d has invalid IRQ (0)\n", i);
> + rc = -EINVAL;
> + goto err_free_irqs;
> + }
> +
> + rc = request_irq(adapter->queue_irq[i], ibmveth_interrupt,
> + 0, netdev->name, &adapter->napi[i]);
> + if (rc) {
> + netdev_err(netdev,
> + "request_irq() failed for irq 0x%x queue %d: %d\n",
> + adapter->queue_irq[i], i, rc);
> + goto err_free_irqs;
> + }
> + }
> +
> + for (i = 0; i < adapter->num_rx_queues; i++)
> + napi_enable(&adapter->napi[i]);
[Severity: Medium]
Could this sequence race with packet reception during interface bring-up?
By calling request_irq() before napi_enable(), the hardware interrupt is
unmasked while the NAPI state is still NAPI_STATE_DISABLE. If an interrupt
fires in this window, could ibmveth_interrupt() fail to schedule the NAPI
poll, potentially leading to dropped events or an interrupt storm?
[ ... ]
> @@ -1136,6 +1176,293 @@ static void __maybe_unused ibmveth_free_rx_qstats(struct ibmveth_adapter *adapte
[ ... ]
> +static int
> +ibmveth_register_logical_lan_queue(struct ibmveth_adapter *adapter,
> + union ibmveth_buf_desc rxq_desc,
> + int queue_index)
> +{
[ ... ]
> + if (lpar_rc == H_FUNCTION) {
> + if (adapter->multi_queue) {
> + netdev_info(adapter->netdev,
> + "Multi queue mode not supported by firmware, falling back to single queue\n");
> + adapter->multi_queue = 0;
> + } else {
> + netdev_err(adapter->netdev,
> + "Unexpected H_FUNCTION for queue %d registration (MQ mode already disabled)\n",
> + queue_index);
> + }
> + return lpar_rc;
[Severity: Medium]
Does returning lpar_rc here cause a regression for the single queue fallback?
Since this error propagates up to ibmveth_register_rx_queues(), it seems it
will unconditionally jump to err_unregister and fail the interface open
rather than allowing it to continue with a single queue.
> + }
> +
> + if (try_again) {
> + try_again = 0;
> + goto retry;
[Severity: Low]
Does this retry loop provide any benefit? Unlike the primary queue which
calls h_free_logical_lan() before retrying to clear stale state, this
jumps back to retry the exact same hypercall with the exact same arguments,
which seems like it will immediately fail again.
> + }
[ ... ]
> +static int
> +ibmveth_register_rx_queues(struct ibmveth_adapter *adapter, u64 mac_address)
> +{
[ ... ]
> + for (i = 1; i < adapter->num_rx_queues; i++) {
> + rc = ibmveth_register_single_rx_queue(adapter, i, mac_address);
> + if (rc) {
> + if (!adapter->queue_handle[i] || !adapter->queue_irq[i]) {
> + netdev_err(netdev,
> + "Invalid hypervisor return for queue %d: handle=0x%llx irq=%u\n",
> + i, adapter->queue_handle[i],
> + adapter->queue_irq[i]);
> + }
[Severity: Low]
Is this validation check placed correctly? Because it sits inside the
if (rc) error block, it will log "Invalid hypervisor return" whenever
registration fails normally (e.g., due to resource constraints). It appears
the hypervisor is not expected to populate the handle and irq on failure.
> + goto err_unregister;
> + }
> + }
^ permalink raw reply
* Re: [PATCH net-next v3 05/15] ibmveth: Refactor RX interrupt control for MQ RX queues
From: Simon Horman @ 2026-07-14 12:43 UTC (permalink / raw)
To: Mingming Cao
Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
linuxppc-dev, maddy, mpe, Dave Marquardt
In-Reply-To: <20260706193603.8039-6-mmc@linux.ibm.com>
On Mon, Jul 06, 2026 at 12:35:53PM -0700, Mingming Cao wrote:
> Queue 0 and subordinate RX queues use different interrupt control
> interfaces in PHYP:
>
> - queue 0: h_vio_signal() after h_register_logical_lan()
> - queue N: H_VIOCTL against the queue handle/hwirq mapping
>
> The current code is single-queue oriented and cannot safely scale to
> multiple RX queues in poll completion and open/close IRQ setup.
>
> Introduce queue-indexed interrupt helpers:
>
> ibmveth_enable_irq(adapter, queue_index)
> ibmveth_disable_irq(adapter, queue_index)
> ibmveth_setup_rx_interrupts()
> ibmveth_cleanup_rx_interrupts()
>
> These helpers centralize queue0-vs-subordinate dispatch and make IRQ
> lifecycle symmetric across open/close and future resize paths.
>
> request_irq() is wired with &adapter->napi[i] as dev_id per queue, so
> interrupt ownership follows the NAPI instance that services that RX
> queue.
>
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>
Although it is added later in this patch set, I believe linux/irqdomain.h
needs to be included in this patch so that irq_dispose_mapping is defined.
Also, I think it would be best to add these helpers in the same patch(es)
that they are first used. As this will avoid temporal compiler warnings
about declared but otherwise unused functions. Similarly for patch 13/15.
Overall, please make sure that when each patch of the series is applied
no new warnings or errors are introduced for allmodconfig W=1 builds.
FWIIW, I exercised this using gcc 16.1 from
https://www.kernel.org/pub/tools/crosstool/
...
--
pw-bot: changes-requested
^ permalink raw reply
* Re: [patch 0/4] entry: Rework syscall skip logic
From: Michal Suchánek @ 2026-07-14 12:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Michael Ellerman, Shrikanth Hegde, linuxppc-dev,
Huacai Chen, loongarch, Paul Walmsley, Palmer Dabbelt,
linux-riscv, Sven Schnelle, linux-s390, x86, Mark Rutland,
Jinjie Ruan, Magnus Lindholm, Mukesh Kumar Chaurasiya (IBM),
Jonathan Corbet, Radu Rendec
In-Reply-To: <20260712134433.549076055@kernel.org>
On Sun, Jul 12, 2026 at 11:25:12PM +0200, Thomas Gleixner wrote:
> This are the reworked leftovers of the larger entry logic rework series,
> which can be found here:
>
> https://lore.kernel.org/lkml/20260707181957.433213175@kernel.org
>
> The undisputed part of the series has been merged into:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/entry
>
> After a fruitless discussion about the most resilent approach, I decided to
> give up wasting my time and reworked the series so it caters to the desire
> of powerpc and s390 to be special.
>
> That results in almost identical behavior except for the case where
> tracing/probe/BPF sets the syscall number to -1 (or any other out of range
> value) and also sets the syscall return value to something different than
> -ENOSYS.
>
> PowerPC and S390 will overwrite that value with -ENOSYS.
>
> Loongarch, RISC-V and x86 will not overwrite it because those architectures
> preset the return value to -ENOSYS and skip out of range syscalls
> completely. Loongarch and RISC-V always did the out of range skip. X86
> gained that in the already applied rework, which means that the final patch
> which splits the skip decision from the syscall number does not longer
> change x86 behavior for the above case.
>
> Documentation is also updated and describes the two implementation variants
> and the subtle difference in the resulting behavior.
>
> The series applies on top of:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/entry
>
> and is also available from git:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git entry/rework
Hello,
applying this on top of 7.2~rc3 and running tests for the cases that
were reported as problematic I see results as expected.
This version of the patchset works for me.
Tested-by: Michal Suchánek <msuchanek@suse.de>
This is certainly not comprehensive, there are many corner cases not
tested. Some were broken to start with as well.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Jason Gunthorpe @ 2026-07-14 12:26 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <yq5aqzl6mc00.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:57:43AM +0530, Aneesh Kumar K.V wrote:
> >
> > But I don't view this as that important, the CC hypervisor is probably
> > going to use the S2 page table to force cachable on all system memory
> > so the non-cached pgprot is a NOP, but the extra vmap is wasteful and
> > it is confusing.. So maybe a little fixme is all that is needed here.
> >
>
> Something like?
Yeah
Jason
^ permalink raw reply
* Re: [PATCH v7 11/22] dma-pool: track decrypted atomic pools and select them via attrs
From: Jason Gunthorpe @ 2026-07-14 12:25 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <yq5atsq2md64.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:32:27AM +0530, Aneesh Kumar K.V wrote:
> If we want to warn about such failures, we should add the warning
> consistently across the code. We may also want to handle decrypt
> failures by encrypting the page again to avoid leaking it.
>
> I will work on that as a tree-wide change in a separate patch.
IMHO the WARN should be inside set_memory_encrypted(), and maybe it
should be a BUG_ON...
"free" functions shouldn't fail, and returning the memory back to
normal is a "free" operation.
Jason
^ permalink raw reply
* Re: [PATCH v7 04/22] dma: free atomic pool pages by physical address
From: Jason Gunthorpe @ 2026-07-14 12:23 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Michael Kelley
In-Reply-To: <yq5awluymdf6.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:27:01AM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@ziepe.ca> writes:
>
> > On Wed, Jul 01, 2026 at 11:19:08AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> dma_direct_alloc_pages() may satisfy atomic allocations from the coherent
> >> atomic pools. The pool allocation is keyed by the virtual address stored in
> >> the gen_pool, but the pages API returns only the backing struct page.
> >>
> >> On architectures with CONFIG_DMA_DIRECT_REMAP, atomic pool chunks are added
> >> to the gen_pool using their remapped virtual address.
> >> dma_direct_free_pages() reconstructs a linear-map address with
> >> page_address(page) and passes that to dma_free_from_pool(). That address
> >> does not match the gen_pool virtual range, so the pool lookup can fail and
> >> the code can fall through to freeing a pool-owned page through the normal
> >> page allocator path.
> >>
> >> Add a page-based pool free helper that looks up the owning pool chunk by
> >> physical address, translates it back to the gen_pool virtual address, and
> >> frees that address to the pool. Use it from dma_direct_free_pages() while
> >> keeping the existing virtual-address helper for coherent allocation frees.
> >>
> >> Tested-by: Michael Kelley <mhklinux@outlook.com>
> >> Tested-by: Mostafa Saleh <smostafa@google.com>
> >> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> >> ---
> >> include/linux/dma-map-ops.h | 1 +
> >> kernel/dma/direct.c | 4 +--
> >> kernel/dma/pool.c | 54 +++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 57 insertions(+), 2 deletions(-)
> >
> > This seems pretty suboptimal?
> >
> > If !CONFIG_DMA_DIRECT_REMAP then page_to_virt() was used to compute
> > the genpool's addr so dma_free_from_pool_page() can use the same
> > logic, which is how things must be working at all today
> >
> > The CONFIG_DMA_DIRECT_REMAP scenario does look broken, so I'm
> > surprised there isn't a Fixes line on this commit? I don't have an
> > opinion on the search, but since alloc_pages() is used there is 8
> > bytes in the struct page that could be used to store the remapped
> > vaddr to avoid the search if someday someone wants to improve
> > this. Maybe a small comment hinting that direction would be a nice
> > addition.
>
> Something like
>
> +/*
> + * FIXME!! We could avoid this by storing the remapped virtual address in
> + * struct page and using that for lookup.
> + */
> bool dma_free_from_pool_page(struct device *dev, struct page *page, size_t size)
Plus some if (!IS_ENABLED()) that does the direct lookup
I would just use : not !! :)
Jason
^ permalink raw reply
* Re: [PATCH v7 02/22] dma-pool: fix page leak in atomic_pool_expand() cleanup
From: Jason Gunthorpe @ 2026-07-14 12:22 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Michael Kelley
In-Reply-To: <yq5azezumef6.fsf@kernel.org>
On Tue, Jul 14, 2026 at 09:05:25AM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@ziepe.ca> writes:
>
> > On Wed, Jul 01, 2026 at 11:19:06AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> @@ -115,8 +116,10 @@ static int atomic_pool_expand(struct gen_pool *pool, size_t pool_size,
> >> */
> >> ret = set_memory_decrypted((unsigned long)page_to_virt(page),
> >> 1 << order);
> >> - if (ret)
> >> + if (ret) {
> >> + leak_pages = true;
> >> goto remove_mapping;
> >> + }
> >
> > Truely these _set_memory_decrypted() things are an insane API. So a if
> > it fails to decrypt it can be in any messy state?
> >
>
> Yes, we could possibly try to encrypt the page again and, if that
> succeeds, avoid leaking it. We might want to do that tree-wide in a
> separate patch.
IMHO it is a horrid API if failure leaves thing in an indeterminate
state. For something like this I don't see why the arch FW implementation
would be unable to restore things back to as they were on failure.
But whatever, everything about set_memory_xx is really bad it could
use a cleaning
Jason
^ permalink raw reply
* Fixes tags need work in the powerpc-fixes tree
From: Mark Brown @ 2026-07-14 11:47 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, PowerPC; +Cc: linux-kernel, linux-next
[-- Attachment #1: Type: text/plain, Size: 308 bytes --]
In commit
1034785975914 ("powerpc/85xx: Add fsl,ifc to common device ids")
Fixes tag
Fixes: 0bf51cc9e9e5 ("powerpc: dts: mpc85xx: remove simple-bus compatible from ifc node")
has these problem(s):
- Subject does not match target commit subject
Just use
git log -1 --format='Fixes: %h ("%s")'
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Fixes tags need work in the powerpc-fixes tree
From: Mark Brown @ 2026-07-14 11:46 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, PowerPC; +Cc: linux-kernel, linux-next
[-- Attachment #1: Type: text/plain, Size: 316 bytes --]
In commit
19dbc55d400b8 ("powerpc/vtime: Initialize starttime at boot for native accounting")
Fixes tag
Fixes: cf9efce0ce31 ("powerpc: Account time using timebase rather than PURR")
has these problem(s):
- Subject does not match target commit subject
Just use
git log -1 --format='Fixes: %h ("%s")'
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/64s: Clarify copy_and_flush() cache sync loop comment
From: Mahesh J Salgaonkar @ 2026-07-14 10:30 UTC (permalink / raw)
To: Nikhil Kumar Singh
Cc: linuxppc-dev, linux-kernel, maddy, mpe, npiggin, chleroy, adityag
In-Reply-To: <20260701182756.273019-1-nikhilks@linux.ibm.com>
On 2026-07-01 23:57:56 Wed, Nikhil Kumar Singh wrote:
> The value loaded into r0 in copy_and_flush() represents the number of
> 8-byte words processed between cache synchronization operations.
>
> The existing comment refers to cache line size, which can make it appear
> that the value is a cache line size in bytes rather than a loop count.
> Clarify the comment to explain that the loop processes 8 words (64 bytes)
> per cache synchronization iteration, and that increasing the value would
> skip cache maintenance for intermediate cache lines.
>
> This is a comment-only change with no functional impact.
>
> Signed-off-by: Nikhil Kumar Singh <nikhilks@linux.ibm.com>
> ---
> arch/powerpc/kernel/head_64.S | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 63432a33ec49..e21c2bce8f7e 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -713,14 +713,14 @@ p_end: .8byte _end - copy_to_here
> _GLOBAL(copy_and_flush)
> addi r5,r5,-8
> addi r6,r6,-8
> -4: li r0,8 /* Use the smallest common */
> - /* denominator cache line */
> - /* size. This results in */
> - /* extra cache line flushes */
> - /* but operation is correct. */
> - /* Can't get cache line size */
> - /* from NACA as it is being */
> - /* moved too. */
> +4: li r0,8 /* r0 is the number of 8-byte words */
> + /* to copy per cache sync iteration. */
> + /* 8 words * 8 bytes = 64 bytes. 64B is */
> + /* the current default cache line size. */
> + /* This is a loop count, not a byte */
> + /* count. Increasing it will skip */
> + /* dcbst/icbi for lines in between and */
> + /* leave stale instructions in icache. */
Looks good to me. The previous comment was misleading which makes one to
think r0 as cache line size which is not the case.
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Thanks,
-Mahesh.
^ permalink raw reply
* Re: [PATCH 01/14] ASoC: codecs: NeoFidelity: repair the kernel-doc format
From: Mark Brown @ 2026-07-14 10:20 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-sound, Jaroslav Kysela, Takashi Iwai, Shengjiu Wang,
Xiubo Li, Fabio Estevam, Nicolin Chen, linuxppc-dev,
Charles Keepax, Maciej Strozek, Bard Liao, Pierre-Louis Bossart,
patches, Srinivas Kandagatla, linux-arm-msm, Liam Girdwood
In-Reply-To: <20260714014445.569992-2-rdunlap@infradead.org>
[-- Attachment #1: Type: text/plain, Size: 460 bytes --]
On Mon, Jul 13, 2026 at 06:44:32PM -0700, Randy Dunlap wrote:
> Don't use "/**" for a non-kernel-doc comment.
> Use kernel-doc notation to document the parameters and return value of
> ntpfw_load().
As mentioned in submitting-patches.rst when submitting a patch series
you should supply a cover letter for that patch series which describes
the overall content of the series. This helps people understand what
they are looking at and how things fit together.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Marek Szyprowski @ 2026-07-14 10:00 UTC (permalink / raw)
To: Aneesh Kumar K.V, Jason Gunthorpe
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
Catalin Marinas, Jiri Pirko, Mostafa Saleh, Petr Tesarik,
Alexey Kardashevskiy, Dan Williams, Xu Yilun, linuxppc-dev,
linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <yq5aldbdnbj8.fsf@kernel.org>
On 14.07.2026 11:52, Aneesh Kumar K.V wrote:
> Marek Szyprowski <m.szyprowski@samsung.com> writes:
>> On 13.07.2026 21:43, Jason Gunthorpe wrote:
>>> On Tue, Jul 07, 2026 at 03:03:48PM +0200, Marek Szyprowski wrote:
>>>> I'm fine with merging on top of the topic branch and I assume that this
>>>> patchset is mature enough to give it a try in linux-next, but first I
>>>> would like to get a review or at least acks from others with good CC
>>>> knowledge or experience.
>>> I think it is good to go from a CC perspective, there are still some
>>> more items to fix up (like the MMIO) but I'd rather they be followups
>>> at this point.
>> Great. Aneesh, any chance You could send v8 with the remaining items
>> fixed till end of this week? I will be happy to push it to
>> dma-mapping-for-next for testing in linux-next asap.
>>
> Sure. You need this rebased on top of the pKVM changes, right?
> Otherwise, we'll end up with conflicts when you do the final pull
> request.
Ah, indeed. I forgot about the pKVM changes. If there is a stable
branch with those patches, please base v8 on top of it.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Aneesh Kumar K.V @ 2026-07-14 9:52 UTC (permalink / raw)
To: Marek Szyprowski, Jason Gunthorpe
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Will Deacon, Marc Zyngier, Steven Price, Suzuki K Poulose,
Catalin Marinas, Jiri Pirko, Mostafa Saleh, Petr Tesarik,
Alexey Kardashevskiy, Dan Williams, Xu Yilun, linuxppc-dev,
linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <cd5d0504-1592-4b48-89ea-1efe8aa57968@samsung.com>
Marek Szyprowski <m.szyprowski@samsung.com> writes:
> On 13.07.2026 21:43, Jason Gunthorpe wrote:
>> On Tue, Jul 07, 2026 at 03:03:48PM +0200, Marek Szyprowski wrote:
>>> I'm fine with merging on top of the topic branch and I assume that this
>>> patchset is mature enough to give it a try in linux-next, but first I
>>> would like to get a review or at least acks from others with good CC
>>> knowledge or experience.
>> I think it is good to go from a CC perspective, there are still some
>> more items to fix up (like the MMIO) but I'd rather they be followups
>> at this point.
>
> Great. Aneesh, any chance You could send v8 with the remaining items
> fixed till end of this week? I will be happy to push it to
> dma-mapping-for-next for testing in linux-next asap.
>
Sure. You need this rebased on top of the pKVM changes, right?
Otherwise, we'll end up with conflicts when you do the final pull
request.
-aneesh
^ permalink raw reply
* Re: [PATCH 23/42] mmc: sdhci-of-bst: Use devm_of_reserved_mem_device_init_by_idx()
From: Adrian Hunter @ 2026-07-14 9:33 UTC (permalink / raw)
To: Mukesh Ojha, Bjorn Andersson, Konrad Dybcio, Liviu Dudau,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Joel Stanley, Andrew Jeffery, Paul Cercueil,
Anitha Chrisanthus, Paul Kocialkowski, Linus Walleij,
Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Alexey Brodkin,
Laurent Pinchart, Tomi Valkeinen, Michal Simek, Daniel Scally,
Jacopo Mondi, Mauro Carvalho Chehab, Eddie James, Tiffany Lin,
Andrew-CT Chen, Yunfei Dong, Minghsiu Tsai, Houlong Wei,
Matthias Brugger, AngeloGioacchino Del Regno, Joseph Liu,
Marvin Lin, Dmitry Osipenko, Krzysztof Kozlowski, Thierry Reding,
Jonathan Hunter, Srinivas Kandagatla, Arnd Bergmann,
Greg Kroah-Hartman, Ge Gordon, Ulf Hansson, Rob Herring,
Saravana Kannan, Mathieu Poirier, Jaroslav Kysela, Takashi Iwai,
Shengjiu Wang, Xiubo Li, Liam Girdwood, Mark Brown, Frank Li,
Sascha Hauer, Peter Ujfalusi, Bard Liao, Daniel Baluta,
Orson Zhai, Baolin Wang, Peter Chen, Fugang Duan
Cc: Ekansh Gupta, BST Linux Kernel Upstream Group, Fabio Estevam,
Nicolin Chen, Pengutronix Kernel Team, Kai Vehmanen,
Pierre-Louis Bossart, Vijendar Mukunda, Chunyan Zhang,
CIX Linux Kernel Upstream Group, linux-arm-msm, linux-kernel,
dri-devel, linux-aspeed, linux-arm-kernel, linux-mips,
linux-sunxi, linux-media, openbmc, linux-mediatek, kernel,
linux-tegra, linux-mmc, devicetree, linux-remoteproc,
linux-staging, linux-sound, linuxppc-dev, imx,
sound-open-firmware
In-Reply-To: <20260703193855.110619-24-mukesh.ojha@oss.qualcomm.com>
On 03/07/2026 22:38, Mukesh Ojha wrote:
> Use the devres-managed devm_of_reserved_mem_device_init_by_idx() instead
> of the manual of_reserved_mem_device_init_by_idx()/
> of_reserved_mem_device_release() pair, letting the device resource
> manager handle cleanup automatically.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-of-bst.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-bst.c b/drivers/mmc/host/sdhci-of-bst.c
> index f8d3df715e1a..304554ced690 100644
> --- a/drivers/mmc/host/sdhci-of-bst.c
> +++ b/drivers/mmc/host/sdhci-of-bst.c
> @@ -405,7 +405,6 @@ static void sdhci_bst_free_bounce_buffer(struct sdhci_host *host)
> host->bounce_buffer, host->bounce_addr);
> host->bounce_buffer = NULL;
> }
> - of_reserved_mem_device_release(mmc_dev(host->mmc));
> }
>
> static int sdhci_bst_alloc_bounce_buffer(struct sdhci_host *host)
> @@ -417,7 +416,7 @@ static int sdhci_bst_alloc_bounce_buffer(struct sdhci_host *host)
> /* Fixed SRAM bounce size to 32KB: verified config under 32-bit DMA addressing limit */
> bounce_size = SZ_32K;
>
> - ret = of_reserved_mem_device_init_by_idx(mmc_dev(mmc), mmc_dev(mmc)->of_node, 0);
> + ret = devm_of_reserved_mem_device_init_by_idx(mmc_dev(mmc), mmc_dev(mmc)->of_node, 0);
> if (ret) {
> dev_err(mmc_dev(mmc), "Failed to initialize reserved memory\n");
> return ret;
> @@ -425,10 +424,8 @@ static int sdhci_bst_alloc_bounce_buffer(struct sdhci_host *host)
>
> host->bounce_buffer = dma_alloc_coherent(mmc_dev(mmc), bounce_size,
> &host->bounce_addr, GFP_KERNEL);
> - if (!host->bounce_buffer) {
> - of_reserved_mem_device_release(mmc_dev(mmc));
> + if (!host->bounce_buffer)
> return -ENOMEM;
> - }
>
> host->bounce_buffer_size = bounce_size;
>
^ permalink raw reply
* Re: [PATCH v2 2/3] powerpc/numa: Allow cpu_to_coregroup_id without PPC_SPLPAR
From: Srikar Dronamraju @ 2026-07-14 9:29 UTC (permalink / raw)
To: Ritesh Harjani
Cc: linuxppc-dev, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, skiboot, arbab,
mahesh, chleroy, linux-kernel
In-Reply-To: <wlv3v6ag.ritesh.list@gmail.com>
* Ritesh Harjani <ritesh.list@gmail.com> [2026-07-10 09:34:55]:
> Srikar Dronamraju <srikar@linux.ibm.com> writes:
>
> > Make cpu_to_coregroup_id() available outside PPC_SPLPAR so it can be
> > used by platforms that do not rely on the SPLPAR-specific VPHN path.
> >
> > Keep the existing fallback behavior by returning the core ID when
> > coregroup information is unavailable.
> >
>
> Looks like this mainly brings the cpu_to_coregroup_id() outside of
> CONFIG_PPC_SPLPAR gate.
>
Yes, thats the intent of the patch
> > Signed-off-by: Srikar Dronamraju <srikar@linux.ibm.com>
> > ---
> > Changelog from v1:
> > - Handle comments from Christophe Leroy; Remove extern key word in
> > declaration
> >
> > arch/powerpc/include/asm/topology.h | 15 +++++++--------
> > arch/powerpc/mm/numa.c | 22 ++++++++++++++++------
> > 2 files changed, 23 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> > index 66ed5fe1b718..385715f1ad56 100644
> > --- a/arch/powerpc/include/asm/topology.h
> > +++ b/arch/powerpc/include/asm/topology.h
> > @@ -71,6 +71,7 @@ extern void map_cpu_to_node(int cpu, int node);
> > extern void unmap_cpu_from_node(unsigned long cpu);
> > #endif /* CONFIG_HOTPLUG_CPU */
> >
> > +int cpu_to_coregroup_id(int cpu);
> > #else
> >
> > static inline int early_cpu_to_node(int cpu) { return 0; }
> > @@ -107,14 +108,6 @@ static inline void map_cpu_to_node(int cpu, int node) {}
> > static inline void unmap_cpu_from_node(unsigned long cpu) {}
> > #endif /* CONFIG_HOTPLUG_CPU */
> > #endif /* CONFIG_SMP */
> > -
> > -#endif /* CONFIG_NUMA */
> > -
> > -#if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
> > -void find_and_update_cpu_nid(int cpu);
> > -extern int cpu_to_coregroup_id(int cpu);
> > -#else
> > -static inline void find_and_update_cpu_nid(int cpu) {}
> > static inline int cpu_to_coregroup_id(int cpu)
> > {
> > #ifdef CONFIG_SMP
> > @@ -124,6 +117,12 @@ static inline int cpu_to_coregroup_id(int cpu)
> > #endif
> > }
> >
> > +#endif /* CONFIG_NUMA */
> > +
> > +#if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
> > +void find_and_update_cpu_nid(int cpu);
> > +#else
> > +static inline void find_and_update_cpu_nid(int cpu) {}
> > #endif /* CONFIG_NUMA && CONFIG_PPC_SPLPAR */
> >
> > #include <asm-generic/topology.h>
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index c44a80d8fc11..9aa71eb7e96b 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -1428,6 +1428,21 @@ void find_and_update_cpu_nid(int cpu)
> > pr_debug("%s:%d cpu %d nid %d\n", __func__, __LINE__, cpu, new_nid);
> > }
> >
> > +static int topology_update_init(void)
> > +{
> > + topology_inited = 1;
> > + return 0;
> > +}
> > +device_initcall(topology_update_init);
> > +
> > +#else
> > +static long vphn_get_associativity(unsigned long cpu,
> > + __be32 *associativity)
> > +{
> > + return -1;
> > +}
> > +#endif /* CONFIG_PPC_SPLPAR */
> > +
> > int cpu_to_coregroup_id(int cpu)
> > {
> > __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> > @@ -1453,10 +1468,5 @@ int cpu_to_coregroup_id(int cpu)
> > return cpu_to_core_id(cpu);
> > }
> >
> > -static int topology_update_init(void)
> > -{
> > - topology_inited = 1;
> > - return 0;
> > + return coregroup_id;
> > }
>
> These above two lines seems to be out of any function scope. So applying
> only patch-1 & patch-2 will give a compile error. This needs fixing.
>
Yes, will fix this
> -ritesh
>
> > -device_initcall(topology_update_init);
> > -#endif /* CONFIG_PPC_SPLPAR */
> > --
> > 2.43.0
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v2 3/3] powerpc/numa: Support coregroup on PowerNV
From: Srikar Dronamraju @ 2026-07-14 9:28 UTC (permalink / raw)
To: Ritesh Harjani
Cc: linuxppc-dev, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, skiboot, arbab,
mahesh, chleroy, linux-kernel
In-Reply-To: <v7anv1ib.ritesh.list@gmail.com>
* Ritesh Harjani <ritesh.list@gmail.com> [2026-07-10 11:18:12]:
> Srikar Dronamraju <srikar@linux.ibm.com> writes:
>
> > Coregroup support on powerpc has so far been limited to PowerVM LPARs.
> > However, PowerNV can also support coregroups when firmware exposes the
> > required coregroup information through the associativity hierarchy.
> >
> > Detect coregroup support by checking whether primary_domain_index is the
> > penultimate domain in the CPU node's ibm,associativity property. On
> > PowerNV, a non-penultimate primary_domain_index indicates that firmware
> > provides an additional level for coregroup information.
> >
> > This keeps the logic compatible with PowerVM systems, where
> > primary_domain_index is likewise not the penultimate associativity
> > domain.
> >
> > Signed-off-by: Srikar Dronamraju <srikar@linux.ibm.com>
> > ---
> > Changelog from v1: https://lkml.kernel.org/r/20260524010017.140408-1-srikar@linux.ibm.com
> > - Handle comments from Christophe Leroy; make code more flat
> >
> > arch/powerpc/mm/numa.c | 56 ++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 46 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index 9aa71eb7e96b..e97b624203ea 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -889,12 +889,32 @@ static int __init numa_setup_drmem_lmb(struct drmem_lmb *lmb,
> > return 0;
> > }
Hi Ritesh,
Thanks for the review.
> >
> > +/*
> > + * If hierarchy extends beyond primary_domain_index + 1, then next
> > + * level corresponds to coregroup.
> > + */
> > +static int detect_and_enable_coregroup(const __be32 *associativity, int index)
>
> Do we care about it's return value? We are not reading that in the
> patch.
Yes, We do care about the return value. If the index is set to -1, we don't
retry enabling the coregroup.
> this function is mainly only needed in __init, can we mark it so.
Yes, this will be done.
>
> > +{
> > + if (!associativity || index == -1)
> > + goto out;
> > +
> > + index = of_read_number(associativity, 1);
> > +
> > + if (index > primary_domain_index + 1) {
> > + coregroup_enabled = 1;
> > + return index;
> > + }
> > +out:
> > + coregroup_enabled = 0;
> > + return -1;
> > +}
>
> For PowerVM, we now have two places which will enable coregroup_enabled
> during mem_topology_setup(). Is there some way we can unify that?
>
For PowerVM, we have two extra associativity properties
ibm,ibm,current-associativity-domains and ibm,max-associativity-domains.
On PowerNV, these two properties are not used/exported.
All we depend is the layout of these properties to determine if coregroup is
enabled. If the layout tells us that there is place after
primary_domain_index for coregroup, we assume coregroup is enabled.
So in this patch, we hook at the place we look at each of the CPU
associativity. This should work for both PowerVM and PowerNV.
So, I can think of two options.
1. Remove the previous logic of depending on PowerVM specific code.
2. Allow the previous logic to be around. Since its not going to hurt
functionally or performance wise.
> This also means we enable coregroup in case of PowerVM with SPLPAR when
> per-cpu VPHN associativity index > primary_domain_index+1. But this
> isn't reflected in your commit msg. The commit msg only says this
> affects PowerNV.
I don't think, I said this affects PowerNV only. But I still don't think
the logic would change. The logic to enable coregroup remains the same.
Just that we may now be depending on the 1st CPU associativity instead of
the PowerVM specific properties.
>
> setup_arch
> mem_topology_setup
> parse_numa_properties
> detect_and_enable_coregroup() {...
> // coregroup_enabled = 0/1
> index = of_read_number(associativity, 1);
>
> if (index > primary_domain_index + 1) {
> coregroup_enabled = 1;
> return index;
> }
>
> }
> <...>
> find_possible_nodes() {...
> prop_length /= sizeof(int);
> if (prop_length > primary_domain_index + 2)
> coregroup_enabled = 1;
> }
>
> > +
> > static int __init parse_numa_properties(void)
> > {
> > struct device_node *memory, *pci;
> > - int default_nid = 0;
> > - unsigned long i;
> > + int default_nid = 0, index = 0;
> > const __be32 *associativity;
> > + unsigned long i;
> >
> > if (numa_enabled == 0) {
> > pr_warn("disabled by user\n");
> > @@ -927,7 +947,6 @@ static int __init parse_numa_properties(void)
> > */
> > for_each_present_cpu(i) {
> > __be32 vphn_assoc[VPHN_ASSOC_BUFSIZE];
> > - struct device_node *cpu;
> > int nid = NUMA_NO_NODE;
> >
> > memset(vphn_assoc, 0, VPHN_ASSOC_BUFSIZE * sizeof(__be32));
> > @@ -935,7 +954,9 @@ static int __init parse_numa_properties(void)
> > if (__vphn_get_associativity(i, vphn_assoc) == 0) {
> > nid = associativity_to_nid(vphn_assoc);
> > initialize_form1_numa_distance(vphn_assoc);
> > + index = detect_and_enable_coregroup(vphn_assoc, index);
> > } else {
> > + struct device_node *cpu;
> >
> > /*
> > * Don't fall back to default_nid yet -- we will plug
> > @@ -948,6 +969,7 @@ static int __init parse_numa_properties(void)
> > associativity = of_get_associativity(cpu);
> > if (associativity) {
> > nid = associativity_to_nid(associativity);
> > + index = detect_and_enable_coregroup(associativity, index);
> > initialize_form1_numa_distance(associativity);
> > }
> > of_node_put(cpu);
> > @@ -1445,7 +1467,9 @@ static long vphn_get_associativity(unsigned long cpu,
> >
> > int cpu_to_coregroup_id(int cpu)
> > {
> > - __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> > + int coregroup_id = cpu_to_core_id(cpu);
> > + struct device_node *cpunode = NULL;
> > + const __be32 *associativity;
> > int index;
> >
> > if (cpu < 0 || cpu > nr_cpu_ids)
> > @@ -1454,19 +1478,31 @@ int cpu_to_coregroup_id(int cpu)
> > if (!coregroup_enabled)
> > goto out;
> >
> > - if (!firmware_has_feature(FW_FEATURE_VPHN))
> > - goto out;
> > + if (firmware_has_feature(FW_FEATURE_VPHN)) {
> > + __be32 tmp[VPHN_ASSOC_BUFSIZE] = {0};
> >
> > - if (vphn_get_associativity(cpu, associativity))
> > + if (vphn_get_associativity(cpu, tmp))
> > + goto out;
> > +
> > + associativity = tmp;
> > +
> > + } else {
> > + cpunode = of_get_cpu_node(cpu, NULL);
> > + if (!cpunode)
> > + goto out;
> > +
> > + associativity = of_get_associativity(cpunode);
> > + }
> > + if (!associativity)
> > goto out;
> >
> > index = of_read_number(associativity, 1);
> > if (index > primary_domain_index + 1)
> > - return of_read_number(&associativity[index - 1], 1);
> > + coregroup_id = of_read_number(&associativity[index - 1], 1);
> >
> > out:
> > - return cpu_to_core_id(cpu);
> > -}
>
> Looks like leftover removed from previous patch. This change should be
> fixed in patch-2 itself.
True, will do the needful.
>
> > + if (cpunode)
> > + of_node_put(cpunode);
> >
> > return coregroup_id;
> > }
> > --
> > 2.43.0
>
> -ritesh
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v7 06/10] ACPI: APEI: GHES: move CXL CPER helpers
From: Ahmed Tiba @ 2026-07-14 9:27 UTC (permalink / raw)
To: Alison Schofield
Cc: Rafael J. Wysocki, Tony Luck, Borislav Petkov, Hanjun Guo,
Mauro Carvalho Chehab, Shuai Xue, Len Brown, Saket Dumbre,
Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Vishal Verma,
Dan Williams, Ira Weiny, Li Ming, Mahesh J Salgaonkar,
Oliver O'Halloran, Bjorn Helgaas, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-acpi, acpica-devel, linux-cxl, linuxppc-dev,
linux-pci, devicetree, linux-edac, linux-doc, Dmitry.Lamerov
In-Reply-To: <alWi5hvXYT0-ZPyV@aschofie-mobl2.lan>
On 14/07/2026 03:45, Alison Schofield wrote:
> On Wed, Jul 08, 2026 at 02:59:05PM +0100, Ahmed Tiba wrote:
>> Move the CXL CPER handling paths out of ghes.c and into ghes_cper.c so the
>> helpers can be reused. The code is moved as-is, with the public
>> prototypes updated so GHES keeps calling into the new translation unit.
>>
>> While moving this code, also add CXL CPER section length checks and use
>> spinlock_irqsave() in CXL register/unregister paths for locking
>> consistency.
>
> NAK on moving and changing in the same patch, and esp in a patch
> whose subject only says MOVE.
Hi Alison,
I understand the concern.
I kept those changes in 06/10 because they address pre-existing issues
reported specifically against this patch by Sashiko, and I wanted to
avoid carrying known defects forward in the series. I did this following
Boris' feedback to address review-reported issues in-series rather than
carry them forward unchanged.
I can also add a dedicated "Sashiko findings addressed" section and
update the commit description to state clearly that this patch is not
pure move.
Please advise your preferred way to address Sashiko findings per patch
in this series.
Thanks,
Ahmed
^ permalink raw reply
* Re: [PATCH 14/14] ASoC: wm8904: don't use "/**" for non-kernel-doc comments
From: Charles Keepax @ 2026-07-14 8:41 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-sound, Jaroslav Kysela, Takashi Iwai, Shengjiu Wang,
Xiubo Li, Fabio Estevam, Nicolin Chen, linuxppc-dev,
Maciej Strozek, Bard Liao, Pierre-Louis Bossart, patches,
Srinivas Kandagatla, linux-arm-msm, Liam Girdwood, Mark Brown
In-Reply-To: <20260714014445.569992-15-rdunlap@infradead.org>
On Mon, Jul 13, 2026 at 06:44:45PM -0700, Randy Dunlap wrote:
> Modify these errant comments to use "/*" since they are not kernel-doc
> comments.
>
> Warning: include/sound/wm8904.h:119 This comment starts with '/**', but isn't a kernel-doc comment.
> * DRC configurations are specified with a label and a set of register
> Warning: ../include/sound/wm8904.h:134 This comment starts with '/**', but isn't a kernel-doc comment.
> * ReTune Mobile configurations are specified with a label, sample
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks,
Charles
^ permalink raw reply
* Re: [PATCH 10/14] ASoC: SDCA: correct enum names and add a missing struct field
From: Charles Keepax @ 2026-07-14 8:40 UTC (permalink / raw)
To: Randy Dunlap
Cc: linux-sound, Jaroslav Kysela, Takashi Iwai, Shengjiu Wang,
Xiubo Li, Fabio Estevam, Nicolin Chen, linuxppc-dev,
Maciej Strozek, Bard Liao, Pierre-Louis Bossart, patches,
Srinivas Kandagatla, linux-arm-msm, Liam Girdwood, Mark Brown
In-Reply-To: <20260714014445.569992-11-rdunlap@infradead.org>
On Mon, Jul 13, 2026 at 06:44:41PM -0700, Randy Dunlap wrote:
> Add a kernel-doc comment for @is_volatile in struct sdca_control.
> Correct 2 malformed enum names to match the enums.
> Fixes 3 warnings:
>
> Warning: include/sound/sdca_function.h:306 expecting prototype for enum sdca_set_index_range. Prototype was for enum sdca_fdl_set_index_range instead
>
> Warning: include/sound/sdca_function.h:829 struct member 'is_volatile' not described in 'sdca_control'
>
> Warning: include/sound/sdca_function.h:1152 expecting prototype for enum sdca_xu_reset_machanism. Prototype was for enum sdca_xu_reset_mechanism instead
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
Thanks for fixing those up.
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks,
Charles
^ permalink raw reply
* Re: [patch 4/4] entry, treewide: Make syscall_enter_from_user_mode[_work]() indicate syscall execution
From: Michal Suchánek @ 2026-07-14 8:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, Michael Ellerman, Shrikanth Hegde, linuxppc-dev,
Huacai Chen, loongarch, Paul Walmsley, Palmer Dabbelt,
linux-riscv, Sven Schnelle, linux-s390, x86, Mark Rutland,
Jinjie Ruan, Magnus Lindholm, Mukesh Kumar Chaurasiya (IBM),
Jonathan Corbet, Radu Rendec
In-Reply-To: <alXlSUcT2TWOlM7r@kunlun.suse.cz>
On Tue, Jul 14, 2026 at 09:29:13AM +0200, Michal Suchánek wrote:
> On Tue, Jul 14, 2026 at 12:20:49AM +0200, Thomas Gleixner wrote:
> > On Mon, Jul 13 2026 at 10:44, Michal Suchánek wrote:
> > > On Sun, Jul 12, 2026 at 11:25:32PM +0200, Thomas Gleixner wrote:
> > >> The return values of syscall_enter_from_user_mode[_work]() are
> > >> non-intuitive. Both functions return the syscall number which should be
> > >> invoked by the architecture specific syscall entry code. The returned
> > >> number can be:
> > >>
> > >> - the unmodified syscall number which was handed in by the caller
> > >>
> > >> - a modified syscall number (ptrace, seccomp, trace/probe/bpf)
> > >>
> > >> That has an additional twist. If the return value is -1L then the caller is
> > >> not allowed to modify the return value as that indicates that the modifying
> > >> entity requests to abort the syscall and set the return value already. That
> > >> can obviously not be differentiated from a syscall which handed in -1 as
> > >> syscall number.
> > >>
> > >> The most trivial way to deal with that is:
> > >>
> > >> set_return_value(regs, -ENOSYS);
> > >> nr = syscall_enter_from_user_mode(regs, nr);
> > >> if (valid(nr))
> > >> handle_syscall(regs, nr);
> > >>
> > >> That's what LOONGARCH, RISCV, and X86 do. But PowerPC and S390 do not
> > >> preset the return value, so when user space hands in -1 and there is
> > >> nothing setting the return value in the entry work code, then the syscall
> > >> is skipped but the return value is whatever random data has been in the
> > >> return value register.
> > >
> > > The reason why PowerPC and S390 do not preset the return value is that
> > > the return value uses the same register as the syscall number. There are
> > > apparently other architectures on which the return value overlaps with
> > > the arguments which also do not preset the return value for that reason.
> > > If they would use the generic entry the same problem would arise.
> >
> > That's an implementation choice of PPC/S390 as I explained before, which
> > could trivially be solved by having an explicit pt_regs->return_val
> > member,
Sorry, I got confused, and was looking at the previous revision of the
patchset.
In this revision I do not see any obvious problem that did not exist
before.
Thanks
Michal
^ permalink raw reply
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