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: <ZzO8jn6OpgFhl4TN@lizhi-Precision-Tower-5810>
Date: Tue, 12 Nov 2024 15:37:34 -0500
From: Frank Li <Frank.li@....com>
To: Niklas Cassel <cassel@...nel.org>
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

On Tue, Nov 12, 2024 at 09:14:49PM +0100, Niklas Cassel wrote:
> 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.

Oh, you said at
https://lore.kernel.org/imx/ZzIVzfkZe-hkAb4G@ryzen/T/#mc10e69e0e1e20cc8d8da9a8808177407d22bce06

I think you give up your idea about helper function, because it is one
for doorbell_offset, the other is for the atu address. bar's struct is
difference with reg. even it is similar,

one if use "addr & (align - 1)" to get offset, the other is "addr & ~algin"
to get base address.

>
> 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.

Do you means add help function, which wrap epc's .align_addr()? I know you
make some improvement about EP's alignment, but I have not realized that
related this thread at all.

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

May "I now see why you did this.
One function is using the db offset, and the other is using the db base."
mis-lead me.

If I understand correct here, I can add wrap function for epc's
.align_addr(). at next version.

Frank
>
>
> Kind regards,
> Niklas