Re: [Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Cornelia Huck]

On Fri, 27 Feb 2015 03:46:25 +0008
Jason Wang <address@hidden> wrote:

> On Thu, Feb 26, 2015 at 9:02 PM, Cornelia Huck 
> <address@hidden> wrote:
> > On Thu, 26 Feb 2015 15:04:39 +0800
> > Jason Wang <address@hidden> wrote:
> > 
> >>  Instead of depending on marco, using a bus specific limit.
> >>  
> >>  Cc: Alexander Graf <address@hidden>
> >>  Cc: Cornelia Huck <address@hidden>
> >>  Cc: Christian Borntraeger <address@hidden>
> >>  Cc: Richard Henderson <address@hidden>
> >>  Signed-off-by: Jason Wang <address@hidden>
> >>  ---
> >>   hw/s390x/s390-virtio-ccw.c |  7 +++++--
> >>   hw/s390x/virtio-ccw.c      | 13 +++++++------
> >>   include/hw/virtio/virtio.h |  1 +
> >>   3 files changed, 13 insertions(+), 8 deletions(-)
> >>  
> >>  diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >>  index 71bafe0..6aeb815 100644
> >>  --- a/hw/s390x/s390-virtio-ccw.c
> >>  +++ b/hw/s390x/s390-virtio-ccw.c
> >>  @@ -43,6 +43,7 @@ void io_subsystem_reset(void)
> >>  
> >>   static int virtio_ccw_hcall_notify(const uint64_t *args)
> >>   {
> >>  +    VirtIODevice *vdev;
> >>       uint64_t subch_id = args[0];
> >>       uint64_t queue = args[1];
> >>       SubchDev *sch;
> >>  @@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const 
> >> uint64_t *args)
> >>       if (!sch || !css_subch_visible(sch)) {
> >>           return -EINVAL;
> >>       }
> >>  -    if (queue >= VIRTIO_PCI_QUEUE_MAX) {
> >>  +
> >>  +    vdev = virtio_ccw_get_vdev(sch);
> >>  +    if (queue >= virtio_get_queue_max(vdev)) {
> > 
> > But we already know we have a virtio_ccw device, right? So why not 
> > just
> > check against VIRTIO_CCW_QUEUE_MAX?
> 
> The problem is whether or not you want to increase the max queue for 
> ccw. If yes, for migration compatibility, you could not just use a 
> marco here since legacy machine type will also get increased. 

Confused. With "legacy machine type", do you mean s390-virtio? It does
not register this hcall.

> Something 
> dynamically like this is need so the machine initialization code can 
> change this limit. 

I fear I don't understand how the machine code comes into play here. Is
the virtio-ccw device code supposed to inherit the limit from the
machine?

Anyway, if we move the queue limit to the VirtIODevice, checking the
limit there instead of using a #define should work out fine.