[PATCH v2 0/4] vfio: eeh: spapr: Compile and compatibility fixes

[PATCH v2 0/4] vfio: eeh: spapr: Compile and compatibility fixes

[Alexey Kardashevskiy]

Here is what I have in my stash which would be nice to have in 3.17.

Alexey Kardashevskiy (2):
  vfio_spapr_eeh: Enable compile as a module
  vfio_pci: spapr: Enable VFIO if EEH is not supported

Gavin Shan (2):
  powerpc/eeh: Export eeh_iommu_group_to_pe()
  vfio: spapr: Fix build error

 arch/powerpc/kernel/eeh.c     |  1 +
 drivers/vfio/Makefile         |  4 ++--
 drivers/vfio/pci/vfio_pci.c   |  6 ++----
 drivers/vfio/vfio_spapr_eeh.c | 16 ++++++++++++++++
 4 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.0.0

[PATCH v2 1/4] powerpc/eeh: Export eeh_iommu_group_to_pe()

[Alexey Kardashevskiy]

From: Gavin Shan <gwshan@linux.vnet.ibm.com>

The function is used by VFIO driver, which might be built as a
dynamic module.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 6043879..59a64f8 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1254,6 +1254,7 @@ struct eeh_pe *eeh_iommu_group_to_pe(struct iommu_group *group)
 
 	return edev->pe;
 }
+EXPORT_SYMBOL_GPL(eeh_iommu_group_to_pe);
 
 #endif /* CONFIG_IOMMU_API */
 
-- 
2.0.0

[PATCH v2 2/4] vfio: spapr: Fix build error

[Alexey Kardashevskiy]

From: Gavin Shan <gwshan@linux.vnet.ibm.com>

The VFIO related components could be built as dynamic modules.
Unfortunately, CONFIG_EEH can't be configured to "m". The patch
fixes the build errors when configuring VFIO related components
as dynamic modules as follows:

  CC [M]  drivers/vfio/vfio_iommu_spapr_tce.o
In file included from drivers/vfio/vfio.c:33:0:
include/linux/vfio.h:101:43: warning: ‘struct pci_dev’ declared \
inside parameter list [enabled by default]
:
  WRAP    arch/powerpc/boot/zImage.pseries
  WRAP    arch/powerpc/boot/zImage.maple
  WRAP    arch/powerpc/boot/zImage.pmac
  WRAP    arch/powerpc/boot/zImage.epapr
  MODPOST 1818 modules
ERROR: ".vfio_spapr_iommu_eeh_ioctl" [drivers/vfio/vfio_iommu_spapr_tce.ko]\
undefined!
ERROR: ".vfio_spapr_pci_eeh_open" [drivers/vfio/pci/vfio-pci.ko] undefined!
ERROR: ".vfio_spapr_pci_eeh_release" [drivers/vfio/pci/vfio-pci.ko] undefined!

Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
[removed include "pci.h" in vfio.c]
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* removed #include <linux/pci.h> from vfio.c and tested

---
 drivers/vfio/Makefile         | 4 ++--
 drivers/vfio/vfio_spapr_eeh.c | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 50e30bc..4891cca 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1,5 +1,5 @@
 obj-$(CONFIG_VFIO) += vfio.o
 obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
-obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
-obj-$(CONFIG_EEH) += vfio_spapr_eeh.o
+obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o \
+				      vfio_spapr_eeh.o
 obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
index f834b4c..1a93e83 100644
--- a/drivers/vfio/vfio_spapr_eeh.c
+++ b/drivers/vfio/vfio_spapr_eeh.c
@@ -14,15 +14,19 @@
 #include <asm/eeh.h>
 
 /* We might build address mapping here for "fast" path later */
+#ifdef CONFIG_EEH
+
 int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
 {
 	return eeh_dev_open(pdev);
 }
+EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_open);
 
 void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
 {
 	eeh_dev_release(pdev);
 }
+EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_release);
 
 long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 				unsigned int cmd, unsigned long arg)
@@ -85,3 +89,5 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(vfio_spapr_iommu_eeh_ioctl);
+#endif /* CONFIG_EEH */
-- 
2.0.0

[PATCH v2 3/4] vfio_spapr_eeh: Enable compile as a module

[Alexey Kardashevskiy]

This adds necessary declarations to the module, otherwise multiple
dynamic linker errors happen:

vfio_spapr_eeh: Unknown symbol eeh_pe_set_option (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_configure (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_reset (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_get_state (err 0)
vfio_spapr_eeh: Unknown symbol eeh_iommu_group_to_pe (err 0)
vfio_spapr_eeh: Unknown symbol eeh_dev_open (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_set_option (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_configure (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_reset (err 0)
vfio_spapr_eeh: Unknown symbol eeh_pe_get_state (err 0)
vfio_spapr_eeh: Unknown symbol eeh_iommu_group_to_pe (err 0)
vfio_spapr_eeh: Unknown symbol eeh_dev_open (err 0)

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 drivers/vfio/vfio_spapr_eeh.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
index 1a93e83..a942dd0 100644
--- a/drivers/vfio/vfio_spapr_eeh.c
+++ b/drivers/vfio/vfio_spapr_eeh.c
@@ -9,10 +9,15 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/module.h>
 #include <linux/uaccess.h>
 #include <linux/vfio.h>
 #include <asm/eeh.h>
 
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "Gavin Shan, IBM Corporation"
+#define DRIVER_DESC     "VFIO IOMMU SPAPR EEH"
+
 /* We might build address mapping here for "fast" path later */
 #ifdef CONFIG_EEH
 
@@ -91,3 +96,8 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 }
 EXPORT_SYMBOL_GPL(vfio_spapr_iommu_eeh_ioctl);
 #endif /* CONFIG_EEH */
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
-- 
2.0.0

[PATCH v2 4/4] vfio_pci: spapr: Enable VFIO if EEH is not supported

[Alexey Kardashevskiy]

The existing vfio_pci_open() fails if there is no EEH support for PCI.
This breaks POWER7's P5IOC2 PHB support which this patch brings back.

It is a warning because this should not normally happen on supported
configurations such as POWER8+IODA so we always want to see it in dmesg.
It is _once() as the message may be triggered by non-privileged userspace
and we do not want to pollute dmesg.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* discussed with Ben Herrenschmidt and did s/pr_warn/pr_warn_once/
* updated commit log
---
 drivers/vfio/pci/vfio_pci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index e2ee80f..68dc8da 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -179,10 +179,8 @@ static int vfio_pci_open(void *device_data)
 			goto error;
 
 		ret = vfio_spapr_pci_eeh_open(vdev->pdev);
-		if (ret) {
-			vfio_pci_disable(vdev);
-			goto error;
-		}
+		if (ret)
+			pr_warn_once("EEH is not supported\n");
 	}
 
 	return 0;
-- 
2.0.0

Re: [PATCH v2 2/4] vfio: spapr: Fix build error

[Alex Williamson]

On Wed, 2014-08-06 at 12:48 +1000, Alexey Kardashevskiy wrote:
> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
> 
> The VFIO related components could be built as dynamic modules.
> Unfortunately, CONFIG_EEH can't be configured to "m". The patch
> fixes the build errors when configuring VFIO related components
> as dynamic modules as follows:
> 
>   CC [M]  drivers/vfio/vfio_iommu_spapr_tce.o
> In file included from drivers/vfio/vfio.c:33:0:
> include/linux/vfio.h:101:43: warning: ‘struct pci_dev’ declared \
> inside parameter list [enabled by default]
> :
>   WRAP    arch/powerpc/boot/zImage.pseries
>   WRAP    arch/powerpc/boot/zImage.maple
>   WRAP    arch/powerpc/boot/zImage.pmac
>   WRAP    arch/powerpc/boot/zImage.epapr
>   MODPOST 1818 modules
> ERROR: ".vfio_spapr_iommu_eeh_ioctl" [drivers/vfio/vfio_iommu_spapr_tce.ko]\
> undefined!
> ERROR: ".vfio_spapr_pci_eeh_open" [drivers/vfio/pci/vfio-pci.ko] undefined!
> ERROR: ".vfio_spapr_pci_eeh_release" [drivers/vfio/pci/vfio-pci.ko] undefined!
> 
> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> [removed include "pci.h" in vfio.c]
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v2:
> * removed #include <linux/pci.h> from vfio.c and tested

I also commented regarding the ifdef around all of vfio_spapr_eeh.c:

        Why not add a new CONFIG_VFIO_SPAPR_EEH option to handle this
        instead?

Did you disagree?  The ifdef is pretty ugly.

> ---
>  drivers/vfio/Makefile         | 4 ++--
>  drivers/vfio/vfio_spapr_eeh.c | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 50e30bc..4891cca 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,5 +1,5 @@
>  obj-$(CONFIG_VFIO) += vfio.o
>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
> -obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
> -obj-$(CONFIG_EEH) += vfio_spapr_eeh.o
> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o \
> +				      vfio_spapr_eeh.o
>  obj-$(CONFIG_VFIO_PCI) += pci/
> diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
> index f834b4c..1a93e83 100644
> --- a/drivers/vfio/vfio_spapr_eeh.c
> +++ b/drivers/vfio/vfio_spapr_eeh.c
> @@ -14,15 +14,19 @@
>  #include <asm/eeh.h>
>  
>  /* We might build address mapping here for "fast" path later */
> +#ifdef CONFIG_EEH
> +
>  int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
>  {
>  	return eeh_dev_open(pdev);
>  }
> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_open);
>  
>  void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
>  {
>  	eeh_dev_release(pdev);
>  }
> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_release);
>  
>  long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>  				unsigned int cmd, unsigned long arg)
> @@ -85,3 +89,5 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(vfio_spapr_iommu_eeh_ioctl);
> +#endif /* CONFIG_EEH */

Re: [PATCH v2 4/4] vfio_pci: spapr: Enable VFIO if EEH is not supported

[Alex Williamson]

On Wed, 2014-08-06 at 12:48 +1000, Alexey Kardashevskiy wrote:
> The existing vfio_pci_open() fails if there is no EEH support for PCI.
> This breaks POWER7's P5IOC2 PHB support which this patch brings back.
> 
> It is a warning because this should not normally happen on supported
> configurations such as POWER8+IODA so we always want to see it in dmesg.
> It is _once() as the message may be triggered by non-privileged userspace
> and we do not want to pollute dmesg.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v2:
> * discussed with Ben Herrenschmidt and did s/pr_warn/pr_warn_once/
> * updated commit log
> ---
>  drivers/vfio/pci/vfio_pci.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index e2ee80f..68dc8da 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -179,10 +179,8 @@ static int vfio_pci_open(void *device_data)
>  			goto error;
>  
>  		ret = vfio_spapr_pci_eeh_open(vdev->pdev);
> -		if (ret) {
> -			vfio_pci_disable(vdev);
> -			goto error;
> -		}
> +		if (ret)
> +			pr_warn_once("EEH is not supported\n");
>  	}
>  
>  	return 0;

Now the next question, what's the point of vfio_spapr_pci_eeh_open()
returning a value?  Couldn't it return void now and this warning can go
into eeh specific code?  Thanks,

Alex

Re: [PATCH v2 2/4] vfio: spapr: Fix build error

[Gavin Shan]

On Tue, Aug 05, 2014 at 09:12:50PM -0600, Alex Williamson wrote:
>On Wed, 2014-08-06 at 12:48 +1000, Alexey Kardashevskiy wrote:
>> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>=20
>> The VFIO related components could be built as dynamic modules.
>> Unfortunately, CONFIG_EEH can't be configured to "m". The patch
>> fixes the build errors when configuring VFIO related components
>> as dynamic modules as follows:
>>=20
>>   CC [M]  drivers/vfio/vfio_iommu_spapr_tce.o
>> In file included from drivers/vfio/vfio.c:33:0:
>> include/linux/vfio.h:101:43: warning: =E2=80=98struct pci_dev=E2=80=99=
 declared \
>> inside parameter list [enabled by default]
>> :
>>   WRAP    arch/powerpc/boot/zImage.pseries
>>   WRAP    arch/powerpc/boot/zImage.maple
>>   WRAP    arch/powerpc/boot/zImage.pmac
>>   WRAP    arch/powerpc/boot/zImage.epapr
>>   MODPOST 1818 modules
>> ERROR: ".vfio_spapr_iommu_eeh_ioctl" [drivers/vfio/vfio_iommu_spapr_tc=
e.ko]\
>> undefined!
>> ERROR: ".vfio_spapr_pci_eeh_open" [drivers/vfio/pci/vfio-pci.ko] undef=
ined!
>> ERROR: ".vfio_spapr_pci_eeh_release" [drivers/vfio/pci/vfio-pci.ko] un=
defined!
>>=20
>> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> [removed include "pci.h" in vfio.c]
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v2:
>> * removed #include <linux/pci.h> from vfio.c and tested

Appreciated for Alexey's help on it :)

>
>I also commented regarding the ifdef around all of vfio_spapr_eeh.c:
>
>        Why not add a new CONFIG_VFIO_SPAPR_EEH option to handle this
>        instead?
>
>Did you disagree?  The ifdef is pretty ugly.
>

I'll introduce CONFIG_VFIO_SPAPR_EEH.

Thanks,
Gavin

>> ---
>>  drivers/vfio/Makefile         | 4 ++--
>>  drivers/vfio/vfio_spapr_eeh.c | 6 ++++++
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>=20
>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>> index 50e30bc..4891cca 100644
>> --- a/drivers/vfio/Makefile
>> +++ b/drivers/vfio/Makefile
>> @@ -1,5 +1,5 @@
>>  obj-$(CONFIG_VFIO) +=3D vfio.o
>>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) +=3D vfio_iommu_type1.o
>> -obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) +=3D vfio_iommu_spapr_tce.o
>> -obj-$(CONFIG_EEH) +=3D vfio_spapr_eeh.o
>> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) +=3D vfio_iommu_spapr_tce.o \
>> +				      vfio_spapr_eeh.o
>>  obj-$(CONFIG_VFIO_PCI) +=3D pci/
>> diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_e=
eh.c
>> index f834b4c..1a93e83 100644
>> --- a/drivers/vfio/vfio_spapr_eeh.c
>> +++ b/drivers/vfio/vfio_spapr_eeh.c
>> @@ -14,15 +14,19 @@
>>  #include <asm/eeh.h>
>> =20
>>  /* We might build address mapping here for "fast" path later */
>> +#ifdef CONFIG_EEH
>> +
>>  int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
>>  {
>>  	return eeh_dev_open(pdev);
>>  }
>> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_open);
>> =20
>>  void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
>>  {
>>  	eeh_dev_release(pdev);
>>  }
>> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_release);
>> =20
>>  long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>>  				unsigned int cmd, unsigned long arg)
>> @@ -85,3 +89,5 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *=
group,
>> =20
>>  	return ret;
>>  }
>> +EXPORT_SYMBOL_GPL(vfio_spapr_iommu_eeh_ioctl);
>> +#endif /* CONFIG_EEH */
>
>
>

Re: [PATCH v2 4/4] vfio_pci: spapr: Enable VFIO if EEH is not supported

[Benjamin Herrenschmidt]

On Tue, 2014-08-05 at 21:44 -0600, Alex Williamson wrote:
> >               ret = vfio_spapr_pci_eeh_open(vdev->pdev);
> > -             if (ret) {
> > -                     vfio_pci_disable(vdev);
> > -                     goto error;
> > -             }
> > +             if (ret)
> > +                     pr_warn_once("EEH is not supported\n");
> >       }
> >  
> >       return 0;
> 
> Now the next question, what's the point of vfio_spapr_pci_eeh_open()
> returning a value?  Couldn't it return void now and this warning can
> go into eeh specific code?  Thanks,

In order to call vfio_pci_disable() when that happens ?

Cheers,
Ben.

Re: [PATCH v2 2/4] vfio: spapr: Fix build error

[Alexey Kardashevskiy]

On 08/06/2014 01:54 PM, Gavin Shan wrote:
> On Tue, Aug 05, 2014 at 09:12:50PM -0600, Alex Williamson wrote:
>> On Wed, 2014-08-06 at 12:48 +1000, Alexey Kardashevskiy wrote:
>>> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>>
>>> The VFIO related components could be built as dynamic modules.
>>> Unfortunately, CONFIG_EEH can't be configured to "m". The patch
>>> fixes the build errors when configuring VFIO related components
>>> as dynamic modules as follows:
>>>
>>>   CC [M]  drivers/vfio/vfio_iommu_spapr_tce.o
>>> In file included from drivers/vfio/vfio.c:33:0:
>>> include/linux/vfio.h:101:43: warning: ‘struct pci_dev’ declared \
>>> inside parameter list [enabled by default]
>>> :
>>>   WRAP    arch/powerpc/boot/zImage.pseries
>>>   WRAP    arch/powerpc/boot/zImage.maple
>>>   WRAP    arch/powerpc/boot/zImage.pmac
>>>   WRAP    arch/powerpc/boot/zImage.epapr
>>>   MODPOST 1818 modules
>>> ERROR: ".vfio_spapr_iommu_eeh_ioctl" [drivers/vfio/vfio_iommu_spapr_tce.ko]\
>>> undefined!
>>> ERROR: ".vfio_spapr_pci_eeh_open" [drivers/vfio/pci/vfio-pci.ko] undefined!
>>> ERROR: ".vfio_spapr_pci_eeh_release" [drivers/vfio/pci/vfio-pci.ko] undefined!
>>>
>>> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>> [removed include "pci.h" in vfio.c]
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> v2:
>>> * removed #include <linux/pci.h> from vfio.c and tested
> 
> Appreciated for Alexey's help on it :)


My bad, that was wrong actually, we still need this:

diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 25a0fbd..224128a 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -98,6 +98,7 @@ extern int vfio_external_user_iommu_id(struct vfio_group
*group);
 extern long vfio_external_check_extension(struct vfio_group *group,
                                          unsigned long arg);

+struct pci_dev;
 #ifdef CONFIG_EEH
 extern int vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
 extern void vfio_spapr_pci_eeh_release(struct pci_dev *pdev);



Otherwise it is "warning: ‘struct pci_dev’ declared inside parameter list".
For some reason I only see this warning when backporting this patch to 3.10
and I do not see it in 3.16-rc7, I guess pci.h gets included somewhere.


> 
>>
>> I also commented regarding the ifdef around all of vfio_spapr_eeh.c:
>>
>>        Why not add a new CONFIG_VFIO_SPAPR_EEH option to handle this
>>        instead?
>>
>> Did you disagree?  The ifdef is pretty ugly.
>>
> 
> I'll introduce CONFIG_VFIO_SPAPR_EEH.


So, Gavin, then the patchset is yours (again), ok?




> Thanks,
> Gavin
> 
>>> ---
>>>  drivers/vfio/Makefile         | 4 ++--
>>>  drivers/vfio/vfio_spapr_eeh.c | 6 ++++++
>>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>>> index 50e30bc..4891cca 100644
>>> --- a/drivers/vfio/Makefile
>>> +++ b/drivers/vfio/Makefile
>>> @@ -1,5 +1,5 @@
>>>  obj-$(CONFIG_VFIO) += vfio.o
>>>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>>> -obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o
>>> -obj-$(CONFIG_EEH) += vfio_spapr_eeh.o
>>> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) += vfio_iommu_spapr_tce.o \
>>> +				      vfio_spapr_eeh.o
>>>  obj-$(CONFIG_VFIO_PCI) += pci/
>>> diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr_eeh.c
>>> index f834b4c..1a93e83 100644
>>> --- a/drivers/vfio/vfio_spapr_eeh.c
>>> +++ b/drivers/vfio/vfio_spapr_eeh.c
>>> @@ -14,15 +14,19 @@
>>>  #include <asm/eeh.h>
>>>  
>>>  /* We might build address mapping here for "fast" path later */
>>> +#ifdef CONFIG_EEH
>>> +
>>>  int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
>>>  {
>>>  	return eeh_dev_open(pdev);
>>>  }
>>> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_open);
>>>  
>>>  void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
>>>  {
>>>  	eeh_dev_release(pdev);
>>>  }
>>> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_release);
>>>  
>>>  long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>>>  				unsigned int cmd, unsigned long arg)
>>> @@ -85,3 +89,5 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>>>  
>>>  	return ret;
>>>  }
>>> +EXPORT_SYMBOL_GPL(vfio_spapr_iommu_eeh_ioctl);
>>> +#endif /* CONFIG_EEH */
>>
>>
>>
> 


-- 
Alexey

Re: [PATCH v2 2/4] vfio: spapr: Fix build error

[Gavin Shan]

On Wed, Aug 06, 2014 at 04:33:29PM +1000, Alexey Kardashevskiy wrote:
>On 08/06/2014 01:54 PM, Gavin Shan wrote:
>> On Tue, Aug 05, 2014 at 09:12:50PM -0600, Alex Williamson wrote:
>>> On Wed, 2014-08-06 at 12:48 +1000, Alexey Kardashevskiy wrote:
>>>> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>>>
>>>> The VFIO related components could be built as dynamic modules.
>>>> Unfortunately, CONFIG_EEH can't be configured to "m". The patch
>>>> fixes the build errors when configuring VFIO related components
>>>> as dynamic modules as follows:
>>>>
>>>>   CC [M]  drivers/vfio/vfio_iommu_spapr_tce.o
>>>> In file included from drivers/vfio/vfio.c:33:0:
>>>> include/linux/vfio.h:101:43: warning: =E2=80=98struct pci_dev=E2=80=99=
 declared \
>>>> inside parameter list [enabled by default]
>>>> :
>>>>   WRAP    arch/powerpc/boot/zImage.pseries
>>>>   WRAP    arch/powerpc/boot/zImage.maple
>>>>   WRAP    arch/powerpc/boot/zImage.pmac
>>>>   WRAP    arch/powerpc/boot/zImage.epapr
>>>>   MODPOST 1818 modules
>>>> ERROR: ".vfio_spapr_iommu_eeh_ioctl" [drivers/vfio/vfio_iommu_spapr_=
tce.ko]\
>>>> undefined!
>>>> ERROR: ".vfio_spapr_pci_eeh_open" [drivers/vfio/pci/vfio-pci.ko] und=
efined!
>>>> ERROR: ".vfio_spapr_pci_eeh_release" [drivers/vfio/pci/vfio-pci.ko] =
undefined!
>>>>
>>>> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>>>> [removed include "pci.h" in vfio.c]
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>> Changes:
>>>> v2:
>>>> * removed #include <linux/pci.h> from vfio.c and tested
>>=20
>> Appreciated for Alexey's help on it :)
>
>
>My bad, that was wrong actually, we still need this:
>

Yep, no worries. I'll fix it in next revision.

>diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>index 25a0fbd..224128a 100644
>--- a/include/linux/vfio.h
>+++ b/include/linux/vfio.h
>@@ -98,6 +98,7 @@ extern int vfio_external_user_iommu_id(struct vfio_gro=
up
>*group);
> extern long vfio_external_check_extension(struct vfio_group *group,
>                                          unsigned long arg);
>
>+struct pci_dev;
> #ifdef CONFIG_EEH
> extern int vfio_spapr_pci_eeh_open(struct pci_dev *pdev);
> extern void vfio_spapr_pci_eeh_release(struct pci_dev *pdev);
>
>
>
>Otherwise it is "warning: =E2=80=98struct pci_dev=E2=80=99 declared insi=
de parameter list".
>For some reason I only see this warning when backporting this patch to 3=
.10
>and I do not see it in 3.16-rc7, I guess pci.h gets included somewhere.
>
>
>>=20
>>>
>>> I also commented regarding the ifdef around all of vfio_spapr_eeh.c:
>>>
>>>        Why not add a new CONFIG_VFIO_SPAPR_EEH option to handle this
>>>        instead?
>>>
>>> Did you disagree?  The ifdef is pretty ugly.
>>>
>>=20
>> I'll introduce CONFIG_VFIO_SPAPR_EEH.
>
>
>So, Gavin, then the patchset is yours (again), ok?
>

Sure. Thanks for your help, Alexey. I was busy with other
bugs with higher priority recently. I'm working on this.

Thanks,
Gavin

>
>
>
>> Thanks,
>> Gavin
>>=20
>>>> ---
>>>>  drivers/vfio/Makefile         | 4 ++--
>>>>  drivers/vfio/vfio_spapr_eeh.c | 6 ++++++
>>>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
>>>> index 50e30bc..4891cca 100644
>>>> --- a/drivers/vfio/Makefile
>>>> +++ b/drivers/vfio/Makefile
>>>> @@ -1,5 +1,5 @@
>>>>  obj-$(CONFIG_VFIO) +=3D vfio.o
>>>>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) +=3D vfio_iommu_type1.o
>>>> -obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) +=3D vfio_iommu_spapr_tce.o
>>>> -obj-$(CONFIG_EEH) +=3D vfio_spapr_eeh.o
>>>> +obj-$(CONFIG_VFIO_IOMMU_SPAPR_TCE) +=3D vfio_iommu_spapr_tce.o \
>>>> +				      vfio_spapr_eeh.o
>>>>  obj-$(CONFIG_VFIO_PCI) +=3D pci/
>>>> diff --git a/drivers/vfio/vfio_spapr_eeh.c b/drivers/vfio/vfio_spapr=
_eeh.c
>>>> index f834b4c..1a93e83 100644
>>>> --- a/drivers/vfio/vfio_spapr_eeh.c
>>>> +++ b/drivers/vfio/vfio_spapr_eeh.c
>>>> @@ -14,15 +14,19 @@
>>>>  #include <asm/eeh.h>
>>>> =20
>>>>  /* We might build address mapping here for "fast" path later */
>>>> +#ifdef CONFIG_EEH
>>>> +
>>>>  int vfio_spapr_pci_eeh_open(struct pci_dev *pdev)
>>>>  {
>>>>  	return eeh_dev_open(pdev);
>>>>  }
>>>> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_open);
>>>> =20
>>>>  void vfio_spapr_pci_eeh_release(struct pci_dev *pdev)
>>>>  {
>>>>  	eeh_dev_release(pdev);
>>>>  }
>>>> +EXPORT_SYMBOL_GPL(vfio_spapr_pci_eeh_release);
>>>> =20
>>>>  long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
>>>>  				unsigned int cmd, unsigned long arg)
>>>> @@ -85,3 +89,5 @@ long vfio_spapr_iommu_eeh_ioctl(struct iommu_group=
 *group,
>>>> =20
>>>>  	return ret;
>>>>  }
>>>> +EXPORT_SYMBOL_GPL(vfio_spapr_iommu_eeh_ioctl);
>>>> +#endif /* CONFIG_EEH */
>>>
>>>
>>>
>>=20
>
>
>--=20
>Alexey
>