* Re: [PATCH v2 1/3] powerpc/booke64: add sync after writing PTE
From: Benjamin Herrenschmidt @ 2013-10-10 23:51 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1381447532.7979.488.camel@snotra.buserror.net>
On Thu, 2013-10-10 at 18:25 -0500, Scott Wood wrote:
> Looking at some of the code in mm/, I suspect that the normal callers of
> set_pte_at() already have an unlock (and thus a sync)
Unlock is lwsync actually...
> already, so we may
> not even be relying on those retries. Certainly some of them do; it
> would take some effort to verify all of them.
>
> Also, without such a sync in map_kernel_page(), even with software
> tablewalk, couldn't we theoretically have a situation where a store to
> pointer X that exposes a new mapping gets reordered before the PTE store
> as seen by another CPU? The other CPU could see non-NULL X and
> dereference it, but get the stale PTE. Callers of ioremap() generally
> don't do a barrier of their own prior to exposing the result.
Hrm, we transition to the new PTE either restricts the access permission
in which case it flushes the TLB (and synchronizes with other CPUs) or
extends access (adds dirty, set pte from 0 -> populated, ...) in which
case the worst case is we see the old one and take a spurrious fault.
So the problem would only be with kernel mappings and in that case I
think we are fine. A driver doing an ioremap shouldn't then start using
that mapping on another CPU before having *informed* that other CPU of
the existence of the mapping and that should be ordered.
Ben.
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc/booke64: add sync after writing PTE
From: Scott Wood @ 2013-10-10 23:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1381444273.7979.473.camel@snotra.buserror.net>
On Thu, 2013-10-10 at 17:31 -0500, Scott Wood wrote:
> On Mon, 2013-09-16 at 19:06 -0500, Scott Wood wrote:
> > On Mon, 2013-09-16 at 07:38 +1000, Benjamin Herrenschmidt wrote:
> > > On Fri, 2013-09-13 at 22:50 -0500, Scott Wood wrote:
> > > > The ISA says that a sync is needed to order a PTE write with a
> > > > subsequent hardware tablewalk lookup. On e6500, without this sync
> > > > we've been observed to die with a DSI due to a PTE write not being seen
> > > > by a subsequent access, even when everything happens on the same
> > > > CPU.
> > >
> > > This is gross, I didn't realize we had that bogosity in the
> > > architecture...
> > >
> > > Did you measure the performance impact ?
> >
> > I didn't see a noticeable impact on the tests I ran, but those were
> > aimed at measuring TLB miss overhead. I'll need to try it with a
> > benchmark that's more oriented around lots of page table updates.
>
> Lmbench's fork test runs about 2% slower with the sync. I've been told
> that nothing relevant has changed since we saw the failure during
> emulation; it's probably luck and/or timing, or maybe a sync got added
> somewhere else since then? I think it's only really a problem for
> kernel page tables, since user page tables will retry if do_page_fault()
> sees a valid PTE. So maybe we should put an mb() in map_kernel_page()
> instead.
Looking at some of the code in mm/, I suspect that the normal callers of
set_pte_at() already have an unlock (and thus a sync) already, so we may
not even be relying on those retries. Certainly some of them do; it
would take some effort to verify all of them.
Also, without such a sync in map_kernel_page(), even with software
tablewalk, couldn't we theoretically have a situation where a store to
pointer X that exposes a new mapping gets reordered before the PTE store
as seen by another CPU? The other CPU could see non-NULL X and
dereference it, but get the stale PTE. Callers of ioremap() generally
don't do a barrier of their own prior to exposing the result.
-Scott
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Mark Lord @ 2013-10-10 23:17 UTC (permalink / raw)
To: Alexander Gordeev, H. Peter Anvin
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <20131010180704.GA15719@dhcp-26-207.brq.redhat.com>
Just to help us all understand "the loop" issue..
Here's an example of driver code which uses the existing MSI-X interfaces,
for a device which can work with either 16, 8, 4, 2, or 1 MSI-X interrupt.
This is from a new driver I'm working on right now:
static int xx_alloc_msix_irqs (struct xx_dev *dev, int nvec)
{
xx_disable_all_irqs(dev);
do {
if (nvec < 2)
xx_prep_for_1_msix_vector(dev);
else if (nvec < 4)
xx_prep_for_2_msix_vectors(dev);
else if (nvec < 8)
xx_prep_for_4_msix_vectors(dev);
else if (nvec < 16)
xx_prep_for_8_msix_vectors(dev);
else
xx_prep_for_16_msix_vectors(dev);
nvec = pci_enable_msix(dev->pdev, dev->irqs, dev->num_vectors);
} while (nvec > 0);
if (nvec) {
kerr(dev->name, "pci_enable_msix() failed, err=%d", nvec);
dev->num_vectors = 0;
return nvec;
}
return 0; /* success */
}
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc/booke64: add sync after writing PTE
From: Scott Wood @ 2013-10-10 22:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1379376371.2536.218.camel@snotra.buserror.net>
On Mon, 2013-09-16 at 19:06 -0500, Scott Wood wrote:
> On Mon, 2013-09-16 at 07:38 +1000, Benjamin Herrenschmidt wrote:
> > On Fri, 2013-09-13 at 22:50 -0500, Scott Wood wrote:
> > > The ISA says that a sync is needed to order a PTE write with a
> > > subsequent hardware tablewalk lookup. On e6500, without this sync
> > > we've been observed to die with a DSI due to a PTE write not being seen
> > > by a subsequent access, even when everything happens on the same
> > > CPU.
> >
> > This is gross, I didn't realize we had that bogosity in the
> > architecture...
> >
> > Did you measure the performance impact ?
>
> I didn't see a noticeable impact on the tests I ran, but those were
> aimed at measuring TLB miss overhead. I'll need to try it with a
> benchmark that's more oriented around lots of page table updates.
Lmbench's fork test runs about 2% slower with the sync. I've been told
that nothing relevant has changed since we saw the failure during
emulation; it's probably luck and/or timing, or maybe a sync got added
somewhere else since then? I think it's only really a problem for
kernel page tables, since user page tables will retry if do_page_fault()
sees a valid PTE. So maybe we should put an mb() in map_kernel_page()
instead.
-Scott
^ permalink raw reply
* Re: Gianfar driver crashes in Kernel v3.10
From: Scott Wood @ 2013-10-10 21:41 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: Thomas Hühn, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <52568A7A.6090909@freescale.com>
On Thu, 2013-10-10 at 14:07 +0300, Claudiu Manoil wrote:
> On 10/4/2013 3:28 PM, Thomas H=C3=BChn wrote:
> >
> > [code]
> > [ 2671.841927] Oops: Exception in kernel mode, sig: 5 [#1]
> > [ 2671.847141] Freescale P1014
> > [ 2671.849925] Modules linked in: ath9k pppoe ppp_async iptable_nat
> > ath9k_common pppox p
> > e xt_tcpudp xt_tcpmss xt_string xt_statistic xt_state xt_recent xt_qu=
ota
> > xt_pkttype xt_o
> > mark xt_connbytes xt_comment xt_addrtype xt_TCPMSS xt_REDIRECT xt_NET=
MAP
> > xt_LOG xt_IPMAR
> > ms_datafab ums_cypress ums_alauda slhc nf_nat_tftp nf_nat_snmp_basic
> > nf_nat_sip nf_nat_r
> > ntrack_sip nf_conntrack_rtsp nf_conntrack_proto_gre nf_conntrack_irc
> > nf_conntrack_h323 n
> > compat_xtables compat ath sch_teql sch_tbf sch_sfq sch_red sch_prio
> > sch_htb sch_gred sc
> > skbedit act_mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route cls_=
fw
> > sch_hfsc sch_ing
> > r usb_storage leds_gpio ohci_hcd ehci_platform ehci_hcd sd_mod scsi_m=
od
> > fsl_mph_dr_of gp
> > [ 2671.988946] CPU: 0 PID: 5209 Comm: iftop Not tainted 3.10.13 #2
> > [ 2671.994859] task: c4b22220 ti: c7ff8000 task.ti: c477e000
> > [ 2672.000250] NIP: c018c7a0 LR: c018c794 CTR: c000b070
> > [ 2672.005206] REGS: c7ff9f10 TRAP: 3202 Not tainted (3.10.13)
> > [ 2672.011028] MSR: 00029000 <CE,EE,ME> CR: 48000024 XER: 20000000
> > [ 2672.017125]
> > GPR00: 000000ff c477fde0 c4b22220 00000000 00000000 000000ff 00000000
> > 70000000
> > GPR08: ffffffff 00000008 00000000 ffffffff 00000046 10022248 00000000
> > 00000008
> > GPR16: c781b3c0 c781b3c0 000000ff 00000000 00000001 0000021c 00000086
> > fffff800
> > GPR24: c7980300 00000000 00000001 00000040 00000003 c4b33000 00000000
> > 00000001
> > [ 2672.046832] NIP [c018c7a0] gfar_poll+0x424/0x520
> > [ 2672.051442] LR [c018c794] gfar_poll+0x418/0x520
> > [ 2672.055962] Call Trace:
> > [ 2672.058402] [c477fde0] [c018c674] gfar_poll+0x2f8/0x520 (unreliabl=
e)
> > [ 2672.064762] [c477fe80] [c01b0ce8] net_rx_action+0x6c/0x158
> > [ 2672.070249] [c477feb0] [c0027dc4] __do_softirq+0xbc/0x16c
> > [ 2672.075642] [c477ff00] [c0027f7c] irq_exit+0x4c/0x68
> > [ 2672.080604] [c477ff10] [c00041f8] do_IRQ+0xf4/0x10c
> > [ 2672.085478] [c477ff40] [c000ca3c] ret_from_except+0x0/0x18
> > [ 2672.090991] --- Exception: 501 at 0x48083c28
> > [ 2672.090991] LR =3D 0x48083bf8
> > [ 2672.098378] Instruction dump:
> > [ 2672.101338] 7f8f2040 419cfcc4 80900000 38a00000 8061004c 7e118378
> > 81c10050 7ffafb78
> > [ 2672.109092] 4bf9eaa1 83810034 7c7e1b78 8361003c <83210038> 83a1004=
c
> > 48000060 41a2004c
> > [ 2672.117021] ---[ end trace 565fb54528d305fa ]---
> > [ 2672.121628]
> > [ 2673.103130] Kernel panic - not syncing: Fatal exception in interru=
pt
> > [ 2673.109474] Rebooting in 3 seconds..
> >
> > U-Boot 2010.12-svn15934 (Dec 11 2012 - 16:23:49)
> > [/code]
> >
>=20
> Hi,
>=20
> Does this show up on a half duplex (100Mb/s) link?
> Could you provide following for the gianfar interface, on your setup:
> # ethtool ethX
> and
> # ethtool -d ethX | grep 500
>=20
> Is there any other indication before this Oops? Like a tx timeout WARN?
It's a watchdog interrupt (CPU watchdog, not netdev). I think it's only
showing up in the gianfar code because that's what's running (unless the
gianfar code is causing the watchdog daemon to not run).
-Scott
^ permalink raw reply
* RE: [PATCH 1/7] powerpc: Add interface to get msi region information
From: Sethi Varun-B16395 @ 2013-10-10 21:13 UTC (permalink / raw)
To: Bhushan Bharat-R65777, joro@8bytes.org, Bjorn Helgaas
Cc: agraf@suse.de, Wood Scott-B07421, linux-pci@vger.kernel.org,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
alex.williamson@redhat.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07196054@039-SN2MPN1-011.039d.mgd.msft.net>
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Bhushan Bharat-R65777
> Sent: Tuesday, October 08, 2013 10:40 PM
> To: joro@8bytes.org; Bjorn Helgaas
> Cc: alex.williamson@redhat.com; benh@kernel.crashing.org;
> galak@kernel.crashing.org; linux-kernel@vger.kernel.org; linuxppc-
> dev@lists.ozlabs.org; linux-pci@vger.kernel.org; agraf@suse.de; Wood
> Scott-B07421; iommu@lists.linux-foundation.org
> Subject: RE: [PATCH 1/7] powerpc: Add interface to get msi region
> information
>=20
>=20
>=20
> > -----Original Message-----
> > From: joro@8bytes.org [mailto:joro@8bytes.org]
> > Sent: Tuesday, October 08, 2013 10:32 PM
> > To: Bjorn Helgaas
> > Cc: Bhushan Bharat-R65777; alex.williamson@redhat.com;
> > benh@kernel.crashing.org; galak@kernel.crashing.org;
> > linux-kernel@vger.kernel.org; linuxppc- dev@lists.ozlabs.org;
> > linux-pci@vger.kernel.org; agraf@suse.de; Wood Scott- B07421;
> > iommu@lists.linux-foundation.org
> > Subject: Re: [PATCH 1/7] powerpc: Add interface to get msi region
> > information
> >
> > On Tue, Oct 08, 2013 at 10:47:49AM -0600, Bjorn Helgaas wrote:
> > > I still have no idea what an "aperture type IOMMU" is, other than
> > > that it is "different."
> >
> > An aperture based IOMMU is basically any GART-like IOMMU which can
> > only remap a small window (the aperture) of the DMA address space. DMA
> > outside of that window is either blocked completly or passed through
> untranslated.
>=20
> It is completely blocked for Freescale PAMU.
> So for this type of iommu what we have to do is to create a MSI mapping
> just after guest physical address, Example: guest have a 512M of memory
> then we create window of 1G (because of power of 2 requirement), then we
> have to FIT MSI just after 512M of guest.
[Sethi Varun-B16395] PAMU (FSL IOMMU) has a concept of primary window and s=
ubwindows. Primary window corresponds to the complete guest iova address sp=
ace (including MSI space), with respect to IOMMU_API this is termed as geom=
etry . IOVA Base of subwindow is determined from the number of subwindows (=
configurable using iommu API). Subwindows allow for handling physically dis=
contiguous memory. PAMU translates device iova accesses to actual physical =
address. MSI mapping would be addressed by a subwindow, with iova base star=
ting at the end of the guest iova space.=20
VFIO code creates a PAMU window (also defines number of subwindow) to map =
the guest iova space + msi space. The interface defined by this patch queri=
es the PAMU driver to get the iova mapping for the msi region assigned to t=
he PCIe device (assigned to the guest).
-Varun
^ permalink raw reply
* Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
From: Alex Williamson @ 2013-10-10 20:41 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, agraf@suse.de,
iommu@lists.linux-foundation.org, Bhushan Bharat-R65777,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D0A52BAC7@039-SN2MPN1-013.039d.mgd.msft.net>
On Thu, 2013-10-10 at 20:09 +0000, Sethi Varun-B16395 wrote:
>
> > -----Original Message-----
> > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> > bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> > Sent: Tuesday, October 08, 2013 8:43 AM
> > To: Bhushan Bharat-R65777
> > Cc: agraf@suse.de; Wood Scott-B07421; linux-pci@vger.kernel.org;
> > galak@kernel.crashing.org; linux-kernel@vger.kernel.org;
> > iommu@lists.linux-foundation.org; benh@kernel.crashing.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
> >
> > On Mon, 2013-10-07 at 05:46 +0000, Bhushan Bharat-R65777 wrote:
> > >
> > > > -----Original Message-----
> > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > Sent: Friday, October 04, 2013 11:42 PM
> > > > To: Bhushan Bharat-R65777
> > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org
> > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > > device
> > > >
> > > > On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote:
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > Sent: Friday, October 04, 2013 10:43 PM
> > > > > > To: Bhushan Bharat-R65777
> > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > > > > > foundation.org
> > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > > > > device
> > > > > >
> > > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote:
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > > Sent: Friday, October 04, 2013 9:15 PM
> > > > > > > > To: Bhushan Bharat-R65777
> > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > > > > > > > foundation.org
> > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain
> > > > > > > > of a device
> > > > > > > >
> > > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777
> > wrote:
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: linux-pci-owner@vger.kernel.org
> > > > > > > > > > [mailto:linux-pci-owner@vger.kernel.org]
> > > > > > > > > > On Behalf Of Alex Williamson
> > > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM
> > > > > > > > > > To: Bhushan Bharat-R65777
> > > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > > > > > > galak@kernel.crashing.org; linux-
> > > > > > > > > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > > > > > > > > > linux- pci@vger.kernel.org; agraf@suse.de; Wood
> > > > > > > > > > Scott-B07421; iommu@lists.linux- foundation.org; Bhushan
> > > > > > > > > > Bharat-R65777
> > > > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get
> > > > > > > > > > iommu_domain of a device
> > > > > > > > > >
> > > > > > > > > > On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> > > > > > > > > > > This api return the iommu domain to which the device is
> > attached.
> > > > > > > > > > > The iommu_domain is required for making API calls
> > > > > > > > > > > related to
> > > > iommu.
> > > > > > > > > > > Follow up patches which use this API to know iommu
> > maping.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Bharat Bhushan
> > > > > > > > > > > <bharat.bhushan@freescale.com>
> > > > > > > > > > > ---
> > > > > > > > > > > drivers/iommu/iommu.c | 10 ++++++++++
> > > > > > > > > > > include/linux/iommu.h | 7 +++++++
> > > > > > > > > > > 2 files changed, 17 insertions(+), 0 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/drivers/iommu/iommu.c
> > > > > > > > > > > b/drivers/iommu/iommu.c index
> > > > > > > > > > > fbe9ca7..6ac5f50 100644
> > > > > > > > > > > --- a/drivers/iommu/iommu.c
> > > > > > > > > > > +++ b/drivers/iommu/iommu.c
> > > > > > > > > > > @@ -696,6 +696,16 @@ void iommu_detach_device(struct
> > > > > > > > > > > iommu_domain *domain, struct device *dev) }
> > > > > > > > > > > EXPORT_SYMBOL_GPL(iommu_detach_device);
> > > > > > > > > > >
> > > > > > > > > > > +struct iommu_domain *iommu_get_dev_domain(struct
> > device *dev) {
> > > > > > > > > > > + struct iommu_ops *ops = dev->bus->iommu_ops;
> > > > > > > > > > > +
> > > > > > > > > > > + if (unlikely(ops == NULL ||
> > > > > > > > > > > +ops->get_dev_iommu_domain ==
> > > > NULL))
> > > > > > > > > > > + return NULL;
> > > > > > > > > > > +
> > > > > > > > > > > + return ops->get_dev_iommu_domain(dev); }
> > > > > > > > > > > +EXPORT_SYMBOL_GPL(iommu_get_dev_domain);
> > > > > > > > > >
> > > > > > > > > > What prevents this from racing iommu_domain_free()?
> > > > > > > > > > There's no references acquired, so there's no reason for
> > > > > > > > > > the caller to assume the
> > > > > > > > pointer is valid.
> > > > > > > > >
> > > > > > > > > Sorry for late query, somehow this email went into a
> > > > > > > > > folder and escaped;
> > > > > > > > >
> > > > > > > > > Just to be sure, there is not lock at generic "struct
> > > > > > > > > iommu_domain", but IP
> > > > > > > > specific structure (link FSL domain) linked in
> > > > > > > > iommu_domain->priv have a lock, so we need to ensure this
> > > > > > > > race in FSL iommu code (say drivers/iommu/fsl_pamu_domain.c),
> > right?
> > > > > > > >
> > > > > > > > No, it's not sufficient to make sure that your use of the
> > > > > > > > interface is race free. The interface itself needs to be
> > > > > > > > designed so that it's difficult to use incorrectly.
> > > > > > >
> > > > > > > So we can define
> > > > > > > iommu_get_dev_domain()/iommu_put_dev_domain();
> > > > > > > iommu_get_dev_domain() will return domain with the lock held,
> > > > > > > and
> > > > > > > iommu_put_dev_domain() will release the lock? And
> > > > > > > iommu_get_dev_domain() must always be followed by
> > > > > > > iommu_get_dev_domain().
> > > > > >
> > > > > > What lock? get/put are generally used for reference counting,
> > > > > > not locking in the kernel.
> > > > > >
> > > > > > > > That's not the case here. This is a backdoor to get the
> > > > > > > > iommu domain from the iommu driver regardless of who is using
> > it or how.
> > > > > > > > The iommu domain is created and managed by vfio, so
> > > > > > > > shouldn't we be looking at how to do this through vfio?
> > > > > > >
> > > > > > > Let me first describe what we are doing here:
> > > > > > > During initialization:-
> > > > > > > - vfio talks to MSI system to know the MSI-page and size
> > > > > > > - vfio then interacts with iommu to map the MSI-page in iommu
> > > > > > > (IOVA is decided by userspace and physical address is the
> > > > > > > MSI-page)
> > > > > > > - So the IOVA subwindow mapping is created in iommu and yes
> > > > > > > VFIO know about
> > > > > > this mapping.
> > > > > > >
> > > > > > > Now do SET_IRQ(MSI/MSIX) ioctl:
> > > > > > > - calls pci_enable_msix()/pci_enable_msi_block(): which is
> > > > > > > supposed to set
> > > > > > MSI address/data in device.
> > > > > > > - So in current implementation (this patchset) msi-subsystem
> > > > > > > gets the IOVA
> > > > > > from iommu via this defined interface.
> > > > > > > - Are you saying that rather than getting this from iommu, we
> > > > > > > should get this
> > > > > > from vfio? What difference does this make?
> > > > > >
> > > > > > Yes, you just said above that vfio knows the msi to iova
> > > > > > mapping, so why go outside of vfio to find it later? The
> > > > > > difference is one case you can have a proper reference to data
> > > > > > structures to make sure the pointer you get back actually has
> > > > > > meaning at the time you're using it vs the code here where
> > > > > > you're defining an API that returns a meaningless value
> > > > >
> > > > > With FSL-PAMU we will always get consistant data from iommu or
> > > > > vfio-data
> > > > structure.
> > > >
> > > > Great, but you're trying to add a generic API to the IOMMU subsystem
> > > > that's difficult to use correctly. The fact that you use it
> > > > correctly does not justify the API.
> > > >
> > > > > > because you can't check or
> > > > > > enforce that an arbitrary caller is using it correctly.
> > > > >
> > > > > I am not sure what is arbitrary caller? pdev is known to vfio, so
> > > > > vfio will only make pci_enable_msix()/pci_enable_msi_block() for
> > this pdev.
> > > > > If anyother code makes then it is some other unexpectedly thing
> > > > > happening in system, no?
> > > >
> > > > What's proposed here is a generic IOMMU API. Anybody can call this.
> > > > What if the host SCSI driver decides to go get the iommu domain for
> > > > it's device (or any other device)? Does that fit your usage model?
> > > >
> > > > > > It's not maintainable.
> > > > > > Thanks,
> > > > >
> > > > > I do not have any issue with this as well, can you also describe
> > > > > the type of API you are envisioning; I can think of defining some
> > > > > function in vfio.c/vfio_iommu*.c, make them global and declare
> > > > > then in include/Linux/vfio.h And include <Linux/vfio.h> in caller
> > > > > file
> > > > > (arch/powerpc/kernel/msi.c)
> > > >
> > > > Do you really want module dependencies between vfio and your core
> > > > kernel MSI setup? Look at the vfio external user interface that
> > we've already defined.
> > > > That allows other components of the kernel to get a proper reference
> > > > to a vfio group. From there you can work out how to get what you
> > > > want. Another alternative is that vfio could register an MSI to
> > > > IOVA mapping with architecture code when the mapping is created.
> > > > The MSI setup path could then do a lookup in architecture code for
> > > > the mapping. You could even store the MSI to IOVA mapping in VFIO
> > > > and create an interface where SET_IRQ passes that mapping into setup
> > code.
> [Sethi Varun-B16395] What you are suggesting is that the MSI setup
> path queries the vfio subsystem for the mapping, rather than directly
> querying the iommu subsystem. So, say if we add an interface for
> getting MSI to IOVA mapping in the msi setup path, wouldn't this again
> be specific to vfio? I reckon that this interface would again ppc
> machine specific interface.
Sure, if this is a generic MSI setup path then clearly vfio should not
be involved. But by that same notion, if this is a generic MSI setup
path, how can the proposed solutions guarantee that the iommu_domain or
iova returned is still valid in all cases? It's racy. If the caller
trying to setup MSI has the information needed, why doesn't it pass it
in as part of the setup? Thanks,
Alex
^ permalink raw reply
* [PATCH] powerpc: fix e500 SPE float SIGFPE generation
From: Joseph S. Myers @ 2013-10-10 20:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-kernel
From: Joseph Myers <joseph@codesourcery.com>
The e500 SPE floating-point emulation code is called from
SPEFloatingPointException and SPEFloatingPointRoundException in
arch/powerpc/kernel/traps.c. Those functions have support for
generating SIGFPE, but do_spe_mathemu and speround_handler don't
generate a return value to indicate that this should be done. Such a
return value should depend on whether an exception is raised that has
been set via prctl to generate SIGFPE. This patch adds the relevant
logic in these functions so that SIGFPE is generated as expected by
the glibc testsuite.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
This patch is not intended to depend on any of my previous patches
<http://lkml.org/lkml/2013/10/4/495>,
<http://lkml.org/lkml/2013/10/4/497>,
<http://lkml.org/lkml/2013/10/8/694>,
<http://lkml.org/lkml/2013/10/8/700> and
<http://lkml.org/lkml/2013/10/8/705>, although testing has been on top
of that patch series and having all six patches will produce the best
results.
diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index 01a0abb..28337c9 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -20,6 +20,7 @@
*/
#include <linux/types.h>
+#include <linux/prctl.h>
#include <asm/uaccess.h>
#include <asm/reg.h>
@@ -691,6 +692,23 @@ update_regs:
pr_debug("va: %08x %08x\n", va.wp[0], va.wp[1]);
pr_debug("vb: %08x %08x\n", vb.wp[0], vb.wp[1]);
+ if (current->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE) {
+ if ((FP_CUR_EXCEPTIONS & FP_EX_DIVZERO)
+ && (current->thread.fpexc_mode & PR_FP_EXC_DIV))
+ return 1;
+ if ((FP_CUR_EXCEPTIONS & FP_EX_OVERFLOW)
+ && (current->thread.fpexc_mode & PR_FP_EXC_OVF))
+ return 1;
+ if ((FP_CUR_EXCEPTIONS & FP_EX_UNDERFLOW)
+ && (current->thread.fpexc_mode & PR_FP_EXC_UND))
+ return 1;
+ if ((FP_CUR_EXCEPTIONS & FP_EX_INEXACT)
+ && (current->thread.fpexc_mode & PR_FP_EXC_RES))
+ return 1;
+ if ((FP_CUR_EXCEPTIONS & FP_EX_INVALID)
+ && (current->thread.fpexc_mode & PR_FP_EXC_INV))
+ return 1;
+ }
return 0;
illegal:
@@ -867,6 +885,8 @@ int speround_handler(struct pt_regs *regs)
pr_debug(" to fgpr: %08x %08x\n", fgpr.wp[0], fgpr.wp[1]);
+ if (current->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE)
+ return (current->thread.fpexc_mode & PR_FP_EXC_RES) ? 1 : 0;
return 0;
}
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply related
* RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
From: Sethi Varun-B16395 @ 2013-10-10 20:09 UTC (permalink / raw)
To: Alex Williamson, Bhushan Bharat-R65777
Cc: Wood Scott-B07421, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, agraf@suse.de,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1381201980.6330.4.camel@ul30vt.home>
> -----Original Message-----
> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> Sent: Tuesday, October 08, 2013 8:43 AM
> To: Bhushan Bharat-R65777
> Cc: agraf@suse.de; Wood Scott-B07421; linux-pci@vger.kernel.org;
> galak@kernel.crashing.org; linux-kernel@vger.kernel.org;
> iommu@lists.linux-foundation.org; benh@kernel.crashing.org; linuxppc-
> dev@lists.ozlabs.org
> Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
>=20
> On Mon, 2013-10-07 at 05:46 +0000, Bhushan Bharat-R65777 wrote:
> >
> > > -----Original Message-----
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Friday, October 04, 2013 11:42 PM
> > > To: Bhushan Bharat-R65777
> > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org
> > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > device
> > >
> > > On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > Sent: Friday, October 04, 2013 10:43 PM
> > > > > To: Bhushan Bharat-R65777
> > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > > > > foundation.org
> > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > > > device
> > > > >
> > > > > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote:
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > Sent: Friday, October 04, 2013 9:15 PM
> > > > > > > To: Bhushan Bharat-R65777
> > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > > > > > > foundation.org
> > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain
> > > > > > > of a device
> > > > > > >
> > > > > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777
> wrote:
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: linux-pci-owner@vger.kernel.org
> > > > > > > > > [mailto:linux-pci-owner@vger.kernel.org]
> > > > > > > > > On Behalf Of Alex Williamson
> > > > > > > > > Sent: Wednesday, September 25, 2013 10:16 PM
> > > > > > > > > To: Bhushan Bharat-R65777
> > > > > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > > > > > galak@kernel.crashing.org; linux-
> > > > > > > > > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > > > > > > > > linux- pci@vger.kernel.org; agraf@suse.de; Wood
> > > > > > > > > Scott-B07421; iommu@lists.linux- foundation.org; Bhushan
> > > > > > > > > Bharat-R65777
> > > > > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get
> > > > > > > > > iommu_domain of a device
> > > > > > > > >
> > > > > > > > > On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> > > > > > > > > > This api return the iommu domain to which the device is
> attached.
> > > > > > > > > > The iommu_domain is required for making API calls
> > > > > > > > > > related to
> > > iommu.
> > > > > > > > > > Follow up patches which use this API to know iommu
> maping.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Bharat Bhushan
> > > > > > > > > > <bharat.bhushan@freescale.com>
> > > > > > > > > > ---
> > > > > > > > > > drivers/iommu/iommu.c | 10 ++++++++++
> > > > > > > > > > include/linux/iommu.h | 7 +++++++
> > > > > > > > > > 2 files changed, 17 insertions(+), 0 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/iommu/iommu.c
> > > > > > > > > > b/drivers/iommu/iommu.c index
> > > > > > > > > > fbe9ca7..6ac5f50 100644
> > > > > > > > > > --- a/drivers/iommu/iommu.c
> > > > > > > > > > +++ b/drivers/iommu/iommu.c
> > > > > > > > > > @@ -696,6 +696,16 @@ void iommu_detach_device(struct
> > > > > > > > > > iommu_domain *domain, struct device *dev) }
> > > > > > > > > > EXPORT_SYMBOL_GPL(iommu_detach_device);
> > > > > > > > > >
> > > > > > > > > > +struct iommu_domain *iommu_get_dev_domain(struct
> device *dev) {
> > > > > > > > > > + struct iommu_ops *ops =3D dev->bus->iommu_ops;
> > > > > > > > > > +
> > > > > > > > > > + if (unlikely(ops =3D=3D NULL ||
> > > > > > > > > > +ops->get_dev_iommu_domain =3D=3D
> > > NULL))
> > > > > > > > > > + return NULL;
> > > > > > > > > > +
> > > > > > > > > > + return ops->get_dev_iommu_domain(dev); }
> > > > > > > > > > +EXPORT_SYMBOL_GPL(iommu_get_dev_domain);
> > > > > > > > >
> > > > > > > > > What prevents this from racing iommu_domain_free()?
> > > > > > > > > There's no references acquired, so there's no reason for
> > > > > > > > > the caller to assume the
> > > > > > > pointer is valid.
> > > > > > > >
> > > > > > > > Sorry for late query, somehow this email went into a
> > > > > > > > folder and escaped;
> > > > > > > >
> > > > > > > > Just to be sure, there is not lock at generic "struct
> > > > > > > > iommu_domain", but IP
> > > > > > > specific structure (link FSL domain) linked in
> > > > > > > iommu_domain->priv have a lock, so we need to ensure this
> > > > > > > race in FSL iommu code (say drivers/iommu/fsl_pamu_domain.c),
> right?
> > > > > > >
> > > > > > > No, it's not sufficient to make sure that your use of the
> > > > > > > interface is race free. The interface itself needs to be
> > > > > > > designed so that it's difficult to use incorrectly.
> > > > > >
> > > > > > So we can define
> > > > > > iommu_get_dev_domain()/iommu_put_dev_domain();
> > > > > > iommu_get_dev_domain() will return domain with the lock held,
> > > > > > and
> > > > > > iommu_put_dev_domain() will release the lock? And
> > > > > > iommu_get_dev_domain() must always be followed by
> > > > > > iommu_get_dev_domain().
> > > > >
> > > > > What lock? get/put are generally used for reference counting,
> > > > > not locking in the kernel.
> > > > >
> > > > > > > That's not the case here. This is a backdoor to get the
> > > > > > > iommu domain from the iommu driver regardless of who is using
> it or how.
> > > > > > > The iommu domain is created and managed by vfio, so
> > > > > > > shouldn't we be looking at how to do this through vfio?
> > > > > >
> > > > > > Let me first describe what we are doing here:
> > > > > > During initialization:-
> > > > > > - vfio talks to MSI system to know the MSI-page and size
> > > > > > - vfio then interacts with iommu to map the MSI-page in iommu
> > > > > > (IOVA is decided by userspace and physical address is the
> > > > > > MSI-page)
> > > > > > - So the IOVA subwindow mapping is created in iommu and yes
> > > > > > VFIO know about
> > > > > this mapping.
> > > > > >
> > > > > > Now do SET_IRQ(MSI/MSIX) ioctl:
> > > > > > - calls pci_enable_msix()/pci_enable_msi_block(): which is
> > > > > > supposed to set
> > > > > MSI address/data in device.
> > > > > > - So in current implementation (this patchset) msi-subsystem
> > > > > > gets the IOVA
> > > > > from iommu via this defined interface.
> > > > > > - Are you saying that rather than getting this from iommu, we
> > > > > > should get this
> > > > > from vfio? What difference does this make?
> > > > >
> > > > > Yes, you just said above that vfio knows the msi to iova
> > > > > mapping, so why go outside of vfio to find it later? The
> > > > > difference is one case you can have a proper reference to data
> > > > > structures to make sure the pointer you get back actually has
> > > > > meaning at the time you're using it vs the code here where
> > > > > you're defining an API that returns a meaningless value
> > > >
> > > > With FSL-PAMU we will always get consistant data from iommu or
> > > > vfio-data
> > > structure.
> > >
> > > Great, but you're trying to add a generic API to the IOMMU subsystem
> > > that's difficult to use correctly. The fact that you use it
> > > correctly does not justify the API.
> > >
> > > > > because you can't check or
> > > > > enforce that an arbitrary caller is using it correctly.
> > > >
> > > > I am not sure what is arbitrary caller? pdev is known to vfio, so
> > > > vfio will only make pci_enable_msix()/pci_enable_msi_block() for
> this pdev.
> > > > If anyother code makes then it is some other unexpectedly thing
> > > > happening in system, no?
> > >
> > > What's proposed here is a generic IOMMU API. Anybody can call this.
> > > What if the host SCSI driver decides to go get the iommu domain for
> > > it's device (or any other device)? Does that fit your usage model?
> > >
> > > > > It's not maintainable.
> > > > > Thanks,
> > > >
> > > > I do not have any issue with this as well, can you also describe
> > > > the type of API you are envisioning; I can think of defining some
> > > > function in vfio.c/vfio_iommu*.c, make them global and declare
> > > > then in include/Linux/vfio.h And include <Linux/vfio.h> in caller
> > > > file
> > > > (arch/powerpc/kernel/msi.c)
> > >
> > > Do you really want module dependencies between vfio and your core
> > > kernel MSI setup? Look at the vfio external user interface that
> we've already defined.
> > > That allows other components of the kernel to get a proper reference
> > > to a vfio group. From there you can work out how to get what you
> > > want. Another alternative is that vfio could register an MSI to
> > > IOVA mapping with architecture code when the mapping is created.
> > > The MSI setup path could then do a lookup in architecture code for
> > > the mapping. You could even store the MSI to IOVA mapping in VFIO
> > > and create an interface where SET_IRQ passes that mapping into setup
> code.
[Sethi Varun-B16395] What you are suggesting is that the MSI setup path que=
ries the vfio subsystem for the mapping, rather than directly querying the =
iommu subsystem. So, say if we add an interface for getting MSI to IOVA map=
ping in the msi setup path, wouldn't this again be specific to vfio? I reck=
on that this interface would again ppc machine specific interface.
-Varun
^ permalink raw reply
* Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
From: Anatolij Gustschin @ 2013-10-10 19:37 UTC (permalink / raw)
To: Brian Norris; +Cc: Gerhard Sittig, linuxppc-dev
In-Reply-To: <CAN8TOE8o-34zFP=0q3NjondqpA6UA1szitN+kgKcbaNOWhrscg@mail.gmail.com>
Hello,
On Thu, 10 Oct 2013 11:23:55 -0700
Brian Norris <computersforpeace@gmail.com> wrote:
...
> > making mpc512x_setup_diu(), mpc512x_release_bootmem(),
> > mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock()
> > should be okay.
>
> And mpc512x_init_diu()?
yes, it can be static, too.
>
> >> Then, you can get the real benefit of IS_ENABLED() by removing the
> >>
> >> #if IS_ENABLED(CONFIG_FB_FSL_DIU)
> >>
> >> from around all the DIU code, and it will automatically be removed by
> >> the compiler when it is not used.
> >>
> >> I think the current patch is necessary for immediate use, and it can be
> >> sent to stable. But I might suggest a follow-up patch or 2 that makes
> >> the functions static and kills the #ifdef entirely.
> >
> > Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in
> > arch/powerpc/sysdev/fsl_soc.h and building should work then.
>
> Still want it around this line, probably, so we'll get compiler errors
> and not linker errors if someone tries to use it?
>
> extern struct platform_diu_data_ops diu_ops;
>
> But otherwise that looks OK to me. Shall I send a patch?
yes, please.
Thanks,
Anatolij
^ permalink raw reply
* Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
From: Brian Norris @ 2013-10-10 18:23 UTC (permalink / raw)
To: Anatolij Gustschin; +Cc: Gerhard Sittig, linuxppc-dev
In-Reply-To: <20131010180948.4350ebda@crub>
Hello,
On Thu, Oct 10, 2013 at 9:09 AM, Anatolij Gustschin <agust@denx.de> wrote:
> On Wed, 9 Oct 2013 12:29:31 -0700
> Brian Norris <computersforpeace@gmail.com> wrote:
> ...
>> > +#else
>> > +void __init mpc512x_setup_diu(void) { /* EMPTY */ }
>> > +void __init mpc512x_init_diu(void) { /* EMPTY */ }
>> > #endif
>> >
>> > void __init mpc512x_init_IRQ(void)
>>
>> I see an alternative solution:
>>
>> Can't almost all of the code in mpc512x_shared.c be declared 'static'?
>
> making mpc512x_setup_diu(), mpc512x_release_bootmem(),
> mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock()
> should be okay.
And mpc512x_init_diu()?
>> Then, you can get the real benefit of IS_ENABLED() by removing the
>>
>> #if IS_ENABLED(CONFIG_FB_FSL_DIU)
>>
>> from around all the DIU code, and it will automatically be removed by
>> the compiler when it is not used.
>>
>> I think the current patch is necessary for immediate use, and it can be
>> sent to stable. But I might suggest a follow-up patch or 2 that makes
>> the functions static and kills the #ifdef entirely.
>
> Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in
> arch/powerpc/sysdev/fsl_soc.h and building should work then.
Still want it around this line, probably, so we'll get compiler errors
and not linker errors if someone tries to use it?
extern struct platform_diu_data_ops diu_ops;
But otherwise that looks OK to me. Shall I send a patch?
Brian
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Alexander Gordeev @ 2013-10-10 18:07 UTC (permalink / raw)
To: H. Peter Anvin
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <5256D5AB.4050105@zytor.com>
On Thu, Oct 10, 2013 at 09:28:27AM -0700, H. Peter Anvin wrote:
> On 10/10/2013 03:17 AM, Alexander Gordeev wrote:
> > On Wed, Oct 09, 2013 at 03:24:08PM +1100, Benjamin Herrenschmidt wrote:
> >
> > Ok, this suggestion sounded in one or another form by several people.
> > What about name it pcim_enable_msix_range() and wrap in couple more
> > helpers to complete an API:
> >
> > int pcim_enable_msix_range(pdev, msix_entries, nvec, minvec);
> > <0 - error code
> > >0 - number of MSIs allocated, where minvec >= result <= nvec
> >
> > int pcim_enable_msix(pdev, msix_entries, nvec);
> > <0 - error code
> > >0 - number of MSIs allocated, where 1 >= result <= nvec
> >
> > int pcim_enable_msix_exact(pdev, msix_entries, nvec);
> > <0 - error code
> > >0 - number of MSIs allocated, where result == nvec
> >
> > The latter's return value seems odd, but I can not help to make
> > it consistent with the first two.
> >
>
> Is there a reason for the wrappers, as opposed to just specifying either
> 1 or nvec as the minimum?
The wrappers are more handy IMO.
I.e. can imagine people start struggling to figure out what minvec to provide:
1 or 0? Why 1? Oh.. okay.. Or should we tolerate 0 (as opposite to -ERANGE)?
Well, do not know.. pcim_enable_msix(pdev, msix_entries, nvec, nvec) is
less readable for me than just pcim_enable_msix_exact(pdev, msix_entries,
nvec).
> -hpa
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: H. Peter Anvin @ 2013-10-10 16:28 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <20131010101704.GC11874@dhcp-26-207.brq.redhat.com>
On 10/10/2013 03:17 AM, Alexander Gordeev wrote:
> On Wed, Oct 09, 2013 at 03:24:08PM +1100, Benjamin Herrenschmidt wrote:
>
> Ok, this suggestion sounded in one or another form by several people.
> What about name it pcim_enable_msix_range() and wrap in couple more
> helpers to complete an API:
>
> int pcim_enable_msix_range(pdev, msix_entries, nvec, minvec);
> <0 - error code
> >0 - number of MSIs allocated, where minvec >= result <= nvec
>
> int pcim_enable_msix(pdev, msix_entries, nvec);
> <0 - error code
> >0 - number of MSIs allocated, where 1 >= result <= nvec
>
> int pcim_enable_msix_exact(pdev, msix_entries, nvec);
> <0 - error code
> >0 - number of MSIs allocated, where result == nvec
>
> The latter's return value seems odd, but I can not help to make
> it consistent with the first two.
>
Is there a reason for the wrappers, as opposed to just specifying either
1 or nvec as the minimum?
-hpa
^ permalink raw reply
* Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
From: Anatolij Gustschin @ 2013-10-10 16:09 UTC (permalink / raw)
To: Brian Norris; +Cc: Gerhard Sittig, linuxppc-dev
In-Reply-To: <20131009192931.GC23337@ld-irv-0074.broadcom.com>
Hi,
On Wed, 9 Oct 2013 12:29:31 -0700
Brian Norris <computersforpeace@gmail.com> wrote:
...
> > +#else
> > +void __init mpc512x_setup_diu(void) { /* EMPTY */ }
> > +void __init mpc512x_init_diu(void) { /* EMPTY */ }
> > #endif
> >
> > void __init mpc512x_init_IRQ(void)
>
> I see an alternative solution:
>
> Can't almost all of the code in mpc512x_shared.c be declared 'static'?
making mpc512x_setup_diu(), mpc512x_release_bootmem(),
mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock()
should be okay.
> Then, you can get the real benefit of IS_ENABLED() by removing the
>
> #if IS_ENABLED(CONFIG_FB_FSL_DIU)
>
> from around all the DIU code, and it will automatically be removed by
> the compiler when it is not used.
>
> I think the current patch is necessary for immediate use, and it can be
> sent to stable. But I might suggest a follow-up patch or 2 that makes
> the functions static and kills the #ifdef entirely.
Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in
arch/powerpc/sysdev/fsl_soc.h and building should work then.
Thanks,
Anatolij
^ permalink raw reply
* Re: [PATCH RFC 50/77] mlx5: Update MSI/MSI-X interrupts enablement code
From: Eli Cohen @ 2013-10-10 15:29 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <20131003194837.GA27636@dhcp-26-207.brq.redhat.com>
On Thu, Oct 03, 2013 at 09:48:39PM +0200, Alexander Gordeev wrote:
>
> pci_enable_msix() may fail, but it can not return a positive number.
>
That is true according to the current logic but the comment on top of
pci_enable_msix() still says:
"A return of < 0 indicates a failure. Or a return of > 0 indicates
that driver request is exceeding the number of irqs or MSI-X vectors
available"
So you're counting on an implementation that may change in the future.
I think leaving the code as it is now is safer.
^ permalink raw reply
* Re: Elbc device driver
From: Scott Wood @ 2013-10-10 15:14 UTC (permalink / raw)
To: Mercier Ivan; +Cc: linuxppc-dev
In-Reply-To: <CAMc2ieqXg1C6twTaqL9irJoLceSw-fFyTseifW5dmCmnXHWLCw@mail.gmail.com>
On Thu, 2013-10-10 at 16:54 +0200, Mercier Ivan wrote:
> Hi,
> I've got a KERN_INFO msg: "fsl-lbc a3p400.11: failed to get memory
> region" corresponding to my elbc device that I try to map.
> a3p400@3,0 {
> compatible = "fsl,elbc";
> reg = <0 0xe0000000 0x1000000>;
> };
The node name and unit address do not match the contents of the node.
The former describes a device that sits on the localbus, while the
latter has a compatible describes the localbus itself. I don't know
what that reg is, but it doesn't match the unit address.
> The corresponding law on ELBC is 0xe0000000 ->0xf0000000.
> I want to map my device on 0xe0000000 ->0xe8000000 as it has 16 adress bits.
>
> Now the controler registers are properly set,but how do I map the device?
Your eLBC node should look something like this (I'm assuming CCSR is at
0xffe000000, and that you actually meant 0x0e0000000 and not
0xfe0000000, though probably one of those assumptions is wrong):
localbus@ffe124000 {
compatible = "fsl,elbc", "simple-bus";
reg = <0xf 0xfe124000 0 0x1000>;
interrupts = <25 2 0 0>;
#address-cells = <2>;
#size-cells = <1>;
ranges = <3 0 0 0xe0000000 0x08000000>;
/* If at all possible, replace "a3p400" in the node name with
something generic */
a3p400@3,0 {
compatible = "something appropriate here";
reg = <3 0 0x08000000>;
};
};
Note that at the dts level, you should be including
arch/powerpc/boot/dts/fsl/p3041-post.dtsi and thus your board dts would
only need to supply reg, ranges, and the a3p400 node (see
arch/powerpc/boot/dts/p3041ds.dts for an example).
-Scott
^ permalink raw reply
* Re: Elbc device driver
From: Mercier Ivan @ 2013-10-10 14:54 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1381271655.7979.290.camel@snotra.buserror.net>
Hi,
I've got a KERN_INFO msg: "fsl-lbc a3p400.11: failed to get memory
region" corresponding to my elbc device that I try to map.
a3p400@3,0 {
compatible = "fsl,elbc";
reg = <0 0xe0000000 0x1000000>;
};
The corresponding law on ELBC is 0xe0000000 ->0xf0000000.
I want to map my device on 0xe0000000 ->0xe8000000 as it has 16 adress bits.
Now the controler registers are properly set,but how do I map the device?
Thanks a lot
2013/10/9 Scott Wood <scottwood@freescale.com>:
> On Tue, 2013-10-08 at 16:06 +0200, Mercier Ivan wrote:
>> Hi,
>>
>> I'm working on a powerpc qoriq p3041 and trying to communicate with a
>> device by elbc bus in gpmc mode.
>>
>> I 've integrated CONFIG_FSL_LBC in Linux which provide the basic functions.
>>
>> Now I'm wondering how can I do read and write operations on the
>> bus.Where is mapped my device?
>
> You'll need to use ioremap() or of_iomap() to map it.
>
>> Should I code .read and .write driver functions?How can I start?
>>
>> How integrates my device in the device tree?
>
> See Documentation/devicetree/bindings/powerpc/fsl/lbc.txt and examples
> such as "board-control" in various device trees.
>
> -Scott
>
>
>
^ permalink raw reply
* Re: Gianfar driver crashes in Kernel v3.10
From: Claudiu Manoil @ 2013-10-10 11:07 UTC (permalink / raw)
To: Thomas Hühn; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <E0CAE3C8-5883-4D20-8A95-CCC88B4A811C@net.t-labs.tu-berlin.de>
On 10/4/2013 3:28 PM, Thomas H=FChn wrote:
>
> [code]
> [ 2671.841927] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 2671.847141] Freescale P1014
> [ 2671.849925] Modules linked in: ath9k pppoe ppp_async iptable_nat
> ath9k_common pppox p
> e xt_tcpudp xt_tcpmss xt_string xt_statistic xt_state xt_recent xt_quot=
a
> xt_pkttype xt_o
> mark xt_connbytes xt_comment xt_addrtype xt_TCPMSS xt_REDIRECT xt_NETMA=
P
> xt_LOG xt_IPMAR
> ms_datafab ums_cypress ums_alauda slhc nf_nat_tftp nf_nat_snmp_basic
> nf_nat_sip nf_nat_r
> ntrack_sip nf_conntrack_rtsp nf_conntrack_proto_gre nf_conntrack_irc
> nf_conntrack_h323 n
> compat_xtables compat ath sch_teql sch_tbf sch_sfq sch_red sch_prio
> sch_htb sch_gred sc
> skbedit act_mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route cls_fw
> sch_hfsc sch_ing
> r usb_storage leds_gpio ohci_hcd ehci_platform ehci_hcd sd_mod scsi_mod
> fsl_mph_dr_of gp
> [ 2671.988946] CPU: 0 PID: 5209 Comm: iftop Not tainted 3.10.13 #2
> [ 2671.994859] task: c4b22220 ti: c7ff8000 task.ti: c477e000
> [ 2672.000250] NIP: c018c7a0 LR: c018c794 CTR: c000b070
> [ 2672.005206] REGS: c7ff9f10 TRAP: 3202 Not tainted (3.10.13)
> [ 2672.011028] MSR: 00029000 <CE,EE,ME> CR: 48000024 XER: 20000000
> [ 2672.017125]
> GPR00: 000000ff c477fde0 c4b22220 00000000 00000000 000000ff 00000000
> 70000000
> GPR08: ffffffff 00000008 00000000 ffffffff 00000046 10022248 00000000
> 00000008
> GPR16: c781b3c0 c781b3c0 000000ff 00000000 00000001 0000021c 00000086
> fffff800
> GPR24: c7980300 00000000 00000001 00000040 00000003 c4b33000 00000000
> 00000001
> [ 2672.046832] NIP [c018c7a0] gfar_poll+0x424/0x520
> [ 2672.051442] LR [c018c794] gfar_poll+0x418/0x520
> [ 2672.055962] Call Trace:
> [ 2672.058402] [c477fde0] [c018c674] gfar_poll+0x2f8/0x520 (unreliable)
> [ 2672.064762] [c477fe80] [c01b0ce8] net_rx_action+0x6c/0x158
> [ 2672.070249] [c477feb0] [c0027dc4] __do_softirq+0xbc/0x16c
> [ 2672.075642] [c477ff00] [c0027f7c] irq_exit+0x4c/0x68
> [ 2672.080604] [c477ff10] [c00041f8] do_IRQ+0xf4/0x10c
> [ 2672.085478] [c477ff40] [c000ca3c] ret_from_except+0x0/0x18
> [ 2672.090991] --- Exception: 501 at 0x48083c28
> [ 2672.090991] LR =3D 0x48083bf8
> [ 2672.098378] Instruction dump:
> [ 2672.101338] 7f8f2040 419cfcc4 80900000 38a00000 8061004c 7e118378
> 81c10050 7ffafb78
> [ 2672.109092] 4bf9eaa1 83810034 7c7e1b78 8361003c <83210038> 83a1004c
> 48000060 41a2004c
> [ 2672.117021] ---[ end trace 565fb54528d305fa ]---
> [ 2672.121628]
> [ 2673.103130] Kernel panic - not syncing: Fatal exception in interrupt
> [ 2673.109474] Rebooting in 3 seconds..
>
> U-Boot 2010.12-svn15934 (Dec 11 2012 - 16:23:49)
> [/code]
>
Hi,
Does this show up on a half duplex (100Mb/s) link?
Could you provide following for the gianfar interface, on your setup:
# ethtool ethX
and
# ethtool -d ethX | grep 500
Is there any other indication before this Oops? Like a tx timeout WARN?
Thanks,
Claudiu
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/scom: Replace debugfs interface with cleaner sysfs one
From: Benjamin Herrenschmidt @ 2013-10-10 10:16 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20131010100640.GA9906@iris.ozlabs.ibm.com>
On Thu, 2013-10-10 at 21:06 +1100, Paul Mackerras wrote:
> On Thu, Oct 10, 2013 at 07:18:35PM +1100, Benjamin Herrenschmidt wrote:
> > The debugfs interface was essentially unused, and racy for anything
> > other than manual use by a developer. This provides a more useful
> > sysfs based one which can be used by programs without racing with
> > each other essentially by providing a file to read/write with an
> > offset being the scom address << 8.
>
> Don't you mean address << 3 (or address * 8)?
Yes, typo in the comment, the code is fine. I was tired :)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Alexander Gordeev @ 2013-10-10 10:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
H. Peter Anvin, linux-s390, Andy King, linux-scsi, linux-rdma,
x86, Ingo Molnar, iss_storagedev, linux-driver, Tejun Heo,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Martin Schwidefsky, linux390, linuxppc-dev
In-Reply-To: <1381292648.645.259.camel@pasglop>
On Wed, Oct 09, 2013 at 03:24:08PM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2013-10-08 at 20:55 -0700, H. Peter Anvin wrote:
> > Why not add a minimum number to pci_enable_msix(), i.e.:
> >
> > pci_enable_msix(pdev, msix_entries, nvec, minvec)
> >
> > ... which means "nvec" is the number of interrupts *requested*, and
> > "minvec" is the minimum acceptable number (otherwise fail).
>
> Which is exactly what Ben (the other Ben :-) suggested and that I
> supports...
Ok, this suggestion sounded in one or another form by several people.
What about name it pcim_enable_msix_range() and wrap in couple more
helpers to complete an API:
int pcim_enable_msix_range(pdev, msix_entries, nvec, minvec);
<0 - error code
>0 - number of MSIs allocated, where minvec >= result <= nvec
int pcim_enable_msix(pdev, msix_entries, nvec);
<0 - error code
>0 - number of MSIs allocated, where 1 >= result <= nvec
int pcim_enable_msix_exact(pdev, msix_entries, nvec);
<0 - error code
>0 - number of MSIs allocated, where result == nvec
The latter's return value seems odd, but I can not help to make
it consistent with the first two.
(Sorry if you see this message twice - my MUA seems struggle with one of CC).
> Cheers,
> Ben.
>
>
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/scom: Replace debugfs interface with cleaner sysfs one
From: Paul Mackerras @ 2013-10-10 10:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1381393115.4330.39.camel@pasglop>
On Thu, Oct 10, 2013 at 07:18:35PM +1100, Benjamin Herrenschmidt wrote:
> The debugfs interface was essentially unused, and racy for anything
> other than manual use by a developer. This provides a more useful
> sysfs based one which can be used by programs without racing with
> each other essentially by providing a file to read/write with an
> offset being the scom address << 8.
Don't you mean address << 3 (or address * 8)?
Paul.
^ permalink raw reply
* Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device tree
From: Mark Rutland @ 2013-10-10 10:03 UTC (permalink / raw)
To: Yuantian.Tang@freescale.com
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1381300704-4238-1-git-send-email-Yuantian.Tang@freescale.com>
On Wed, Oct 09, 2013 at 07:38:24AM +0100, Yuantian.Tang@freescale.com wrote:
> From: Tang Yuantian <yuantian.tang@freescale.com>
>
> The following SoCs will be affected: p2041, p3041, p4080,
> p5020, p5040, b4420, b4860, t4240
>
> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> v5:
> - refine the binding document
> - update the compatible string
> v4:
> - add binding document
> - update compatible string
> - update the reg property
> v3:
> - fix typo
> v2:
> - add t4240, b4420, b4860 support
> - remove pll/4 clock from p2041, p3041 and p5020 board
>
> .../devicetree/bindings/clock/corenet-clock.txt | 111 ++++++++++++++++++++
> arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++
> arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 +
> arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++
> arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++
> arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++
> arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 +++++++++++++++++++++
> arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++
> arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++
> arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 +
> arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++
> arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 ++++++++++++++++
> arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++
> 17 files changed, 640 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/corenet-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/corenet-clock.txt
> new file mode 100644
> index 0000000..8efc62d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt
> @@ -0,0 +1,111 @@
> +* Clock Block on Freescale CoreNet Platforms
> +
> +Freescale CoreNet chips take primary clocking input from the external
> +SYSCLK signal. The SYSCLK input (frequency) is multiplied using
> +multiple phase locked loops (PLL) to create a variety of frequencies
> +which can then be passed to a variety of internal logic, including
> +cores and peripheral IP blocks.
> +Please refer to the Reference Manual for details.
> +
> +1. Clock Block Binding
> +
> +Required properties:
> +- compatible: Should include one or more of the following:
> + - "fsl,<chip>-clockgen": for chip specific clock block
> + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock
While I can see that "fsl,<chip>-clockgen" might have a large set of
strings that we may never deal with in th kernel, I'd prefer that the
basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" variants) were
listed explicitly here.
Given they only seem to be "fsl,qoriq-clockgen-1.0" and
"fsl,qoriq-clockgen-2.0" this shouldn't be too difficult to list and
describe.
> +- reg: Offset and length of the clock register set
> +- clock-frequency: Indicates input clock frequency of clock block.
> + Will be set by u-boot
Why does the fact this is set by u-boot matter to the binding?
> +
> +Recommended properties:
> +- #ddress-cells: Specifies the number of cells used to represent
> + physical base addresses. Must be present if the device has
> + sub-nodes and set to 1 if present
Typo: #address-cells
In the example it looks like the address cells of child nodes are
offsets within the unit, rather than absolute physical addresses. Could
the code not treat them as absolute addresses? Then we'd only need to
document that #address-cells, #size-cells and ranges must be present and
have values suitable for mapping child nodes into the address space of
the parent.
> +- #size-cells: Specifies the number of cells used to represent
> + the size of an address. Must be present if the device has
> + sub-nodes and set to 1 if present
It's not really the size of an address, it's the size of a region
identified by a reg entry.
I think this can be simplified by my suggestion above.
> +
> +2. Clock Provider/Consumer Binding
> +
> +Most of the binding are from the common clock binding[1].
> + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : Should include one or more of the following:
> + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock device
> + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer clock
> + device; divided from the core PLL clock
As above, I'd prefer a complete list of the basic strings we expect.
> + - "fixed-clock": From common clock binding; indicates output clock
> + of oscillator
> + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock
Here too.
> +- #clock-cells: From common clock binding; indicates the number of
> + output clock. 0 is for one output clock; 1 for more than one clock
If a clock source has multiple outputs, what those outputs are and what
values in clock-cells they correspond to should be described here.
> +
> +Recommended properties:
> +- clocks: Should be the phandle of input parent clock
> +- clock-names: From common clock binding, indicates the clock name
That description's a bit opaque.
What's the name of the clock input on these units? That's what
clock-names should contain, and that should be documented.
Do we not _always_ need the parent clock?
If we have a clock do we need a clock-names entry for it?
> +- clock-output-names: From common clock binding, indicates the names of
> + output clocks
> +- reg: Should be the offset and length of clock block base address.
> + The length should be 4.
> +
> +Example for clock block and clock provider:
> +/ {
> + clockgen: global-utilities@e1000 {
> + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0";
> + reg = <0xe1000 0x1000>;
> + clock-frequency = <0>;
That looks odd.
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + sysclk: sysclk {
> + #clock-cells = <0>;
> + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock";
We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was
compatible with "fixed-clock" and should have "fixed-clock" in the
compatible list...
> + clock-output-names = "sysclk";
> + }
> +
> + pll0: pll0@800 {
> + #clock-cells = <1>;
> + reg = <0x800 0x4>;
> + compatible = "fsl,qoriq-core-pll-1.0";
> + clocks = <&sysclk>;
> + clock-output-names = "pll0", "pll0-div2";
> + };
> +
> + pll1: pll1@820 {
> + #clock-cells = <1>;
> + reg = <0x820 0x4>;
> + compatible = "fsl,qoriq-core-pll-1.0";
> + clocks = <&sysclk>;
> + clock-output-names = "pll1", "pll1-div2";
> + };
> +
> + mux0: mux0@0 {
> + #clock-cells = <0>;
> + reg = <0x0 0x4>;
> + compatible = "fsl,qoriq-core-mux-1.0";
> + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>;
> + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1";
> + clock-output-names = "cmux0";
> + };
> +
> + mux1: mux1@20 {
> + #clock-cells = <0>;
> + reg = <0x20 0x4>;
> + compatible = "fsl,qoriq-core-mux-1.0";
> + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>;
> + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1";
> + clock-output-names = "cmux1";
How does the mux choose which input clock to use at a point in time?
Cheers,
Mark.
^ permalink raw reply
* Loading and executing non-Linux kernel outside Linux memory.
From: Sumesh Kaana @ 2013-10-10 8:47 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
[-- Attachment #1: Type: text/plain, Size: 369 bytes --]
Hi,
I'm running Linux on a PowerPC-44x based board with 256 Megabytes of RAM at 0x5000.0000 - 0x6000.0000.Upon an error interrupt, I should load my recovery module(non-Linux kernel) at 0x4000.0000 and jump the execution to this.
Any suggestions for this implementation?
Would kexec allow me to load an image ouside linux's memory?
Thanks,Sumesh.
[-- Attachment #2: Type: text/html, Size: 753 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts
From: Anshuman Khandual @ 2013-10-10 8:50 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, mikey
In-Reply-To: <20131009060321.GB28160@concordia>
On 10/09/2013 11:33 AM, Michael Ellerman wrote:
> On Wed, Oct 09, 2013 at 10:16:32AM +0530, Anshuman Khandual wrote:
>> On 10/09/2013 06:51 AM, Michael Ellerman wrote:
>>> On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
>>>> On 10/08/2013 09:51 AM, Michael Ellerman wrote:
>>>>> On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
>>>>>> Right now the `config_bhrb` PMU specific call happens after write_mmcr0
>>>>>> which actually enables the PMU for event counting and interrupt. So
>>>>>> there is a small window of time where the PMU and BHRB runs without the
>>>>>> required HW branch filter (if any) enabled in BHRB. This can cause some
>>>>>> of the branch samples to be collected through BHRB without any filter
>>>>>> being applied and hence affecting the correctness of the results. This
>>>>>> patch moves the BHRB config function call before enabling the interrupts.
>>>>>
>>>>> Patch looks good.
>>>>>
>>>>> But it reminds me I have an item in my TODO list:
>>>>> - "Why can't config_bhrb() be done in compute_mmcr()" ?
>>>>>
>>>>
>>>> compute_mmcr() function deals with generic MMCR* configs for normal PMU
>>>> events. Even if BHRB config touches MMCRA register, it's configuration
>>>> does not interfere with the PMU config for general events. So its best
>>>> to keep them separate.
>>>
>>> I'm unconvinced. If they'd been together to begin with this bug never
>>> would have happened.
>>
>> This is an ordering of configuration problem. Putting them together in the
>> same function does not rule out the chances of this ordering problem. Could
>> you please kindly explain how this could have been avoided ?
>
> The existing code already makes sure to write MMCRA before MMCR0.
>
Thats not true. One example being here at power_pmu_enable function.
write_mmcr0(cpuhw, mmcr0);
/*
* Enable instruction sampling if necessary
*/
if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
mb();
mtspr(SPRN_MMCRA, cpuhw->mmcr[2]);
}
Even I think this is not right. Instruction sampling should have been
enabled before we enable PMU interrupts. Else there is a small window
of time where we could have the PMU enabled with events (which requires
sampling) without the sampling itself being enabled in MMCRA.
The only dependency BHRB and generic events have with each other is that
they both are ready for action once the PMU interrupt has been enabled
with MMCR0_PMXE bit.
Regards
Anshuman
^ permalink raw reply
* [PATCH 3/3] powerpc/powernv: Add support for indirect XSCOM via sysfs
From: Benjamin Herrenschmidt @ 2013-10-10 8:19 UTC (permalink / raw)
To: linuxppc-dev
Indirect XSCOM addresses normally have the top bit set (of the 64-bit
address). This doesn't work via the normal sysfs interface, so we use
a different encoding, which we need to convert before calling OPAL.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/platforms/powernv/opal-xscom.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c
index 09a90d8..23e52f3 100644
--- a/arch/powerpc/platforms/powernv/opal-xscom.c
+++ b/arch/powerpc/platforms/powernv/opal-xscom.c
@@ -71,11 +71,33 @@ static int opal_xscom_err_xlate(int64_t rc)
}
}
+static u64 opal_scom_unmangle(u64 reg)
+{
+ /*
+ * XSCOM indirect addresses have the top bit set. Additionally
+ * the reset of the top 3 nibbles is always 0.
+ *
+ * Because the sysfs interface uses signed offsets and shifts
+ * the address left by 3, we basically cannot use the top 4 bits
+ * of the 64-bit address, and thus cannot use the indirect bit.
+ *
+ * To deal with that, we support the indirect bit being in bit
+ * 4 (IBM notation) instead of bit 0 in this API, we do the
+ * conversion here. To leave room for further xscom address
+ * expansion, we only clear out the top byte
+ *
+ */
+ if (reg & (1ull << 59))
+ reg = (reg & ~(0xffull << 56)) | (1ull << 63);
+ return reg;
+}
+
static int opal_scom_read(scom_map_t map, u64 reg, u64 *value)
{
struct opal_scom_map *m = map;
int64_t rc;
+ reg = opal_scom_unmangle(reg);
rc = opal_xscom_read(m->chip, m->addr + reg, (uint64_t *)__pa(value));
return opal_xscom_err_xlate(rc);
}
@@ -85,6 +107,7 @@ static int opal_scom_write(scom_map_t map, u64 reg, u64 value)
struct opal_scom_map *m = map;
int64_t rc;
+ reg = opal_scom_unmangle(reg);
rc = opal_xscom_write(m->chip, m->addr + reg, value);
return opal_xscom_err_xlate(rc);
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox