Re: [Qemu-devel] [PATCH v4 3/5] s390x: Add I/O adapter registration.

[Cornelia Huck]

On Wed, 9 Apr 2014 17:53:40 +0200
Alexander Graf <address@hidden> wrote:

> 
> 
> > Am 09.04.2014 um 17:35 schrieb Cornelia Huck <address@hidden>:
> > 
> > On Wed, 09 Apr 2014 16:30:33 +0200
> > Alexander Graf <address@hidden> wrote:
> > 
> >> 
> >>> On 09.04.14 16:24, Cornelia Huck wrote:
> >>> On Wed, 09 Apr 2014 16:05:00 +0200
> >>> Alexander Graf <address@hidden> wrote:
> >>> 
> >>>>> On 09.04.14 13:34, Cornelia Huck wrote:
> >>>>> Register an I/O adapter interrupt source for when virtio-ccw devices 
> >>>>> start
> >>>>> using adapter interrupts.
> >>>>> 
> >>>>> Reviewed-by: Thomas Huth <address@hidden>
> >>>>> Signed-off-by: Cornelia Huck <address@hidden>
> >>>>> ---
> >>>>>   hw/intc/s390_flic.c   |   59 
> >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>   hw/s390x/css.c        |   51 
> >>>>> ++++++++++++++++++++++++++++++++++++++++++
> >>>>>   hw/s390x/css.h        |    4 ++++
> >>>>>   hw/s390x/virtio-ccw.c |    4 ++++
> >>>>>   hw/s390x/virtio-ccw.h |    1 +
> >>>>>   target-s390x/cpu.h    |   33 +++++++++++++++++++++++++++
> >>>>>   6 files changed, 152 insertions(+)
> >>>>> 
> >>>>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> >>>>> index 2bf0af8..1193682 100644
> >>>>> --- a/hw/s390x/virtio-ccw.c
> >>>>> +++ b/hw/s390x/virtio-ccw.c
> >>>>> @@ -522,6 +522,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> >>>>>                   dev->thinint_isc = thinint->isc;
> >>>>>                   dev->ind_bit = thinint->ind_bit;
> >>>>>                   cpu_physical_memory_unmap(thinint, hw_len, 0, hw_len);
> >>>>> +                ret = css_register_io_adapter(CSS_IO_ADAPTER_VIRTIO,
> >>>>> +                                              dev->thinint_isc, true, 
> >>>>> false,
> >>>>> +                                              &dev->adapter_id);
> >>>> In all other machines the machine file is the one creating the link
> >>>> between a device and the interrupt controller. Can we do something
> >>>> similar for s390?
> >>> Hm. This would imply we'd need to add a virtio I/O adapter for each isc
> >>> (0-7) at startup, regardless whether the guest enables adapter
> >>> interrupts on any of those iscs. Moreover, we'd need to do the same for
> >>> each type (on each isc) if we add more types of I/O adapters later
> >>> (should we want to support one of the other adapter-interrupt using
> >>> devices). I'd prefer to add an I/O adapter only when needed.
> >> 
> >> I'm not sure I can follow you here. Instead of registering the interrupt 
> >> vector on the fly, you would still register it on the fly, but after the 
> >> virtio-ccw device got created, no?
> > 
> > You mean register-at-device-creation instead of
> > register-while-interpreting-ccw? We'd end up with the same problem: We
> > don't know which isc the guest wants to use for adapter interrupts at
> > that point in time, so we would need to register for all iscs. I don't
> > think that is what we want.
> 
> Well, if we only assign a route to the interrupt delivery path, the guest can 
> then configure that previously set up route to the respective isc, or am I 
> missing something obvious?

The guest might want to use different iscs for different devices.
Currently, the first caller for an isc registers the adapter, further
users just use it. If we had all devices register for (say) isc 0, we'd
have one adapter for that. If the first guest devices uses isc 3, we
could switch this one to isc 3, but it would still be one adapter. If
now another guest device uses isc 7, we'd need to register a new
adapter for isc 7 anyway.