linux-kernel - Re: [PATCH v6 3/5] PCI: endpoint: pci-epf-test: Add doorbell test support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZzO3OcCNtHUfm867@x1-carbon>
Date: Tue, 12 Nov 2024 21:14:49 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Frank Li <Frank.Li@....com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	imx@...ts.linux.dev, dlemoal@...nel.org, maz@...nel.org,
	tglx@...utronix.de, jdmason@...zu.us
Subject: Re: [PATCH v6 3/5] PCI: endpoint: pci-epf-test: Add doorbell test
 support

Hello Frank,

On Tue, Nov 12, 2024 at 12:48:16PM -0500, Frank Li wrote:
> Add three registers: doorbell_bar, doorbell_addr, and doorbell_data,
> along with doorbell_done. Use pci_epf_alloc_doorbell() to allocate a
> doorbell address space.
> 
> Enable the Root Complex (RC) side driver to trigger pci-epc-test's doorbell
> callback handler by writing doorbell_data to the mapped doorbell_bar's
> address space.
> 
> Set doorbell_done in the doorbell callback to indicate completion.
> 
> To avoid broken compatibility, add new command COMMAND_ENABLE_DOORBELL
> and COMMAND_DISABLE_DOORBELL. Host side need send COMMAND_ENABLE_DOORBELL
> to map one bar's inbound address to MSI space. the command
> COMMAND_DISABLE_DOORBELL to recovery original inbound address mapping.
> 
> 	 	Host side new driver	Host side old driver
> 
> EP: new driver      S				F
> EP: old driver      F				F
> 
> S: If EP side support MSI, 'pcitest -B' return success.
>    If EP side doesn't support MSI, the same to 'F'.
> 
> F: 'pcitest -B' return failure, other case as usual.
> 
> Tested-by: Niklas Cassel <cassel@...nel.org>
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
> Change from v5 to v6
> - rename doorbell_addr to doorbell_offset

Is there a reason why you chose to not incorporate the helper function
that I suggested here:
https://lore.kernel.org/linux-pci/ZzMtKUFi30_o6SwL@ryzen/

I didn't see any reply from you to that message.

Personally I think that it is nice to have the alignment code in a single
function, rather than duplicating the code. The helper also looks quite
similar to how we do outbound address translation alignment:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=endpoint&id=e73ea1c2d4d8f7ba5daaf7aa51171f63cf79bcd8
so that people will recognize the pattern more easily.

But I guess you didn't like my suggestion?
(Which is fine, but I would have expected some motivation.)


Kind regards,
Niklas