Re: [Qemu-devel] [PATCH 1/3] s390: Add PCI bus support

[Frank Blaschka]

On Tue, Nov 25, 2014 at 01:14:01PM +0100, Alexander Graf wrote:
> 
> 
> On 25.11.14 11:11, Frank Blaschka wrote:
> > On Tue, Nov 18, 2014 at 06:00:40PM +0100, Alexander Graf wrote:
> >>
> >>
> >> On 18.11.14 13:50, Frank Blaschka wrote:
> >>> On Mon, Nov 10, 2014 at 04:14:16PM +0100, Alexander Graf wrote:
> >>>>
> >>>>
> >>>> On 10.11.14 15:20, Frank Blaschka wrote:
> >>>>> From: Frank Blaschka <address@hidden>
> >>>>>
> >>>>> This patch implements a pci bus for s390x together with infrastructure
> >>>>> to generate and handle hotplug events, to configure/unconfigure via
> >>>>> sclp instruction, to do iommu translations and provide s390 support for
> >>>>> MSI/MSI-X notification processing.
> >>>>>
> >>>>> Signed-off-by: Frank Blaschka <address@hidden>
> >>
> >> [...]
> >>
> >>>>> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> >>>>> new file mode 100644
> >>>>> index 0000000..f2fa6ba
> >>>>> --- /dev/null
> >>>>> +++ b/hw/s390x/s390-pci-bus.c
> >>>>> @@ -0,0 +1,485 @@
> >>>>> +/*
> >>>>> + * s390 PCI BUS
> >>>>> + *
> >>>>> + * Copyright 2014 IBM Corp.
> >>>>> + * Author(s): Frank Blaschka <address@hidden>
> >>>>> + *            Hong Bo Li <address@hidden>
> >>>>> + *            Yi Min Zhao <address@hidden>
> >>>>> + *
> >>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or 
> >>>>> (at
> >>>>> + * your option) any later version. See the COPYING file in the 
> >>>>> top-level
> >>>>> + * directory.
> >>>>> + */
> >>>>> +
> >>>>> +#include <hw/pci/pci.h>
> >>>>> +#include <hw/pci/pci_bus.h>
> >>>>> +#include <hw/s390x/css.h>
> >>>>> +#include <hw/s390x/sclp.h>
> >>>>> +#include <hw/pci/msi.h>
> >>>>> +#include "qemu/error-report.h"
> >>>>> +#include "s390-pci-bus.h"
> >>>>> +
> >>>>> +/* #define DEBUG_S390PCI_BUS */
> >>>>> +#ifdef DEBUG_S390PCI_BUS
> >>>>> +#define DPRINTF(fmt, ...) \
> >>>>> +    do { fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); } while 
> >>>>> (0)
> >>>>> +#else
> >>>>> +#define DPRINTF(fmt, ...) \
> >>>>> +    do { } while (0)
> >>>>> +#endif
> >>>>> +
> >>>>> +static const unsigned long be_to_le = BITS_PER_LONG - 1;
> >>>>> +static QTAILQ_HEAD(, SeiContainer) pending_sei =
> >>>>> +    QTAILQ_HEAD_INITIALIZER(pending_sei);
> >>>>> +static QTAILQ_HEAD(, S390PCIBusDevice) device_list =
> >>>>> +    QTAILQ_HEAD_INITIALIZER(device_list);
> >>>>
> >>>> Please get rid of all statics ;). All state has to live in objects.
> >>>>
> >>>
> >>> be_to_le was misleading and unnecesary will remove this one but
> >>> static QTAILQ_HEAD seems to be a common practice for list anchors.
> >>> If you really want me to change this do you have any prefered way,
> >>> or can you point me to some code doing this?
> >>
> >> For PCI devices, I don't think you need a list at all. Your PHB device
> >> should already have a proper qbus that knows about all its child devices.
> > 
> > OK
> > 
> >>
> >> As for pending_sei, what is this about?
> >>
> > 
> > This is a queue to store events (StoreEventInformation) used for hotplug
> > support. In case a device is pluged/unpluged an event is stored to this 
> > queue
> > and the guest is notified. Then the guest pick up the event information via
> > chsc instruction.
> 
> Is this for overall CCW or only for PCI? Depending on the answer, you
> can put the sei event list into the respective parent device.
>

An NT2 event is pci specific. So I moved the queue for NT2 events to the PHB as 
well.

> 
> Alex
>