On Tue, Nov 24, 2015 at 10:00:15AM +0100, Alexander Graf wrote:
>
>
> On 19.11.15 05:29, David Gibson wrote:
> > The EEH operations in the spapr-vfio-pci-host-bridge no longer rely on the
> > special groupid field in sPAPRPHBVFIOState. So we can simplify, removing
> > the class specific callbacks with direct calls based on a simple
> > spapr_phb_eeh_enabled() helper. For now we implement that in terms of
> > a boolean in the class, but we'll continue to clean that up later.
> >
> > On its own this is a rather strange way of doing things, but it's a useful
> > intermediate step to further cleanups.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr_pci.c | 44
> > ++++++++++++++++++++++----------------------
> > hw/ppc/spapr_pci_vfio.c | 18 +++++++-----------
> > include/hw/pci-host/spapr.h | 13 +++++++++----
> > 3 files changed, 38 insertions(+), 37 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 55fa8db..9203d15 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -91,6 +91,13 @@ PCIDevice *spapr_pci_find_dev(sPAPRMachineState *spapr,
> > uint64_t buid,
> > return pci_find_device(phb->bus, bus_num, devfn);
> > }
> >
> > +static bool spapr_phb_eeh_available(sPAPRPHBState *sphb)
> > +{
> > + sPAPRPHBClass *spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> > +
> > + return spc->eeh_available;
> > +}
> > +
> > static uint32_t rtas_pci_cfgaddr(uint32_t arg)
> > {
> > /* This handles the encoding of extended config space addresses */
> > @@ -430,7 +437,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> > target_ulong rets)
> > {
> > sPAPRPHBState *sphb;
> > - sPAPRPHBClass *spc;
> > uint32_t addr, option;
> > uint64_t buid;
> > int ret;
> > @@ -448,12 +454,11 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
> > goto param_error_exit;
> > }
> >
> > - spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
> > - if (!spc->eeh_set_option) {
> > + if (!spapr_phb_eeh_available(sphb)) {
> > goto param_error_exit;
> > }
> >
> > - ret = spc->eeh_set_option(sphb, addr, option);
> > + ret = spapr_phb_vfio_eeh_set_option(sphb, addr, option);
>
> First of all, I think the direction you're taking with this series is sound.
>
> However the hunk above would break compilation on systems that don't
> have VFIO enabled (such as Windows hosts for example).
Ah drat, good point.
> The same holds true for the defines you're using later in the series.
> Things like VFIO_EEH_PE_GET_STATE are not defined for non-Linux hosts.
>
> I think the best path to get to where we really want to be is to define
> qemu internal defines and eeh helpers for emulated phb mode. Make sure
> the constants match the VFIO constants and use them interchangibly (with
> a big fat comment saying that they are, and a number of ifdefs making
> sure they stay identical).
I'll see what I can do.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
