Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Frank Li <Frank.li@nxp.com>
Cc: "Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	imx@lists.linux.dev, dlemoal@kernel.org, maz@kernel.org,
	tglx@linutronix.de, jdmason@kudzu.us
Subject: Re: [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
Date: Thu, 7 Nov 2024 00:57:16 +0100	[thread overview]
Message-ID: <ZywCXOjuTTiayIxd@ryzen> (raw)
In-Reply-To: <ZyujpT+4bd7TwbcM@lizhi-Precision-Tower-5810>

On Wed, Nov 06, 2024 at 12:13:09PM -0500, Frank Li wrote:
> On Wed, Nov 06, 2024 at 10:36:42AM +0100, Niklas Cassel wrote:
> > On Wed, Nov 06, 2024 at 10:15:27AM +0100, Niklas Cassel wrote:
> > >
> > > I do get a domain, but I do not get any IRQ on the EP side when the RC side is
> > > writing the doorbell (as part of pcitest -B),
> > >
> > > [    7.978984] pci_epc_alloc_doorbell: num_db: 1
> > > [    7.979545] pci_epf_test_bind: doorbell_addr: 0x40
> > > [    7.979978] pci_epf_test_bind: doorbell_data: 0x0
> > > [    7.980397] pci_epf_test_bind: doorbell_bar: 0x1
> > > [   21.114613] pci_epf_enable_doorbell db_bar: 1
> > > [   21.115001] pci_epf_enable_doorbell: doorbell_addr: 0xfe650040
> > > [   21.115512] pci_epf_enable_doorbell: phys_addr: 0xfe650000
> > > [   21.115994] pci_epf_enable_doorbell: success
> > >
> > > # cat /proc/interrupts | grep epc
> > > 117:          0          0          0          0          0          0          0          0  ITS-pMSI-a40000000.pcie-ep   0 Edge      pci-epc-doorbell0
> > >
> > > Even if I try to write the doorbell manually from EP side using devmem:
> > >
> > > # devmem 0xfe670040 32 1
> >
> > Sorry, this should of course have been:
> > # devmem 0xfe650040 32 1
> 
> Thank you test it. You can't write it at EP side. ITS identify the bus
> master. master ID (streamid) of CPU is the diffference with PCI master's
> ID (streamid). You set msi-parent = <&its0 0x0000>, not sure if 0x0000 is
> validate stream.

I see, this makes sense since the ITS converts BDF to an MSI specifier.


> 
> You have to run at RC side, "devmem (Bar1+0x40) 32 0".  So PCIe EP
> controller can use EP streamid.
> 
> some system need special register to config stream id, you can refer host
> side's settings.

> <&its0 0x0000>,  second argument is your PCIe controller's stream ID. You
> can ref RC side.

The RC node looks like this:
msi-map = <0x0000 &its1 0x0000 0x1000>;
So it does indeed use 0x0 as the MSI specifier.


> 
> >
> > Considering that the RC node is using &its1, that is probably
> > also what should be used in the EP node when running the controller
> > in EP mode instead of RC mode.
> 
> Generally,  RC node should use smmu-map, instead &its1. Or your PCI
> controller direct use 16bit RID as streamid.

smmu-map? Do you mean iommu-map?

I don't see why we would need to have the SMMU enabled to use ITS.
The iommu is currently disabled on my platform.

I did enable the iommu, and all BAR tests, read tests, write tests,
and copy tests pass. However I get an iommu error when the RC is
writing the doorbell. Perhaps you need to do dma_map_single() on
the address that you are setting the inbound address translation to?



Without the IOMMU, if I modify pci_endpoint_test.c to not send the
DISABLE_DOORBELL command on error (so that EP side still has DB enabled),
I can read all BARs except BAR1 (which was used for the doorbell):
[   21.077417] pci 0000:01:00.0: BAR 0 [mem 0xf0300000-0xf03fffff]: assigned
[   21.078029] pci 0000:01:00.0: BAR 1 [mem 0xf0400000-0xf04fffff]: assigned
[   21.078640] pci 0000:01:00.0: BAR 2 [mem 0xf0500000-0xf05fffff]: assigned
[   21.079250] pci 0000:01:00.0: BAR 3 [mem 0xf0600000-0xf06fffff]: assigned
[   21.079860] pci 0000:01:00.0: BAR 5 [mem 0xf0700000-0xf07fffff]: assigned
# pcitest -B
[   25.156623] COMMAND_ENABLE_DOORBELL complete - status: 0x440
[   25.157131] db_bar: 1 addr: 0x40 data: 0x0
[   25.157501] setting irq_type to: 1
[   25.157802] writing: 0x0 to offset: 0x40 in BAR: 1
[   35.300676] we wrote to the BAR, status is now: 0x0

status is not updated after writing to DB,
and /proc/interrupts on EP side is not incrementing.

# devmem 0xf0300000
0x00000000
# devmem 0xf0400040
0xFFFFFFFF
# devmem 0xf0500000
0x00000000
# devmem 0xf0600000
0x00000000
# devmem 0xf0700000
0x00000000

So there does seem to be something wrong with the inbound translation,
at least when testing on rk3588 which only uses 1MB fixed size BARs:
https://github.com/torvalds/linux/blob/v6.12-rc6/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L276-L281



You also didn't answer which imx platform that you are using to test this,
I don't see a single imx platform that specifies "msi-parent".


Kind regards,
Niklas

  reply	other threads:[~2024-11-06 23:57 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-31 16:26 [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Frank Li
2024-10-31 16:27 ` [PATCH v4 1/5] PCI: endpoint: Add pci_epc_get_fn() API for customizable filtering Frank Li
2024-10-31 16:27 ` [PATCH v4 2/5] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller Frank Li
2024-11-08  0:53   ` Krishna Chaitanya Chundru
2024-10-31 16:27 ` [PATCH v4 3/5] PCI: endpoint: pci-epf-test: Add doorbell test support Frank Li
2024-11-07 21:52   ` Niklas Cassel
2024-11-07 22:44     ` Frank Li
2024-11-07 22:48       ` Niklas Cassel
2024-10-31 16:27 ` [PATCH v4 4/5] misc: pci_endpoint_test: Add doorbell test case Frank Li
2024-11-07 21:42   ` Niklas Cassel
2024-11-07 22:45     ` Frank Li
2024-10-31 16:27 ` [PATCH v4 5/5] tools: PCI: Add 'B' option for test doorbell Frank Li
2024-11-06  9:15 ` [PATCH v4 0/5] PCI: EP: Add RC-to-EP doorbell with platform MSI controller Niklas Cassel
2024-11-06  9:36   ` Niklas Cassel
2024-11-06 17:13     ` Frank Li
2024-11-06 23:57       ` Niklas Cassel [this message]
2024-11-07  0:41         ` Frank Li
2024-11-08  0:07           ` Niklas Cassel
2024-11-07  0:46         ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZywCXOjuTTiayIxd@ryzen \
    --to=cassel@kernel.org \
    --cc=Frank.li@nxp.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=dlemoal@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=imx@lists.linux.dev \
    --cc=jdmason@kudzu.us \
    --cc=kishon@kernel.org \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=maz@kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox