* Re: [PATCH v2 14/22] MIPS/Xlp/MSI: Use MSI chip framework to configure MSI/MSI-X irq
From: Thierry Reding @ 2014-09-25 7:36 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, sparclinux,
linux-arch, linux-s390, Russell King, Joerg Roedel, x86,
Sebastian Ott, xen-devel, arnab.basu, Arnd Bergmann,
Konrad Rzeszutek Wilk, Chris Metcalf, Bjorn Helgaas,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-15-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
On Thu, Sep 25, 2014 at 11:14:24AM +0800, Yijing Wang wrote:
> Use MSI chip framework instead of arch MSI functions to configure
> MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
Nit: s/irq/IRQ/ in the above.
> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
> ---
> arch/mips/pci/msi-xlp.c | 14 ++++++++++++--
> 1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/pci/msi-xlp.c b/arch/mips/pci/msi-xlp.c
> index e469dc7..6b791ef 100644
> --- a/arch/mips/pci/msi-xlp.c
> +++ b/arch/mips/pci/msi-xlp.c
> @@ -245,7 +245,7 @@ static struct irq_chip xlp_msix_chip = {
> .irq_unmask = unmask_msi_irq,
> };
>
> -void arch_teardown_msi_irq(unsigned int irq)
> +void xlp_teardown_msi_irq(unsigned int irq)
Should this not be static now as well?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 12/22] MIPS/Octeon/MSI: Use MSI chip framework to configure MSI/MSI-X irq
From: Thierry Reding @ 2014-09-25 7:34 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, sparclinux,
linux-arch, linux-s390, Russell King, Joerg Roedel, x86,
Sebastian Ott, xen-devel, arnab.basu, Arnd Bergmann,
Konrad Rzeszutek Wilk, Chris Metcalf, Bjorn Helgaas,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-13-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]
On Thu, Sep 25, 2014 at 11:14:22AM +0800, Yijing Wang wrote:
[...]
> diff --git a/arch/mips/pci/msi-octeon.c b/arch/mips/pci/msi-octeon.c
[...]
> @@ -132,12 +132,12 @@ msi_irq_allocated:
> /* Make sure the search for available interrupts didn't fail */
> if (irq >= 64) {
> if (request_private_bits) {
> - pr_err("arch_setup_msi_irq: Unable to find %d free interrupts, trying just one",
> + pr_err("octeon_setup_msi_irq: Unable to find %d free interrupts, trying just one",
> 1 << request_private_bits);
Perhaps while at it make this (and other similar changes in this patch):
pr_err("%s(): Unable to ...", __func__, ...);
So that it becomes more resilient against this kind of rename?
> request_private_bits = 0;
> goto try_only_one;
> } else
> - panic("arch_setup_msi_irq: Unable to find a free MSI interrupt");
> + panic("octeon_setup_msi_irq: Unable to find a free MSI interrupt");
> @@ -210,14 +210,13 @@ int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>
> return 0;
> }
> -
This...
> @@ -240,7 +239,7 @@ void arch_teardown_msi_irq(unsigned int irq)
> */
> number_irqs = 0;
> while ((irq0 + number_irqs < 64) &&
> - (msi_multiple_irq_bitmask[index]
> + (msi_multiple_irq_bitmask[index]
... and this seem like unrelated whitespace changes.
> & (1ull << (irq0 + number_irqs))))
> number_irqs++;
> number_irqs++;
> @@ -249,8 +248,8 @@ void arch_teardown_msi_irq(unsigned int irq)
> /* Shift the mask to the correct bit location */
> bitmask <<= irq0;
> if ((msi_free_irq_bitmask[index] & bitmask) != bitmask)
> - panic("arch_teardown_msi_irq: Attempted to teardown MSI "
> - "interrupt (%d) not in use", irq);
> + panic("octeon_teardown_msi_irq: Attempted to teardown MSI "
> + "interrupt (%d) not in use", irq);
And the second line here also needlessly changes the indentation.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 06/22] PCI/MSI: Introduce weak arch_find_msi_chip() to find MSI chip
From: Thierry Reding @ 2014-09-25 7:26 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, sparclinux,
linux-arch, linux-s390, Russell King, Joerg Roedel, x86,
Sebastian Ott, xen-devel, arnab.basu, Arnd Bergmann,
Konrad Rzeszutek Wilk, Chris Metcalf, Bjorn Helgaas,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-7-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 925 bytes --]
On Thu, Sep 25, 2014 at 11:14:16AM +0800, Yijing Wang wrote:
> Introduce weak arch_find_msi_chip() to find the match msi_chip.
> Currently, MSI chip associates pci bus to msi_chip. Because in
> ARM platform, there may be more than one MSI controller in system.
> Associate pci bus to msi_chip help pci device to find the match
> msi_chip and setup MSI/MSI-X irq correctly. But in other platform,
> like in x86. we only need one MSI chip, because all device use
> the same MSI address/data and irq etc. So it's no need to associate
> pci bus to MSI chip, just use a arch function, arch_find_msi_chip()
> to return the MSI chip for simplicity. The default weak
> arch_find_msi_chip() used in ARM platform, find the MSI chip
> by pci bus.
Can't x86 simply set the bus' .msi field anyway? It would seem to be
easy to do and unifies the code rather than driving it further apart
using even more of the __weak functions.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v2] powerpc/powernv: Override dma_get_required_mask()
From: Gavin Shan @ 2014-09-25 7:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
When using bypass window on IODA2, the incorrect DMA operations
"dma_iommu_ops" is used by devices. The device driver calls
dma_get_required_mask() to determine using 32-bits or 64-bits
bypass DMA window. Unfortunately, the returned DMA mask always
forces the driver to use 32-bits DMA window. The problem was
reported on the device as follows:
0004:03:00.0 0107: 1000:0087 (rev 05)
0004:03:00.0 Serial Attached SCSI controller: LSI Logic / Symbios \
Logic SAS2308 PCI-Express Fusion-MPT SAS-2 (rev 05)
The patch fixes above issue by overriding dma_get_required_mask(),
which returns mask corresponding to bypass window base. Otherwise,
"dma_iommu_ops::get_required_mask" will be called to return mask
corresponding to 32-bits DMA window.
Reported-by: Murali N. Iyer <mniyer@us.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/dma-mapping.h | 1 +
arch/powerpc/kernel/dma.c | 14 ++++++++++----
arch/powerpc/platforms/powernv/pci-ioda.c | 23 +++++++++++++++++++++++
arch/powerpc/platforms/powernv/pci.c | 11 +++++++++++
arch/powerpc/platforms/powernv/pci.h | 2 ++
arch/powerpc/platforms/powernv/powernv.h | 6 ++++++
arch/powerpc/platforms/powernv/setup.c | 9 +++++++++
7 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
index 150866b..894d538 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -135,6 +135,7 @@ static inline int dma_supported(struct device *dev, u64 mask)
extern int dma_set_mask(struct device *dev, u64 dma_mask);
extern int __dma_set_mask(struct device *dev, u64 dma_mask);
+extern u64 __dma_get_required_mask(struct device *dev);
#define dma_alloc_coherent(d,s,h,f) dma_alloc_attrs(d,s,h,f,NULL)
diff --git a/arch/powerpc/kernel/dma.c b/arch/powerpc/kernel/dma.c
index ee78f6e..210ff9d 100644
--- a/arch/powerpc/kernel/dma.c
+++ b/arch/powerpc/kernel/dma.c
@@ -202,6 +202,7 @@ int __dma_set_mask(struct device *dev, u64 dma_mask)
*dev->dma_mask = dma_mask;
return 0;
}
+
int dma_set_mask(struct device *dev, u64 dma_mask)
{
if (ppc_md.dma_set_mask)
@@ -210,13 +211,10 @@ int dma_set_mask(struct device *dev, u64 dma_mask)
}
EXPORT_SYMBOL(dma_set_mask);
-u64 dma_get_required_mask(struct device *dev)
+u64 __dma_get_required_mask(struct device *dev)
{
struct dma_map_ops *dma_ops = get_dma_ops(dev);
- if (ppc_md.dma_get_required_mask)
- return ppc_md.dma_get_required_mask(dev);
-
if (unlikely(dma_ops == NULL))
return 0;
@@ -225,6 +223,14 @@ u64 dma_get_required_mask(struct device *dev)
return DMA_BIT_MASK(8 * sizeof(dma_addr_t));
}
+
+u64 dma_get_required_mask(struct device *dev)
+{
+ if (ppc_md.dma_get_required_mask)
+ return ppc_md.dma_get_required_mask(dev);
+
+ return __dma_get_required_mask(dev);
+}
EXPORT_SYMBOL_GPL(dma_get_required_mask);
static int __init dma_init(void)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 36b1a7a..380ebc9 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -890,6 +890,28 @@ static int pnv_pci_ioda_dma_set_mask(struct pnv_phb *phb,
return 0;
}
+static u64 pnv_pci_ioda_dma_get_required_mask(struct pnv_phb *phb,
+ struct pci_dev *pdev)
+{
+ struct pci_dn *pdn = pci_get_pdn(pdev);
+ struct pnv_ioda_pe *pe;
+ u64 end, mask;
+
+ if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
+ return 0;
+
+ pe = &phb->ioda.pe_array[pdn->pe_number];
+ if (!pe->tce_bypass_enabled)
+ return __dma_get_required_mask(&pdev->dev);
+
+
+ end = pe->tce_bypass_base + memblock_end_of_DRAM();
+ mask = 1ULL << (fls64(end) - 1);
+ mask += mask - 1;
+
+ return mask;
+}
+
static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
struct pci_bus *bus,
bool add_to_iommu_group)
@@ -1782,6 +1804,7 @@ void __init pnv_pci_init_ioda_phb(struct device_node *np,
/* Setup TCEs */
phb->dma_dev_setup = pnv_pci_ioda_dma_dev_setup;
phb->dma_set_mask = pnv_pci_ioda_dma_set_mask;
+ phb->dma_get_required_mask = pnv_pci_ioda_dma_get_required_mask;
/* Setup shutdown function for kexec */
phb->shutdown = pnv_pci_ioda_shutdown;
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index b854b57..e9f509b 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -761,6 +761,17 @@ int pnv_pci_dma_set_mask(struct pci_dev *pdev, u64 dma_mask)
return __dma_set_mask(&pdev->dev, dma_mask);
}
+u64 pnv_pci_dma_get_required_mask(struct pci_dev *pdev)
+{
+ struct pci_controller *hose = pci_bus_to_host(pdev->bus);
+ struct pnv_phb *phb = hose->private_data;
+
+ if (phb && phb->dma_get_required_mask)
+ return phb->dma_get_required_mask(phb, pdev);
+
+ return __dma_get_required_mask(&pdev->dev);
+}
+
void pnv_pci_shutdown(void)
{
struct pci_controller *hose;
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index b54af34..aa95fcb 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -124,6 +124,8 @@ struct pnv_phb {
void (*dma_dev_setup)(struct pnv_phb *phb, struct pci_dev *pdev);
int (*dma_set_mask)(struct pnv_phb *phb, struct pci_dev *pdev,
u64 dma_mask);
+ u64 (*dma_get_required_mask)(struct pnv_phb *phb,
+ struct pci_dev *pdev);
void (*fixup_phb)(struct pci_controller *hose);
u32 (*bdfn_to_pe)(struct pnv_phb *phb, struct pci_bus *bus, u32 devfn);
void (*shutdown)(struct pnv_phb *phb);
diff --git a/arch/powerpc/platforms/powernv/powernv.h b/arch/powerpc/platforms/powernv/powernv.h
index 75501bf..6c8e2d1 100644
--- a/arch/powerpc/platforms/powernv/powernv.h
+++ b/arch/powerpc/platforms/powernv/powernv.h
@@ -13,6 +13,7 @@ struct pci_dev;
extern void pnv_pci_init(void);
extern void pnv_pci_shutdown(void);
extern int pnv_pci_dma_set_mask(struct pci_dev *pdev, u64 dma_mask);
+extern u64 pnv_pci_dma_get_required_mask(struct pci_dev *pdev);
#else
static inline void pnv_pci_init(void) { }
static inline void pnv_pci_shutdown(void) { }
@@ -21,6 +22,11 @@ static inline int pnv_pci_dma_set_mask(struct pci_dev *pdev, u64 dma_mask)
{
return -ENODEV;
}
+
+static inline u64 pnv_pci_dma_get_required_mask(struct pci_dev *pdev)
+{
+ return 0;
+}
#endif
extern void pnv_lpc_init(void);
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index 5a0e2dc..0cb3a07 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -173,6 +173,14 @@ static int pnv_dma_set_mask(struct device *dev, u64 dma_mask)
return __dma_set_mask(dev, dma_mask);
}
+static u64 pnv_dma_get_required_mask(struct device *dev)
+{
+ if (dev_is_pci(dev))
+ return pnv_pci_dma_get_required_mask(to_pci_dev(dev));
+
+ return __dma_get_required_mask(dev);
+}
+
static void pnv_shutdown(void)
{
/* Let the PCI code clear up IODA tables */
@@ -335,6 +343,7 @@ define_machine(powernv) {
.power_save = power7_idle,
.calibrate_decr = generic_calibrate_decr,
.dma_set_mask = pnv_dma_set_mask,
+ .dma_get_required_mask = pnv_dma_get_required_mask,
#ifdef CONFIG_KEXEC
.kexec_cpu_down = pnv_kexec_cpu_down,
#endif
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH v2 03/22] MSI: Remove the redundant irq_set_chip_data()
From: Thierry Reding @ 2014-09-25 7:19 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, sparclinux,
linux-arch, linux-s390, Russell King, Joerg Roedel, x86,
Sebastian Ott, xen-devel, arnab.basu, Arnd Bergmann,
Konrad Rzeszutek Wilk, Chris Metcalf, Bjorn Helgaas,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-4-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 491 bytes --]
On Thu, Sep 25, 2014 at 11:14:13AM +0800, Yijing Wang wrote:
> Currently, pcie-designware, pcie-rcar, pci-tegra drivers
> use irq chip_data to save the msi_chip pointer. They
> already call irq_set_chip_data() in their own MSI irq map
> functions. So irq_set_chip_data() in arch_setup_msi_irq()
> is useless.
Again, I think this should be the other way around. If drivers do
something that's already handled by the core, then the duplicate code
should be dropped from the drivers.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 01/22] PCI/MSI: Clean up struct msi_chip argument
From: Thierry Reding @ 2014-09-25 7:15 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, sparclinux,
linux-arch, linux-s390, Russell King, Joerg Roedel, x86,
Sebastian Ott, xen-devel, arnab.basu, Arnd Bergmann,
Konrad Rzeszutek Wilk, Chris Metcalf, Bjorn Helgaas,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-2-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 741 bytes --]
On Thu, Sep 25, 2014 at 11:14:11AM +0800, Yijing Wang wrote:
> Msi_chip functions setup_irq/teardown_irq rarely use msi_chip
> argument.
That's not true. Out of the four drivers that you modify two use the
parameter. And the two that don't probably should be using it too.
50% is not "rarely". =)
> We can look up msi_chip pointer by the device pointer
> or irq number, so clean up msi_chip argument.
I don't like this particular change. The idea was to keep the API object
oriented so that drivers wouldn't have to know where to get the MSI chip
from. It also makes it more resilient against code reorganizations since
the core code is the only place that needs to know where to get the chip
from.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment
From: Thierry Reding @ 2014-09-25 7:06 UTC (permalink / raw)
To: Yijing Wang
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, sparclinux,
linux-arch, linux-s390, Russell King, Joerg Roedel, x86,
Sebastian Ott, xen-devel, arnab.basu, Arnd Bergmann,
Konrad Rzeszutek Wilk, Chris Metcalf, Bjorn Helgaas,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-3-git-send-email-wangyijing@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 727 bytes --]
On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote:
> Currently, PCI drivers will initialize bus->msi in
> pcibios_add_bus(). pcibios_add_bus() will be called
> in every pci bus initialization. So the bus->msi
> assignment in pci_alloc_child_bus() is useless.
I think this should be the other way around. The default should be to
inherit bus->msi from the parent. That way drivers don't typically have
to do it, yet they can still opt to override it if they need to.
For Tegra for example I think it would work if we assigned the MSI chip
to the root bus (in tegra_pcie_scan_bus()) and then have it propagated
to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus()
can be removed altogether.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/3] drivers/of: add of_changeset_apply_locked
From: Stephen Rothwell @ 2014-09-25 7:01 UTC (permalink / raw)
To: Cyril Bur; +Cc: linuxppc-dev
In-Reply-To: <1411627290-20302-2-git-send-email-cyril.bur@au1.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 490 bytes --]
Hi Cyril,
Nit:
On Thu, 25 Sep 2014 16:41:28 +1000 Cyril Bur <cyril.bur@au1.ibm.com> wrote:
>
> Due to the requirement of of_changesets that of_changeset_apply be called
> holding the of_mutex and that the of_mutex cannot be accessed nicely outside
> the of code, added a wrapper which grabs the lock and called
^^^^^^
calls
> of_changeset_apply.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] selftests/powerpc: Add test of load_unaligned_zero_pad()
From: Michael Ellerman @ 2014-09-25 6:45 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard
It is a rarely exercised case, so we want to have a test to ensure it
works as required.
Signed-off-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/word-at-a-time.h | 13 +-
tools/testing/selftests/powerpc/Makefile | 2 +-
.../testing/selftests/powerpc/primitives/Makefile | 17 +++
.../selftests/powerpc/primitives/asm/asm-compat.h | 1 +
.../selftests/powerpc/primitives/asm/ppc-opcode.h | 0
.../powerpc/primitives/load_unaligned_zeropad.c | 147 +++++++++++++++++++++
.../selftests/powerpc/primitives/word-at-a-time.h | 1 +
7 files changed, 179 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/primitives/Makefile
create mode 120000 tools/testing/selftests/powerpc/primitives/asm/asm-compat.h
create mode 100644 tools/testing/selftests/powerpc/primitives/asm/ppc-opcode.h
create mode 100644 tools/testing/selftests/powerpc/primitives/load_unaligned_zeropad.c
create mode 120000 tools/testing/selftests/powerpc/primitives/word-at-a-time.h
diff --git a/arch/powerpc/include/asm/word-at-a-time.h b/arch/powerpc/include/asm/word-at-a-time.h
index 07cc121e8a79..ea52b51be401 100644
--- a/arch/powerpc/include/asm/word-at-a-time.h
+++ b/arch/powerpc/include/asm/word-at-a-time.h
@@ -116,6 +116,15 @@ static inline unsigned long prep_zero_mask(unsigned long a, unsigned long bits,
#endif
+/*
+ * We use load_unaligned_zero() in a selftest, which builds a userspace
+ * program. Some linker scripts seem to discard the .fixup section, so allow
+ * the test code to use a different section name.
+ */
+#ifndef FIXUP_SECTION
+#define FIXUP_SECTION ".fixup"
+#endif
+
static inline unsigned long load_unaligned_zeropad(const void *addr)
{
unsigned long ret, offset, tmp;
@@ -123,7 +132,7 @@ static inline unsigned long load_unaligned_zeropad(const void *addr)
asm(
"1: " PPC_LL "%[ret], 0(%[addr])\n"
"2:\n"
- ".section .fixup,\"ax\"\n"
+ ".section " FIXUP_SECTION ",\"ax\"\n"
"3: "
#ifdef __powerpc64__
"clrrdi %[tmp], %[addr], 3\n\t"
@@ -156,4 +165,6 @@ static inline unsigned long load_unaligned_zeropad(const void *addr)
return ret;
}
+#undef FIXUP_SECTION
+
#endif /* _ASM_WORD_AT_A_TIME_H */
diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 74a78cedce37..f6ff90a76bd7 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -13,7 +13,7 @@ CFLAGS := -Wall -O2 -flto -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CUR
export CC CFLAGS
-TARGETS = pmu copyloops mm tm
+TARGETS = pmu copyloops mm tm primitives
endif
diff --git a/tools/testing/selftests/powerpc/primitives/Makefile b/tools/testing/selftests/powerpc/primitives/Makefile
new file mode 100644
index 000000000000..ea737ca01732
--- /dev/null
+++ b/tools/testing/selftests/powerpc/primitives/Makefile
@@ -0,0 +1,17 @@
+CFLAGS += -I$(CURDIR)
+
+PROGS := load_unaligned_zeropad
+
+all: $(PROGS)
+
+$(PROGS): ../harness.c
+
+run_tests: all
+ @-for PROG in $(PROGS); do \
+ ./$$PROG; \
+ done;
+
+clean:
+ rm -f $(PROGS) *.o
+
+.PHONY: all run_tests clean
diff --git a/tools/testing/selftests/powerpc/primitives/asm/asm-compat.h b/tools/testing/selftests/powerpc/primitives/asm/asm-compat.h
new file mode 120000
index 000000000000..b14255e15a25
--- /dev/null
+++ b/tools/testing/selftests/powerpc/primitives/asm/asm-compat.h
@@ -0,0 +1 @@
+../.././../../../../arch/powerpc/include/asm/asm-compat.h
\ No newline at end of file
diff --git a/tools/testing/selftests/powerpc/primitives/asm/ppc-opcode.h b/tools/testing/selftests/powerpc/primitives/asm/ppc-opcode.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tools/testing/selftests/powerpc/primitives/load_unaligned_zeropad.c b/tools/testing/selftests/powerpc/primitives/load_unaligned_zeropad.c
new file mode 100644
index 000000000000..d1b647509596
--- /dev/null
+++ b/tools/testing/selftests/powerpc/primitives/load_unaligned_zeropad.c
@@ -0,0 +1,147 @@
+/*
+ * Userspace test harness for load_unaligned_zeropad. Creates two
+ * pages and uses mprotect to prevent access to the second page and
+ * a SEGV handler that walks the exception tables and runs the fixup
+ * routine.
+ *
+ * The results are compared against a normal load that is that is
+ * performed while access to the second page is enabled via mprotect.
+ *
+ * Copyright (C) 2014 Anton Blanchard <anton@au.ibm.com>, IBM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <signal.h>
+#include <unistd.h>
+#include <sys/mman.h>
+
+#define FIXUP_SECTION ".ex_fixup"
+
+#include "word-at-a-time.h"
+
+#include "utils.h"
+
+
+static int page_size;
+static char *mem_region;
+
+static int protect_region(void)
+{
+ if (mprotect(mem_region + page_size, page_size, PROT_NONE)) {
+ perror("mprotect");
+ return 1;
+ }
+
+ return 0;
+}
+
+static int unprotect_region(void)
+{
+ if (mprotect(mem_region + page_size, page_size, PROT_READ|PROT_WRITE)) {
+ perror("mprotect");
+ return 1;
+ }
+
+ return 0;
+}
+
+extern char __start___ex_table[];
+extern char __stop___ex_table[];
+
+#if defined(__powerpc64__)
+#define UCONTEXT_NIA(UC) (UC)->uc_mcontext.gp_regs[PT_NIP]
+#elif defined(__powerpc__)
+#define UCONTEXT_NIA(UC) (UC)->uc_mcontext.uc_regs->gregs[PT_NIP]
+#else
+#error implement UCONTEXT_NIA
+#endif
+
+static int segv_error;
+
+static void segv_handler(int signr, siginfo_t *info, void *ptr)
+{
+ ucontext_t *uc = (ucontext_t *)ptr;
+ unsigned long addr = (unsigned long)info->si_addr;
+ unsigned long *ip = &UCONTEXT_NIA(uc);
+ unsigned long *ex_p = (unsigned long *)__start___ex_table;
+
+ while (ex_p < (unsigned long *)__stop___ex_table) {
+ unsigned long insn, fixup;
+
+ insn = *ex_p++;
+ fixup = *ex_p++;
+
+ if (insn == *ip) {
+ *ip = fixup;
+ return;
+ }
+ }
+
+ printf("No exception table match for NIA %lx ADDR %lx\n", *ip, addr);
+ segv_error++;
+}
+
+static void setup_segv_handler(void)
+{
+ struct sigaction action;
+
+ memset(&action, 0, sizeof(action));
+ action.sa_sigaction = segv_handler;
+ action.sa_flags = SA_SIGINFO;
+ sigaction(SIGSEGV, &action, NULL);
+}
+
+static int do_one_test(char *p, int page_offset)
+{
+ unsigned long should;
+ unsigned long got;
+
+ FAIL_IF(unprotect_region());
+ should = *(unsigned long *)p;
+ FAIL_IF(protect_region());
+
+ got = load_unaligned_zeropad(p);
+
+ if (should != got)
+ printf("offset %u load_unaligned_zeropad returned 0x%lx, should be 0x%lx\n", page_offset, got, should);
+
+ return 0;
+}
+
+static int test_body(void)
+{
+ unsigned long i;
+
+ page_size = getpagesize();
+ mem_region = mmap(NULL, page_size * 2, PROT_READ|PROT_WRITE,
+ MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+
+ FAIL_IF(mem_region == MAP_FAILED);
+
+ for (i = 0; i < page_size; i++)
+ mem_region[i] = i;
+
+ memset(mem_region+page_size, 0, page_size);
+
+ setup_segv_handler();
+
+ for (i = 0; i < page_size; i++)
+ FAIL_IF(do_one_test(mem_region+i, i));
+
+ FAIL_IF(segv_error);
+
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(test_body, "load_unaligned_zeropad");
+}
diff --git a/tools/testing/selftests/powerpc/primitives/word-at-a-time.h b/tools/testing/selftests/powerpc/primitives/word-at-a-time.h
new file mode 120000
index 000000000000..eb74401b591f
--- /dev/null
+++ b/tools/testing/selftests/powerpc/primitives/word-at-a-time.h
@@ -0,0 +1 @@
+../../../../../arch/powerpc/include/asm/word-at-a-time.h
\ No newline at end of file
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/3] powerpc/pseries: create rtas buffer accessor
From: Cyril Bur @ 2014-09-25 6:41 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1411627290-20302-1-git-send-email-cyril.bur@au1.ibm.com>
Added simple accessor functions for rtas in memory buffers which performs
accesses of appropriate type and performs endian conversions.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
---
arch/powerpc/platforms/pseries/Makefile | 4 +-
arch/powerpc/platforms/pseries/pseries.h | 41 +++++++++
arch/powerpc/platforms/pseries/rtas_buffer.c | 126 +++++++++++++++++++++++++++
3 files changed, 170 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/pseries/rtas_buffer.c
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 0348079..7eb7c46 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -3,7 +3,9 @@ ccflags-$(CONFIG_PPC_PSERIES_DEBUG) += -DDEBUG
obj-y := lpar.o hvCall.o nvram.o reconfig.o \
setup.o iommu.o event_sources.o ras.o \
- firmware.o power.o dlpar.o mobility.o rng.o
+ firmware.o power.o dlpar.o mobility.o \
+ rng.o rtas_buffer.o
+
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_SCANLOG) += scanlog.o
obj-$(CONFIG_EEH) += eeh_pseries.o
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 361add6..f24e352 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -66,4 +66,45 @@ int pseries_root_bridge_prepare(struct pci_host_bridge *bridge);
unsigned long pseries_memory_block_size(void);
+/* Manipulation of the in memory data returned from an RTAS call */
+
+/* Data pointed to by ptr is in big endian */
+struct rtas_buffer {
+ void *ptr;
+ int len;
+ int pos;
+};
+
+/* Buffer is already zeroed */
+int make_rtas_buf(struct rtas_buffer *b, size_t size);
+void free_rtas_buf(struct rtas_buffer *b);
+
+/* Return pointer to the buffer being used */
+void *get_rtas_buf(struct rtas_buffer *b);
+
+/* Checks if the buffer exists and the read position is less than the length*/
+bool check_rtas_buf(struct rtas_buffer *b);
+size_t get_rtas_buf_size(struct rtas_buffer *b);
+
+/* Advance the internal position of the buffer by size bytes */
+bool advance_rtas_buf(struct rtas_buffer *b, size_t size);
+
+/* Put a value val into the buffer at position pos. Function expect val in cpu
+ * endian. Returns true if the write to the buffer was successful.
+ */
+bool put_rtas_buf_32(struct rtas_buffer *b, u32 val, int pos);
+
+/* Grab the byte at the current position of the buffer without incrementing
+ * the internal position of the buffer */
+bool peek_rtas_buf(struct rtas_buffer *b, u8 *c);
+
+/* Accessor functions return true if access succeeded and value is written to
+ * val in cpu endian. Automatically advances its reference into the buffer by
+ * the requested amount.
+ */
+bool get_rtas_buf_32(struct rtas_buffer *b, u32 *val);
+bool get_rtas_buf_64(struct rtas_buffer *b, u64 *val);
+bool get_rtas_buf_mem(struct rtas_buffer *b, void **p, size_t len);
+bool get_rtas_buf_str(struct rtas_buffer *b, char **s);
+
#endif /* _PSERIES_PSERIES_H */
diff --git a/arch/powerpc/platforms/pseries/rtas_buffer.c b/arch/powerpc/platforms/pseries/rtas_buffer.c
new file mode 100644
index 0000000..f06b73c
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/rtas_buffer.c
@@ -0,0 +1,126 @@
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+#include "pseries.h"
+
+
+int make_rtas_buf(struct rtas_buffer *b, size_t sz)
+{
+ b->ptr = kzalloc(sz, GFP_KERNEL);
+ b->len = sz;
+ b->pos = 0;
+ return (!b->ptr) ? -ENOMEM : 0;
+}
+
+void free_rtas_buf(struct rtas_buffer *b)
+{
+ kfree(b->ptr);
+}
+
+void *get_rtas_buf(struct rtas_buffer *b)
+{
+ return (b) ? b->ptr : NULL;
+}
+
+size_t get_rtas_buf_size(struct rtas_buffer *b)
+{
+ return (b) ? b->len : 0;
+}
+
+bool check_rtas_buf(struct rtas_buffer *b)
+{
+ return (b && b->ptr && b->pos < b->len);
+}
+
+static inline void *buf_pos(struct rtas_buffer *b)
+{
+ return (b && b->ptr) ? b->ptr + b->pos : NULL;
+}
+
+bool peek_rtas_buf(struct rtas_buffer *b, u8 *c)
+{
+ if (!b || !c || b->pos >= b->len)
+ return false;
+
+ *c = *(u8 *)buf_pos(b);
+
+ return true;
+}
+
+bool put_rtas_buf_32(struct rtas_buffer *b, u32 val, int pos)
+{
+ if (!b || b->pos >= b->len)
+ return false;
+
+ *((__be32 *)buf_pos(b)) = cpu_to_be32(val);
+
+ return true;
+}
+
+bool get_rtas_buf_32(struct rtas_buffer *b, u32 *val)
+{
+ if (!b || !val || b->len - b->pos < sizeof(u32))
+ return false;
+
+ *val = be32_to_cpu(*((__be32 *)buf_pos(b)));
+ b->pos += sizeof(u32);
+ return true;
+}
+
+bool get_rtas_buf_64(struct rtas_buffer *b, u64 *val)
+{
+ if (!b || !val || b->len - b->pos < sizeof(u64))
+ return false;
+
+ *val = be64_to_cpu(*((__be64 *)buf_pos(b)));
+ b->pos += sizeof(u64);
+ return true;
+}
+
+bool get_rtas_buf_str(struct rtas_buffer *b, char **s)
+{
+ int i;
+ if (!b || !s)
+ return false;
+
+ /* Get length of string */
+ i = b->pos;
+ while (i < b->len) {
+ if (*(char *)(b->ptr + i) == '\0') {
+ *s = (char *)buf_pos(b);
+ if (!*s)
+ return false;
+
+ b->pos = i + 1;
+ return true;
+ }
+ i++;
+ }
+
+ return false;
+}
+
+bool get_rtas_buf_mem(struct rtas_buffer *b, void **p, size_t len)
+{
+ if (!b || !p || b->len - b->pos < len)
+ return false;
+
+ *p = buf_pos(b);
+ if (!*p)
+ return false;
+
+ b->pos += len;
+
+ return true;
+}
+
+bool advance_rtas_buf(struct rtas_buffer *b, size_t len)
+{
+ if (!b || b->len - b->pos < len)
+ return false;
+
+ b->pos += len;
+
+ return true;
+}
+
--
1.9.1
^ permalink raw reply related
* [PATCH v2 3/3] powerpc/pseries: fix bugs in mobility RTAS calls
From: Cyril Bur @ 2014-09-25 6:41 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1411627290-20302-1-git-send-email-cyril.bur@au1.ibm.com>
These calls use a shared memory buffer to communicate device tree updates and
PAPR specifies that RTAS buffers are to be written in big endian. Used the
rtas buffer accessor to help solve both endian problems and standard buffer
access problems.
It seems more sane to stop trying to parse a buffer on errors. Attempting to
continue opens up the possibility of overflows and/or garbage reads. Used
of_changesets throughout to avoid leaving the device tree in an inconsitent
state on error.
Don't warn about failed allcations when the amount was taken from the buffer,
assume the value was incorrect, don't needlessly concern the user.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 406 ++++++++++++++++++------------
1 file changed, 243 insertions(+), 163 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index e7cb6d4..37d16da 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -20,16 +20,11 @@
#include <asm/machdep.h>
#include <asm/rtas.h>
+
#include "pseries.h"
-static struct kobject *mobility_kobj;
-struct update_props_workarea {
- u32 phandle;
- u32 state;
- u64 reserved;
- u32 nprops;
-} __packed;
+static struct kobject *mobility_kobj;
#define NODE_ACTION_MASK 0xff000000
#define NODE_COUNT_MASK 0x00ffffff
@@ -40,7 +35,7 @@ struct update_props_workarea {
#define MIGRATION_SCOPE (1)
-static int mobility_rtas_call(int token, char *buf, s32 scope)
+static int mobility_rtas_call(int token, void *buf, s32 scope)
{
int rc;
@@ -54,171 +49,193 @@ static int mobility_rtas_call(int token, char *buf, s32 scope)
return rc;
}
-static int delete_dt_node(u32 phandle)
+static int create_property(struct rtas_buffer *rtas_buf, struct property **prop,
+ const char *name, int length)
{
- struct device_node *dn;
-
- dn = of_find_node_by_phandle(phandle);
- if (!dn)
- return -ENOENT;
+ void *prop_value;
+ void *new_value;
- dlpar_detach_node(dn);
- of_node_put(dn);
- return 0;
-}
+ struct property *working_prop = *prop;
-static int update_dt_property(struct device_node *dn, struct property **prop,
- const char *name, u32 vd, char *value)
-{
- struct property *new_prop = *prop;
- int more = 0;
+ if (!get_rtas_buf_mem(rtas_buf, &prop_value, length))
+ return -EOVERFLOW;
- /* A negative 'vd' value indicates that only part of the new property
- * value is contained in the buffer and we need to call
- * ibm,update-properties again to get the rest of the value.
- *
- * A negative value is also the two's compliment of the actual value.
- */
- if (vd & 0x80000000) {
- vd = ~vd + 1;
- more = 1;
- }
-
- if (new_prop) {
- /* partial property fixup */
- char *new_data = kzalloc(new_prop->length + vd, GFP_KERNEL);
- if (!new_data)
- return -ENOMEM;
-
- memcpy(new_data, new_prop->value, new_prop->length);
- memcpy(new_data + new_prop->length, value, vd);
-
- kfree(new_prop->value);
- new_prop->value = new_data;
- new_prop->length += vd;
- } else {
- new_prop = kzalloc(sizeof(*new_prop), GFP_KERNEL);
- if (!new_prop)
- return -ENOMEM;
-
- new_prop->name = kstrdup(name, GFP_KERNEL);
- if (!new_prop->name) {
- kfree(new_prop);
+ if (!working_prop) {
+ working_prop = kzalloc(sizeof(*working_prop), GFP_KERNEL);
+ if (!working_prop) {
return -ENOMEM;
}
- new_prop->length = vd;
- new_prop->value = kzalloc(new_prop->length, GFP_KERNEL);
- if (!new_prop->value) {
- kfree(new_prop->name);
- kfree(new_prop);
+ working_prop->name = kstrdup(name, GFP_KERNEL | __GFP_NOWARN);
+ if (!working_prop->name) {
+ kfree(working_prop);
return -ENOMEM;
}
-
- memcpy(new_prop->value, value, vd);
- *prop = new_prop;
+ *prop = working_prop;
}
- if (!more) {
- of_update_property(dn, new_prop);
+ new_value = krealloc(working_prop->value, working_prop->length + length, GFP_KERNEL | __GFP_NOWARN);
+ if (!new_value) {
+ kfree(working_prop->value);
+ kfree(working_prop->name);
+ kfree(working_prop);
*prop = NULL;
+ return -ENOMEM;
}
+ working_prop->value = new_value;
+ memcpy(working_prop->value + working_prop->length, prop_value, length);
+ working_prop->length += length;
+
return 0;
}
-static int update_dt_node(u32 phandle, s32 scope)
+static int parse_ibm_update_properties(struct rtas_buffer *rtas_buf, struct of_changeset *cs,
+ struct device_node *dn, struct property **r_prop)
{
- struct update_props_workarea *upwa;
- struct device_node *dn;
+ int i;
+ int rc = 0;
+
struct property *prop = NULL;
- int i, rc, rtas_rc;
- char *prop_data;
- char *rtas_buf;
- int update_properties_token;
+
+ char peek;
+ u32 nprops;
u32 vd;
- update_properties_token = rtas_token("ibm,update-properties");
- if (update_properties_token == RTAS_UNKNOWN_SERVICE)
+ if (!rtas_buf || !cs || !dn || !r_prop)
return -EINVAL;
- rtas_buf = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
- if (!rtas_buf)
- return -ENOMEM;
+ /* Advance the buffer by 16 bytes which contain the phandle (4 bytes),
+ * 4 bytes of state variable and 8 bytes of reserved (zero) bytes.
+ * As the state variable must be sent on subsequent RTAS calls ensure
+ * don't touch it so the caller can reuse this buffer.
+ */
+ if (!advance_rtas_buf(rtas_buf, 16) || !get_rtas_buf_32(rtas_buf, &nprops))
+ return -EOVERFLOW;
- dn = of_find_node_by_phandle(phandle);
- if (!dn) {
- kfree(rtas_buf);
- return -ENOENT;
+ /* On the first call to ibm,update-properties for a node the
+ * the first property value descriptor contains an empty
+ * property name, the property value length encoded as u32,
+ * and the property value is the node path being updated.
+ *
+ * Discard this information as the node was looked up by its phandle and
+ * passed in.
+ */
+ if (!peek_rtas_buf(rtas_buf, &peek))
+ return -EOVERFLOW;
+ if (peek == '\0') {
+ if (!advance_rtas_buf(rtas_buf, sizeof(u32)) ||
+ !get_rtas_buf_32(rtas_buf, &vd) ||
+ !advance_rtas_buf(rtas_buf, vd))
+ return -EOVERFLOW;
+ nprops--;
}
- upwa = (struct update_props_workarea *)&rtas_buf[0];
- upwa->phandle = phandle;
+ for (i = 0; i < nprops; i++) {
+ char *prop_name = NULL;
+ int prop_len;
- do {
- rtas_rc = mobility_rtas_call(update_properties_token, rtas_buf,
- scope);
- if (rtas_rc < 0)
+ if (!get_rtas_buf_str(rtas_buf, &prop_name) ||
+ !get_rtas_buf_32(rtas_buf, &vd))
+ return -EOVERFLOW;
+
+ switch (vd) {
+ case 0x00000000:
+ /* name only property, nothing to do */
break;
- prop_data = rtas_buf + sizeof(*upwa);
-
- /* On the first call to ibm,update-properties for a node the
- * the first property value descriptor contains an empty
- * property name, the property value length encoded as u32,
- * and the property value is the node path being updated.
- */
- if (*prop_data == 0) {
- prop_data++;
- vd = *(u32 *)prop_data;
- prop_data += vd + sizeof(vd);
- upwa->nprops--;
+ case 0x80000000:
+ prop = of_find_property(dn, prop_name, NULL);
+ of_changeset_remove_property(cs, dn, prop);
+ prop = NULL;
+ break;
+
+ default:
+ /* A negative 'vd' value indicates that only part of the new property
+ * value is contained in the buffer and we need to call
+ * ibm,update-properties again to get the rest of the value.
+ *
+ * A negative value is also the two's compliment of the actual value.
+ */
+ prop_len = vd & 0x80000000 ? ~vd + 1 : vd;
+
+ rc = create_property(rtas_buf, &prop, prop_name, prop_len);
+ if (rc) {
+ printk(KERN_ERR "Could not update %s property\n", prop_name);
+ /* Could try to continue but failure in that function likely means
+ * we have bigger problems.
+ */
+ return rc;
+ }
+
+ if (prop && !(vd & 0x80000000)) {
+ of_changeset_update_property(cs, dn, prop);
+ prop = NULL;
+ }
}
+ }
- for (i = 0; i < upwa->nprops; i++) {
- char *prop_name;
+ if (prop)
+ *r_prop = prop;
+ return rc;
+}
- prop_name = prop_data;
- prop_data += strlen(prop_name) + 1;
- vd = *(u32 *)prop_data;
- prop_data += sizeof(vd);
+static int pseries_devicetree_update_props(struct of_changeset *cs, struct device_node *dn,
+ s32 scope)
+{
+ int rc;
+ int error;
+ int update_properties_token;
- switch (vd) {
- case 0x00000000:
- /* name only property, nothing to do */
- break;
+ void *buf;
+ struct rtas_buffer rtas_buf;
- case 0x80000000:
- prop = of_find_property(dn, prop_name, NULL);
- of_remove_property(dn, prop);
- prop = NULL;
- break;
+ struct property *prop = NULL;
- default:
- rc = update_dt_property(dn, &prop, prop_name,
- vd, prop_data);
- if (rc) {
- printk(KERN_ERR "Could not update %s"
- " property\n", prop_name);
- }
+ if (!cs || !dn)
+ return -EINVAL;
- prop_data += vd;
- }
+ update_properties_token = rtas_token("ibm,update-properties");
+ if (update_properties_token == RTAS_UNKNOWN_SERVICE)
+ return -EINVAL;
+
+ rc = make_rtas_buf(&rtas_buf, RTAS_DATA_BUF_SIZE);
+ if (rc)
+ return -ENOMEM;
+
+ put_rtas_buf_32(&rtas_buf, dn->phandle, 0);
+
+ buf = get_rtas_buf(&rtas_buf);
+ if (!buf) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ do {
+
+ rc = mobility_rtas_call(update_properties_token, buf, scope);
+ if (rc < 0)
+ goto out;
+
+ error = parse_ibm_update_properties(&rtas_buf, cs, dn, &prop);
+ if (error) {
+ rc = error;
+ goto out;
}
- } while (rtas_rc == 1);
- of_node_put(dn);
- kfree(rtas_buf);
- return 0;
+ } while (rc == 1);
+
+out:
+ free_rtas_buf(&rtas_buf);
+ return rc;
}
-static int add_dt_node(u32 parent_phandle, u32 drc_index)
+static int pseries_devicetree_update_node(struct of_changeset *cs, struct device_node *parent_dn,
+ u32 drc_index)
{
struct device_node *dn;
- struct device_node *parent_dn;
int rc;
- parent_dn = of_find_node_by_phandle(parent_phandle);
if (!parent_dn)
return -ENOENT;
@@ -230,59 +247,122 @@ static int add_dt_node(u32 parent_phandle, u32 drc_index)
if (rc)
dlpar_free_cc_nodes(dn);
- of_node_put(parent_dn);
return rc;
}
+static int parse_ibm_update_nodes(struct rtas_buffer *rtas_buf, struct of_changeset *cs, s32 scope)
+{
+ u32 node;
+ int rc = 0;
+
+ if (!rtas_buf || !cs)
+ return -EINVAL;
+
+ /* Advance the buffer by 16 bytes which encompases 4 bytes of state
+ * variable and 12 reserved (zeroed) bytes.
+ * As the state variable must be sent on subsequent RTAS calls ensure
+ * don't touch it so the caller can reuse this buffer.
+ */
+ if (!advance_rtas_buf(rtas_buf, 16) ||
+ !get_rtas_buf_32(rtas_buf, &node))
+ goto out;
+
+ while (node & NODE_ACTION_MASK) {
+ int i;
+ u32 action = node & NODE_ACTION_MASK;
+ int node_count = node & NODE_COUNT_MASK;
+ struct device_node *dn;
+
+ for (i = 0; i < node_count; i++) {
+ u32 phandle;
+ u32 drc_index;
+
+ if (!get_rtas_buf_32(rtas_buf, &phandle))
+ goto out;
+
+ dn = of_find_node_by_phandle(phandle);
+ if (!dn)
+ return -ENOENT;
+
+ switch (action) {
+ case DELETE_DT_NODE:
+ of_changeset_detach_node(cs, dn);
+ break;
+ case UPDATE_DT_NODE:
+ rc = pseries_devicetree_update_props(cs, dn, scope);
+ break;
+ case ADD_DT_NODE:
+ if (!get_rtas_buf_32(rtas_buf, &drc_index)) {
+ of_node_put(dn);
+ goto out;
+ }
+ rc = pseries_devicetree_update_node(cs, dn, drc_index);
+ break;
+ default:
+ of_node_put(dn);
+ /* Bogus action */
+ return -EINVAL; /* Break */
+ }
+ of_node_put(dn);
+ }
+ if (!get_rtas_buf_32(rtas_buf, &node))
+ goto out;
+ }
+
+ return rc;
+out:
+ printk(KERN_ERR "RTAS buffer length exceeded\n");
+ return -EOVERFLOW;
+}
+
int pseries_devicetree_update(s32 scope)
{
- char *rtas_buf;
- u32 *data;
+ struct rtas_buffer rtas_buf;
+ struct of_changeset cs;
+ void *buf;
int update_nodes_token;
int rc;
+ int error;
update_nodes_token = rtas_token("ibm,update-nodes");
if (update_nodes_token == RTAS_UNKNOWN_SERVICE)
return -EINVAL;
- rtas_buf = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
- if (!rtas_buf)
- return -ENOMEM;
+ of_changeset_init(&cs);
+
+ rc = make_rtas_buf(&rtas_buf, RTAS_DATA_BUF_SIZE);
+ if (rc) {
+ of_changeset_destroy(&cs);
+ return rc;
+ }
+
+ buf = get_rtas_buf(&rtas_buf);
+ if (!buf) {
+ rc = -ENOMEM;
+ goto out;
+ }
do {
- rc = mobility_rtas_call(update_nodes_token, rtas_buf, scope);
+ rc = mobility_rtas_call(update_nodes_token, buf, scope);
if (rc && rc != 1)
break;
- data = (u32 *)rtas_buf + 4;
- while (*data & NODE_ACTION_MASK) {
- int i;
- u32 action = *data & NODE_ACTION_MASK;
- int node_count = *data & NODE_COUNT_MASK;
-
- data++;
-
- for (i = 0; i < node_count; i++) {
- u32 phandle = *data++;
- u32 drc_index;
-
- switch (action) {
- case DELETE_DT_NODE:
- delete_dt_node(phandle);
- break;
- case UPDATE_DT_NODE:
- update_dt_node(phandle, scope);
- break;
- case ADD_DT_NODE:
- drc_index = *data++;
- add_dt_node(phandle, drc_index);
- break;
- }
- }
- }
+ error = parse_ibm_update_nodes(&rtas_buf, &cs, scope);
+ if (error)
+ break;
+
} while (rc == 1);
- kfree(rtas_buf);
+ if (error)
+ rc = error;
+
+ if (rc == 0)
+ rc = of_changeset_apply_locked(&cs);
+
+out:
+ of_changeset_destroy(&cs);
+ free_rtas_buf(&rtas_buf);
+
return rc;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/3] drivers/of: add of_changeset_apply_locked
From: Cyril Bur @ 2014-09-25 6:41 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1411627290-20302-1-git-send-email-cyril.bur@au1.ibm.com>
Due to the requirement of of_changesets that of_changeset_apply be called
holding the of_mutex and that the of_mutex cannot be accessed nicely outside
the of code, added a wrapper which grabs the lock and called
of_changeset_apply.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
---
drivers/of/dynamic.c | 11 +++++++++++
include/linux/of.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 54fecc4..cbff2a2 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -542,6 +542,17 @@ void of_changeset_destroy(struct of_changeset *ocs)
__of_changeset_entry_destroy(ce);
}
+int of_changeset_apply_locked(struct of_changeset *ocs)
+{
+ int ret;
+
+ mutex_lock(&of_mutex);
+ ret = of_changeset_apply(ocs);
+ mutex_unlock(&of_mutex);
+
+ return ret;
+}
+
/**
* of_changeset_apply - Applies a changeset
*
diff --git a/include/linux/of.h b/include/linux/of.h
index 6c4363b..f5c48fa 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -827,6 +827,7 @@ struct of_changeset {
extern void of_changeset_init(struct of_changeset *ocs);
extern void of_changeset_destroy(struct of_changeset *ocs);
extern int of_changeset_apply(struct of_changeset *ocs);
+extern int of_changeset_apply_locked(struct of_changeset *ocs);
extern int of_changeset_revert(struct of_changeset *ocs);
extern int of_changeset_action(struct of_changeset *ocs,
unsigned long action, struct device_node *np,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 0/3] fix bugs in mobility RTAS calls
From: Cyril Bur @ 2014-09-25 6:41 UTC (permalink / raw)
To: linuxppc-dev
This patchset addresses endian issues and bugs in device tree update for
ibm,update-nodes and ibm,update-properties RTAS calls.
A subseqent patchset will deal with issues in device tree node addition
(ibm,configure-connector RTAS call) as well as more robust handling of
deleting critical device tree nodes.
v1 attempted to keep the structure of the existing code.
v2 rewrites the relevent sections of mobility.c.
Cyril Bur (3):
drivers/of: add of_changeset_apply_locked
powerpc/pseries: create rtas buffer accessor
powerpc/pseries: fix bugs in mobility RTAS calls
arch/powerpc/platforms/pseries/Makefile | 4 +-
arch/powerpc/platforms/pseries/mobility.c | 406 ++++++++++++++++-----------
arch/powerpc/platforms/pseries/pseries.h | 41 +++
arch/powerpc/platforms/pseries/rtas_buffer.c | 126 +++++++++
drivers/of/dynamic.c | 11 +
include/linux/of.h | 1 +
6 files changed, 425 insertions(+), 164 deletions(-)
create mode 100644 arch/powerpc/platforms/pseries/rtas_buffer.c
--
1.9.1
^ permalink raw reply
* [PATCH v3] powerpc/iommu/ddw: Fix endianness
From: Alexey Kardashevskiy @ 2014-09-25 6:39 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: devicetree, linux-kernel, stable, anton, linuxppc-dev
rtas_call() accepts and returns values in CPU endianness.
The ddw_query_response and ddw_create_response structs members are
defined and treated as BE but as they are passed to rtas_call() as
(u32 *) and they get byteswapped automatically, the data is CPU-endian.
This fixes ddw_query_response and ddw_create_response definitions and use.
of_read_number() is designed to work with device tree cells - it assumes
the input is big-endian and returns data in CPU-endian. However due
to the ddw_create_response struct fix, create.addr_hi/lo are already
CPU-endian so do not byteswap them.
ddw_avail is a pointer to the "ibm,ddw-applicable" property which contains
3 cells which are big-endian as it is a device tree. rtas_call() accepts
a RTAS token in CPU-endian. This makes use of of_property_read_u32_array
to byte swap and avoid the need for a number of be32_to_cpu calls.
Cc: stable@vger.kernel.org # v3.13
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: Anton Blanchard <anton@samba.org>
[aik: folded Anton's patch with of_property_read_u32_array]
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* of_property_read_u32_array() is used for ddw_avail[]
v2:
* updated commit log
* fixed definition of ddw_query_response and ddw_create_response
---
arch/powerpc/platforms/pseries/iommu.c | 51 +++++++++++++++++++---------------
1 file changed, 28 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 4642d6a..de1ec54 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -329,16 +329,16 @@ struct direct_window {
/* Dynamic DMA Window support */
struct ddw_query_response {
- __be32 windows_available;
- __be32 largest_available_block;
- __be32 page_size;
- __be32 migration_capable;
+ u32 windows_available;
+ u32 largest_available_block;
+ u32 page_size;
+ u32 migration_capable;
};
struct ddw_create_response {
- __be32 liobn;
- __be32 addr_hi;
- __be32 addr_lo;
+ u32 liobn;
+ u32 addr_hi;
+ u32 addr_lo;
};
static LIST_HEAD(direct_window_list);
@@ -725,16 +725,18 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
{
struct dynamic_dma_window_prop *dwp;
struct property *win64;
- const u32 *ddw_avail;
+ u32 ddw_avail[3];
u64 liobn;
- int len, ret = 0;
+ int ret = 0;
+
+ ret = of_property_read_u32_array(np, "ibm,ddw-applicable",
+ &ddw_avail[0], 3);
- ddw_avail = of_get_property(np, "ibm,ddw-applicable", &len);
win64 = of_find_property(np, DIRECT64_PROPNAME, NULL);
if (!win64)
return;
- if (!ddw_avail || len < 3 * sizeof(u32) || win64->length < sizeof(*dwp))
+ if (ret || win64->length < sizeof(*dwp))
goto delprop;
dwp = win64->value;
@@ -872,8 +874,9 @@ static int create_ddw(struct pci_dev *dev, const u32 *ddw_avail,
do {
/* extra outputs are LIOBN and dma-addr (hi, lo) */
- ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create, cfg_addr,
- BUID_HI(buid), BUID_LO(buid), page_shift, window_shift);
+ ret = rtas_call(ddw_avail[1], 5, 4, (u32 *)create,
+ cfg_addr, BUID_HI(buid), BUID_LO(buid),
+ page_shift, window_shift);
} while (rtas_busy_delay(ret));
dev_info(&dev->dev,
"ibm,create-pe-dma-window(%x) %x %x %x %x %x returned %d "
@@ -910,7 +913,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
int page_shift;
u64 dma_addr, max_addr;
struct device_node *dn;
- const u32 *uninitialized_var(ddw_avail);
+ u32 ddw_avail[3];
struct direct_window *window;
struct property *win64;
struct dynamic_dma_window_prop *ddwprop;
@@ -942,8 +945,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
* for the given node in that order.
* the property is actually in the parent, not the PE
*/
- ddw_avail = of_get_property(pdn, "ibm,ddw-applicable", &len);
- if (!ddw_avail || len < 3 * sizeof(u32))
+ ret = of_property_read_u32_array(pdn, "ibm,ddw-applicable",
+ &ddw_avail[0], 3);
+ if (ret)
goto out_failed;
/*
@@ -966,11 +970,11 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
dev_dbg(&dev->dev, "no free dynamic windows");
goto out_failed;
}
- if (be32_to_cpu(query.page_size) & 4) {
+ if (query.page_size & 4) {
page_shift = 24; /* 16MB */
- } else if (be32_to_cpu(query.page_size) & 2) {
+ } else if (query.page_size & 2) {
page_shift = 16; /* 64kB */
- } else if (be32_to_cpu(query.page_size) & 1) {
+ } else if (query.page_size & 1) {
page_shift = 12; /* 4kB */
} else {
dev_dbg(&dev->dev, "no supported direct page size in mask %x",
@@ -980,7 +984,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
/* verify the window * number of ptes will map the partition */
/* check largest block * page size > max memory hotplug addr */
max_addr = memory_hotplug_max();
- if (be32_to_cpu(query.largest_available_block) < (max_addr >> page_shift)) {
+ if (query.largest_available_block < (max_addr >> page_shift)) {
dev_dbg(&dev->dev, "can't map partiton max 0x%llx with %u "
"%llu-sized pages\n", max_addr, query.largest_available_block,
1ULL << page_shift);
@@ -1006,8 +1010,9 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
if (ret != 0)
goto out_free_prop;
- ddwprop->liobn = create.liobn;
- ddwprop->dma_base = cpu_to_be64(of_read_number(&create.addr_hi, 2));
+ ddwprop->liobn = cpu_to_be32(create.liobn);
+ ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) |
+ create.addr_lo);
ddwprop->tce_shift = cpu_to_be32(page_shift);
ddwprop->window_shift = cpu_to_be32(len);
@@ -1039,7 +1044,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
list_add(&window->list, &direct_window_list);
spin_unlock(&direct_window_list_lock);
- dma_addr = of_read_number(&create.addr_hi, 2);
+ dma_addr = be64_to_cpu(ddwprop->dma_base);
goto out_unlock;
out_free_window:
--
2.0.0
^ permalink raw reply related
* Re: [RFC PATCH] powerpc/powernv: Fix dma_ops for bypass window
From: Gavin Shan @ 2014-09-25 6:03 UTC (permalink / raw)
To: Gavin Shan; +Cc: linuxppc-dev
In-Reply-To: <1411618460-8153-1-git-send-email-gwshan@linux.vnet.ibm.com>
On Thu, Sep 25, 2014 at 02:14:20PM +1000, Gavin Shan wrote:
Please ignore this. I was told by Ben some drivers don't call
set_dev_mask(). So I have to fix it in another way.
Thanks,
Gavin
>When using bypass window on IODA2, the incorrect DMA operations
>"dma_iommu_ops" is used by devices. The device driver calls
>dma_get_required_mask() to determine using 32-bits or bypass DMA
>window. Unfortunately, the returned DMA mask always forces the
>driver to use 32-bits DMA window. The problem was reported on
>the device as follows:
>
>0004:03:00.0 0107: 1000:0087 (rev 05)
>0004:03:00.0 Serial Attached SCSI controller: LSI Logic / Symbios \
> Logic SAS2308 PCI-Express Fusion-MPT SAS-2 (rev 05)
>
>The patch fixes above issue by keeping things consistent: when
>enabling bypass window, we switch to "dma_direct_ops". Instead,
>switch to "pci_dma_ops" when disabling bypass window.
>
>Reported-by: Murali N. Iyer <mniyer@us.ibm.com>
>Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>---
> arch/powerpc/platforms/powernv/pci-ioda.c | 76 +++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 30 deletions(-)
>
>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>index 36b1a7a..60e44d9 100644
>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>@@ -883,13 +883,29 @@ static int pnv_pci_ioda_dma_set_mask(struct pnv_phb *phb,
> set_dma_offset(&pdev->dev, pe->tce_bypass_base);
> } else {
> dev_info(&pdev->dev, "Using 32-bit DMA via iommu\n");
>- set_dma_ops(&pdev->dev, &dma_iommu_ops);
>+ set_dma_ops(&pdev->dev, get_pci_dma_ops());
> set_iommu_table_base(&pdev->dev, &pe->tce32_table);
> }
> *pdev->dev.dma_mask = dma_mask;
> return 0;
> }
>
>+static void pnv_ioda_setup_dev_dma(struct pnv_ioda_pe *pe,
>+ struct pci_dev *pdev,
>+ bool add_to_iommu_group)
>+{
>+ if (pe->tce_bypass_enabled) {
>+ set_dma_ops(&pdev->dev, &dma_direct_ops);
>+ set_dma_offset(&pdev->dev, pe->tce_bypass_base);
>+ } else {
>+ set_dma_ops(&pdev->dev, get_pci_dma_ops());
>+ set_iommu_table_base(&pdev->dev, &pe->tce32_table);
>+ }
>+
>+ if (add_to_iommu_group)
>+ iommu_add_device(&pdev->dev);
>+}
>+
> static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
> struct pci_bus *bus,
> bool add_to_iommu_group)
>@@ -897,11 +913,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
> struct pci_dev *dev;
>
> list_for_each_entry(dev, &bus->devices, bus_list) {
>- if (add_to_iommu_group)
>- set_iommu_table_base_and_group(&dev->dev,
>- &pe->tce32_table);
>- else
>- set_iommu_table_base(&dev->dev, &pe->tce32_table);
>+ pnv_ioda_setup_dev_dma(pe, dev, add_to_iommu_group);
>
> if (dev->subordinate)
> pnv_ioda_setup_bus_dma(pe, dev->subordinate,
>@@ -909,6 +921,15 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
> }
> }
>
>+static void pnv_ioda_setup_pe_dma(struct pnv_ioda_pe *pe,
>+ bool add_to_iommu_group)
>+{
>+ if (pe->pdev)
>+ pnv_ioda_setup_dev_dma(pe, pe->pdev, add_to_iommu_group);
>+ else
>+ pnv_ioda_setup_bus_dma(pe, pe->pbus, add_to_iommu_group);
>+}
>+
> static void pnv_pci_ioda1_tce_invalidate(struct pnv_ioda_pe *pe,
> struct iommu_table *tbl,
> __be64 *startp, __be64 *endp, bool rm)
>@@ -1080,11 +1101,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
> iommu_init_table(tbl, phb->hose->node);
> iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
>
>- if (pe->pdev)
>- set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
>- else
>- pnv_ioda_setup_bus_dma(pe, pe->pbus, true);
>-
>+ pnv_ioda_setup_pe_dma(pe, true);
> return;
> fail:
> /* XXX Failure: Try to fallback to 64-bit only ? */
>@@ -1101,7 +1118,13 @@ static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
> uint16_t window_id = (pe->pe_number << 1 ) + 1;
> int64_t rc;
>
>+ /* Check if we really need do something */
>+ if (pe->tce_bypass_enabled == enable)
>+ return;
>+
> pe_info(pe, "%sabling 64-bit DMA bypass\n", enable ? "En" : "Dis");
>+ pe->tce_bypass_enabled = enable;
>+
> if (enable) {
> phys_addr_t top = memblock_end_of_DRAM();
>
>@@ -1117,22 +1140,15 @@ static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
> window_id,
> pe->tce_bypass_base,
> 0);
>-
>- /*
>- * EEH needs the mapping between IOMMU table and group
>- * of those VFIO/KVM pass-through devices. We can postpone
>- * resetting DMA ops until the DMA mask is configured in
>- * host side.
>- */
>- if (pe->pdev)
>- set_iommu_table_base(&pe->pdev->dev, tbl);
>- else
>- pnv_ioda_setup_bus_dma(pe, pe->pbus, false);
> }
>- if (rc)
>+ if (rc != OPAL_SUCCESS) {
> pe_err(pe, "OPAL error %lld configuring bypass window\n", rc);
>- else
>- pe->tce_bypass_enabled = enable;
>+ return;
>+ }
>+
>+ /* Update base and operations */
>+ pe->tce_bypass_enabled = enable;
>+ pnv_ioda_setup_pe_dma(pe, false);
> }
>
> static void pnv_pci_ioda2_setup_bypass_pe(struct pnv_phb *phb,
>@@ -1213,12 +1229,12 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
> iommu_init_table(tbl, phb->hose->node);
> iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
>
>- if (pe->pdev)
>- set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
>- else
>- pnv_ioda_setup_bus_dma(pe, pe->pbus, true);
>+ /* If the bypass window fails to be created, we still
>+ * can use 32-bits window.
>+ */
>+ pnv_ioda_setup_pe_dma(pe, true);
>
>- /* Also create a bypass window */
>+ /* Create bypass window */
> pnv_pci_ioda2_setup_bypass_pe(phb, pe);
> return;
> fail:
>--
>1.8.3.2
>
^ permalink raw reply
* Re: [PATCH v2 03/13] powerpc/spapr: vfio: Implement spapr_tce_iommu_ops
From: Alexey Kardashevskiy @ 2014-09-25 5:19 UTC (permalink / raw)
To: Alex Williamson; +Cc: linuxppc-dev, Gavin Shan, kvm, linux-kernel
In-Reply-To: <1411504921.24563.20.camel@ul30vt.home>
On 09/24/2014 06:42 AM, Alex Williamson wrote:
> On Tue, 2014-09-23 at 13:00 +1000, Alexey Kardashevskiy wrote:
>> Modern IBM POWERPC systems support multiple IOMMU tables per PE
>> so we need a more reliable way (compared to container_of()) to get
>> a PE pointer from the iommu_table struct pointer used in IOMMU functions.
>>
>> At the moment IOMMU group data points to an iommu_table struct. This
>> introduces a spapr_tce_iommu_group struct which keeps an iommu_owner
>> and a spapr_tce_iommu_ops struct. For IODA, iommu_owner is a pointer to
>> the pnv_ioda_pe struct, for others it is still a pointer to
>> the iommu_table struct. The ops structs correspond to the type which
>> iommu_owner points to.
>>
>> This defines a get_table() callback which returns an iommu_table
>> by its number.
>>
>> As the IOMMU group data pointer points to variable type instead of
>> iommu_table, VFIO SPAPR TCE driver is updated to use the new type.
>> This changes the tce_container struct to store iommu_group instead of
>> iommu_table.
>>
>> So, it was:
>> - iommu_table points to iommu_group via iommu_table::it_group;
>> - iommu_group points to iommu_table via iommu_group_get_iommudata();
>>
>> now it is:
>> - iommu_table points to iommu_group via iommu_table::it_group;
>> - iommu_group points to spapr_tce_iommu_group via
>> iommu_group_get_iommudata();
>> - spapr_tce_iommu_group points to either (depending on .get_table()):
>> - iommu_table;
>> - pnv_ioda_pe;
>>
>> This uses pnv_ioda1_iommu_get_table for both IODA1&2 but IODA2 will
>> have own pnv_ioda2_iommu_get_table soon and pnv_ioda1_iommu_get_table
>> will only be used for IODA1.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> arch/powerpc/include/asm/iommu.h | 6 ++
>> arch/powerpc/include/asm/tce.h | 13 +++
>> arch/powerpc/kernel/iommu.c | 35 ++++++-
>> arch/powerpc/platforms/powernv/pci-ioda.c | 31 +++++-
>> arch/powerpc/platforms/powernv/pci-p5ioc2.c | 1 +
>> arch/powerpc/platforms/powernv/pci.c | 2 +-
>> arch/powerpc/platforms/pseries/iommu.c | 10 +-
>> drivers/vfio/vfio_iommu_spapr_tce.c | 148 ++++++++++++++++++++++------
>> 8 files changed, 208 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
>> index 42632c7..84ee339 100644
>> --- a/arch/powerpc/include/asm/iommu.h
>> +++ b/arch/powerpc/include/asm/iommu.h
>> @@ -108,13 +108,19 @@ extern void iommu_free_table(struct iommu_table *tbl, const char *node_name);
>> */
>> extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
>> int nid);
>> +
>> +struct spapr_tce_iommu_ops;
>> #ifdef CONFIG_IOMMU_API
>> extern void iommu_register_group(struct iommu_table *tbl,
>> + void *iommu_owner,
>> + struct spapr_tce_iommu_ops *ops,
>> int pci_domain_number, unsigned long pe_num);
>> extern int iommu_add_device(struct device *dev);
>> extern void iommu_del_device(struct device *dev);
>> #else
>> static inline void iommu_register_group(struct iommu_table *tbl,
>> + void *iommu_owner,
>> + struct spapr_tce_iommu_ops *ops,
>> int pci_domain_number,
>> unsigned long pe_num)
>> {
>> diff --git a/arch/powerpc/include/asm/tce.h b/arch/powerpc/include/asm/tce.h
>> index 743f36b..9f159eb 100644
>> --- a/arch/powerpc/include/asm/tce.h
>> +++ b/arch/powerpc/include/asm/tce.h
>> @@ -50,5 +50,18 @@
>> #define TCE_PCI_READ 0x1 /* read from PCI allowed */
>> #define TCE_VB_WRITE 0x1 /* write from VB allowed */
>>
>> +struct spapr_tce_iommu_group;
>> +
>> +struct spapr_tce_iommu_ops {
>> + struct iommu_table *(*get_table)(
>> + struct spapr_tce_iommu_group *data,
>> + int num);
>> +};
>> +
>> +struct spapr_tce_iommu_group {
>> + void *iommu_owner;
>> + struct spapr_tce_iommu_ops *ops;
>> +};
>> +
>> #endif /* __KERNEL__ */
>> #endif /* _ASM_POWERPC_TCE_H */
>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>> index b378f78..1c5dae7 100644
>> --- a/arch/powerpc/kernel/iommu.c
>> +++ b/arch/powerpc/kernel/iommu.c
>> @@ -878,24 +878,53 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
>> */
>> static void group_release(void *iommu_data)
>> {
>> - struct iommu_table *tbl = iommu_data;
>> - tbl->it_group = NULL;
>> + kfree(iommu_data);
>> }
>>
>> +static struct iommu_table *spapr_tce_default_get_table(
>> + struct spapr_tce_iommu_group *data, int num)
>> +{
>> + struct iommu_table *tbl = data->iommu_owner;
>> +
>> + switch (num) {
>> + case 0:
>> + if (tbl->it_size)
>> + return tbl;
>> + /* fallthru */
>> + default:
>> + return NULL;
>> + }
>> +}
>> +
>> +static struct spapr_tce_iommu_ops spapr_tce_default_ops = {
>> + .get_table = spapr_tce_default_get_table
>> +};
>> +
>> void iommu_register_group(struct iommu_table *tbl,
>> + void *iommu_owner, struct spapr_tce_iommu_ops *ops,
>> int pci_domain_number, unsigned long pe_num)
>> {
>> struct iommu_group *grp;
>> char *name;
>> + struct spapr_tce_iommu_group *data;
>> +
>> + data = kzalloc(sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return;
>> +
>> + data->iommu_owner = iommu_owner ? iommu_owner : tbl;
>> + data->ops = ops ? ops : &spapr_tce_default_ops;
>>
>> grp = iommu_group_alloc();
>> if (IS_ERR(grp)) {
>> pr_warn("powerpc iommu api: cannot create new group, err=%ld\n",
>> PTR_ERR(grp));
>> + kfree(data);
>> return;
>> }
>> +
>> tbl->it_group = grp;
>> - iommu_group_set_iommudata(grp, tbl, group_release);
>> + iommu_group_set_iommudata(grp, data, group_release);
>> name = kasprintf(GFP_KERNEL, "domain%d-pe%lx",
>> pci_domain_number, pe_num);
>> if (!name)
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index 136e765..2d32a1c 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -23,6 +23,7 @@
>> #include <linux/io.h>
>> #include <linux/msi.h>
>> #include <linux/memblock.h>
>> +#include <linux/iommu.h>
>>
>> #include <asm/sections.h>
>> #include <asm/io.h>
>> @@ -988,6 +989,26 @@ static void pnv_pci_ioda2_tce_invalidate(struct pnv_ioda_pe *pe,
>> }
>> }
>>
>> +static struct iommu_table *pnv_ioda1_iommu_get_table(
>> + struct spapr_tce_iommu_group *data,
>> + int num)
>> +{
>> + struct pnv_ioda_pe *pe = data->iommu_owner;
>> +
>> + switch (num) {
>> + case 0:
>> + if (pe->tce32.table.it_size)
>> + return &pe->tce32.table;
>> + /* fallthru */
>> + default:
>> + return NULL;
>> + }
>> +}
>> +
>> +static struct spapr_tce_iommu_ops pnv_pci_ioda1_ops = {
>> + .get_table = pnv_ioda1_iommu_get_table,
>> +};
>> +
>> static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>> struct pnv_ioda_pe *pe, unsigned int base,
>> unsigned int segs)
>> @@ -1067,7 +1088,8 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
>> TCE_PCI_SWINV_PAIR);
>> }
>> iommu_init_table(tbl, phb->hose->node);
>> - iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
>> + iommu_register_group(tbl, pe, &pnv_pci_ioda1_ops,
>> + phb->hose->global_number, pe->pe_number);
>>
>> if (pe->pdev)
>> set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
>> @@ -1137,6 +1159,10 @@ static void pnv_pci_ioda2_setup_bypass_pe(struct pnv_phb *phb,
>> pnv_pci_ioda2_set_bypass(&pe->tce32.table, true);
>> }
>>
>> +static struct spapr_tce_iommu_ops pnv_pci_ioda2_ops = {
>> + .get_table = pnv_ioda1_iommu_get_table,
>> +};
>> +
>> static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
>> struct pnv_ioda_pe *pe)
>> {
>> @@ -1202,7 +1228,8 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
>> tbl->it_type |= (TCE_PCI_SWINV_CREATE | TCE_PCI_SWINV_FREE);
>> }
>> iommu_init_table(tbl, phb->hose->node);
>> - iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
>> + iommu_register_group(tbl, pe, &pnv_pci_ioda2_ops,
>> + phb->hose->global_number, pe->pe_number);
>>
>> if (pe->pdev)
>> set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
>> diff --git a/arch/powerpc/platforms/powernv/pci-p5ioc2.c b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
>> index 94ce348..b79066d 100644
>> --- a/arch/powerpc/platforms/powernv/pci-p5ioc2.c
>> +++ b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
>> @@ -89,6 +89,7 @@ static void pnv_pci_p5ioc2_dma_dev_setup(struct pnv_phb *phb,
>> if (phb->p5ioc2.iommu_table.it_map == NULL) {
>> iommu_init_table(&phb->p5ioc2.iommu_table, phb->hose->node);
>> iommu_register_group(&phb->p5ioc2.iommu_table,
>> + NULL, NULL,
>> pci_domain_nr(phb->hose->bus), phb->opal_id);
>> }
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index 97895d4..6ffac79 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -723,7 +723,7 @@ static struct iommu_table *pnv_pci_setup_bml_iommu(struct pci_controller *hose)
>> pnv_pci_setup_iommu_table(tbl, __va(be64_to_cpup(basep)),
>> be32_to_cpup(sizep), 0, IOMMU_PAGE_SHIFT_4K);
>> iommu_init_table(tbl, hose->node);
>> - iommu_register_group(tbl, pci_domain_nr(hose->bus), 0);
>> + iommu_register_group(tbl, NULL, NULL, pci_domain_nr(hose->bus), 0);
>>
>> /* Deal with SW invalidated TCEs when needed (BML way) */
>> swinvp = of_get_property(hose->dn, "linux,tce-sw-invalidate-info",
>> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
>> index 4642d6a..b95f8cf 100644
>> --- a/arch/powerpc/platforms/pseries/iommu.c
>> +++ b/arch/powerpc/platforms/pseries/iommu.c
>> @@ -616,7 +616,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
>>
>> iommu_table_setparms(pci->phb, dn, tbl);
>> pci->iommu_table = iommu_init_table(tbl, pci->phb->node);
>> - iommu_register_group(tbl, pci_domain_nr(bus), 0);
>> + iommu_register_group(tbl, NULL, NULL, pci_domain_nr(bus), 0);
>>
>> /* Divide the rest (1.75GB) among the children */
>> pci->phb->dma_window_size = 0x80000000ul;
>> @@ -661,7 +661,7 @@ static void pci_dma_bus_setup_pSeriesLP(struct pci_bus *bus)
>> ppci->phb->node);
>> iommu_table_setparms_lpar(ppci->phb, pdn, tbl, dma_window);
>> ppci->iommu_table = iommu_init_table(tbl, ppci->phb->node);
>> - iommu_register_group(tbl, pci_domain_nr(bus), 0);
>> + iommu_register_group(tbl, NULL, NULL, pci_domain_nr(bus), 0);
>> pr_debug(" created table: %p\n", ppci->iommu_table);
>> }
>> }
>> @@ -688,7 +688,8 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
>> phb->node);
>> iommu_table_setparms(phb, dn, tbl);
>> PCI_DN(dn)->iommu_table = iommu_init_table(tbl, phb->node);
>> - iommu_register_group(tbl, pci_domain_nr(phb->bus), 0);
>> + iommu_register_group(tbl, NULL, NULL,
>> + pci_domain_nr(phb->bus), 0);
>> set_iommu_table_base_and_group(&dev->dev,
>> PCI_DN(dn)->iommu_table);
>> return;
>> @@ -1105,7 +1106,8 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
>> pci->phb->node);
>> iommu_table_setparms_lpar(pci->phb, pdn, tbl, dma_window);
>> pci->iommu_table = iommu_init_table(tbl, pci->phb->node);
>> - iommu_register_group(tbl, pci_domain_nr(pci->phb->bus), 0);
>> + iommu_register_group(tbl, NULL, NULL,
>> + pci_domain_nr(pci->phb->bus), 0);
>> pr_debug(" created table: %p\n", pci->iommu_table);
>> } else {
>> pr_debug(" found DMA window, table: %p\n", pci->iommu_table);
>> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
>> index 730b4ef..a8adfbd 100644
>> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
>> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
>> @@ -43,17 +43,51 @@ static void tce_iommu_detach_group(void *iommu_data,
>> */
>> struct tce_container {
>> struct mutex lock;
>> - struct iommu_table *tbl;
>> + struct iommu_group *grp;
>> + long windows_num;
>> bool enabled;
>> };
>>
>> +static struct iommu_table *spapr_tce_find_table(
>> + struct tce_container *container,
>> + struct spapr_tce_iommu_group *data,
>> + phys_addr_t ioba)
>> +{
>> + long i;
>> + struct iommu_table *ret = NULL;
>> +
>> + mutex_lock(&container->lock);
>> + for (i = 0; i < container->windows_num; ++i) {
>> + struct iommu_table *tbl = data->ops->get_table(data, i);
>> +
>> + if (tbl) {
>> + unsigned long entry = ioba >> tbl->it_page_shift;
>> + unsigned long start = tbl->it_offset;
>> + unsigned long end = start + tbl->it_size;
>> +
>> + if ((start <= entry) && (entry < end)) {
>> + ret = tbl;
>> + break;
>> + }
>> + }
>> + }
>> + mutex_unlock(&container->lock);
>> +
>> + return ret;
>> +}
>> +
>> static int tce_iommu_enable(struct tce_container *container)
>> {
>> int ret = 0;
>> unsigned long locked, lock_limit, npages;
>> - struct iommu_table *tbl = container->tbl;
>> + struct iommu_table *tbl;
>> + struct spapr_tce_iommu_group *data;
>>
>> - if (!container->tbl)
>> + if (!container->grp)
>> + return -ENXIO;
>> +
>> + data = iommu_group_get_iommudata(container->grp);
>> + if (!data || !data->iommu_owner || !data->ops->get_table)
>> return -ENXIO;
>>
>> if (!current->mm)
>> @@ -80,6 +114,10 @@ static int tce_iommu_enable(struct tce_container *container)
>> * that would effectively kill the guest at random points, much better
>> * enforcing the limit based on the max that the guest can map.
>> */
>> + tbl = data->ops->get_table(data, 0);
>
> Can we define an enum to avoid sprinkling magic zeros in the code?
Right. Missed it in one of iterations :-/
>> + if (!tbl)
>> + return -ENXIO;
>> +
>> down_write(¤t->mm->mmap_sem);
>> npages = (tbl->it_size << IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT;
>> locked = current->mm->locked_vm + npages;
>> @@ -89,7 +127,6 @@ static int tce_iommu_enable(struct tce_container *container)
>> rlimit(RLIMIT_MEMLOCK));
>> ret = -ENOMEM;
>> } else {
>> -
>> current->mm->locked_vm += npages;
>> container->enabled = true;
>> }
>> @@ -100,16 +137,27 @@ static int tce_iommu_enable(struct tce_container *container)
>>
>> static void tce_iommu_disable(struct tce_container *container)
>> {
>> + struct spapr_tce_iommu_group *data;
>> + struct iommu_table *tbl;
>> +
>> if (!container->enabled)
>> return;
>>
>> container->enabled = false;
>>
>> - if (!container->tbl || !current->mm)
>> + if (!container->grp || !current->mm)
>> + return;
>> +
>> + data = iommu_group_get_iommudata(container->grp);
>> + if (!data || !data->iommu_owner || !data->ops->get_table)
>> + return;
>> +
>> + tbl = data->ops->get_table(data, 0);
>> + if (!tbl)
>> return;
>>
>> down_write(¤t->mm->mmap_sem);
>> - current->mm->locked_vm -= (container->tbl->it_size <<
>> + current->mm->locked_vm -= (tbl->it_size <<
>> IOMMU_PAGE_SHIFT_4K) >> PAGE_SHIFT;
>> up_write(¤t->mm->mmap_sem);
>> }
>> @@ -129,6 +177,8 @@ static void *tce_iommu_open(unsigned long arg)
>>
>> mutex_init(&container->lock);
>>
>> + container->windows_num = 1;
>> +
>> return container;
>> }
>>
>> @@ -136,11 +186,11 @@ static void tce_iommu_release(void *iommu_data)
>> {
>> struct tce_container *container = iommu_data;
>>
>> - WARN_ON(container->tbl && !container->tbl->it_group);
>> + WARN_ON(container->grp);
>> tce_iommu_disable(container);
>>
>> - if (container->tbl && container->tbl->it_group)
>> - tce_iommu_detach_group(iommu_data, container->tbl->it_group);
>> + if (container->grp)
>> + tce_iommu_detach_group(iommu_data, container->grp);
>>
>> mutex_destroy(&container->lock);
>>
>> @@ -169,8 +219,17 @@ static long tce_iommu_ioctl(void *iommu_data,
>>
>> case VFIO_IOMMU_SPAPR_TCE_GET_INFO: {
>> struct vfio_iommu_spapr_tce_info info;
>> - struct iommu_table *tbl = container->tbl;
>> + struct iommu_table *tbl;
>> + struct spapr_tce_iommu_group *data;
>>
>> + if (WARN_ON(!container->grp))
>> + return -ENXIO;
>> +
>> + data = iommu_group_get_iommudata(container->grp);
>> + if (WARN_ON(!data || !data->iommu_owner || !data->ops))
>> + return -ENXIO;
>> +
>> + tbl = data->ops->get_table(data, 0);
>> if (WARN_ON(!tbl))
>> return -ENXIO;
>>
>> @@ -194,13 +253,16 @@ static long tce_iommu_ioctl(void *iommu_data,
>> }
>> case VFIO_IOMMU_MAP_DMA: {
>> struct vfio_iommu_type1_dma_map param;
>> - struct iommu_table *tbl = container->tbl;
>> + struct iommu_table *tbl;
>> + struct spapr_tce_iommu_group *data;
>> unsigned long tce, i;
>>
>> - if (!tbl)
>> + if (WARN_ON(!container->grp))
>
> If a user can get here by their own mistake, return an error and be
> done, no warning. If a user can get here via a kernel ordering issue,
> it's a problem in the design. Which is it?
I'll remove here and below these WARN's. I just cannot force myself to
start thinking of malicious userspace which can trigger many of those :)
>
>> return -ENXIO;
>>
>> - BUG_ON(!tbl->it_group);
>> + data = iommu_group_get_iommudata(container->grp);
>> + if (WARN_ON(!data || !data->iommu_owner || !data->ops))
>> + return -ENXIO;
>
> Same
>
>>
>> minsz = offsetofend(struct vfio_iommu_type1_dma_map, size);
>>
>> @@ -225,6 +287,11 @@ static long tce_iommu_ioctl(void *iommu_data,
>> if (param.flags & VFIO_DMA_MAP_FLAG_WRITE)
>> tce |= TCE_PCI_WRITE;
>>
>> + tbl = spapr_tce_find_table(container, data, param.iova);
>
> Why aren't we using ops->find_table() here?
I am trying to keep spapr_tce_iommu_ops as simple as possible and now it
only provides a search-by-window-number callback and the helper to search
by IOBA uses it.
--
Alexey
^ permalink raw reply
* [PATCH] powerpc: Print instruction when logging unhandled exceptions
From: Anton Blanchard @ 2014-09-25 5:05 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev
It is often useful to see the instruction that caused an unhandled
exception.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
arch/powerpc/kernel/traps.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 0dc43f9..27e30c8 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -36,10 +36,10 @@
#include <linux/debugfs.h>
#include <linux/ratelimit.h>
#include <linux/context_tracking.h>
+#include <linux/uaccess.h>
#include <asm/emulated_ops.h>
#include <asm/pgtable.h>
-#include <asm/uaccess.h>
#include <asm/io.h>
#include <asm/machdep.h>
#include <asm/rtas.h>
@@ -242,9 +242,9 @@ void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
{
siginfo_t info;
const char fmt32[] = KERN_INFO "%s[%d]: unhandled signal %d " \
- "at %08lx nip %08lx lr %08lx code %x\n";
+ "at %08lx nip %08lx lr %08lx code %x insn %08x\n";
const char fmt64[] = KERN_INFO "%s[%d]: unhandled signal %d " \
- "at %016lx nip %016lx lr %016lx code %x\n";
+ "at %016lx nip %016lx lr %016lx code %x insn %08x\n";
if (!user_mode(regs)) {
die("Exception in kernel mode", regs, signr);
@@ -252,9 +252,18 @@ void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
}
if (show_unhandled_signals && unhandled_signal(current, signr)) {
+ u32 __user *nia = (u32 __user *)regs->nip;
+ u32 insn = 0;
+
+ pagefault_disable();
+ if (!access_ok(VERIFY_READ, nia, sizeof(*nia)) ||
+ __get_user_inatomic(insn, nia))
+ insn = 0xffffffffUL;
+ pagefault_enable();
+
printk_ratelimited(regs->msr & MSR_64BIT ? fmt64 : fmt32,
current->comm, current->pid, signr,
- addr, regs->nip, regs->link, code);
+ addr, regs->nip, regs->link, code, insn);
}
if (arch_irqs_disabled() && !arch_irq_disabled_regs(regs))
--
1.9.1
^ permalink raw reply related
* Re: [1/4] powerpc/powernv: Sync header with firmware
From: Gavin Shan @ 2014-09-25 4:56 UTC (permalink / raw)
To: Michael Ellerman; +Cc: qiudayu, linuxppc-dev, Gavin Shan
In-Reply-To: <20140925042747.8E2AE14016A@ozlabs.org>
On Thu, Sep 25, 2014 at 02:27:47PM +1000, Michael Ellerman wrote:
>On Tue, 2014-26-08 at 07:56:16 UTC, Gavin Shan wrote:
>> From: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>
>> The patch synchronizes firmware header file (opal.h) for PCI error
>> injection.
>>
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index 4593a93..9113653 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -200,6 +201,33 @@ enum OpalPciErrorSeverity {
>> OPAL_EEH_SEV_INF = 5
>> };
>>
>> +enum OpalErrinjctType {
>> + OpalErrinjctTypeIoaBusError = 0,
>> + OpalErrinjctTypeIoaBusError64 = 1,
>> +
>> + /* IoaBusError & IoaBusError64 */
>> + OpalEjtIoaLoadMemAddr = 0,
>> + OpalEjtIoaLoadMemData = 1,
>> + OpalEjtIoaLoadIoAddr = 2,
>> + OpalEjtIoaLoadIoData = 3,
>> + OpalEjtIoaLoadConfigAddr = 4,
>> + OpalEjtIoaLoadConfigData = 5,
>> + OpalEjtIoaStoreMemAddr = 6,
>> + OpalEjtIoaStoreMemData = 7,
>> + OpalEjtIoaStoreIoAddr = 8,
>> + OpalEjtIoaStoreIoData = 9,
>> + OpalEjtIoaStoreConfigAddr = 10,
>> + OpalEjtIoaStoreConfigData = 11,
>> + OpalEjtIoaDmaReadMemAddr = 12,
>> + OpalEjtIoaDmaReadMemData = 13,
>> + OpalEjtIoaDmaReadMemMaster = 14,
>> + OpalEjtIoaDmaReadMemTarget = 15,
>> + OpalEjtIoaDmaWriteMemAddr = 16,
>> + OpalEjtIoaDmaWriteMemData = 17,
>> + OpalEjtIoaDmaWriteMemMaster = 18,
>> + OpalEjtIoaDmaWriteMemTarget = 19,
>> +};
>
>I realise these come from the skiboot source, but they're just too ugly.
>
>Please use kernel style naming, like most of the rest of the file, eg:
>
> OPAL_ERR_INJECT_IOA_BUS_ERR
>
>Also this enum seems to contain two separate types, the first two values are
>the "type", and the rest are "functions".
>
Yes, two separate types: One is major error type, another one is
specific error type in that domain.
>The only usage I see is:
>
> /* Sanity check on error type */
> if (type < OpalErrinjctTypeIoaBusError ||
> type > OpalErrinjctTypeIoaBusError64 ||
> function < OpalEjtIoaLoadMemAddr ||
> function > OpalEjtIoaDmaWriteMemTarget) {
> pr_warn("%s: Invalid error type %d-%d\n",
> __func__, type, function);
> return -ERANGE;
> }
>
>So we could also just do:
>
># define OPAL_ERR_INJECT_TYPE_MIN 0
># define OPAL_ERR_INJECT_TYPE_MAX 1
>
># define OPAL_ERR_INJECT_FUNC_MIN 0
># define OPAL_ERR_INJECT_FUNC_MAX 19
>
> if (type < OPAL_ERR_INJECT_TYPE_MIN ||
> type > OPAL_ERR_INJECT_TYPE_MAX ||
> function < OPAL_ERR_INJECT_FUNC_MIN ||
> function > OPAL_ERR_INJECT_FUNC_MIN)
> {
> pr_warn("%s: Invalid error type %d-%d\n", __func__, type, function);
> return -ERANGE;
> }
>
Ok. I'll take this and put it into next revision.
Thanks,
Gavin
^ permalink raw reply
* Re: [2/5] powerpc/eeh: Add eeh_pe_state sysfs entry
From: Gavin Shan @ 2014-09-25 4:47 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <20140925040958.4D2C414016A@ozlabs.org>
On Thu, Sep 25, 2014 at 02:09:58PM +1000, Michael Ellerman wrote:
>On Sun, 2014-17-08 at 03:02:26 UTC, Gavin Shan wrote:
>> The patch adds sysfs entry "eeh_pe_state". Reading on it returns
>> the PE's state while writing to it clears the frozen state. It's
>> used to check or clear the PE frozen state from userland for
>> debugging purpose.
>>
>> diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
>> index e2595ba..e69bcbb 100644
>> --- a/arch/powerpc/kernel/eeh_sysfs.c
>> +++ b/arch/powerpc/kernel/eeh_sysfs.c
>> @@ -54,6 +54,63 @@ EEH_SHOW_ATTR(eeh_mode, mode, "0x%x");
>> EEH_SHOW_ATTR(eeh_config_addr, config_addr, "0x%x");
>> EEH_SHOW_ATTR(eeh_pe_config_addr, pe_config_addr, "0x%x");
>>
>> +static ssize_t eeh_pe_state_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> + struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
>> + int state;
>> +
>> + if (!edev || !edev->pe)
>> + return 0;
>> +
>> + state = eeh_ops->get_state(edev->pe, NULL);
>> + return sprintf(buf, "PHB#%d-PE#%d: 0x%08x 0x%08x\n",
>> + edev->pe->phb->global_number,
>> + edev->pe->addr, state, edev->pe->state);
>
>Shouldn't this only display the state, ie not the number and addr etc.
>
Yes, I'll remove PHB#%d-PE#%d in next revision. Another sysfs entry
gives the PE number: /sys/bus/pci/devices/xxxx:xx:xx.x/eeh_pe_config_addr
>And why are there two states, state and edev->pe->state ?
>
state is from hardware, edev->pe->state is software maintained state.
>> +static ssize_t eeh_pe_state_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(dev);
>> + struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
>> + int ret;
>> +
>> + if (!edev || !edev->pe)
>> + return 0;
>
>Shouldn't that be an error?
>
>> + /* Nothing to do if it's not frozen */
>> + if (!(edev->pe->state & EEH_PE_ISOLATED))
>> + return 0;
>> +
>> + /* Enable MMIO */
>> + ret = eeh_pci_enable(edev->pe, EEH_OPT_THAW_MMIO);
>> + if (ret) {
>> + pr_warn("%s: Failure %d enabling MMIO for PHB#%d-PE#%d\n",
>> + __func__, ret, edev->pe->phb->global_number,
>> + edev->pe->addr);
>> + return 0;
>
>Error ?
>
>> + }
>> +
>> + /* Enable DMA */
>> + ret = eeh_pci_enable(edev->pe, EEH_OPT_THAW_DMA);
>> + if (ret) {
>> + pr_warn("%s: Failure %d enabling DMA for PHB#%d-PE#%d\n",
>> + __func__, ret, edev->pe->phb->global_number,
>> + edev->pe->addr);
>> + return 0;
>
>Error?
>
Yes, I'll fix all "Error" cases.
>And should it roll back, ie. unthaw MMIO?
>
It's not necessary as it's only for debugging purpose. The main
purpose is to keep dumping the PE hardware/software state when
recovering one specific PE.
Thanks,
Gavin
^ permalink raw reply
* Re: [1/4] powerpc/powernv: Sync header with firmware
From: Michael Ellerman @ 2014-09-25 4:27 UTC (permalink / raw)
To: Gavin Shan, linuxppc-dev; +Cc: qiudayu, Gavin Shan
In-Reply-To: <1409039779-392-2-git-send-email-gwshan@linux.vnet.ibm.com>
On Tue, 2014-26-08 at 07:56:16 UTC, Gavin Shan wrote:
> From: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>
> The patch synchronizes firmware header file (opal.h) for PCI error
> injection.
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 4593a93..9113653 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -200,6 +201,33 @@ enum OpalPciErrorSeverity {
> OPAL_EEH_SEV_INF = 5
> };
>
> +enum OpalErrinjctType {
> + OpalErrinjctTypeIoaBusError = 0,
> + OpalErrinjctTypeIoaBusError64 = 1,
> +
> + /* IoaBusError & IoaBusError64 */
> + OpalEjtIoaLoadMemAddr = 0,
> + OpalEjtIoaLoadMemData = 1,
> + OpalEjtIoaLoadIoAddr = 2,
> + OpalEjtIoaLoadIoData = 3,
> + OpalEjtIoaLoadConfigAddr = 4,
> + OpalEjtIoaLoadConfigData = 5,
> + OpalEjtIoaStoreMemAddr = 6,
> + OpalEjtIoaStoreMemData = 7,
> + OpalEjtIoaStoreIoAddr = 8,
> + OpalEjtIoaStoreIoData = 9,
> + OpalEjtIoaStoreConfigAddr = 10,
> + OpalEjtIoaStoreConfigData = 11,
> + OpalEjtIoaDmaReadMemAddr = 12,
> + OpalEjtIoaDmaReadMemData = 13,
> + OpalEjtIoaDmaReadMemMaster = 14,
> + OpalEjtIoaDmaReadMemTarget = 15,
> + OpalEjtIoaDmaWriteMemAddr = 16,
> + OpalEjtIoaDmaWriteMemData = 17,
> + OpalEjtIoaDmaWriteMemMaster = 18,
> + OpalEjtIoaDmaWriteMemTarget = 19,
> +};
I realise these come from the skiboot source, but they're just too ugly.
Please use kernel style naming, like most of the rest of the file, eg:
OPAL_ERR_INJECT_IOA_BUS_ERR
Also this enum seems to contain two separate types, the first two values are
the "type", and the rest are "functions".
The only usage I see is:
/* Sanity check on error type */
if (type < OpalErrinjctTypeIoaBusError ||
type > OpalErrinjctTypeIoaBusError64 ||
function < OpalEjtIoaLoadMemAddr ||
function > OpalEjtIoaDmaWriteMemTarget) {
pr_warn("%s: Invalid error type %d-%d\n",
__func__, type, function);
return -ERANGE;
}
So we could also just do:
# define OPAL_ERR_INJECT_TYPE_MIN 0
# define OPAL_ERR_INJECT_TYPE_MAX 1
# define OPAL_ERR_INJECT_FUNC_MIN 0
# define OPAL_ERR_INJECT_FUNC_MAX 19
if (type < OPAL_ERR_INJECT_TYPE_MIN ||
type > OPAL_ERR_INJECT_TYPE_MAX ||
function < OPAL_ERR_INJECT_FUNC_MIN ||
function > OPAL_ERR_INJECT_FUNC_MIN)
{
pr_warn("%s: Invalid error type %d-%d\n", __func__, type, function);
return -ERANGE;
}
cheers
^ permalink raw reply
* [RFC PATCH] powerpc/powernv: Fix dma_ops for bypass window
From: Gavin Shan @ 2014-09-25 4:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
When using bypass window on IODA2, the incorrect DMA operations
"dma_iommu_ops" is used by devices. The device driver calls
dma_get_required_mask() to determine using 32-bits or bypass DMA
window. Unfortunately, the returned DMA mask always forces the
driver to use 32-bits DMA window. The problem was reported on
the device as follows:
0004:03:00.0 0107: 1000:0087 (rev 05)
0004:03:00.0 Serial Attached SCSI controller: LSI Logic / Symbios \
Logic SAS2308 PCI-Express Fusion-MPT SAS-2 (rev 05)
The patch fixes above issue by keeping things consistent: when
enabling bypass window, we switch to "dma_direct_ops". Instead,
switch to "pci_dma_ops" when disabling bypass window.
Reported-by: Murali N. Iyer <mniyer@us.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/pci-ioda.c | 76 +++++++++++++++++++------------
1 file changed, 46 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 36b1a7a..60e44d9 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -883,13 +883,29 @@ static int pnv_pci_ioda_dma_set_mask(struct pnv_phb *phb,
set_dma_offset(&pdev->dev, pe->tce_bypass_base);
} else {
dev_info(&pdev->dev, "Using 32-bit DMA via iommu\n");
- set_dma_ops(&pdev->dev, &dma_iommu_ops);
+ set_dma_ops(&pdev->dev, get_pci_dma_ops());
set_iommu_table_base(&pdev->dev, &pe->tce32_table);
}
*pdev->dev.dma_mask = dma_mask;
return 0;
}
+static void pnv_ioda_setup_dev_dma(struct pnv_ioda_pe *pe,
+ struct pci_dev *pdev,
+ bool add_to_iommu_group)
+{
+ if (pe->tce_bypass_enabled) {
+ set_dma_ops(&pdev->dev, &dma_direct_ops);
+ set_dma_offset(&pdev->dev, pe->tce_bypass_base);
+ } else {
+ set_dma_ops(&pdev->dev, get_pci_dma_ops());
+ set_iommu_table_base(&pdev->dev, &pe->tce32_table);
+ }
+
+ if (add_to_iommu_group)
+ iommu_add_device(&pdev->dev);
+}
+
static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
struct pci_bus *bus,
bool add_to_iommu_group)
@@ -897,11 +913,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) {
- if (add_to_iommu_group)
- set_iommu_table_base_and_group(&dev->dev,
- &pe->tce32_table);
- else
- set_iommu_table_base(&dev->dev, &pe->tce32_table);
+ pnv_ioda_setup_dev_dma(pe, dev, add_to_iommu_group);
if (dev->subordinate)
pnv_ioda_setup_bus_dma(pe, dev->subordinate,
@@ -909,6 +921,15 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
}
}
+static void pnv_ioda_setup_pe_dma(struct pnv_ioda_pe *pe,
+ bool add_to_iommu_group)
+{
+ if (pe->pdev)
+ pnv_ioda_setup_dev_dma(pe, pe->pdev, add_to_iommu_group);
+ else
+ pnv_ioda_setup_bus_dma(pe, pe->pbus, add_to_iommu_group);
+}
+
static void pnv_pci_ioda1_tce_invalidate(struct pnv_ioda_pe *pe,
struct iommu_table *tbl,
__be64 *startp, __be64 *endp, bool rm)
@@ -1080,11 +1101,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
iommu_init_table(tbl, phb->hose->node);
iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
- if (pe->pdev)
- set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
- else
- pnv_ioda_setup_bus_dma(pe, pe->pbus, true);
-
+ pnv_ioda_setup_pe_dma(pe, true);
return;
fail:
/* XXX Failure: Try to fallback to 64-bit only ? */
@@ -1101,7 +1118,13 @@ static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
uint16_t window_id = (pe->pe_number << 1 ) + 1;
int64_t rc;
+ /* Check if we really need do something */
+ if (pe->tce_bypass_enabled == enable)
+ return;
+
pe_info(pe, "%sabling 64-bit DMA bypass\n", enable ? "En" : "Dis");
+ pe->tce_bypass_enabled = enable;
+
if (enable) {
phys_addr_t top = memblock_end_of_DRAM();
@@ -1117,22 +1140,15 @@ static void pnv_pci_ioda2_set_bypass(struct iommu_table *tbl, bool enable)
window_id,
pe->tce_bypass_base,
0);
-
- /*
- * EEH needs the mapping between IOMMU table and group
- * of those VFIO/KVM pass-through devices. We can postpone
- * resetting DMA ops until the DMA mask is configured in
- * host side.
- */
- if (pe->pdev)
- set_iommu_table_base(&pe->pdev->dev, tbl);
- else
- pnv_ioda_setup_bus_dma(pe, pe->pbus, false);
}
- if (rc)
+ if (rc != OPAL_SUCCESS) {
pe_err(pe, "OPAL error %lld configuring bypass window\n", rc);
- else
- pe->tce_bypass_enabled = enable;
+ return;
+ }
+
+ /* Update base and operations */
+ pe->tce_bypass_enabled = enable;
+ pnv_ioda_setup_pe_dma(pe, false);
}
static void pnv_pci_ioda2_setup_bypass_pe(struct pnv_phb *phb,
@@ -1213,12 +1229,12 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
iommu_init_table(tbl, phb->hose->node);
iommu_register_group(tbl, phb->hose->global_number, pe->pe_number);
- if (pe->pdev)
- set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
- else
- pnv_ioda_setup_bus_dma(pe, pe->pbus, true);
+ /* If the bypass window fails to be created, we still
+ * can use 32-bits window.
+ */
+ pnv_ioda_setup_pe_dma(pe, true);
- /* Also create a bypass window */
+ /* Create bypass window */
pnv_pci_ioda2_setup_bypass_pe(phb, pe);
return;
fail:
--
1.8.3.2
^ permalink raw reply related
* [PATCH 2/3] powerpc/powernv: Fetch frozen PE on top level
From: Gavin Shan @ 2014-09-25 4:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1411618387-7920-1-git-send-email-gwshan@linux.vnet.ibm.com>
It should have been part of commit 1ad7a72c5 ("powerpc/eeh: Report
frozen parent PE prior to child PE"). There are 2 ways to report
EEH errors: proactively polling triggered by PCI config or IO
accesses, or interrupt driven event. We missed to report and handle
parent frozen PE prior to child frozen PE for the later case on
PowerNV platform.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/eeh-ioda.c | 48 ++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index f248586..57de63c 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -880,14 +880,12 @@ static int ioda_eeh_get_pe(struct pci_controller *hose,
* the master PE because slave PE is invisible
* to EEH core.
*/
- if (phb->get_pe_state) {
- pnv_pe = &phb->ioda.pe_array[pe_no];
- if (pnv_pe->flags & PNV_IODA_PE_SLAVE) {
- pnv_pe = pnv_pe->master;
- WARN_ON(!pnv_pe ||
- !(pnv_pe->flags & PNV_IODA_PE_MASTER));
- pe_no = pnv_pe->pe_number;
- }
+ pnv_pe = &phb->ioda.pe_array[pe_no];
+ if (pnv_pe->flags & PNV_IODA_PE_SLAVE) {
+ pnv_pe = pnv_pe->master;
+ WARN_ON(!pnv_pe ||
+ !(pnv_pe->flags & PNV_IODA_PE_MASTER));
+ pe_no = pnv_pe->pe_number;
}
/* Find the PE according to PE# */
@@ -898,15 +896,37 @@ static int ioda_eeh_get_pe(struct pci_controller *hose,
if (!dev_pe)
return -EEXIST;
- /*
- * At this point, we're sure the compound PE should
- * be put into frozen state.
- */
+ /* Freeze the (compound) PE */
*pe = dev_pe;
- if (phb->freeze_pe &&
- !(dev_pe->state & EEH_PE_ISOLATED))
+ if (!(dev_pe->state & EEH_PE_ISOLATED))
phb->freeze_pe(phb, pe_no);
+ /*
+ * At this point, we're sure the (compound) PE should
+ * have been frozen. However, we still need poke until
+ * hitting the frozen PE on top level.
+ */
+ dev_pe = dev_pe->parent;
+ while (dev_pe && !(dev_pe->type & EEH_PE_PHB)) {
+ int ret;
+ int active_flags = (EEH_STATE_MMIO_ACTIVE |
+ EEH_STATE_DMA_ACTIVE);
+
+ ret = eeh_ops->get_state(dev_pe, NULL);
+ if (ret <= 0 || (ret & active_flags) == active_flags) {
+ dev_pe = dev_pe->parent;
+ continue;
+ }
+
+ /* Frozen parent PE */
+ *pe = dev_pe;
+ if (!(dev_pe->state & EEH_PE_ISOLATED))
+ phb->freeze_pe(phb, dev_pe->addr);
+
+ /* Next one */
+ dev_pe = dev_pe->parent;
+ }
+
return 0;
}
--
1.8.3.2
^ permalink raw reply related
* [PATCH 1/3] powerpc/eeh: Dump PCI config space for all child devices
From: Gavin Shan @ 2014-09-25 4:13 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Gavin Shan
The PEs can be organized as nested. Current implementation doesn't
dump PCI config space for subordinate devices of child PEs. However,
the frozen PE could be caused by those subordinate devices of its
child PEs.
The patch dumps PCI config space for all subordinate devices of the
problematic PE.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
arch/powerpc/kernel/eeh.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 6c88d781..03d75cb 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -117,7 +117,7 @@ static DEFINE_MUTEX(eeh_dev_mutex);
* not dynamically alloced, so that it ends up in RMO where RTAS
* can access it.
*/
-#define EEH_PCI_REGS_LOG_LEN 4096
+#define EEH_PCI_REGS_LOG_LEN 8192
static unsigned char pci_regs_buf[EEH_PCI_REGS_LOG_LEN];
/*
@@ -148,16 +148,12 @@ static int __init eeh_setup(char *str)
}
__setup("eeh=", eeh_setup);
-/**
- * eeh_gather_pci_data - Copy assorted PCI config space registers to buff
- * @edev: device to report data for
- * @buf: point to buffer in which to log
- * @len: amount of room in buffer
- *
- * This routine captures assorted PCI configuration space data,
- * and puts them into a buffer for RTAS error logging.
+/*
+ * This routine captures assorted PCI configuration space data
+ * for the indicated PCI device, and puts them into a buffer
+ * for RTAS error logging.
*/
-static size_t eeh_gather_pci_data(struct eeh_dev *edev, char *buf, size_t len)
+static size_t eeh_dump_dev_log(struct eeh_dev *edev, char *buf, size_t len)
{
struct device_node *dn = eeh_dev_to_of_node(edev);
u32 cfg;
@@ -255,6 +251,19 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char *buf, size_t len)
return n;
}
+static void *eeh_dump_pe_log(void *data, void *flag)
+{
+ struct eeh_pe *pe = data;
+ struct eeh_dev *edev, *tmp;
+ size_t *plen = flag;
+
+ eeh_pe_for_each_dev(pe, edev, tmp)
+ *plen += eeh_dump_dev_log(edev, pci_regs_buf + *plen,
+ EEH_PCI_REGS_LOG_LEN - *plen);
+
+ return NULL;
+}
+
/**
* eeh_slot_error_detail - Generate combined log including driver log and error log
* @pe: EEH PE
@@ -268,7 +277,6 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char *buf, size_t len)
void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
{
size_t loglen = 0;
- struct eeh_dev *edev, *tmp;
/*
* When the PHB is fenced or dead, it's pointless to collect
@@ -286,10 +294,7 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
eeh_pe_restore_bars(pe);
pci_regs_buf[0] = 0;
- eeh_pe_for_each_dev(pe, edev, tmp) {
- loglen += eeh_gather_pci_data(edev, pci_regs_buf + loglen,
- EEH_PCI_REGS_LOG_LEN - loglen);
- }
+ eeh_pe_traverse(pe, eeh_dump_pe_log, &loglen);
}
eeh_ops->get_log(pe, severity, pci_regs_buf, loglen);
--
1.8.3.2
^ permalink raw reply related
* Re: [2/5] powerpc/eeh: Add eeh_pe_state sysfs entry
From: Michael Ellerman @ 2014-09-25 4:09 UTC (permalink / raw)
To: Gavin Shan, linuxppc-dev; +Cc: Gavin Shan
In-Reply-To: <1408244549-10221-3-git-send-email-gwshan@linux.vnet.ibm.com>
On Sun, 2014-17-08 at 03:02:26 UTC, Gavin Shan wrote:
> The patch adds sysfs entry "eeh_pe_state". Reading on it returns
> the PE's state while writing to it clears the frozen state. It's
> used to check or clear the PE frozen state from userland for
> debugging purpose.
>
> diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
> index e2595ba..e69bcbb 100644
> --- a/arch/powerpc/kernel/eeh_sysfs.c
> +++ b/arch/powerpc/kernel/eeh_sysfs.c
> @@ -54,6 +54,63 @@ EEH_SHOW_ATTR(eeh_mode, mode, "0x%x");
> EEH_SHOW_ATTR(eeh_config_addr, config_addr, "0x%x");
> EEH_SHOW_ATTR(eeh_pe_config_addr, pe_config_addr, "0x%x");
>
> +static ssize_t eeh_pe_state_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
> + int state;
> +
> + if (!edev || !edev->pe)
> + return 0;
> +
> + state = eeh_ops->get_state(edev->pe, NULL);
> + return sprintf(buf, "PHB#%d-PE#%d: 0x%08x 0x%08x\n",
> + edev->pe->phb->global_number,
> + edev->pe->addr, state, edev->pe->state);
Shouldn't this only display the state, ie not the number and addr etc.
And why are there two states, state and edev->pe->state ?
> +static ssize_t eeh_pe_state_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
> + int ret;
> +
> + if (!edev || !edev->pe)
> + return 0;
Shouldn't that be an error?
> + /* Nothing to do if it's not frozen */
> + if (!(edev->pe->state & EEH_PE_ISOLATED))
> + return 0;
> +
> + /* Enable MMIO */
> + ret = eeh_pci_enable(edev->pe, EEH_OPT_THAW_MMIO);
> + if (ret) {
> + pr_warn("%s: Failure %d enabling MMIO for PHB#%d-PE#%d\n",
> + __func__, ret, edev->pe->phb->global_number,
> + edev->pe->addr);
> + return 0;
Error ?
> + }
> +
> + /* Enable DMA */
> + ret = eeh_pci_enable(edev->pe, EEH_OPT_THAW_DMA);
> + if (ret) {
> + pr_warn("%s: Failure %d enabling DMA for PHB#%d-PE#%d\n",
> + __func__, ret, edev->pe->phb->global_number,
> + edev->pe->addr);
> + return 0;
Error?
And should it roll back, ie. unthaw MMIO?
cheers
^ permalink raw reply
* [PATCH v2 08/22] x86/MSI: Use MSI chip framework to configure MSI/MSI-X irq
From: Yijing Wang @ 2014-09-25 3:14 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-mips, linux-ia64, linux-pci, Bharat.Bhushan, Yijing Wang,
Thierry Reding, sparclinux, linux-arch, linux-s390, Russell King,
Joerg Roedel, x86, Sebastian Ott, xen-devel, arnab.basu,
Arnd Bergmann, Konrad Rzeszutek Wilk, Chris Metcalf,
Thomas Gleixner, linux-arm-kernel, Thomas Petazzoni, Xinwei Hu,
Tony Luck, Sergei Shtylyov, linux-kernel, Ralf Baechle, iommu,
David Vrabel, Wuyun, linuxppc-dev, David S. Miller, Lucas Stach
In-Reply-To: <1411614872-4009-1-git-send-email-wangyijing@huawei.com>
Use MSI chip framework instead of arch MSI functions to configure
MSI/MSI-X irq. So we can manage MSI/MSI-X irq in a unified framework.
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
arch/x86/include/asm/pci.h | 1 +
arch/x86/kernel/apic/io_apic.c | 12 ++++++++++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 0892ea0..878a06d 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -101,6 +101,7 @@ void native_teardown_msi_irq(unsigned int irq);
void native_restore_msi_irqs(struct pci_dev *dev);
int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
unsigned int irq_base, unsigned int irq_offset);
+extern struct msi_chip *x86_msi_chip;
#else
#define native_setup_msi_irqs NULL
#define native_teardown_msi_irq NULL
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 2a2ec28..882b95e 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3337,6 +3337,18 @@ int default_setup_hpet_msi(unsigned int irq, unsigned int id)
}
#endif
+struct msi_chip apic_msi_chip = {
+ .setup_irqs = native_setup_msi_irqs,
+ .teardown_irq = native_teardown_msi_irq,
+};
+
+struct msi_chip *arch_find_msi_chip(struct pci_dev *dev)
+{
+ return x86_msi_chip;
+}
+
+struct msi_chip *x86_msi_chip = &apic_msi_chip;
+
#endif /* CONFIG_PCI_MSI */
/*
* Hypertransport interrupt support
--
1.7.1
^ 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