linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thierry Reding <treding@nvidia.com>
To: Vidya Sagar <vidyas@nvidia.com>
Cc: "Bjorn Helgaas" <helgaas@kernel.org>,
	bhelgaas@google.com, linux-tegra@vger.kernel.org,
	linux-pci@vger.kernel.org, kthota@nvidia.com, swarren@nvidia.com,
	mmaddireddy@nvidia.com, "Michal Simek" <michal.simek@xilinx.com>,
	"Sören Brinkmann" <soren.brinkmann@xilinx.com>,
	"Simon Horman" <horms@verge.net.au>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Arnd Bergmann" <arnd@arndb.de>
Subject: Re: [PATCH] PCI: tegra: limit MSI target address to 32-bit
Date: Fri, 10 Nov 2017 10:37:37 +0100	[thread overview]
Message-ID: <20171110093737.GA25067@ulmo> (raw)
In-Reply-To: <7727b9bc-a44b-4cbe-1839-7e4dd7c2c186@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]

On Fri, Nov 10, 2017 at 12:11:05AM +0530, Vidya Sagar wrote:
> 
> 
> On Thursday 09 November 2017 11:44 PM, Bjorn Helgaas wrote:
> > [+cc Lorenzo]
> > 
> > On Thu, Nov 09, 2017 at 12:48:14PM +0530, Vidya Sagar wrote:
> > > On Thursday 09 November 2017 02:55 AM, Bjorn Helgaas wrote:
> > > > On Mon, Nov 06, 2017 at 11:33:07PM +0530, Vidya Sagar wrote:
> > > > > limits MSI target address to only 32-bit region to enable
> > > > > some of the PCIe end points where only 32-bit MSIs
> > > > > are supported work properly.
> > > > > One example being Marvel SATA controller
> > > > > 
> > > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> > > > > ---
> > > > >   drivers/pci/host/pci-tegra.c | 2 +-
> > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > > > index 1987fec1f126..03d3dcdd06c2 100644
> > > > > --- a/drivers/pci/host/pci-tegra.c
> > > > > +++ b/drivers/pci/host/pci-tegra.c
> > > > > @@ -1531,7 +1531,7 @@ static int tegra_pcie_enable_msi(struct tegra_pcie *pcie)
> > > > >   	}
> > > > >   	/* setup AFI/FPCI range */
> > > > > -	msi->pages = __get_free_pages(GFP_KERNEL, 0);
> > > > > +	msi->pages = __get_free_pages(GFP_DMA, 0);
> > > > >   	msi->phys = virt_to_phys((void *)msi->pages);
> > > > Should this be GFP_DMA32?  See the comment above the GFP_DMA
> > > > definition.
> > > looking at the comments for both GFP_DMA32 and GFP_DMA, I thought GFP_DMA32
> > > is the correct one to use, but, even with that I got >32-bit addresses.
> > > GFP_DMA always gives addresses in <4GB boundary (i.e. 32-bit).
> > > I didn't dig into it to find out why is this the case.
> > This sounds worth looking into (but maybe we don't need the
> > __get_free_pages() at all; see below).  Maybe there's some underlying
> > bug.  My laptop shows this, which looks like it might be related:
> > 
> >    Zone ranges:
> >      DMA      [mem 0x0000000000001000-0x0000000000ffffff]
> >      DMA32    [mem 0x0000000001000000-0x00000000ffffffff]
> >      Normal   [mem 0x0000000100000000-0x00000004217fffff]
> >      Device   empty
> > 
> > What does your machine show?
> I see following in my linux box
>  Zone ranges:
>    DMA      [mem 0x0000000000001000-0x0000000000ffffff]
>    DMA32    [mem 0x0000000001000000-0x00000000ffffffff]
>    Normal   [mem 0x0000000100000000-0x000000106effffff]
>    Device   empty
> 
> and following in my T210 Tegra platform
> Zone ranges:
>   DMA      [mem 0x0000000080000000-0x00000000ffffffff]
>   Normal   [mem 0x0000000100000000-0x000000017fffffff]

This seems to be happening because 64-bit ARM doesn't have the
ZONE_DMA32 Kconfig option, which seems to cause the DMA32 zone
to default to the normal zone (see include/linux/gfp.h).

That's very confusing in conjunction with the kerneldoc comment
for GFP_DMA32 because it isn't actually guaranteed to give you
32-bit addresses for !ZONE_DMA32.

Cc'ing Arnd who knows about these things.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2017-11-10  9:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-06 18:03 [PATCH] PCI: tegra: limit MSI target address to 32-bit Vidya Sagar
2017-11-08 21:25 ` Bjorn Helgaas
2017-11-09  7:18   ` Vidya Sagar
2017-11-09 18:14     ` Bjorn Helgaas
2017-11-09 18:41       ` Vidya Sagar
2017-11-10  9:37         ` Thierry Reding [this message]
2017-11-10 11:57           ` Arnd Bergmann
2017-11-10  9:47         ` David Laight
2018-03-16 17:23         ` Lorenzo Pieralisi
2017-11-10  0:47       ` subrahmanya_lingappa
2017-11-10  9:44         ` Thierry Reding
2017-11-10 11:22       ` Lorenzo Pieralisi
2017-11-10 12:04         ` Robin Murphy
2017-11-10 13:07           ` Thierry Reding
2017-11-20 17:07             ` Lorenzo Pieralisi
2017-11-13 11:06           ` 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=20171110093737.GA25067@ulmo \
    --to=treding@nvidia.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=horms@verge.net.au \
    --cc=kthota@nvidia.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=michal.simek@xilinx.com \
    --cc=mmaddireddy@nvidia.com \
    --cc=soren.brinkmann@xilinx.com \
    --cc=swarren@nvidia.com \
    --cc=vidyas@nvidia.com \
    /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).