LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v9] PPC: POWERNV: move iommu_add_device earlier
From: Jeremy Kerr @ 2013-11-21  5:41 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev
  Cc: Bharat Bhushan, Alex Graf, linux-kernel
In-Reply-To: <1384324220-30109-1-git-send-email-aik@ozlabs.ru>

Hi Alexey,

> This patch does 2 things:
> 1. removes the loop in which PCI devices were added to groups and
> adds explicit iommu_add_device() calls to add devices as soon as they get
> the iommu_table pointer assigned to them.
> 2. moves a bus notifier to powernv code in order to avoid conflict with
> the notifier from Freescale driver.

This breaks when building with !IOMMU_API for me, as the
iommu_add_device function is declared but not defined. We'd need
something like the following (on top of your change) to work:

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 426d0ec0..04d2abbe 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -102,10 +102,27 @@ extern void iommu_free_table(struct iommu_table *tbl, cons
  */
 extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
                                            int nid);
+
+#ifdef CONFIG_IOMMU_API
 extern void iommu_register_group(struct iommu_table *tbl,
                                 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,
+                                int pci_domain_number, unsigned long pe_num)
+{
+}
+
+static inline int iommu_add_device(struct device *dev)
+{
+       return 0;
+}
+
+static inline void iommu_del_device(struct device *dev)
+{
+}
+#endif
 
 static inline void set_iommu_table_base_and_group(struct device *dev,
                                                  void *base)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 170b2182..5a02a50f 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1212,11 +1212,4 @@ void iommu_del_device(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_del_device);
 
-#else
-
-void iommu_register_group(struct iommu_table *tbl,
-               int pci_domain_number, unsigned long pe_num)
-{
-}
-
 #endif /* CONFIG_IOMMU_API */


Cheers,


Jeremy

^ permalink raw reply related

* [PATCH] powerpc: allyesconfig should not select CONFIG_CPU_LITTLE_ENDIAN
From: Anton Blanchard @ 2013-11-21  5:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, paulus, fengguang.wu, Stephen Rothwell,
	imunsie
  Cc: linux-next, linuxppc-dev, linux-kernel


Stephen reported a failure in an allyesconfig build.  
CONFIG_CPU_LITTLE_ENDIAN=y gets set but his toolchain is not
new enough to support little endian. We really want to
default to a big endian build; Ben suggested using a choice
which defaults to CPU_BIG_ENDIAN.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/platforms/Kconfig.cputype
===================================================================
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -404,13 +404,27 @@ config PPC_DOORBELL
 
 endmenu
 
-config CPU_LITTLE_ENDIAN
-	bool "Build little endian kernel"
-	default n
+choice
+	prompt "Endianness selection"
+	default CPU_BIG_ENDIAN
 	help
 	  This option selects whether a big endian or little endian kernel will
 	  be built.
 
+config CPU_BIG_ENDIAN
+	bool "Build big endian kernel"
+	help
+	  Build a big endian kernel.
+
+	  If unsure, select this option.
+
+config CPU_LITTLE_ENDIAN
+	bool "Build little endian kernel"
+	help
+	  Build a little endian kernel.
+
 	  Note that if cross compiling a little endian kernel,
 	  CROSS_COMPILE must point to a toolchain capable of targeting
 	  little endian powerpc.
+
+endchoice

^ permalink raw reply

* powerpc: Fix missing includes needed for chroma
From: Michael Neuling @ 2013-11-21  4:40 UTC (permalink / raw)
  To: benh; +Cc: Linux PPC dev, Jimi Xenidis

chroma_defconfig is horribly broken currently, so add a bunch of
#includes to fix it.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
So when are we dropping arch/powerpc/platforms/wsp?

diff --git a/arch/powerpc/platforms/wsp/chroma.c b/arch/powerpc/platforms/wsp/chroma.c
index 8ef53bc..aaa46b3 100644
--- a/arch/powerpc/platforms/wsp/chroma.c
+++ b/arch/powerpc/platforms/wsp/chroma.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/smp.h>
 #include <linux/time.h>
+#include <linux/of_fdt.h>
 
 #include <asm/machdep.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/wsp/h8.c b/arch/powerpc/platforms/wsp/h8.c
index d18e6cc..a3c87f3 100644
--- a/arch/powerpc/platforms/wsp/h8.c
+++ b/arch/powerpc/platforms/wsp/h8.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/io.h>
+#include <linux/of_address.h>
 
 #include "wsp.h"
 
diff --git a/arch/powerpc/platforms/wsp/ics.c b/arch/powerpc/platforms/wsp/ics.c
index 2d3b1dd..3b782ce 100644
--- a/arch/powerpc/platforms/wsp/ics.c
+++ b/arch/powerpc/platforms/wsp/ics.c
@@ -18,6 +18,8 @@
 #include <linux/smp.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>
diff --git a/arch/powerpc/platforms/wsp/opb_pic.c b/arch/powerpc/platforms/wsp/opb_pic.c
index cb565bf..3f67298 100644
--- a/arch/powerpc/platforms/wsp/opb_pic.c
+++ b/arch/powerpc/platforms/wsp/opb_pic.c
@@ -15,6 +15,8 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/time.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 
 #include <asm/reg_a2.h>
 #include <asm/irq.h>
diff --git a/arch/powerpc/platforms/wsp/psr2.c b/arch/powerpc/platforms/wsp/psr2.c
index 508ec82..a87b414 100644
--- a/arch/powerpc/platforms/wsp/psr2.c
+++ b/arch/powerpc/platforms/wsp/psr2.c
@@ -15,6 +15,7 @@
 #include <linux/of.h>
 #include <linux/smp.h>
 #include <linux/time.h>
+#include <linux/of_fdt.h>
 
 #include <asm/machdep.h>
 #include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/wsp/scom_wsp.c b/arch/powerpc/platforms/wsp/scom_wsp.c
index 8928507..6538b4d 100644
--- a/arch/powerpc/platforms/wsp/scom_wsp.c
+++ b/arch/powerpc/platforms/wsp/scom_wsp.c
@@ -14,6 +14,7 @@
 #include <linux/of.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <linux/of_address.h>
 
 #include <asm/cputhreads.h>
 #include <asm/reg_a2.h>
diff --git a/arch/powerpc/platforms/wsp/wsp.c b/arch/powerpc/platforms/wsp/wsp.c
index ddb6efe..58cd1f0 100644
--- a/arch/powerpc/platforms/wsp/wsp.c
+++ b/arch/powerpc/platforms/wsp/wsp.c
@@ -13,6 +13,7 @@
 #include <linux/smp.h>
 #include <linux/delay.h>
 #include <linux/time.h>
+#include <linux/of_address.h>
 
 #include <asm/scom.h>
 

^ permalink raw reply related

* [PATCH] powerpc: Fix error when cross building TAGS & cscope
From: Michael Neuling @ 2013-11-21  3:59 UTC (permalink / raw)
  To: benh; +Cc: Linux PPC dev, Ian Munsie, anton

Currently if I cross build TAGS or cscope from x86 I get this:
  % make ARCH=3Dpowerpc TAGS
  gcc-4.8.real: error: unrecognized command line option =E2=80=98-mbig-endi=
an=E2=80=99
  GEN     TAGS
  %=20

I'm not setting CROSS_COMPILE=3D as logically I shouldn't need to and I
haven't needed to in the past when building TAGS or cscope.  Also, the
above completess correct as the error is not fatal to the build.

This was caused by:
    commit d72b08017161ab385d4ae080ea415c9eb7ceef83
    Author: Ian Munsie <imunsie@au1.ibm.com>
    powerpc: Add ability to build little endian kernels

The below fixes this by testing for the -mbig-endian option before
adding it.

I've not done the same thing in the little endian case as if
-mlittle-endian doesn't exist, we probably want to fail quickly as you
probably have an old big endian compiler.

Signed-off-by: Michael Neuling <mikey@neuling.org>

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 8a24636..273ace7 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -75,8 +75,10 @@ LDEMULATION	:=3D lppc
 GNUTARGET	:=3D powerpcle
 MULTIPLEWORD	:=3D -mno-multiple
 else
+ifeq ($(call cc-option-yn,-mbig-endian),y)
 override CC	+=3D -mbig-endian
 override AS	+=3D -mbig-endian
+endif
 override LD	+=3D -EB
 LDEMULATION	:=3D ppc
 GNUTARGET	:=3D powerpc

^ permalink raw reply related

* Re: linux-next: build failure after merge of the final tree (Linus' tree related - powerpc)
From: Anton Blanchard @ 2013-11-21  3:33 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linux-next, linuxppc-dev, linux-kernel
In-Reply-To: <20131121134622.a853064a1b5a8355e5077737@canb.auug.org.au>


Hi,

> After merging the final tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
> 
> <stdin>:1:0: error: -mcall-aixdesc must be big endian

Urgh, allyesconfig is building an LE kernel. Ian: do we need to reverse
the polarity of the option, and call it CONFIG_CPU_BIG_ENDIAN?

Anton

^ permalink raw reply

* linux-next: build failure after merge of the final tree (Linus' tree related - powerpc)
From: Stephen Rothwell @ 2013-11-21  2:46 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev
  Cc: linux-next, linux-kernel, Anton Blanchard

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

Hi all,

After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:

<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
scripts/gcc-version.sh: line 31: printf: #: invalid number
scripts/gcc-version.sh: line 31: printf: #: invalid number
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
scripts/gcc-version.sh: line 31: printf: #: invalid number
scripts/gcc-version.sh: line 31: printf: #: invalid number
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
scripts/gcc-version.sh: line 29: printf: #: invalid number
scripts/gcc-version.sh: line 29: printf: #: invalid number
scripts/gcc-version.sh: line 29: printf: #: invalid number
/bin/sh: line 0: test: 0001 6000 0160: integer expression expected
scripts/mod/empty.c:1:0: error: -mcall-aixdesc must be big endian
 /* empty file to figure out endianness / word size */
 ^
scripts/mod/devicetable-offsets.c:1:0: error: -mcall-aixdesc must be big endian
 #include <linux/kbuild.h>
 ^
kernel/bounds.c:1:0: error: -mcall-aixdesc must be big endian
 /*
 ^

Caused by commit 7c105b63bd98 ("powerpc: Add CONFIG_CPU_LITTLE_ENDIAN
kernel config option").

I have reverted that commit for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH 2/2] powerpc: mm: change pgtable index size for 64K page
From: Liu Ping Fan @ 2013-11-21  2:17 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1385000275-5988-1-git-send-email-pingfank@linux.vnet.ibm.com>

For 64K page, we waste half of the pte_t page. With this patch, after
changing PGD_INDEX_SIZE from 12 to 11, PTE_INDEX_SIZE from 8 to 9,
we can improve the usage of pte_t page and shrink the continuous phys
size for pgd_t.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pgtable-ppc64-64k.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable-ppc64-64k.h b/arch/powerpc/include/asm/pgtable-ppc64-64k.h
index a56b82f..f6955ff 100644
--- a/arch/powerpc/include/asm/pgtable-ppc64-64k.h
+++ b/arch/powerpc/include/asm/pgtable-ppc64-64k.h
@@ -4,10 +4,10 @@
 #include <asm-generic/pgtable-nopud.h>
 
 
-#define PTE_INDEX_SIZE  8
+#define PTE_INDEX_SIZE  9
 #define PMD_INDEX_SIZE  10
 #define PUD_INDEX_SIZE	0
-#define PGD_INDEX_SIZE  12
+#define PGD_INDEX_SIZE  11
 
 #ifndef __ASSEMBLY__
 #define PTE_TABLE_SIZE	(sizeof(real_pte_t) << PTE_INDEX_SIZE)
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH 1/2] powerpc: mm: use macro PGTABLE_EADDR_SIZE instead of digital
From: Liu Ping Fan @ 2013-11-21  2:17 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras

In case of extending the eaddr in future, use this macro
PGTABLE_EADDR_SIZE to ease the maintenance of the code.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 arch/powerpc/mm/slb_low.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
index 17aa6df..e0b3cf4 100644
--- a/arch/powerpc/mm/slb_low.S
+++ b/arch/powerpc/mm/slb_low.S
@@ -35,7 +35,7 @@ _GLOBAL(slb_allocate_realmode)
 	 * check for bad kernel/user address
 	 * (ea & ~REGION_MASK) >= PGTABLE_RANGE
 	 */
-	rldicr. r9,r3,4,(63 - 46 - 4)
+	rldicr. r9,r3,4,(63 - PGTABLE_EADDR_SIZE - 4)
 	bne-	8f
 
 	srdi	r9,r3,60		/* get region */
-- 
1.8.1.4

^ permalink raw reply related

* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Li Zhong @ 2013-11-21  1:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: PowerPC email list, Russell King - ARM Linux, Paul Mackerras
In-Reply-To: <1384993347.26969.124.camel@pasglop>

On Thu, 2013-11-21 at 11:22 +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2013-11-21 at 00:08 +0000, Russell King - ARM Linux wrote:
> > On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:
> > > On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> > > > Li Zong's patch works around the issue of a failing dma_set_mask(),
> > > > but as I've already said elsewhere, the real fix is to get whatever
> > > > created the struct device to initialise the dev->dma_mask with a
> > > > bus default.
> > > > 
> > > > Using dma_coerce_xxx() merely makes the problem "go away" papering
> > > > over the issue - it's fine to do it this way, but someone should still
> > > > fix the broken code creating these devices...
> > > 
> > > Ok, they are created by the vio bus core, so it should be doing the
> > > job here of setting the dma_mask pointer to a proper value.
> > > 
> > > Li, can you take care of that ? Look at other bus types we have in
> > > there such as the macio bus etc...
> > 
> > Oh, hang on a moment, this is the "bus" code.
> > 
> > In which case, the question becomes: do vio devices ever need to have
> > a separate streaming DMA mask from a coherent DMA mask?  If not, then
> > something like the following is what's needed here, and I should've
> > never have used dma_set_mask_and_coherent().
> 
> No, a single mask.
> 
> > dma_set_mask_and_coherent() (and the other dma_set_mask() functions)
> > are really supposed to be used by drivers only.
> > 
> >  arch/powerpc/kernel/vio.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> > index e7d0c88f621a..d771778f398e 100644
> > --- a/arch/powerpc/kernel/vio.c
> > +++ b/arch/powerpc/kernel/vio.c
> > @@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
> >  
> >  		/* needed to ensure proper operation of coherent allocations
> >  		 * later, in case driver doesn't set it explicitly */
> > -		dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
> > +		viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
> > +		viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask;
> >  	}
> >  
> >  	/* register with generic device framework */
> 
> Right that's exactly what I had in mind. Li, can you test this please ?

Sure, and it works.

Tested-by: Li Zhong <zhong@linux.vnet.ibm.com>

> 
> The previous "fix" using dma_coerce_mask_and_coherent() is already on
> its way to Linus, so we'll rework the above patch to undo it but for
> now please test.
> 
> Cheers,
> Ben.
> 
> 

^ permalink raw reply

* Re: [PATCH v3] KVM: PPC: vfio kvm device: support spapr tce
From: Alexey Kardashevskiy @ 2013-11-21  1:22 UTC (permalink / raw)
  To: Alex Williamson; +Cc: kvm-ppc, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <1384981031.2879.389.camel@ul30vt.home>

On 11/21/2013 07:57 AM, Alex Williamson wrote:
> On Wed, 2013-11-20 at 16:18 +1100, Alexey Kardashevskiy wrote:
>> In addition to the external VFIO user API, a VFIO KVM device
>> has been introduced recently.
>>
>> sPAPR TCE IOMMU is para-virtualized and the guest does map/unmap
>> via hypercalls which take a logical bus id (LIOBN) as a target IOMMU
>> identifier. LIOBNs are made up and linked to IOMMU groups by the user
>> space. In order to accelerate IOMMU operations in the KVM, we need
>> to tell KVM the information about LIOBN-to-group mapping.
>>
>> For that, a new KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN parameter
>> is added. It accepts a pair of a VFIO group fd and LIOBN.
>>
>> This also adds a new kvm_vfio_find_group_by_liobn() function which
>> receives kvm struct, LIOBN and a callback. As it increases the IOMMU
>> group use counter, the KVMr is required to pass a callback which
>> called when the VFIO group is about to be removed VFIO-KVM tracking so
>> the KVM is able to call iommu_group_put() to release the IOMMU group.
>>
>> The KVM uses kvm_vfio_find_group_by_liobn() once per KVM run and caches
>> the result in kvm_arch. iommu_group_put() for all groups will be called
>> when KVM finishes (in the SPAPR TCE in KVM enablement patch).
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v3:
>> * total rework
>> * added a release callback into kvm_vfio_find_group_by_liobn so now
>> the user of the API can get a notification if the group is about to
>> disappear
>> ---
>>  Documentation/virtual/kvm/devices/vfio.txt |  19 ++++-
>>  arch/powerpc/kvm/Kconfig                   |   1 +
>>  arch/powerpc/kvm/Makefile                  |   3 +
>>  include/linux/kvm_host.h                   |  18 +++++
>>  include/uapi/linux/kvm.h                   |   7 ++
>>  virt/kvm/vfio.c                            | 116 ++++++++++++++++++++++++++++-
>>  6 files changed, 161 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
>> index ef51740..7ecb3b2 100644
>> --- a/Documentation/virtual/kvm/devices/vfio.txt
>> +++ b/Documentation/virtual/kvm/devices/vfio.txt
>> @@ -16,7 +16,22 @@ Groups:
>>  
>>  KVM_DEV_VFIO_GROUP attributes:
>>    KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
>> +	kvm_device_attr.addr points to an int32_t file descriptor
>> +	for the VFIO group.
>> +
>>    KVM_DEV_VFIO_GROUP_DEL: Remove a VFIO group from VFIO-KVM device tracking
>> +	kvm_device_attr.addr points to an int32_t file descriptor
>> +	for the VFIO group.
>> +
>> +  KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN: sets a liobn for a VFIO group
>> +	kvm_device_attr.addr points to a struct:
>> +		struct kvm_vfio_spapr_tce_liobn {
>> +			__u32	argsz;
>> +			__u32	fd;
> 
> fds are signed, __s32
> 
>> +			__u32	liobn;
>> +		};
>> +		where
>> +		@argsz is a struct size;
>> +		@fd is a file descriptor for a VFIO group;
>> +		@liobn is a logical bus id to be associated with the group.
>>  
>> -For each, kvm_device_attr.addr points to an int32_t file descriptor
>> -for the VFIO group.
>> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
>> index 61b3535..d1b7f64 100644
>> --- a/arch/powerpc/kvm/Kconfig
>> +++ b/arch/powerpc/kvm/Kconfig
>> @@ -60,6 +60,7 @@ config KVM_BOOK3S_64
>>  	select KVM_BOOK3S_64_HANDLER
>>  	select KVM
>>  	select SPAPR_TCE_IOMMU
>> +	select KVM_VFIO
>>  	---help---
>>  	  Support running unmodified book3s_64 and book3s_32 guest kernels
>>  	  in virtual machines on book3s_64 host processors.
>> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
>> index 6646c95..2438d2e 100644
>> --- a/arch/powerpc/kvm/Makefile
>> +++ b/arch/powerpc/kvm/Makefile
>> @@ -87,6 +87,9 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>>  kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>>  	book3s_xics.o
>>  
>> +kvm-book3s_64-objs-$(CONFIG_KVM_VFIO) += \
>> +	$(KVM)/vfio.o \
>> +
>>  kvm-book3s_64-module-objs := \
>>  	$(KVM)/kvm_main.o \
>>  	$(KVM)/eventfd.o \
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 88ff96a..1d2ad5e 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -1112,5 +1112,23 @@ static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
>>  }
>>  
>>  #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
>> +
>> +typedef void (*kvm_vfio_release_group_callback)(struct kvm *kvm,
>> +		unsigned long liobn);
> 
> liobn was said to be __u32 in kvm_vfio_spapr_tce_liobn above, here it's
> unsigned long?


PAPR spec says it is 32 bit and kvm_vfio_spapr_tce_liobn is a binary
interface (ABI?) so I want it to be precise.

However kvmppc_get_gpr() (used to parse hypercall parameters such as liobn)
return unsigned long. So inside the kernel I use unsigned long.

So what does make sense to change here?


> 
>> +
>> +#if defined(CONFIG_KVM_VFIO) && defined(CONFIG_SPAPR_TCE_IOMMU)
>> +
>> +extern struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
>> +		unsigned long liobn, kvm_vfio_release_group_callback cb);
>> +
>> +#else
>> +
>> +static inline struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
>> +		unsigned long liobn, ikvm_vfio_release_group_callback cb)
>> +{
>> +	return NULL;
>> +}
>> +#endif /* CONFIG_KVM_VFIO && CONFIG_SPAPR_TCE_IOMMU */
>> +
>>  #endif
>>  
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 7c1a349..3d77dde 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -847,6 +847,13 @@ struct kvm_device_attr {
>>  #define  KVM_DEV_VFIO_GROUP			1
>>  #define   KVM_DEV_VFIO_GROUP_ADD			1
>>  #define   KVM_DEV_VFIO_GROUP_DEL			2
>> +#define   KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN	3
>> +
>> +struct kvm_vfio_spapr_tce_liobn {
>> +	__u32	argsz;
>> +	__u32	fd;
>> +	__u32	liobn;
>> +};
>>  
>>  /*
>>   * ioctls for VM fds
>> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
>> index ca4260e..448910d 100644
>> --- a/virt/kvm/vfio.c
>> +++ b/virt/kvm/vfio.c
>> @@ -22,6 +22,12 @@
>>  struct kvm_vfio_group {
>>  	struct list_head node;
>>  	struct vfio_group *vfio_group;
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> +	struct {
>> +		unsigned long liobn;
>> +		kvm_vfio_release_group_callback cb;
>> +	} spapr_tce;
>> +#endif
>>  };
>>  
>>  struct kvm_vfio {
>> @@ -59,6 +65,51 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
>>  	symbol_put(vfio_group_put_external_user);
>>  }
>>  
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> +struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
>> +		unsigned long liobn, kvm_vfio_release_group_callback cb)
>> +{
>> +	struct kvm_vfio_group *kvg;
>> +	int group_id;
>> +	struct iommu_group *grp;
>> +	struct kvm_vfio *kv = NULL;
>> +	struct kvm_device *tmp;
>> +
>> +	if (!cb)
>> +		return NULL;
> 
> Is it worthwhile to use ERR_PTR(-EINVAL) here and the caller can use
> IS_ERR()?
> 
>> +
>> +	/* Find a VFIO KVM device */
>> +	list_for_each_entry(tmp, &kvm->devices, vm_node) {
>> +		if (tmp->ops != &kvm_vfio_ops)
>> +			continue;
>> +
>> +		kv = tmp->private;
>> +		break;
>> +	}
>> +
>> +	if (!kv)
>> +		return NULL;
> 
> ERR_PTR(-EFAULT)?  EIO?
> 
>> +
>> +	/* Find a group */
> 
> Still ignoring kv->lock
> 
>> +	list_for_each_entry(kvg, &kv->group_list, node) {
>> +		if (kvg->spapr_tce.liobn != liobn)
>> +			continue;
>> +
>> +		if (kvg->spapr_tce.cb)
>> +			return NULL;
> 
> ERR_PTR(-EBUSY)?
> 
>> +
>> +		kvg->spapr_tce.cb = cb;
>> +		group_id = vfio_external_user_iommu_id(kvg->vfio_group);
>> +		grp = iommu_group_get_by_id(group_id);
>> +
>> +		return grp;
>> +	}
>> +
>> +	return NULL;
> 
> ERR_PTR(-ENODEV)?
> 
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_vfio_find_group_by_liobn);
>> +#endif
>> +
>>  /*
>>   * Groups can use the same or different IOMMU domains.  If the same then
>>   * adding a new group may change the coherency of groups we've previously
>> @@ -170,6 +221,11 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>>  				continue;
>>  
>>  			list_del(&kvg->node);
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> +			if (kvg->spapr_tce.cb)
>> +				kvg->spapr_tce.cb(dev->kvm,
>> +						kvg->spapr_tce.liobn);
>> +#endif
>>  			kvm_vfio_group_put_external_user(kvg->vfio_group);
>>  			kfree(kvg);
>>  			ret = 0;
>> @@ -183,6 +239,62 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>>  		kvm_vfio_update_coherency(dev);
>>  
>>  		return ret;
>> +
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> +	case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN: {
>> +		struct kvm_vfio_spapr_tce_liobn param;
>> +		unsigned long minsz;
>> +		struct kvm_vfio *kv = dev->private;
>> +		struct vfio_group *vfio_group;
>> +		struct kvm_vfio_group *kvg;
>> +		struct fd f;
>> +
>> +		minsz = offsetofend(struct kvm_vfio_spapr_tce_liobn, liobn);
>> +
>> +		if (copy_from_user(&param, (void __user *)arg, minsz))
>> +			return -EFAULT;
>> +
>> +		if (param.argsz < minsz)
>> +			return -EINVAL;
>> +
>> +		if (copy_from_user(&param, (void __user *)arg, minsz))
>> +			return -EFAULT;
> 
> copy_from_user twice?  Extra copy here?


Oh. All I wanted was to read minsz first but I cut-n-pasted too blindely :)

> 
>> +
>> +		f = fdget(param.fd);
>> +		if (!f.file)
>> +			return -EBADF;
>> +
>> +		vfio_group = kvm_vfio_group_get_external_user(f.file);
>> +		fdput(f);
>> +
>> +		if (IS_ERR(vfio_group))
>> +			return PTR_ERR(vfio_group);
>> +
>> +		ret = -ENOENT;
>> +
>> +		mutex_lock(&kv->lock);
>> +
>> +		list_for_each_entry(kvg, &kv->group_list, node) {
>> +			if (kvg->vfio_group != vfio_group)
>> +				continue;
>> +
>> +			if (kvg->spapr_tce.liobn) {
>> +				ret = -EBUSY;
>> +				break;
>> +			}
> 
> Is zero not an liobn that can be used by userspace?

Good point, thanks. I thought zero cannot be used but by spec -1 is reserved


>  Is it intentional
> that there's no way to unset or change the group/liobn mapping?  Thanks,


I do not see why we would want to disable once enabled acceleration. Group
removal should clear it though so we are good.

Thanks for the review and patience :)


> 
> Alex
> 
>> +
>> +			kvg->spapr_tce.liobn = param.liobn;
>> +			ret = 0;
>> +			break;
>> +		}
>> +
>> +		mutex_unlock(&kv->lock);
>> +
>> +		kvm_vfio_group_put_external_user(vfio_group);
>> +
>> +		return ret;
>> +	}
>> +#endif /* CONFIG_SPAPR_TCE_IOMMU */
>>  	}
>>  
>>  	return -ENXIO;
>> @@ -207,9 +319,11 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
>>  		switch (attr->attr) {
>>  		case KVM_DEV_VFIO_GROUP_ADD:
>>  		case KVM_DEV_VFIO_GROUP_DEL:
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> +		case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN:
>> +#endif
>>  			return 0;
>>  		}
>> -
>>  		break;
>>  	}
>>  
> 
> 
> 


-- 
Alexey

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-11-21  0:34 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list

Hi Linus !

Since you pulled my previous one in less than 20mn (the LE stuff),
there's no point waiting for tomorrow for these fixes I mentioned
earlier so here they are.

This is a small collection of random bug fixes and a few improvements
of Oops output which I deemed valuable enough to include as well.

The fixes are essentially recent build breakage and regressions,
and a couple of older bugs such as the DTL log duplication, the
EEH issue with PCI_COMMAND_MASTER and the problem with small
contexts passed to get/set_context with VSX enabled.

Cheers,
Ben.
The following changes since commit b4789b8e6be3151a955ade74872822f30e8cd914:

  aacraid: prevent invalid pointer dereference (2013-11-19 16:27:39 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

for you to fetch changes up to c13f20ac48328b05cd3b8c19e31ed6c132b44b42:

  powerpc/signals: Mark VSX not saved with small contexts (2013-11-21 10:33:45 +1100)

----------------------------------------------------------------
Aneesh Kumar K.V (1):
      powerpc: booke: Fix build failures

Anton Blanchard (5):
      powerpc: Print DAR and DSISR on machine check oopses
      powerpc: Remove a few lines of oops output
      powerpc/pseries: Duplicate dtl entries sometimes sent to userspace
      powerpc: Only print PACATMSCRATCH in oops when TM is active
      powerpc: ppc64 address space capped at 32TB, mmap randomisation disabled

Gavin Shan (2):
      powerpc/eeh: Enable PCI_COMMAND_MASTER for PCI bridges
      powerpc/eeh: More accurate log

Heiko Carstens (1):
      powerpc: Fix __get_user_pages_fast() irq handling

Li Zhong (1):
      powerpc/vio: Fix a dma_mask issue of vio

Michael Ellerman (2):
      powerpc: Make cpu_to_chip_id() available when SMP=n
      powerpc/pseries: Fix SMP=n build of rng.c

Michael Neuling (1):
      powerpc/signals: Mark VSX not saved with small contexts

 arch/powerpc/include/asm/smp.h        |  2 +-
 arch/powerpc/kernel/eeh.c             |  9 +++++++++
 arch/powerpc/kernel/eeh_event.c       |  9 +++++++--
 arch/powerpc/kernel/process.c         | 21 +++++++++++----------
 arch/powerpc/kernel/prom.c            | 20 ++++++++++++++++++++
 arch/powerpc/kernel/signal_32.c       | 10 +++++++++-
 arch/powerpc/kernel/smp.c             | 16 ----------------
 arch/powerpc/kernel/time.c            |  4 ++--
 arch/powerpc/kernel/vio.c             |  2 +-
 arch/powerpc/mm/gup.c                 |  5 +++--
 arch/powerpc/mm/slice.c               |  2 +-
 arch/powerpc/platforms/powernv/rng.c  |  1 +
 arch/powerpc/platforms/pseries/rng.c  |  1 +
 arch/powerpc/platforms/wsp/chroma.c   |  1 +
 arch/powerpc/platforms/wsp/h8.c       |  1 +
 arch/powerpc/platforms/wsp/ics.c      |  2 ++
 arch/powerpc/platforms/wsp/opb_pic.c  |  2 ++
 arch/powerpc/platforms/wsp/psr2.c     |  1 +
 arch/powerpc/platforms/wsp/scom_wsp.c |  1 +
 arch/powerpc/platforms/wsp/wsp.c      |  1 +
 20 files changed, 75 insertions(+), 36 deletions(-)

^ permalink raw reply

* Re: [PATCH] powerpc/gpio: Fix the wrong GPIO input data on MPC8572/MPC8536
From: Scott Wood @ 2013-11-21  0:32 UTC (permalink / raw)
  To: Liu Gang; +Cc: linux-gpio, linus.walleij, linuxppc-dev, r61911, b07421
In-Reply-To: <1384916079.16677.26.camel@linux>

On Wed, 2013-11-20 at 10:54 +0800, Liu Gang wrote:
> On Tue, 2013-11-19 at 16:51 -0600, Scott Wood wrote:
> > > @@ -71,6 +71,7 @@ static int mpc8572_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> > >  	struct mpc8xxx_gpio_chip *mpc8xxx_gc = to_mpc8xxx_gpio_chip(mm);
> > >  
> > >  	val = in_be32(mm->regs + GPIO_DAT) & ~in_be32(mm->regs + GPIO_DIR);
> > > +	mpc8xxx_gc->data &= in_be32(mm->regs + GPIO_DIR);
> > >  
> > >  	return (val | mpc8xxx_gc->data) & mpc8xxx_gpio2mask(gpio);
> > >  }
> > 
> > It seems odd to update ->data in a function that's supposed to be
> > reading things...  Perhaps it would be better to keep ->data in a good
> > state from the beginning.
> > 
> > -Scott
> 
> Yes, keeping the ->data in a good state from the beginning will be
> better. But this will need more code in different functions to cover
> all the scenarios.
> First, we should check the direct of the pin in the function
> "mpc8xxx_gpio_set", and clean the input bit in ->data after setting
> operation.

For userspace value setting, it looks like gpiolib blocks the write if
the pin if FLAG_IS_OUT is set.  This suggests that this is an error
condition for other uses as well.  Though, I notice that
mpc8xxx_gpio_dir_out() calls gpio_set() before actually changing the
direction.  So it may be useful to avoid races where the wrong value is
output briefly after the direction is changed (especially in open drain
situations, where the signal could have a meaningful default even before
we begin outputting).  But that raises the question of how you'd do that
from userspace, and it also renders the to-be-output value as write-only
data (until the direction is actually changed), since a readback would
get the input value instead.

> So maybe it's better to eliminate the effects of the ->data to the
> input pins when reading the status, regardless of the possible changes
> of the pins and the data.
> Do you think so?

Perhaps, but that doesn't require you to modify ->data in the get()
function.

-Scott

^ permalink raw reply

* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Benjamin Herrenschmidt @ 2013-11-21  0:22 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <20131121000819.GU16735@n2100.arm.linux.org.uk>

On Thu, 2013-11-21 at 00:08 +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:
> > On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> > > Li Zong's patch works around the issue of a failing dma_set_mask(),
> > > but as I've already said elsewhere, the real fix is to get whatever
> > > created the struct device to initialise the dev->dma_mask with a
> > > bus default.
> > > 
> > > Using dma_coerce_xxx() merely makes the problem "go away" papering
> > > over the issue - it's fine to do it this way, but someone should still
> > > fix the broken code creating these devices...
> > 
> > Ok, they are created by the vio bus core, so it should be doing the
> > job here of setting the dma_mask pointer to a proper value.
> > 
> > Li, can you take care of that ? Look at other bus types we have in
> > there such as the macio bus etc...
> 
> Oh, hang on a moment, this is the "bus" code.
> 
> In which case, the question becomes: do vio devices ever need to have
> a separate streaming DMA mask from a coherent DMA mask?  If not, then
> something like the following is what's needed here, and I should've
> never have used dma_set_mask_and_coherent().

No, a single mask.

> dma_set_mask_and_coherent() (and the other dma_set_mask() functions)
> are really supposed to be used by drivers only.
> 
>  arch/powerpc/kernel/vio.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> index e7d0c88f621a..d771778f398e 100644
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>  
>  		/* needed to ensure proper operation of coherent allocations
>  		 * later, in case driver doesn't set it explicitly */
> -		dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
> +		viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
> +		viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask;
>  	}
>  
>  	/* register with generic device framework */

Right that's exactly what I had in mind. Li, can you test this please ?

The previous "fix" using dma_coerce_mask_and_coherent() is already on
its way to Linus, so we'll rework the above patch to undo it but for
now please test.

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Russell King - ARM Linux @ 2013-11-21  0:08 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <1384992102.26969.120.camel@pasglop>

On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> > Li Zong's patch works around the issue of a failing dma_set_mask(),
> > but as I've already said elsewhere, the real fix is to get whatever
> > created the struct device to initialise the dev->dma_mask with a
> > bus default.
> > 
> > Using dma_coerce_xxx() merely makes the problem "go away" papering
> > over the issue - it's fine to do it this way, but someone should still
> > fix the broken code creating these devices...
> 
> Ok, they are created by the vio bus core, so it should be doing the
> job here of setting the dma_mask pointer to a proper value.
> 
> Li, can you take care of that ? Look at other bus types we have in
> there such as the macio bus etc...

Oh, hang on a moment, this is the "bus" code.

In which case, the question becomes: do vio devices ever need to have
a separate streaming DMA mask from a coherent DMA mask?  If not, then
something like the following is what's needed here, and I should've
never have used dma_set_mask_and_coherent().

dma_set_mask_and_coherent() (and the other dma_set_mask() functions)
are really supposed to be used by drivers only.

 arch/powerpc/kernel/vio.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index e7d0c88f621a..d771778f398e 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
 
 		/* needed to ensure proper operation of coherent allocations
 		 * later, in case driver doesn't set it explicitly */
-		dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
+		viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
+		viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask;
 	}
 
 	/* register with generic device framework */

^ permalink raw reply related

* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Benjamin Herrenschmidt @ 2013-11-21  0:01 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <20131120232337.GT16735@n2100.arm.linux.org.uk>

On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> Li Zong's patch works around the issue of a failing dma_set_mask(),
> but as I've already said elsewhere, the real fix is to get whatever
> created the struct device to initialise the dev->dma_mask with a
> bus default.
> 
> Using dma_coerce_xxx() merely makes the problem "go away" papering
> over the issue - it's fine to do it this way, but someone should still
> fix the broken code creating these devices...

Ok, they are created by the vio bus core, so it should be doing the
job here of setting the dma_mask pointer to a proper value.

Li, can you take care of that ? Look at other bus types we have in
there such as the macio bus etc...

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Russell King - ARM Linux @ 2013-11-20 23:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <1384910882.26969.57.camel@pasglop>

On Wed, Nov 20, 2013 at 12:28:02PM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2013-11-19 at 16:11 +0800, Li Zhong wrote:
> > I encountered following issue:
> > [    0.283035] ibmvscsi 30000015: couldn't initialize event pool
> > [    5.688822] ibmvscsi: probe of 30000015 failed with error -1
> > 
> > which prevents the storage from being recognized, and the machine from
> > booting.
> > 
> > After some digging, it seems that it is caused by commit 4886c399da
> > 
> > as dma_mask pointer in viodev->dev is not set, so in
> > dma_set_mask_and_coherent(), dma_set_coherent_mask() is not called
> > because dma_set_mask(), which is dma_set_mask_pSeriesLP() returned EIO.
> > While before the commit, dma_set_coherent_mask() is always called. 
> > 
> > I tried to replace dma_set_mask_and_coherent() with
> > dma_coerce_mask_and_coherent(), and the machine could boot again. 
> > 
> > But I'm not sure whether this is the correct fix...
> 
> Russell, care to chime in ? I can't make sense of the semantics...
> 
> The original commit was fairly clear:
> 
> <<
>     Replace the following sequence:
>     
>     	dma_set_mask(dev, mask);
>     	dma_set_coherent_mask(dev, mask);
>     
>     with a call to the new helper dma_set_mask_and_coherent().
> >>
> 
> It all makes sense so far ... but doesn't work for some odd reason,
> and the "fix" uses a function whose name doesn't make much sense to
> me ... what is the difference between "setting" and "coercing"
> the mask ? And why doe replacing two "set" with a "set both" doesn't
> work and require a coerce ?
> 
> I'm asking because I'm worried about breakage elsewhere...

I'd expect that the reason it doesn't work is that the dma_set_mask()
is failing, which means we don't go on to set the coherent mask.

Li Zong's patch works around the issue of a failing dma_set_mask(),
but as I've already said elsewhere, the real fix is to get whatever
created the struct device to initialise the dev->dma_mask with a
bus default.

Using dma_coerce_xxx() merely makes the problem "go away" papering
over the issue - it's fine to do it this way, but someone should still
fix the broken code creating these devices...

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-11-20 23:10 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list

Hi Linus !

With my previous pull request I mentioned some remaining Little Endian
patches, notably support for our new ABI, which I was sitting on making
sure it was all finalized.

The toolchain folks confirmed it now, the new ABI is stable and merged
with gcc, so we are all good. Oh and we actually missed the actual
Kconfig switch for LE so here it is, along with a couple more bug fixes.

I have more fixes but not related to LE so I'll send them as a separate
pull request tomorrow, let's get this one out of the way.

Note that this supports running user space binaries using the new ABI,
but the kernel itself still needs to be built with the old one. We'll
bring fixes for that after -rc1.

Here's Anton log that goes with this series:

<<<
This patch series adds support for the new ABI, LPAR support
for H_SET_MODE and finally adds a kconfig option and defconfig.

ABIv2 support was recently committed to binutils and gcc, and
should be merged into glibc soon. There are a number of
very nice improvements including the removal of function
descriptors. Rusty's kernel patches allow binaries of either
ABI to work, easing the transition.
>>>

Cheers,
Ben.

The following changes since commit 0c4888ef1d8a8b82c29075ce7e257ff795af15c7:

  powerpc: Fix fatal SLB miss when restoring PPR (2013-11-06 14:13:53 +1100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git next

for you to fetch changes up to 280270828f108be56f0c486def58acabb070244f:

  powerpc: Wrong DWARF CFI in the kernel vdso for little-endian / ELFv2 (2013-11-21 09:19:23 +1100)

----------------------------------------------------------------
Alistair Popple (1):
      powerpc: Don't use ELFv2 ABI to build the kernel

Anton Blanchard (4):
      powerpc/pseries: Fix endian issues in pseries EEH code
      pseries: Add H_SET_MODE to change exception endianness
      powerpc: Add CONFIG_CPU_LITTLE_ENDIAN kernel config option.
      powerpc: Add pseries_le_defconfig

Rusty Russell (4):
      powerpc: Add TIF_ELF2ABI flag.
      powerpc: Set eflags correctly for ELF ABIv2 core dumps.
      powerpc: ELF2 binaries launched directly.
      powerpc: ELF2 binaries signal handling

Ulrich Weigand (1):
      powerpc: Wrong DWARF CFI in the kernel vdso for little-endian / ELFv2

 arch/powerpc/Makefile                        |   1 +
 arch/powerpc/configs/pseries_le_defconfig    | 352 +++++++++++++++++++++++++++
 arch/powerpc/include/asm/elf.h               |   4 +
 arch/powerpc/include/asm/hvcall.h            |   2 +
 arch/powerpc/include/asm/plpar_wrappers.h    |  26 ++
 arch/powerpc/include/asm/thread_info.h       |   9 +
 arch/powerpc/kernel/process.c                |  50 ++--
 arch/powerpc/kernel/signal_64.c              |  25 +-
 arch/powerpc/kernel/vdso64/sigtramp.S        |  16 +-
 arch/powerpc/platforms/Kconfig.cputype       |  11 +
 arch/powerpc/platforms/pseries/eeh_pseries.c |  21 +-
 arch/powerpc/platforms/pseries/lpar.c        |  17 ++
 arch/powerpc/platforms/pseries/setup.c       |  42 ++++
 13 files changed, 542 insertions(+), 34 deletions(-)
 create mode 100644 arch/powerpc/configs/pseries_le_defconfig

^ permalink raw reply

* Re: [PATCH v2] offb: make the screen properties endian safe
From: Benjamin Herrenschmidt @ 2013-11-20 22:50 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <1383212192-3622-1-git-send-email-clg@fr.ibm.com>

On Thu, 2013-10-31 at 10:36 +0100, Cédric Le Goater wrote:
> The "screen" properties : depth, width, height, linebytes need
> to be converted to the host endian order when read from the device
> tree.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---

Did you actually test that ? IE, using emulated VGA in qemu for
example ?

I'm asking because there are a few interesting nits here...

 - fbdev *generally* assume native endian framebuffer, but of course
under qemu today, the adapter will use a big endian frame buffer
aperture. You can compile in support for foreign endian but I don't know
how that actually works.

 - The setcolreg fix ... the "value" is used 2 lines above your endian
swap, is that correct ?

Cheers
Ben.


> Changes in v2:
> 
>  - replaced be32_to_cpu() by be32_to_cpup() 
>  - fixed setcolreg ops 
> 
>  drivers/video/offb.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/offb.c b/drivers/video/offb.c
> index 0c4f343..68e8415 100644
> --- a/drivers/video/offb.c
> +++ b/drivers/video/offb.c
> @@ -120,7 +120,7 @@ static int offb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>  			mask <<= info->var.transp.offset;
>  			value |= mask;
>  		}
> -		pal[regno] = value;
> +		pal[regno] = cpu_to_be32(value);
>  		return 0;
>  	}
>  
> @@ -536,7 +536,7 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
>  	unsigned int flags, rsize, addr_prop = 0;
>  	unsigned long max_size = 0;
>  	u64 rstart, address = OF_BAD_ADDR;
> -	const u32 *pp, *addrp, *up;
> +	const __be32 *pp, *addrp, *up;
>  	u64 asize;
>  	int foreign_endian = 0;
>  
> @@ -552,25 +552,25 @@ static void __init offb_init_nodriver(struct device_node *dp, int no_real_node)
>  	if (pp == NULL)
>  		pp = of_get_property(dp, "depth", &len);
>  	if (pp && len == sizeof(u32))
> -		depth = *pp;
> +		depth = be32_to_cpup(pp);
>  
>  	pp = of_get_property(dp, "linux,bootx-width", &len);
>  	if (pp == NULL)
>  		pp = of_get_property(dp, "width", &len);
>  	if (pp && len == sizeof(u32))
> -		width = *pp;
> +		width = be32_to_cpup(pp);
>  
>  	pp = of_get_property(dp, "linux,bootx-height", &len);
>  	if (pp == NULL)
>  		pp = of_get_property(dp, "height", &len);
>  	if (pp && len == sizeof(u32))
> -		height = *pp;
> +		height = be32_to_cpup(pp);
>  
>  	pp = of_get_property(dp, "linux,bootx-linebytes", &len);
>  	if (pp == NULL)
>  		pp = of_get_property(dp, "linebytes", &len);
>  	if (pp && len == sizeof(u32) && (*pp != 0xffffffffu))
> -		pitch = *pp;
> +		pitch = be32_to_cpup(pp);
>  	else
>  		pitch = width * ((depth + 7) / 8);
>  

^ permalink raw reply

* Re: [RFC][PATCH] powerpc/CoreNet64: compile with CONFIG_E{5,6}500_CPU well
From: Scott Wood @ 2013-11-20 22:03 UTC (permalink / raw)
  To: Kumar Gala; +Cc: Tiejun Chen, linuxppc-dev
In-Reply-To: <CC3170D1-8CF3-4A84-B585-BC753023DE6E@kernel.crashing.org>

On Wed, 2013-11-20 at 12:47 -0600, Kumar Gala wrote:
> On Nov 20, 2013, at 10:41 AM, Scott Wood <scottwood@freescale.com> wrot=
e:
>=20
> > On Wed, 2013-11-20 at 16:35 +0800, Tiejun Chen wrote:
> >> CONFIG_ALTIVEC is always enabled for CoreNet64.
> >=20
> > In the defconfig perhaps, but this isn't a generally true statement.
> >=20
> >> And if we select CONFIG_E{5,6}500_CPU this may introduce -mcpu=3De50=
0mc64
> >> into $CFLAGS. But Altivec and Spe options not allowed with
> >> e500mc64, so :
> >=20
> > Sigh.
> >=20
> >>  CC      arch/powerpc/lib/xor_vmx.o
> >> arch/powerpc/lib/xor_vmx.c:1:0: error: AltiVec not supported in this=
 target
> >> make[1]: *** [arch/powerpc/lib/xor_vmx.o] Error 1
> >> make: *** [arch/powerpc/lib] Error 2
> >>=20
> >> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> >> ---
> >> arch/powerpc/lib/Makefile |    3 +++
> >> 1 file changed, 3 insertions(+)
> >>=20
> >> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> >> index 95a20e1..641a77d 100644
> >> --- a/arch/powerpc/lib/Makefile
> >> +++ b/arch/powerpc/lib/Makefile
> >> @@ -40,5 +40,8 @@ obj-y			+=3D code-patching.o
> >> obj-y			+=3D feature-fixups.o
> >> obj-$(CONFIG_FTR_FIXUP_SELFTEST) +=3D feature-fixups-test.o
> >>=20
> >> +# Altivec and Spe options not allowed with e500mc64 in GCC.
> >> +ifeq ($(call cc-option-yn,-mcpu=3De500mc64),n)
> >> obj-$(CONFIG_ALTIVEC)	+=3D xor_vmx.o
> >> CFLAGS_xor_vmx.o +=3D -maltivec -mabi=3Daltivec
> >> +endif
> >=20
> > This does not seem like the right fix.  What if GCC supports both
> > -mcpu=3De500mc64 and -mcpu=3De6500, and we're using the latter?  Or f=
or that
> > matter, if we're using -mcpu=3Dwhatever-ibm-chip-has-this?
> >=20
> > Plus, wouldn't you need to do something to prevent code in that file
> > from being called?
> >=20
> > -Scott
>=20
> Why does -mcpu=3De500mc64 get you spe enabled?  It shouldn=E2=80=99t as=
 no e500mc or greater part has spe.  Can you try using -mno-spe -maltivec=
?

I don't think SPE is relevant.  The problem is that GCC refuses to allow
-maltivec to be combined with -mcpu=3De500mc64.  -mno-spe doesn't make a
difference.

-Scott

^ permalink raw reply

* Re: [PATCH v3] KVM: PPC: vfio kvm device: support spapr tce
From: Alex Williamson @ 2013-11-20 20:57 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: kvm-ppc, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <1384924730-22762-1-git-send-email-aik@ozlabs.ru>

On Wed, 2013-11-20 at 16:18 +1100, Alexey Kardashevskiy wrote:
> In addition to the external VFIO user API, a VFIO KVM device
> has been introduced recently.
> 
> sPAPR TCE IOMMU is para-virtualized and the guest does map/unmap
> via hypercalls which take a logical bus id (LIOBN) as a target IOMMU
> identifier. LIOBNs are made up and linked to IOMMU groups by the user
> space. In order to accelerate IOMMU operations in the KVM, we need
> to tell KVM the information about LIOBN-to-group mapping.
> 
> For that, a new KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN parameter
> is added. It accepts a pair of a VFIO group fd and LIOBN.
> 
> This also adds a new kvm_vfio_find_group_by_liobn() function which
> receives kvm struct, LIOBN and a callback. As it increases the IOMMU
> group use counter, the KVMr is required to pass a callback which
> called when the VFIO group is about to be removed VFIO-KVM tracking so
> the KVM is able to call iommu_group_put() to release the IOMMU group.
> 
> The KVM uses kvm_vfio_find_group_by_liobn() once per KVM run and caches
> the result in kvm_arch. iommu_group_put() for all groups will be called
> when KVM finishes (in the SPAPR TCE in KVM enablement patch).
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v3:
> * total rework
> * added a release callback into kvm_vfio_find_group_by_liobn so now
> the user of the API can get a notification if the group is about to
> disappear
> ---
>  Documentation/virtual/kvm/devices/vfio.txt |  19 ++++-
>  arch/powerpc/kvm/Kconfig                   |   1 +
>  arch/powerpc/kvm/Makefile                  |   3 +
>  include/linux/kvm_host.h                   |  18 +++++
>  include/uapi/linux/kvm.h                   |   7 ++
>  virt/kvm/vfio.c                            | 116 ++++++++++++++++++++++++++++-
>  6 files changed, 161 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
> index ef51740..7ecb3b2 100644
> --- a/Documentation/virtual/kvm/devices/vfio.txt
> +++ b/Documentation/virtual/kvm/devices/vfio.txt
> @@ -16,7 +16,22 @@ Groups:
>  
>  KVM_DEV_VFIO_GROUP attributes:
>    KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
> +	kvm_device_attr.addr points to an int32_t file descriptor
> +	for the VFIO group.
> +
>    KVM_DEV_VFIO_GROUP_DEL: Remove a VFIO group from VFIO-KVM device tracking
> +	kvm_device_attr.addr points to an int32_t file descriptor
> +	for the VFIO group.
> +
> +  KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN: sets a liobn for a VFIO group
> +	kvm_device_attr.addr points to a struct:
> +		struct kvm_vfio_spapr_tce_liobn {
> +			__u32	argsz;
> +			__u32	fd;

fds are signed, __s32

> +			__u32	liobn;
> +		};
> +		where
> +		@argsz is a struct size;
> +		@fd is a file descriptor for a VFIO group;
> +		@liobn is a logical bus id to be associated with the group.
>  
> -For each, kvm_device_attr.addr points to an int32_t file descriptor
> -for the VFIO group.
> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
> index 61b3535..d1b7f64 100644
> --- a/arch/powerpc/kvm/Kconfig
> +++ b/arch/powerpc/kvm/Kconfig
> @@ -60,6 +60,7 @@ config KVM_BOOK3S_64
>  	select KVM_BOOK3S_64_HANDLER
>  	select KVM
>  	select SPAPR_TCE_IOMMU
> +	select KVM_VFIO
>  	---help---
>  	  Support running unmodified book3s_64 and book3s_32 guest kernels
>  	  in virtual machines on book3s_64 host processors.
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index 6646c95..2438d2e 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -87,6 +87,9 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>  kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>  	book3s_xics.o
>  
> +kvm-book3s_64-objs-$(CONFIG_KVM_VFIO) += \
> +	$(KVM)/vfio.o \
> +
>  kvm-book3s_64-module-objs := \
>  	$(KVM)/kvm_main.o \
>  	$(KVM)/eventfd.o \
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 88ff96a..1d2ad5e 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1112,5 +1112,23 @@ static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
>  }
>  
>  #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
> +
> +typedef void (*kvm_vfio_release_group_callback)(struct kvm *kvm,
> +		unsigned long liobn);

liobn was said to be __u32 in kvm_vfio_spapr_tce_liobn above, here it's
unsigned long?

> +
> +#if defined(CONFIG_KVM_VFIO) && defined(CONFIG_SPAPR_TCE_IOMMU)
> +
> +extern struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
> +		unsigned long liobn, kvm_vfio_release_group_callback cb);
> +
> +#else
> +
> +static inline struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
> +		unsigned long liobn, ikvm_vfio_release_group_callback cb)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_KVM_VFIO && CONFIG_SPAPR_TCE_IOMMU */
> +
>  #endif
>  
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 7c1a349..3d77dde 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -847,6 +847,13 @@ struct kvm_device_attr {
>  #define  KVM_DEV_VFIO_GROUP			1
>  #define   KVM_DEV_VFIO_GROUP_ADD			1
>  #define   KVM_DEV_VFIO_GROUP_DEL			2
> +#define   KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN	3
> +
> +struct kvm_vfio_spapr_tce_liobn {
> +	__u32	argsz;
> +	__u32	fd;
> +	__u32	liobn;
> +};
>  
>  /*
>   * ioctls for VM fds
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index ca4260e..448910d 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -22,6 +22,12 @@
>  struct kvm_vfio_group {
>  	struct list_head node;
>  	struct vfio_group *vfio_group;
> +#ifdef CONFIG_SPAPR_TCE_IOMMU
> +	struct {
> +		unsigned long liobn;
> +		kvm_vfio_release_group_callback cb;
> +	} spapr_tce;
> +#endif
>  };
>  
>  struct kvm_vfio {
> @@ -59,6 +65,51 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
>  	symbol_put(vfio_group_put_external_user);
>  }
>  
> +#ifdef CONFIG_SPAPR_TCE_IOMMU
> +struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
> +		unsigned long liobn, kvm_vfio_release_group_callback cb)
> +{
> +	struct kvm_vfio_group *kvg;
> +	int group_id;
> +	struct iommu_group *grp;
> +	struct kvm_vfio *kv = NULL;
> +	struct kvm_device *tmp;
> +
> +	if (!cb)
> +		return NULL;

Is it worthwhile to use ERR_PTR(-EINVAL) here and the caller can use
IS_ERR()?

> +
> +	/* Find a VFIO KVM device */
> +	list_for_each_entry(tmp, &kvm->devices, vm_node) {
> +		if (tmp->ops != &kvm_vfio_ops)
> +			continue;
> +
> +		kv = tmp->private;
> +		break;
> +	}
> +
> +	if (!kv)
> +		return NULL;

ERR_PTR(-EFAULT)?  EIO?

> +
> +	/* Find a group */

Still ignoring kv->lock

> +	list_for_each_entry(kvg, &kv->group_list, node) {
> +		if (kvg->spapr_tce.liobn != liobn)
> +			continue;
> +
> +		if (kvg->spapr_tce.cb)
> +			return NULL;

ERR_PTR(-EBUSY)?

> +
> +		kvg->spapr_tce.cb = cb;
> +		group_id = vfio_external_user_iommu_id(kvg->vfio_group);
> +		grp = iommu_group_get_by_id(group_id);
> +
> +		return grp;
> +	}
> +
> +	return NULL;

ERR_PTR(-ENODEV)?

> +}
> +EXPORT_SYMBOL_GPL(kvm_vfio_find_group_by_liobn);
> +#endif
> +
>  /*
>   * Groups can use the same or different IOMMU domains.  If the same then
>   * adding a new group may change the coherency of groups we've previously
> @@ -170,6 +221,11 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  				continue;
>  
>  			list_del(&kvg->node);
> +#ifdef CONFIG_SPAPR_TCE_IOMMU
> +			if (kvg->spapr_tce.cb)
> +				kvg->spapr_tce.cb(dev->kvm,
> +						kvg->spapr_tce.liobn);
> +#endif
>  			kvm_vfio_group_put_external_user(kvg->vfio_group);
>  			kfree(kvg);
>  			ret = 0;
> @@ -183,6 +239,62 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>  		kvm_vfio_update_coherency(dev);
>  
>  		return ret;
> +
> +#ifdef CONFIG_SPAPR_TCE_IOMMU
> +	case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN: {
> +		struct kvm_vfio_spapr_tce_liobn param;
> +		unsigned long minsz;
> +		struct kvm_vfio *kv = dev->private;
> +		struct vfio_group *vfio_group;
> +		struct kvm_vfio_group *kvg;
> +		struct fd f;
> +
> +		minsz = offsetofend(struct kvm_vfio_spapr_tce_liobn, liobn);
> +
> +		if (copy_from_user(&param, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (param.argsz < minsz)
> +			return -EINVAL;
> +
> +		if (copy_from_user(&param, (void __user *)arg, minsz))
> +			return -EFAULT;

copy_from_user twice?  Extra copy here?

> +
> +		f = fdget(param.fd);
> +		if (!f.file)
> +			return -EBADF;
> +
> +		vfio_group = kvm_vfio_group_get_external_user(f.file);
> +		fdput(f);
> +
> +		if (IS_ERR(vfio_group))
> +			return PTR_ERR(vfio_group);
> +
> +		ret = -ENOENT;
> +
> +		mutex_lock(&kv->lock);
> +
> +		list_for_each_entry(kvg, &kv->group_list, node) {
> +			if (kvg->vfio_group != vfio_group)
> +				continue;
> +
> +			if (kvg->spapr_tce.liobn) {
> +				ret = -EBUSY;
> +				break;
> +			}

Is zero not an liobn that can be used by userspace?  Is it intentional
that there's no way to unset or change the group/liobn mapping?  Thanks,

Alex

> +
> +			kvg->spapr_tce.liobn = param.liobn;
> +			ret = 0;
> +			break;
> +		}
> +
> +		mutex_unlock(&kv->lock);
> +
> +		kvm_vfio_group_put_external_user(vfio_group);
> +
> +		return ret;
> +	}
> +#endif /* CONFIG_SPAPR_TCE_IOMMU */
>  	}
>  
>  	return -ENXIO;
> @@ -207,9 +319,11 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
>  		switch (attr->attr) {
>  		case KVM_DEV_VFIO_GROUP_ADD:
>  		case KVM_DEV_VFIO_GROUP_DEL:
> +#ifdef CONFIG_SPAPR_TCE_IOMMU
> +		case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN:
> +#endif
>  			return 0;
>  		}
> -
>  		break;
>  	}
>  

^ permalink raw reply

* [PATCH] powerpc: Wrong DWARF CFI in the kernel vdso for little-endian / ELFv2
From: Anton Blanchard @ 2013-11-20 20:38 UTC (permalink / raw)
  To: benh, paulus, rusty, Ulrich.Weigand, alistair; +Cc: linuxppc-dev

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>

I've finally tracked down why my CR signal-unwind test case still
fails on little-endian.  The problem turned to be that the kernel
installs a signal trampoline in the vDSO, and provides a DWARF CFI
record for that trampoline.  This CFI describes the save location
for CR:

  rsave (70, 38*RSIZE + (RSIZE - CRSIZE))

which is correct for big-endian, but points to the wrong word on
little-endian.   This is wrong no matter which ABI.

In addition, for the ELFv2 ABI, we should not only provide a CFI
record for register 70 (cr2), but for all CR fields separately.
Strictly speaking, I guess this would mean providing two separate
vDSO images, one for ELFv1 processes and one for ELFv2 processes (or
maybe playing some tricks with conditional DWARF expressions).
However, having CFI records for the other CR fields in ELFv1 is not
actually wrong, they just will be ignored.   So it seems the simplest
fix would be just to always provide CFI for all the fields.

Signed-off-by: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Signed-off-by: Anton Blanchard <anton@samba.org>
---
 arch/powerpc/kernel/vdso64/sigtramp.S | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/vdso64/sigtramp.S b/arch/powerpc/kernel/vdso64/sigtramp.S
index 45ea281..542c6f42 100644
--- a/arch/powerpc/kernel/vdso64/sigtramp.S
+++ b/arch/powerpc/kernel/vdso64/sigtramp.S
@@ -142,6 +142,13 @@ V_FUNCTION_END(__kernel_sigtramp_rt64)
 /* Size of CR reg in DWARF unwind info. */
 #define CRSIZE	4
 
+/* Offset of CR reg within a full word. */
+#ifdef __LITTLE_ENDIAN__
+#define CROFF 0
+#else
+#define CROFF (RSIZE - CRSIZE)
+#endif
+
 /* This is the offset of the VMX reg pointer.  */
 #define VREGS	48*RSIZE+33*8
 
@@ -181,7 +188,14 @@ V_FUNCTION_END(__kernel_sigtramp_rt64)
   rsave (31, 31*RSIZE);							\
   rsave (67, 32*RSIZE);		/* ap, used as temp for nip */		\
   rsave (65, 36*RSIZE);		/* lr */				\
-  rsave (70, 38*RSIZE + (RSIZE - CRSIZE)) /* cr */
+  rsave (68, 38*RSIZE + CROFF);	/* cr fields */				\
+  rsave (69, 38*RSIZE + CROFF);						\
+  rsave (70, 38*RSIZE + CROFF);						\
+  rsave (71, 38*RSIZE + CROFF);						\
+  rsave (72, 38*RSIZE + CROFF);						\
+  rsave (73, 38*RSIZE + CROFF);						\
+  rsave (74, 38*RSIZE + CROFF);						\
+  rsave (75, 38*RSIZE + CROFF)
 
 /* Describe where the FP regs are saved.  */
 #define EH_FRAME_FP \
-- 
1.8.3.2

^ permalink raw reply related

* Re: [RFC][PATCH] powerpc/CoreNet64: compile with CONFIG_E{5, 6}500_CPU well
From: Kumar Gala @ 2013-11-20 18:47 UTC (permalink / raw)
  To: Scott Wood; +Cc: Tiejun Chen, linuxppc-dev
In-Reply-To: <1384965671.1403.414.camel@snotra.buserror.net>


On Nov 20, 2013, at 10:41 AM, Scott Wood <scottwood@freescale.com> =
wrote:

> On Wed, 2013-11-20 at 16:35 +0800, Tiejun Chen wrote:
>> CONFIG_ALTIVEC is always enabled for CoreNet64.
>=20
> In the defconfig perhaps, but this isn't a generally true statement.
>=20
>> And if we select CONFIG_E{5,6}500_CPU this may introduce =
-mcpu=3De500mc64
>> into $CFLAGS. But Altivec and Spe options not allowed with
>> e500mc64, so :
>=20
> Sigh.
>=20
>>  CC      arch/powerpc/lib/xor_vmx.o
>> arch/powerpc/lib/xor_vmx.c:1:0: error: AltiVec not supported in this =
target
>> make[1]: *** [arch/powerpc/lib/xor_vmx.o] Error 1
>> make: *** [arch/powerpc/lib] Error 2
>>=20
>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>> ---
>> arch/powerpc/lib/Makefile |    3 +++
>> 1 file changed, 3 insertions(+)
>>=20
>> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
>> index 95a20e1..641a77d 100644
>> --- a/arch/powerpc/lib/Makefile
>> +++ b/arch/powerpc/lib/Makefile
>> @@ -40,5 +40,8 @@ obj-y			+=3D code-patching.o
>> obj-y			+=3D feature-fixups.o
>> obj-$(CONFIG_FTR_FIXUP_SELFTEST) +=3D feature-fixups-test.o
>>=20
>> +# Altivec and Spe options not allowed with e500mc64 in GCC.
>> +ifeq ($(call cc-option-yn,-mcpu=3De500mc64),n)
>> obj-$(CONFIG_ALTIVEC)	+=3D xor_vmx.o
>> CFLAGS_xor_vmx.o +=3D -maltivec -mabi=3Daltivec
>> +endif
>=20
> This does not seem like the right fix.  What if GCC supports both
> -mcpu=3De500mc64 and -mcpu=3De6500, and we're using the latter?  Or =
for that
> matter, if we're using -mcpu=3Dwhatever-ibm-chip-has-this?
>=20
> Plus, wouldn't you need to do something to prevent code in that file
> from being called?
>=20
> -Scott

Why does -mcpu=3De500mc64 get you spe enabled?  It shouldn=92t as no =
e500mc or greater part has spe.  Can you try using -mno-spe -maltivec?

- k=

^ permalink raw reply

* Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
From: Alex Williamson @ 2013-11-20 18:47 UTC (permalink / raw)
  To: Bharat Bhushan
  Cc: linux-pci, joro, agraf, stuart.yoder, Bharat Bhushan, scottwood,
	iommu, bhelgaas, linuxppc-dev, linux-kernel
In-Reply-To: <1384838233-24847-1-git-send-email-Bharat.Bhushan@freescale.com>

On Tue, 2013-11-19 at 10:47 +0530, Bharat Bhushan wrote:
> From: Bharat Bhushan <bharat.bhushan@freescale.com>
> 
> PAMU (FSL IOMMU) has a concept of primary window and subwindows.
> Primary window corresponds to the complete guest iova address space
> (including MSI space), with respect to IOMMU_API this is termed as
> geometry. IOVA Base of subwindow is determined from the number of
> subwindows (configurable using iommu API).
> MSI I/O page must be within the geometry and maximum supported
> subwindows, so MSI IO-page is setup just after guest memory iova space.
> 
> So patch 1/9-4/9(inclusive) are for defining the interface to get:
>   - Number of MSI regions (which is number of MSI banks for powerpc)
>   - MSI-region address range: Physical page which have the
>     address/addresses used for generating MSI interrupt
>     and size of the page.
> 
> Patch 5/9-7/9(inclusive) is defining the interface of setting up
> MSI iova-base for a msi region(bank) for a device. so that when
> msi-message will be composed then this configured iova will be used.
> Earlier we were using iommu interface for getting the configured iova
> which was not currect and Alex Williamson suggeested this type of interface.
> 
> patch 8/9 moves some common functions in a separate file so that these
> can be used by FSL_PAMU implementation (next patch uses this).
> These will be used later for iommu-none implementation. I believe we
> can do more of this but will take step by step.
> 
> Finally last patch actually adds the support for FSL-PAMU :)

Patches 1-3: msi_get_region needs to return an error an error (probably
-EINVAL) if called on a path where there's no backend implementation.
Otherwise the caller doesn't know that the data in the region pointer
isn't valid.

Patches 5&6: same as above for msi_set_iova, return an error if no
backend implementation.

Patch 7: Why does fsl_msi_del_iova_device bother to return anything if
it's always zero?  Return -ENODEV when not found?

Patch 9:

vfio_handle_get_attr() passes random kernel data back to userspace in
the event of iommu_domain_get_attr() error.

vfio_handle_set_attr(): I don't see any data validation happening, is
iommu_domain_set_attr() really that safe?

For both of those, drop the pr_err on unknown attribute, it's sufficient
to return error.

Is VFIO_IOMMU_PAMU_GET_MSI_BANK_COUNT per aperture (ie. each vfio user
has $COUNT regions at their disposal exclusively)?  Thanks,

Alex

> v1->v2
>  - Added interface for setting msi iova for a msi region for a device.
>    Earlier I added iommu interface for same but as per comment that is
>    removed and now created a direct interface between vfio and msi.
>  - Incorporated review comments (details is in individual patch)
> 
> Bharat Bhushan (9):
>   pci:msi: add weak function for returning msi region info
>   pci: msi: expose msi region information functions
>   powerpc: pci: Add arch specific msi region interface
>   powerpc: msi: Extend the msi region interface to get info from
>     fsl_msi
>   pci/msi: interface to set an iova for a msi region
>   powerpc: pci: Extend msi iova page setup to arch specific
>   pci: msi: Extend msi iova setting interface to powerpc arch
>   vfio: moving some functions in common file
>   vfio pci: Add vfio iommu implementation for FSL_PAMU
> 
>  arch/powerpc/include/asm/machdep.h |   10 +
>  arch/powerpc/kernel/msi.c          |   28 +
>  arch/powerpc/sysdev/fsl_msi.c      |  132 +++++-
>  arch/powerpc/sysdev/fsl_msi.h      |   25 +-
>  drivers/pci/msi.c                  |   35 ++
>  drivers/vfio/Kconfig               |    6 +
>  drivers/vfio/Makefile              |    5 +-
>  drivers/vfio/vfio_iommu_common.c   |  227 ++++++++
>  drivers/vfio/vfio_iommu_common.h   |   27 +
>  drivers/vfio/vfio_iommu_fsl_pamu.c | 1003 ++++++++++++++++++++++++++++++++++++
>  drivers/vfio/vfio_iommu_type1.c    |  206 +--------
>  include/linux/msi.h                |   14 +
>  include/linux/pci.h                |   21 +
>  include/uapi/linux/vfio.h          |  100 ++++
>  14 files changed, 1623 insertions(+), 216 deletions(-)
>  create mode 100644 drivers/vfio/vfio_iommu_common.c
>  create mode 100644 drivers/vfio/vfio_iommu_common.h
>  create mode 100644 drivers/vfio/vfio_iommu_fsl_pamu.c
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: BUG: Patch "Convert some mftb/mftbu into mfspr" breaks MPC885
From: Scott Wood @ 2013-11-20 16:57 UTC (permalink / raw)
  To: leroy christophe; +Cc: LinuxPPC-dev, linux-kernel
In-Reply-To: <528C82D1.4090803@c-s.fr>

On Wed, 2013-11-20 at 10:37 +0100, leroy christophe wrote:
> Scott,
> 
> The patch "Convert some mftb/mftbu into
> mfspr" (beb2dc0a7a84be003ce54e98b95d65cc66e6e536) breaks startup on
> MPC885.
> 
> The CPU traps (SoftwareEmulation trap) at sched_clock() when trying to
> read TBU with mfspr.
> 
> Reverting the patch solves the issue.
> 
> What's the prefered way to fix this ?

I guess we need to add an ifdef.

-Scott

^ permalink raw reply

* Re: [RFC][PATCH] powerpc/CoreNet64: compile with CONFIG_E{5,6}500_CPU well
From: Scott Wood @ 2013-11-20 16:41 UTC (permalink / raw)
  To: Tiejun Chen; +Cc: linuxppc-dev
In-Reply-To: <1384936551-8494-1-git-send-email-tiejun.chen@windriver.com>

On Wed, 2013-11-20 at 16:35 +0800, Tiejun Chen wrote:
> CONFIG_ALTIVEC is always enabled for CoreNet64.

In the defconfig perhaps, but this isn't a generally true statement.

> And if we select CONFIG_E{5,6}500_CPU this may introduce -mcpu=e500mc64
> into $CFLAGS. But Altivec and Spe options not allowed with
> e500mc64, so :

Sigh.

>   CC      arch/powerpc/lib/xor_vmx.o
> arch/powerpc/lib/xor_vmx.c:1:0: error: AltiVec not supported in this target
> make[1]: *** [arch/powerpc/lib/xor_vmx.o] Error 1
> make: *** [arch/powerpc/lib] Error 2
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
>  arch/powerpc/lib/Makefile |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 95a20e1..641a77d 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -40,5 +40,8 @@ obj-y			+= code-patching.o
>  obj-y			+= feature-fixups.o
>  obj-$(CONFIG_FTR_FIXUP_SELFTEST) += feature-fixups-test.o
>  
> +# Altivec and Spe options not allowed with e500mc64 in GCC.
> +ifeq ($(call cc-option-yn,-mcpu=e500mc64),n)
>  obj-$(CONFIG_ALTIVEC)	+= xor_vmx.o
>  CFLAGS_xor_vmx.o += -maltivec -mabi=altivec
> +endif

This does not seem like the right fix.  What if GCC supports both
-mcpu=e500mc64 and -mcpu=e6500, and we're using the latter?  Or for that
matter, if we're using -mcpu=whatever-ibm-chip-has-this?

Plus, wouldn't you need to do something to prevent code in that file
from being called?

-Scott

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox