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:   Thu, 24 Sep 2020 09:09:51 -0600
From:   Rob Herring <robh@...nel.org>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     Jisheng Zhang <Jisheng.Zhang@...aptics.com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Jingoo Han <jingoohan1@...il.com>,
        Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
        Thierry Reding <treding@...dia.com>,
        Vidya Sagar <vidyas@...dia.com>,
        PCI <linux-pci@...r.kernel.org>,
        linux-omap <linux-omap@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] PCI: dwc: Move allocate and map page for msi out of dw_pcie_msi_init()

On Thu, Sep 24, 2020 at 8:33 AM Ard Biesheuvel <ardb@...nel.org> wrote:
>
> On Thu, 24 Sep 2020 at 15:45, Ard Biesheuvel <ardb@...nel.org> wrote:
> >
> > On Thu, 24 Sep 2020 at 15:28, Rob Herring <robh@...nel.org> wrote:
> > >
> > > On Thu, Sep 24, 2020 at 5:00 AM Ard Biesheuvel <ardb@...nel.org> wrote:
> > > >
> > > > On Wed, 23 Sep 2020 at 08:28, Jisheng Zhang <Jisheng.Zhang@...aptics.com> wrote:
> > > > >
> > > > > Currently, dw_pcie_msi_init() allocates and maps page for msi, then
> > > > > program the PCIE_MSI_ADDR_LO and PCIE_MSI_ADDR_HI. The Root Complex
> > > > > may lose power during suspend-to-RAM, so when we resume, we want to
> > > > > redo the latter but not the former. If designware based driver (for
> > > > > example, pcie-tegra194.c) calls dw_pcie_msi_init() in resume path, the
> > > > > previous msi page will be leaked.
> > > > >
> > > > > Move the allocate and map msi page from dw_pcie_msi_init() to
> > > > > dw_pcie_host_init() to fix this problem.
> > > > >
> > > > > Fixes: 56e15a238d92 ("PCI: tegra: Add Tegra194 PCIe support")
> > > > > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@...aptics.com>
> > > >
> > > > Why do you allocate a page for this in the first place? Isn't
> > > > PCIE_MSI_ADDR_HI:PCIE_MSI_ADDR_LO simply a magic DMA address that
> > > > never gets forwarded across to the CPU side of the host bridge, and
> > > > triggers a SPI instead, which gets handled by reading
> > > > PCIE_MSI_INTR0_STATUS ?
> > >
> > > My question too after digging into this some more. I've asked the
> > > question on the thread that further complicated all this changing from
> > > virt_to_phys() to dma_map_page()[1].
> > >
> > > > Couldn't you just map the zero page instead?
> > >
> > > Why a page even? You could use PCIE_MSI_ADDR_LO address itself even.
> > > Or just an address in the driver data which is what some other drivers
> > > do.
> > >
> >
> > PCIE_MSI_ADDR_LO itself could collide with an actual DRAM address if
> > any translation is applied on inbound transactions. Using an actual
> > DRAM address avoids that.

Good point, and the inbound windows could be less than all DRAM if
there's any restrictions. However, the DWC doesn't do any inbound
setup (at least for platforms with iATU), so I guess the default is
all traffic is forwarded.

> ... although the MSI doorbell register on the GIC, for instance, needs
> to be DMA addressable as well, of course.

There's at least one DWC driver that has its own doorbell register
too. I suppose there's a way for the host driver to retrieve the GIC
address if configuration is needed. Doesn't seem to be needed on DWC
given the above.

Rob

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ