linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Marek Behún" <kabel@kernel.org>, "Marc Zyngier" <maz@kernel.org>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 11/23] PCI: aardvark: Fix setting MSI address
Date: Thu, 24 Feb 2022 13:59:01 +0100	[thread overview]
Message-ID: <20220224125901.bmfqrby3lrda36p3@pali> (raw)
In-Reply-To: <20220223181312.GA141319@bhelgaas>

On Wednesday 23 February 2022 12:13:12 Bjorn Helgaas wrote:
> On Fri, Feb 18, 2022 at 03:43:29PM +0100, Marek Behún wrote:
> > On Thu, 17 Feb 2022 11:14:52 -0600
> > Bjorn Helgaas <helgaas@kernel.org> wrote:
> > 
> > > > +	phys_addr_t msi_addr;
> > > >  	u32 reg;
> > > >  	int i;
> > > >  
> > > > @@ -561,6 +561,11 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
> > > >  	reg |= LANE_COUNT_1;
> > > >  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > > >  
> > > > +	/* Set MSI address */
> > > > +	msi_addr = virt_to_phys(pcie);  
> > > 
> > > Strictly speaking, msi_addr should be a pci_bus_addr_t, not a
> > > phys_addr_t, and virt_to_phys() doesn't return a bus address.
> > 
> > the problem here is that as far as we know currently there is no
> > function that converts a virtual address to pci_bus_addr_t like
> > virt_to_phys() does to convert to phys_addr_t.
> > 
> > On Armada 3720 there are PCIe Controller Address Decoder Registers,
> > which such a translating function would need to consult to do the
> > translation. But the default settings of these registers is to map PCIe
> > addresses 1 to 1 to physical addresses, and no driver changes these
> > registers.
> 
> The poorly-named pcibios_resource_to_bus() (I think the name is my
> fault) is the way to convert a CPU physical address to a PCI bus
> address.

But here it is needed to do something different. It is needed to do
inverse mapping of function which converts PCI bus address to CPU
physical address of CPU memory. So to converting CPU physical address of
CPU memory to PCI bus address from PCI bus point of view.

I think that this information is stored in dma_ranges member of struct
pci_host_bridge. But function pcibios_resource_to_bus() looks at the
->windows member which converts CPU physical address of PCI memory (not
CPU memory) to PCI bus address, which is something different. So
pcibios_resource_to_bus() would not work here and it may return
incorrect values (as PCI memory may be different from CPU point of view
and PCI bus point of view).

> This is implemented in terms of the host bridge windows and the
> translation offset in struct resource_entry, which should be set up
> via the pci_add_resource_offset() called from
> devm_of_pci_get_host_bridge_resources().
> 
> > Pali says that other drivers also use phys_addr_t, and most hardware
> > maps 1 to 1 by default.
> 
> Yes.  I think they're all technically incorrect.  Most systems do map
> CPU phys == PCI bus, but I point it out because it's a case where
> copying that pattern to new drivers will eventually bite us.

I agree, it is incorrect but I do not see a way how to do it correctly
because of missing function (which for pci-aardvark should return
identity).

> > So we think that until at least an API for such a function exists, we
> > shuld leave it as it is. I am not against converting the phys_addr_to
> > to a pci_bus_addr_t, but Pali thinks that for now we should leave even
> > that as it is, because the virt_to_phys() function returns phys_addr_t.
> > 
> > We can add a comment there explaining this, if you want.
> > 
> > What do you think?
> > 
> > Marek

  reply	other threads:[~2022-02-24 12:59 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-10  1:49 [PATCH v2 00/23] PCI: aardvark controller fixes BATCH 4 Marek Behún
2022-01-10  1:49 ` [PATCH v2 01/23] PCI: aardvark: Replace custom PCIE_CORE_INT_* macros with PCI_INTERRUPT_* Marek Behún
2022-01-10 17:07   ` Bjorn Helgaas
2022-01-10  1:49 ` [PATCH v2 02/23] PCI: aardvark: Fix reading MSI interrupt number Marek Behún
2022-02-04 17:24   ` Lorenzo Pieralisi
2022-02-05 10:53     ` Marc Zyngier
2022-01-10  1:49 ` [PATCH v2 03/23] PCI: aardvark: Fix support for MSI interrupts Marek Behún
2022-01-10  1:49 ` [PATCH v2 04/23] PCI: aardvark: Rewrite IRQ code to chained IRQ handler Marek Behún
2022-01-10  1:50 ` [PATCH v2 05/23] PCI: aardvark: Check return value of generic_handle_domain_irq() when processing INTx IRQ Marek Behún
2022-01-10  1:50 ` [PATCH v2 06/23] PCI: aardvark: Make MSI irq_chip structures static driver structures Marek Behún
2022-01-10  1:50 ` [PATCH v2 07/23] PCI: aardvark: Make msi_domain_info structure a static driver structure Marek Behún
2022-01-10  1:50 ` [PATCH v2 08/23] PCI: aardvark: Use dev_fwnode() instead of of_node_to_fwnode(dev->of_node) Marek Behún
2022-01-10  1:50 ` [PATCH v2 09/23] PCI: aardvark: Refactor unmasking summary MSI interrupt Marek Behún
2022-01-10  1:50 ` [PATCH v2 10/23] PCI: aardvark: Add support for masking MSI interrupts Marek Behún
2022-01-10  1:50 ` [PATCH v2 11/23] PCI: aardvark: Fix setting MSI address Marek Behún
2022-02-17 17:14   ` Bjorn Helgaas
2022-02-18 14:43     ` Marek Behún
2022-02-23 18:13       ` Bjorn Helgaas
2022-02-24 12:59         ` Pali Rohár [this message]
2022-02-24 19:43           ` Bjorn Helgaas
2022-01-10  1:50 ` [PATCH v2 12/23] PCI: aardvark: Enable MSI-X support Marek Behún
2022-01-10  1:50 ` [PATCH v2 13/23] PCI: aardvark: Add support for ERR interrupt on emulated bridge Marek Behún
2022-01-10  1:50 ` [PATCH v2 14/23] PCI: aardvark: Fix reading PCI_EXP_RTSTA_PME bit " Marek Behún
2022-01-10  1:50 ` [PATCH v2 15/23] PCI: aardvark: Optimize writing PCI_EXP_RTCTL_PMEIE and PCI_EXP_RTSTA_PME " Marek Behún
2022-01-10  1:50 ` [PATCH v2 16/23] PCI: aardvark: Add support for PME interrupts Marek Behún
2022-01-10  1:50 ` [PATCH v2 17/23] PCI: aardvark: Fix support for PME requester on emulated bridge Marek Behún
2022-01-10  1:50 ` [PATCH v2 18/23] PCI: aardvark: Use separate INTA interrupt for emulated root bridge Marek Behún
2022-01-10  1:50 ` [PATCH v2 19/23] PCI: aardvark: Remove irq_mask_ack callback for INTx interrupts Marek Behún
2022-01-10  1:50 ` [PATCH v2 20/23] PCI: aardvark: Don't mask irq when mapping Marek Behún
2022-01-10  1:50 ` [PATCH v2 21/23] PCI: aardvark: Drop __maybe_unused from advk_pcie_disable_phy() Marek Behún
2022-01-10  1:50 ` [PATCH v2 22/23] PCI: aardvark: Update comment about link going down after link-up Marek Behún
2022-01-10  1:50 ` [PATCH v2 23/23] PCI: aardvark: Make main irq_chip structure a static driver structure Marek Behún
2022-01-10  9:28   ` Marc Zyngier
2022-01-10 10:23     ` Marek Behún
2022-01-10 10:53     ` Pali Rohár
2022-01-10 14:44       ` Marc Zyngier
2022-01-10 15:19         ` Marek Behún
2022-02-08 10:50 ` (subset) [PATCH v2 00/23] PCI: aardvark controller fixes BATCH 4 Lorenzo Pieralisi

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=20220224125901.bmfqrby3lrda36p3@pali \
    --to=pali@kernel.org \
    --cc=helgaas@kernel.org \
    --cc=kabel@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=maz@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).