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

[Jason Wang <jasowang@redhat.com>]

On Fri, Feb 27, 2015 at 5:41 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Fri, 27 Feb 2015 03:46:25 +0008
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  On Thu, Feb 26, 2015 at 9:02 PM, Cornelia Huck 
>>  <cornelia.huck@de.ibm.com> wrote:
>>  > On Thu, 26 Feb 2015 15:04:39 +0800
>>  > Jason Wang <jasowang@redhat.com> wrote:
>>  > 
>>  >>  Instead of depending on marco, using a bus specific limit.
>>  >>  
>>  >>  Cc: Alexander Graf <agraf@suse.de>
>>  >>  Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>>  >>  Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>  >>  Cc: Richard Henderson <rth@twiddle.net>
>>  >>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  >>  ---
>>  >>   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.

Ok, I thought s390 may have something like pc had to keep the migration 
compatibility for migration. But seems not.

> 
> 
>>  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?

As I said in the previous reply. The machine code is used to keep 
migration compatibility. We don't want to break the migration from 2.3 
to 2.2. If the limit of ccw will be increased in the future and we want 
to keep migration compatibility. The machine codes may do something 
similar to virtio pci.


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