From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 61BD24EBAC7 for ; Thu, 3 Sep 2026 18:10:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788459032; cv=none; b=AosUDMQ2dpDAYLyp0o/40MMO9f6CZs2VaWQBMnEJLB4f9viiu3+1FFB5/YKtvhQk8Qzy0t0KffKzgvYKyZ9he11GodG220r4R7j3hEQtiFh1vC3xmVEXd2RNxKDrkQbyPXt7LZWDMfxvovm9JVRxyN50EwYJW/W/l/oFLxkdZiw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788459032; c=relaxed/simple; bh=+SFVnoKlIRD+ZTq+mhuCnbpjPN1Wj67ah+jO05IeITs=; h=Subject:From:To:Cc:Date:Message-ID:In-Reply-To:References: Content-Type:MIME-Version; b=Qne1pVoP3z1wy+c5itHMn4jY+zXpvj5eOk9rCCci6N65lznOQx2+eQIsl94yj5w7p3HJ2mZfjcDOOT4gMN85KZRXyDWCXFnOP8nm6g49b787mCOS6BiMYTOlcJW40Ro0oh0sgfB5m5GRRd4uw/ygufO7a+Xp6wsAJhiQJgfRpuM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=V+fHpkUm; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="V+fHpkUm" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1B0951F00A3D; Thu, 3 Sep 2026 18:10:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1788459031; bh=NlRDybBs4oFnFdw0mKBg+OGXTcovPntU/gWkRhWZePI=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=V+fHpkUm+RCpDXrEErTITR2ZdmWdnnKWAZQV1V4kNjOJZtt/AhhtPdJajmKiN96tb ideBysxS1OgO8MBm8CS0E2zX3QiYAX/fRV9JiJRiBTMuH5jgOM0JAwqmdWz9WTP+rB JL8HAzUI40qilXNLmI+SDP/bLwQZW03aQXfzfql+cXXHoX4hL35PHEtdqskX+yPwSS YS9jLqNZapNxwFK//fUuMjDiwbWr4nPYftZFXv6WzgpS0crwfVZSosQ0Lmp7gG2Zyg 63PPZJaNIWgylp5eqJQD+0WKIQFZhqNlyTdi672t4qVv2K6aedtETRrCC3lLkHHdTo ipS6HlWF0T1fg== Subject: Re: [net-next,v6,01/15] ibmveth: Add MQ RX hypercall wrappers and call definitions From: netdev-bot+sashiko@kernel.org To: mmc@linux.ibm.com Cc: netdev@vger.kernel.org, davem@davemloft.net, kuba@kernel.org, horms@kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, nnac123@linux.ibm.com, maddy@linux.ibm.com, mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org, haren@linux.ibm.com, ricklind@linux.ibm.com, davemarq@linux.ibm.com, bjking1@linux.ibm.com, shaik.abdulla1@ibm.com Date: Thu, 03 Sep 2026 18:10:29 +0000 Message-ID: <178845902960.3394541.10707165174722907578@kernel.org> In-Reply-To: References: X-sashiko-severity: Low Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 3 - [Low] The commit message body contains a verbatim duplicated sentence. - [Low] kernel-doc for the three new MQ hcall wrappers added by this patch is inconsistent about the H_FUNCTION (unsupported-hcall) return. - [Low] The in-tree perf hypercall decoder is not updated for the three opcodes added by this patch. -- ibmveth: Add MQ RX hypercall wrappers and call definitions This adds the PAPR 11.20.00 hcall IDs H_REG_LOGICAL_LAN_QUEUE (0x49C), H_ADD_LOGICAL_LAN_BUFFERS_QUEUE (0x4A0) and H_FREE_LOGICAL_LAN_QUEUE (0x4A8), raising MAX_HCALL_OPCODE, and adds matching static inline wrappers in ibmveth.h plus h_register_logical_lan_with_handle() so queue 0 can capture the PHYP queue handle. A few questions below, all on the changelog text and the kernel-doc. > Add ibmveth.h wrapper helpers (h_register_logical_lan_queue(), > h_add_logical_lan_buffers_queue(), h_free_logical_lan_queue()) with > argument ordering and return semantics matching the existing ibmveth > hcall wrappers. h_free_logical_lan_queue() uses plpar_hcall_norets() > like h_free_logical_lan(). Also add h_register_logical_lan_with_handle() > so queue 0 can capture the PHYP queue handle in MQ mode. Both new > registration wrappers use plpar_hcall() rather than plpar_hcall9(), so > they do not read unwritten stack slots. Both new > registration wrappers use plpar_hcall() rather than plpar_hcall9(), so > they do not read unwritten stack slots. [Severity: Low] This isn't a bug, but the sentence "Both new registration wrappers use plpar_hcall() rather than plpar_hcall9(), so they do not read unwritten stack slots." appears twice in a row here. Was the second copy meant to be dropped when the v6 rationale was folded into the changelog body? > diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h > index dff90a7d7f70b..cb0ea53491e6c 100644 > --- a/arch/powerpc/include/asm/hvcall.h > +++ b/arch/powerpc/include/asm/hvcall.h > @@ -362,7 +362,11 @@ > #define H_GUEST_DELETE 0x488 > #define H_PKS_WRAP_OBJECT 0x490 > #define H_PKS_UNWRAP_OBJECT 0x494 > -#define MAX_HCALL_OPCODE H_PKS_UNWRAP_OBJECT > +/* 0x498 reserved; 0x4A4 = H_FREE_LOGICAL_LAN_BUFFER_QUEUE (unused here) */ > +#define H_REG_LOGICAL_LAN_QUEUE 0x49C > +#define H_ADD_LOGICAL_LAN_BUFFERS_QUEUE 0x4A0 > +#define H_FREE_LOGICAL_LAN_QUEUE 0x4A8 > +#define MAX_HCALL_OPCODE H_FREE_LOGICAL_LAN_QUEUE [Severity: Low] Should tools/perf/scripts/python/powerpc-hcalls.py get entries for these three opcodes too? Its hcall_table currently tracks hvcall.h up to the previous MAX_HCALL_OPCODE: 1168: 'H_PKS_WRAP_OBJECT', 1172: 'H_PKS_UNWRAP_OBJECT', # Platform-specific hcalls used by the Ultravisor 61184: 'H_SVM_PAGE_IN', There is nothing for 1180 (0x49C), 1184 (0x4A0) or 1192 (0x4A8), so tracing the new MQ RX path with that script prints the raw numbers rather than the names. > diff --git a/drivers/net/ethernet/ibm/ibmveth.h b/drivers/net/ethernet/ibm/ibmveth.h > index d87713668ed30..08504d1cafd55 100644 > --- a/drivers/net/ethernet/ibm/ibmveth.h > +++ b/drivers/net/ethernet/ibm/ibmveth.h > @@ -66,6 +66,145 @@ static inline long h_add_logical_lan_buffers(unsigned long unit_address, > desc5, desc6, desc7, desc8); > } > > +/** > + * h_register_logical_lan_queue - Register a subordinate receive queue > + * @unit_address: Device unit address > + * @buffer_list: DMA address of 4KB page for tracking registered buffers > + * @rec_queue: Buffer descriptor of receive queue > + * @queue_handle: Output queue handle on success (may be NULL) > + * @irq: Output hypervisor IRQ number on success (may be NULL) > + * > + * Registers a subordinate receive queue with the hypervisor. > + * > + * Return: > + * H_SUCCESS (0) on success > + * H_PARAMETER if parameters are invalid [Severity: Low] Should H_FUNCTION be listed here as well? H_REG_LOGICAL_LAN_QUEUE (0x49C) is new in PAPR 11.20.00, so firmware predating it returns H_FUNCTION, and h_register_logical_lan_queue() passes the hypervisor status straight back. The later commit in this series ("ibmveth: Add RX queue register helpers for MQ") does distinguish that case: if (lpar_rc == H_FUNCTION) netdev_err(adapter->netdev, "h_register_logical_lan_queue H_FUNCTION for queue %d (firmware MQ unsupported)\n", so a caller reading only this Return: section has no documented way to tell "firmware has no MQ support, fall back to single queue" from a hard error. > +static inline long > +h_register_logical_lan_queue(unsigned long unit_address, > + unsigned long buffer_list, > + unsigned long rec_queue, > + unsigned long *queue_handle, > + unsigned long *irq) > +{ [ ... ] > + * Return: > + * H_SUCCESS - All buffers added successfully > + * H_PARAMETER - Invalid parameters > + * H_HARDWARE - Hardware error > + * H_FUNCTION - Firmware does not support this hcall > + */ > +static inline long h_add_logical_lan_buffers_queue(unsigned long unit_address, [ ... ] > + * Deregisters and frees all structures associated with the subordinate queue. > + * > + * Return: > + * H_SUCCESS - Queue freed successfully > + * H_PARAMETER - Invalid parameters > + * H_HARDWARE - Hardware error > + * H_STATE - VIOA not in valid state > + * H_BUSY / H_LONG_BUSY_* - Resource busy, retry > + */ > +static inline long h_free_logical_lan_queue(unsigned long unit_address, > + unsigned long queue_handle) > +{ > + return plpar_hcall_norets(H_FREE_LOGICAL_LAN_QUEUE, > + unit_address, queue_handle); > +} Same question for h_free_logical_lan_queue(): 0x4A8 is equally new, and plpar_hcall_norets() returns H_FUNCTION unchanged on firmware that lacks it, but only h_add_logical_lan_buffers_queue() documents that status. Was the difference between the three Return: sections intentional? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1788102125.git.mmc%40linux.ibm.com