linux-kernel - Re: [PATCH v7 3/9] PCI: endpoint: functions/pci-epf-test: Add second entry

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]

Date:   Fri, 4 May 2018 11:28:58 +0530
From:   Kishon Vijay Abraham I <kishon@...com>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>
CC:     Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "Joao.Pinto@...opsys.com" <Joao.Pinto@...opsys.com>,
        "jingoohan1@...il.com" <jingoohan1@...il.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v7 3/9] PCI: endpoint: functions/pci-epf-test: Add second
 entry

Hi Lorenzo,

On Thursday 03 May 2018 07:46 PM, Lorenzo Pieralisi wrote:
> On Thu, May 03, 2018 at 12:03:15PM +0530, Kishon Vijay Abraham I wrote:
> 
> [...]
> 
>>>> Since the linkup notifier and BAR index (where auxiliary registers are
>>>> located) may be configurable and is something platform dependent,
>>>> perhaps the configuration of this variables should be done by module
>>>> parameter and not by configfs, leaving this configuration
>>>> responsibility in charge of each platform.
>>>
>>> They are platform dependent because they depend on the EP controller.
>>> That's why I said that those are EP controller parameters. I do not
>>> think they are module parameters either - they should be part of HW
>>> description in firmware.
>>
>> The problem is because pci-epf-test cannot be described in HW. pci-epf-test is
>> also not automatically bound to the EP controller but is bound by the user like
>> below
>> ln -s functions/pci_epf_test/func1 controllers/51000000.pcie_ep/
>>
>> So given that user anyways has to bind the epf device to the controller, based
>> on the platform the user can use a different configfs entry like below
>> ln -s functions/pci_epf_test_dw/func1 controllers/51000000.pcie_ep/ or
>> ln -s functions/pci_epf_test_k2g/func1 controllers/21800000.pcie-ep/
>>
>> If the epf can be described in dt, then something like below can be done
>> pcie1_ep: pcie_ep@...00000 {
>> 	compatible = "ti,dra7-pcie-ep";
>> 	interrupts = <0 232 0x4>;
>> 	num-lanes = <1>;
>> 	num-ib-windows = <4>;
>> 	num-ob-windows = <16>;
>> 	phys = <&pcie1_phy>;
>> 	phy-names = "pcie-phy0";
>> 	pci_epf_test: pci_epf_test@0 {
>> 		name = "pci_epf_test_dw";
>> 		<other properties>;
>> 	}
>> };
>>
>> With this pci-dra7xx.c driver should create pci_epf_device using
>> pci_epf_create("pci_epf_test_dw");
>>
>> Then the driver_data corresponding to "pci_epf_test_dw" will select linkup
>> notifier or BAR index etc.
> 
> Those two properties are properties of the EP controller (it is not 100%
> clear to me how the test BAR register is defined), is this correct ?

Right, these properties are specific to a platform. In some of the platforms
like K2G (BAR0 is reserved i.e it is used to map PCIe app registers and cannot
be used by pci_epf_test. In such cases we should use a BAR other than BAR0).
> 
> If yes, given that those properties are not useful before an EPF is
> bound to an EPC, can't they be retrieved at bind time from the EPC
> controller data (that we can add through DT bindings) ?

hmm..

We can have quirk in pci_epc, something like below

struct pci_epc {
	.
	.
	unsigned int quirks;
	.
	.
};

#define EPC_QUIRKS_NO_LINKUP_NOTIFIER	BIT(0)
#define EPC_QUIRKS_BAR0_RESERVED	BIT(1)
#define EPC_QUIRKS_BAR1_RESERVED	BIT(2)
#define EPC_QUIRKS_BAR2_RESERVED	BIT(3)
#define EPC_QUIRKS_BAR3_RESERVED	BIT(4)
#define EPC_QUIRKS_BAR4_RESERVED	BIT(5)
#define EPC_QUIRKS_BAR5_RESERVED	BIT(6)

The controller driver can set the appropriate quirks
epc->quirks |= EPC_QUIRKS_NO_LINKUP_NOTIFIER | EPC_QUIRKS_BAR0_RESERVED;

Then pci-epf-test driver can checks the quirks to see the supported EPC features.

Does something like above looks okay to you?

Thanks
Kishon