Message-ID: <5f978a20-3f28-4282-8688-b05f3a1f21b8@socionext.com>
Date: Mon, 23 Dec 2024 20:51:42 +0900
From: Kunihiko Hayashi <hayashi.kunihiko@...ionext.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>, Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] misc: pci_endpoint_test: Set reserved BARs for each
 SoCs

Hi Niklas,

On 2024/12/19 22:08, Niklas Cassel wrote:
> On Thu, Dec 19, 2024 at 08:17:50PM +0900, Kunihiko Hayashi wrote:
>> On 2024/12/17 17:19, Niklas Cassel wrote:
>>
>>> If you simply add code that disables all BARs by default in am654, you
>>> should be able to remove these ugly is_am654_pci_dev() checks in the
>>> host
>>> driver, and the host driver should not be able to write to these
>>> reserved
>>> BARs, as they will never get enabled by pci-epf-test.c.
>>
>> However, dw_pcie_ep_reset_bar() only clears BAR registers to 0x0. BAR
>> doesn't have any "disabled" field, so I think that it means "32-bit,
>> memory,
>> non-prefetchable".
> 
>  From the DWC databook, 4.60a, section "6.1.2 BAR Details",
> heading "Disabling a BAR".
> 
> "To disable a BAR (in any of the three schemes), your application can
> write ‘0’ to the LSB of the BAR mask register."
> 
> dw_pcie_ep_reset_bar() calls __dw_pcie_ep_reset_bar(), which will
> write a zero to the LSB of the BAR mask register:
> https://github.com/torvalds/linux/blob/v6.13-rc3/drivers/pci/controller/dwc/pcie-designware-ep.c#L50

Now I missed it, and also find this description in the databook.

>>
>> https://github.com/torvalds/linux/blob/v6.13-rc3/drivers/pci/controller/dwc/pcie-designware-ep.c#L47-L52
>>
>> And even if each endpoint driver marks "BAR_RESERVED" to the features, it
>> is
>> only referred to as excluded BARs when searching for free BARs. So the
>> host
>> will recognize this "reserved" BAR.
> 
> A BAR that has been disabled on the EP side, will not have a size/
> be visible on host side.
> 
> Like I said, rk3588 calls dw_pcie_ep_reset_bar() on all BARs in EP init,
> like most DWC based EPC drivers, and marks BAR4 as reserved.
> This is how it looks on the host side during enumeration:
> 
> [   25.496645] pci 0000:01:00.0: [1d87:3588] type 00 class 0xff0000 PCIe
> Endpoint
> [   25.497322] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x000fffff]
> [   25.497863] pci 0000:01:00.0: BAR 1 [mem 0x00000000-0x000fffff]
> [   25.498400] pci 0000:01:00.0: BAR 2 [mem 0x00000000-0x000fffff]
> [   25.498937] pci 0000:01:00.0: BAR 3 [mem 0x00000000-0x000fffff]
> [   25.499498] pci 0000:01:00.0: BAR 5 [mem 0x00000000-0x000fffff]
> [   25.500036] pci 0000:01:00.0: ROM [mem 0x00000000-0x0000ffff pref]
> [   25.500861] pci 0000:01:00.0: supports D1 D2
> [   25.501240] pci 0000:01:00.0: PME# supported from D0 D1 D3hot

Surely writing 0 to dbi2.BARn (BAR mask) register disables BARn.
I tried to do that using UniPhier SoC, and found disabling any BARs in
the same way.

On the other hand, some other SoCs might have BAR masks fixed by the DWC
IP configuration. These BARs will be exposed to the host even if the BAR
mask is set to 0. However, such case hasn't been upstreamed, so there is
no need to worry about them now.

The am654 driver just doesn't call dw_pcie_ep_reset_bar(), so this case
probably doesn't apply.

Thank you,

---
Best Regards
Kunihiko Hayashi

Powered by blists - more mailing lists