linux-kernel - Re: [PATCH 1/2] PCI: endpoint: Introduce 'get

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: <CAAEEuhp+ZtjrU1986CJE5nmFy97YPdnfd1Myoufr+6TgjRODeA@mail.gmail.com>
Date: Fri, 26 Jul 2024 13:21:32 +0200
From: Rick Wertenbroek <rick.wertenbroek@...il.com>
To: Damien Le Moal <dlemoal@...nel.org>
Cc: Niklas Cassel <cassel@...nel.org>, 
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, rick.wertenbroek@...g-vd.ch, 
	alberto.dassatti@...g-vd.ch, Krzysztof Wilczyński <kw@...ux.com>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, Frank Li <Frank.Li@....com>, 
	Lars-Peter Clausen <lars@...afoo.de>, linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH 1/2] PCI: endpoint: Introduce 'get_bar' to map fixed
 address BARs in EPC

On Fri, Jul 26, 2024 at 12:53 AM Damien Le Moal <dlemoal@...nel.org> wrote:
>
> On 7/26/24 06:52, Niklas Cassel wrote:
> > On Thu, Jul 25, 2024 at 10:06:52PM +0530, Manivannan Sadhasivam wrote:
> >>
> >> I vary with you here. IMO EPF drivers have no business in knowing the BAR
> >> location as they are independent of controller (mostly except drivers like MHI).
> >> So an EPF driver should call a single API that just allocates/configures the
> >> BAR. For fixed address BAR, EPC core should be able to figure it out using the
> >> EPC features.
> >>
> >> For naming, we have 3 proposals as of now:
> >>
> >> 1. pci_epf_setup_bar() - This looks good, but somewhat collides with the
> >> existing pci_epc_set_bar() API.
> >>
> >> 2. pci_epc_alloc_set_bar() - Looks ugly, but aligns with the existing APIs.
> >>
> >> 3. pci_epc_get_bar() - Also looks good, but the usage of 'get' gives the
> >> impression that the BAR is fetched from somewhere, which is true for fixed
> >> address BAR, but not for dynamic BAR.
> >
> > pci_epc_configure_bar() ?
> > we could name the 'struct pci_epf_bar *' param 'conf'
>
> +1
>
> But let's spell this out: pci_epc_configure_bar(), to be sure to avoid any
> possible confusion (config could mean configure or configuration...).
>
> --
> Damien Le Moal
> Western Digital Research
>

+1

One thing to keep in mind is that 'struct pci_epf_bar 'conf' would be
an 'inout' parameter, where 'conf' gets changed in case of a fixed
address BAR or fixed 64-bit etc. This means the EPF code needs to
check 'conf' after the call. Also, if the caller sets flags and the
controller only handles different flags, do we return an error, or
configure the BAR with the only possible flags and let the caller
check if those flags are ok for the endpoint function ?

This is a bit unclear for me for the moment.

One solution could be to pass two 'pci_epf_bar' structs, one is to be
filed by the function call, and the other an optional parameter
'desired_conf' (i.e. can be 'NULL') if NULL it would mean let the
controller decide, if not NULL 'desired_conf' describes the EPF
requirements (e.g., chosen BAR address or certain flags, size, etc.)
and 'pci_epc_configure_bar()' would fail if it cannot satisfy the
'desired_conf' requirements.

But still 'desired_conf' is a structure with several things that could
be independently desired, e.g., a given address or BAR size, flags,
but not all of them might be a necessity.

Let me know what you think.

Have a nice week-end.

Rick