| Message ID | 1456486323-8047-5-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
On 02/26/2016 10:31 PM, David Gibson wrote: > Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure() > into rtas_ibm_configure_pe(). > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > hw/ppc/spapr_pci.c | 11 +++++++++-- > hw/ppc/spapr_pci_vfio.c | 12 ------------ > include/hw/pci-host/spapr.h | 1 - > 3 files changed, 9 insertions(+), 15 deletions(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index d1e5222..fa633a9 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -42,6 +42,9 @@ > #include "hw/ppc/spapr_drc.h" > #include "sysemu/device_tree.h" > > +#include "hw/vfio/vfio.h" > +#include <linux/vfio.h> > + > /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ > #define RTAS_QUERY_FN 0 > #define RTAS_CHANGE_FN 1 > @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu, > goto param_error_exit; > } > > - ret = spapr_phb_vfio_eeh_configure(sphb); > - rtas_st(rets, 0, ret); > + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); > + if (ret < 0) { > + goto param_error_exit; > + } > + > + rtas_st(rets, 0, RTAS_OUT_SUCCESS); > return; > > param_error_exit: > diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c > index 10fa88a..4ac5736 100644 > --- a/hw/ppc/spapr_pci_vfio.c > +++ b/hw/ppc/spapr_pci_vfio.c > @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) > return RTAS_OUT_SUCCESS; > } > > -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) > -{ > - int ret; > - > - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); > - if (ret < 0) { > - return RTAS_OUT_PARAM_ERROR; > - } > - > - return RTAS_OUT_SUCCESS; > -} > - > static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index cc1b82c..f02ef51 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, > unsigned int addr, int option); > int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state); > int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option); > -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb); > > #endif /* __HW_SPAPR_PCI_H__ */ >
On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote: > On 02/26/2016 10:31 PM, David Gibson wrote: >> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure() >> into rtas_ibm_configure_pe(). >> >> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> Aaaand this breaks mingw32: CC ppc64-softmmu/hw/ppc/spapr_pci.o /home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:46:24: fatal error: linux/vfio.h: No such file or directory compilation terminated. /home/aik/p/qemu-dwg-eeh/rules.mak:57: recipe for target 'hw/ppc/spapr_pci.o' failed make[1]: *** [hw/ppc/spapr_pci.o] Error 1 make[1]: *** Waiting for unfinished jobs.... Makefile:186: recipe for target 'subdir-ppc64-softmmu' failed make: *** [subdir-ppc64-softmmu] Error 2 make: Leaving directory '/scratch/aik/p/qemu-dwg-eeh--ppc64_mingw32-build' > >> --- >> hw/ppc/spapr_pci.c | 11 +++++++++-- >> hw/ppc/spapr_pci_vfio.c | 12 ------------ >> include/hw/pci-host/spapr.h | 1 - >> 3 files changed, 9 insertions(+), 15 deletions(-) >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index d1e5222..fa633a9 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -42,6 +42,9 @@ >> #include "hw/ppc/spapr_drc.h" >> #include "sysemu/device_tree.h" >> >> +#include "hw/vfio/vfio.h" >> +#include <linux/vfio.h> >> + >> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ >> #define RTAS_QUERY_FN 0 >> #define RTAS_CHANGE_FN 1 >> @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu, >> goto param_error_exit; >> } >> >> - ret = spapr_phb_vfio_eeh_configure(sphb); >> - rtas_st(rets, 0, ret); >> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); >> + if (ret < 0) { >> + goto param_error_exit; >> + } >> + >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> return; >> >> param_error_exit: >> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c >> index 10fa88a..4ac5736 100644 >> --- a/hw/ppc/spapr_pci_vfio.c >> +++ b/hw/ppc/spapr_pci_vfio.c >> @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, >> int option) >> return RTAS_OUT_SUCCESS; >> } >> >> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) >> -{ >> - int ret; >> - >> - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); >> - if (ret < 0) { >> - return RTAS_OUT_PARAM_ERROR; >> - } >> - >> - return RTAS_OUT_SUCCESS; >> -} >> - >> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc = DEVICE_CLASS(klass); >> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h >> index cc1b82c..f02ef51 100644 >> --- a/include/hw/pci-host/spapr.h >> +++ b/include/hw/pci-host/spapr.h >> @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, >> unsigned int addr, int option); >> int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state); >> int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option); >> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb); >> >> #endif /* __HW_SPAPR_PCI_H__ */ >> > >
On 02/29/2016 02:45 PM, Alexey Kardashevskiy wrote: > On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote: >> On 02/26/2016 10:31 PM, David Gibson wrote: >>> Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure() >>> into rtas_ibm_configure_pe(). >>> >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >> >> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > Aaaand this breaks mingw32: > > CC ppc64-softmmu/hw/ppc/spapr_pci.o > /home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:46:24: fatal error: > linux/vfio.h: No such file or directory > compilation terminated. > /home/aik/p/qemu-dwg-eeh/rules.mak:57: recipe for target > 'hw/ppc/spapr_pci.o' failed > make[1]: *** [hw/ppc/spapr_pci.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > Makefile:186: recipe for target 'subdir-ppc64-softmmu' failed > make: *** [subdir-ppc64-softmmu] Error 2 > make: Leaving directory '/scratch/aik/p/qemu-dwg-eeh--ppc64_mingw32-build' > > > >> >>> --- >>> hw/ppc/spapr_pci.c | 11 +++++++++-- >>> hw/ppc/spapr_pci_vfio.c | 12 ------------ >>> include/hw/pci-host/spapr.h | 1 - >>> 3 files changed, 9 insertions(+), 15 deletions(-) >>> >>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >>> index d1e5222..fa633a9 100644 >>> --- a/hw/ppc/spapr_pci.c >>> +++ b/hw/ppc/spapr_pci.c >>> @@ -42,6 +42,9 @@ >>> #include "hw/ppc/spapr_drc.h" >>> #include "sysemu/device_tree.h" >>> >>> +#include "hw/vfio/vfio.h" >>> +#include <linux/vfio.h> >>> + This is missing: #ifdef CONFIG_LINUX #include <linux/vfio.h> #endif and below where you use symbols from linux/vfio.h. My version of this rework did convert class callbacks to exported helpers and keep these helpers (plus stubs) in spapr_pci_vfio.c, with one #ifdef CONFIG_LINUX. Looked quite clean to me... >>> /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ >>> #define RTAS_QUERY_FN 0 >>> #define RTAS_CHANGE_FN 1 >>> @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu, >>> goto param_error_exit; >>> } >>> >>> - ret = spapr_phb_vfio_eeh_configure(sphb); >>> - rtas_st(rets, 0, ret); >>> + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); >>> + if (ret < 0) { >>> + goto param_error_exit; >>> + } >>> + >>> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >>> return; >>> >>> param_error_exit: >>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c >>> index 10fa88a..4ac5736 100644 >>> --- a/hw/ppc/spapr_pci_vfio.c >>> +++ b/hw/ppc/spapr_pci_vfio.c >>> @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, >>> int option) >>> return RTAS_OUT_SUCCESS; >>> } >>> >>> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) >>> -{ >>> - int ret; >>> - >>> - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); >>> - if (ret < 0) { >>> - return RTAS_OUT_PARAM_ERROR; >>> - } >>> - >>> - return RTAS_OUT_SUCCESS; >>> -} >>> - >>> static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) >>> { >>> DeviceClass *dc = DEVICE_CLASS(klass); >>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h >>> index cc1b82c..f02ef51 100644 >>> --- a/include/hw/pci-host/spapr.h >>> +++ b/include/hw/pci-host/spapr.h >>> @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, >>> unsigned int addr, int option); >>> int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state); >>> int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option); >>> -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb); >>> >>> #endif /* __HW_SPAPR_PCI_H__ */
On Mon, Feb 29, 2016 at 03:25:24PM +1100, Alexey Kardashevskiy wrote: > On 02/29/2016 02:45 PM, Alexey Kardashevskiy wrote: > >On 02/29/2016 12:43 PM, Alexey Kardashevskiy wrote: > >>On 02/26/2016 10:31 PM, David Gibson wrote: > >>>Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure() > >>>into rtas_ibm_configure_pe(). > >>> > >>>Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > >> > >>Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > > >Aaaand this breaks mingw32: > > > > CC ppc64-softmmu/hw/ppc/spapr_pci.o > >/home/aik/p/qemu-dwg-eeh/hw/ppc/spapr_pci.c:46:24: fatal error: > >linux/vfio.h: No such file or directory > >compilation terminated. > >/home/aik/p/qemu-dwg-eeh/rules.mak:57: recipe for target > >'hw/ppc/spapr_pci.o' failed > >make[1]: *** [hw/ppc/spapr_pci.o] Error 1 > >make[1]: *** Waiting for unfinished jobs.... > >Makefile:186: recipe for target 'subdir-ppc64-softmmu' failed > >make: *** [subdir-ppc64-softmmu] Error 2 > >make: Leaving directory '/scratch/aik/p/qemu-dwg-eeh--ppc64_mingw32-build' > > > > > > > >> > >>>--- > >>> hw/ppc/spapr_pci.c | 11 +++++++++-- > >>> hw/ppc/spapr_pci_vfio.c | 12 ------------ > >>> include/hw/pci-host/spapr.h | 1 - > >>> 3 files changed, 9 insertions(+), 15 deletions(-) > >>> > >>>diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > >>>index d1e5222..fa633a9 100644 > >>>--- a/hw/ppc/spapr_pci.c > >>>+++ b/hw/ppc/spapr_pci.c > >>>@@ -42,6 +42,9 @@ > >>> #include "hw/ppc/spapr_drc.h" > >>> #include "sysemu/device_tree.h" > >>> > >>>+#include "hw/vfio/vfio.h" > >>>+#include <linux/vfio.h> > >>>+ > > This is missing: > > #ifdef CONFIG_LINUX > #include <linux/vfio.h> > #endif > > and below where you use symbols from linux/vfio.h. > > > My version of this rework did convert class callbacks to exported helpers > and keep these helpers (plus stubs) in spapr_pci_vfio.c, with one #ifdef > CONFIG_LINUX. Looked quite clean to me... Yeah, good idea. I'd forgotten the case of non-Linux builds. I'll do something similar in v2.
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index d1e5222..fa633a9 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -42,6 +42,9 @@ #include "hw/ppc/spapr_drc.h" #include "sysemu/device_tree.h" +#include "hw/vfio/vfio.h" +#include <linux/vfio.h> + /* Copied from the kernel arch/powerpc/platforms/pseries/msi.c */ #define RTAS_QUERY_FN 0 #define RTAS_CHANGE_FN 1 @@ -628,8 +631,12 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu, goto param_error_exit; } - ret = spapr_phb_vfio_eeh_configure(sphb); - rtas_st(rets, 0, ret); + ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); + if (ret < 0) { + goto param_error_exit; + } + + rtas_st(rets, 0, RTAS_OUT_SUCCESS); return; param_error_exit: diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c index 10fa88a..4ac5736 100644 --- a/hw/ppc/spapr_pci_vfio.c +++ b/hw/ppc/spapr_pci_vfio.c @@ -221,18 +221,6 @@ int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option) return RTAS_OUT_SUCCESS; } -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb) -{ - int ret; - - ret = vfio_eeh_as_op(&sphb->iommu_as, VFIO_EEH_PE_CONFIGURE); - if (ret < 0) { - return RTAS_OUT_PARAM_ERROR; - } - - return RTAS_OUT_SUCCESS; -} - static void spapr_phb_vfio_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h index cc1b82c..f02ef51 100644 --- a/include/hw/pci-host/spapr.h +++ b/include/hw/pci-host/spapr.h @@ -140,6 +140,5 @@ int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb, unsigned int addr, int option); int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state); int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option); -int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb); #endif /* __HW_SPAPR_PCI_H__ */
Simplify the sPAPR PCI code by folding spapr_phb_vfio_eeh_configure() into rtas_ibm_configure_pe(). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr_pci.c | 11 +++++++++-- hw/ppc/spapr_pci_vfio.c | 12 ------------ include/hw/pci-host/spapr.h | 1 - 3 files changed, 9 insertions(+), 15 deletions(-)