On 8/17/26 6:47 PM, Jakub Kicinski wrote:
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Hi Jakub,
|Thanks for the v5 review. v6 is
posted:https://lore.kernel.org/r/[email protected] |
|Per-comment replies below.|
---
ibmveth: Add MQ RX hypercall wrappers and call definitions
This adds the PHYP call IDs for H_REG_LOGICAL_LAN_QUEUE,
H_ADD_LOGICAL_LAN_BUFFERS_QUEUE and H_FREE_LOGICAL_LAN_QUEUE, raising
MAX_HCALL_OPCODE accordingly, and adds matching static inline wrappers in
ibmveth.h. It is plumbing only, with no runtime behaviour change until a
later commit enables multi_queue.
diff --git a/drivers/net/ethernet/ibm/ibmveth.h
b/drivers/net/ethernet/ibm/ibmveth.h
index d87713668ed30..c0ef03cad9b9a 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -66,6 +66,142 @@ static inline long h_add_logical_lan_buffers(unsigned long
unit_address,
[ ... ]
+/**
+ * h_add_logical_lan_buffers_queue - Add buffers to subordinate queue
+ * @unit_address: Device unit address
+ * @queue_handle: Queue handle from h_register_logical_lan_queue()
[Severity: Low]
This isn't a bug, but do these two kernel-doc blocks in the same header
contradict each other about which handles are valid here?
h_add_logical_lan_buffers_queue() is documented as "Add buffers to
subordinate queue" with "@queue_handle: Queue handle from
h_register_logical_lan_queue()", while h_register_logical_lan_with_handle()
below says its queue 0 handle:
* Registers the primary receive queue (queue 0) with the hypervisor and
* returns the queue handle. This is needed in multi-queue mode to use
* h_add_logical_lan_buffers_queue() for all queues including queue 0.
By the end of the series the second wording is the one that matches the
code: ibmveth_add_logical_lan_buffers() in
drivers/net/ethernet/ibm/ibmveth.c passes
adapter->queue_handle[queue_index] for every index, queue_index == 0
included, and that handle comes from h_register_logical_lan_with_handle().
Would it be clearer to widen the add-buffers kdoc to say the handle may
come from either h_register_logical_lan_queue() or
h_register_logical_lan_with_handle(), so a future caller doesn't conclude
queue 0 must use the legacy buffer hcalls?
Agreed. v6 widens @queue_handle on both add-buffers and free-queue so
the handle may come from either register helper.
+ * @buffersznum: Buffer size (upper 32 bits) | count (lower 32 bits)
+ * @ioba12: Buffer addresses 1 and 2 packed ((addr1 << 32) | addr2)
+ * @ioba34: Buffer addresses 3 and 4 packed
+ * @ioba56: Buffer addresses 5 and 6 packed
+ * @ioba78: Buffer addresses 7 and 8 packed
+ * @ioba910: Buffer addresses 9 and 10 packed
+ * @ioba1112: Buffer addresses 11 and 12 packed
+ *
+ * Return:
+ * H_SUCCESS - All buffers added successfully
+ * H_PARAMETER - Invalid parameters
+ * H_HARDWARE - Hardware error
+ */
+static inline long h_add_logical_lan_buffers_queue(unsigned long unit_address,
+ unsigned long queue_handle,
[ ... ]
+/**
+ * h_register_logical_lan_with_handle - Register primary queue and get handle
+ * @unit_address: Device unit address
+ * @buffer_list: DMA address of buffer list
+ * @rec_queue: Buffer descriptor of receive queue
+ * @filter_list: DMA address of filter list
+ * @mac_address: MAC address
+ * @queue_handle: Output parameter for queue handle (may be NULL)
+ *
+ * Registers the primary receive queue (queue 0) with the hypervisor and
+ * returns the queue handle. This is needed in multi-queue mode to use
+ * h_add_logical_lan_buffers_queue() for all queues including queue 0.
+ *
+ * Return: H_SUCCESS (0) on success, error code otherwise
+ */
[ ... ]
Cross-instance finding from sashiko-gemini
(34d1289184ae332db8c101e251d3dec96e9969f2e8dee8d98c3efff05039c9d0):
[Severity: High]
Calling plpar_hcall9() with missing arguments causes an out-of-bounds stack
read, leaking uninitialized kernel stack memory to the hypervisor.
Yes. Fixed in v6. Both new registration wrappers use plpar_hcall().
h_add_logical_lan_buffers_queue() still needs all nine args. I left
the pre-existing h_send_logical_lan() underfill alone; happy to send
that separately.
Thanks
Mingming
