[Qemu-devel] [PATCH 0/1] virtio-scsi-ccw: fix iotest 068 on s390x

[Qemu-devel] [PATCH 0/1] virtio-scsi-ccw: fix iotest 068 on s390x

[QingFeng Hao]

This commit fixes iotest 068 for s390x as s390x uses virtio-scsi-ccw.
The related commit is c324fd0a39c for other platforms by Stefan Hajnoczi. 
Thanks!

QingFeng Hao (1):
  virtio-scsi-ccw: use ioeventfd even when KVM is disabled

 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/kvm.c    | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.11.2

[Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/kvm.c    | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb9ff..35896eb007 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
         sch->cssid, sch->ssid, sch->schid, sch->devno,
         ccw_dev->devno.valid ? "user-configured" : "auto-configured");
 
-    if (!kvm_eventfds_enabled()) {
+    if (kvm_enabled() && !kvm_eventfds_enabled()) {
         dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
     }
 
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a3d00196f4..c37f9c3b9e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
         .addr = sch,
         .len = 8,
     };
+    if (!kvm_enabled()) {
+        return 0;
+    }
     if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
         return -ENOSYS;
     }
-- 
2.11.2

Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Christian Borntraeger]

On 07/03/2017 09:38 AM, QingFeng Hao wrote:
> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
> qtest or TCG mode.
> 
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
> 
> I have tested that virtio-scsi-ccw works under tcg both with and without
> iothread.
> 
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
> 
> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

cut'n'paste mistake of adding Stefans signoff?

Otherwise it looks good.
> ---
>  hw/s390x/virtio-ccw.c | 2 +-
>  target/s390x/kvm.c    | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>          sch->cssid, sch->ssid, sch->schid, sch->devno,
>          ccw_dev->devno.valid ? "user-configured" : "auto-configured");
> 
> -    if (!kvm_eventfds_enabled()) {
> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
>          dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>      }
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a3d00196f4..c37f9c3b9e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>          .addr = sch,
>          .len = 8,
>      };
> +    if (!kvm_enabled()) {
> +        return 0;
> +    }
>      if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>          return -ENOSYS;
>      }
> 

Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/7/3 15:41, Christian Borntraeger 写道:
> On 07/03/2017 09:38 AM, QingFeng Hao wrote:
>> Do not check kvm_eventfds_enabled() when KVM is disabled since it
>> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
>> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
>> qtest or TCG mode.
>>
>> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
>> when KVM is disabled.
>>
>> I have tested that virtio-scsi-ccw works under tcg both with and without
>> iothread.
>>
>> This patch fixes qemu-iotests 068, which was accidentally merged early
>> despite the dependency on ioeventfd.
>>
>> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> cut'n'paste mistake of adding Stefans signoff?
>
> Otherwise it looks good.
I just want to mark that this patch is related with the former one from 
Stefan.
Is that ok to add this sign-off? thanks!
>> ---
>>   hw/s390x/virtio-ccw.c | 2 +-
>>   target/s390x/kvm.c    | 3 +++
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index 90d37cb9ff..35896eb007 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>>           sch->cssid, sch->ssid, sch->schid, sch->devno,
>>           ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>>
>> -    if (!kvm_eventfds_enabled()) {
>> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
>>           dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>>       }
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index a3d00196f4..c37f9c3b9e 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>           .addr = sch,
>>           .len = 8,
>>       };
>> +    if (!kvm_enabled()) {
>> +        return 0;
>> +    }
>>       if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>>           return -ENOSYS;
>>       }
>>

-- 
Regards
QingFeng Hao

Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Christian Borntraeger]

On 07/03/2017 10:08 AM, QingFeng Hao wrote:
> 
> 
> 在 2017/7/3 15:41, Christian Borntraeger 写道:
>> On 07/03/2017 09:38 AM, QingFeng Hao wrote:
>>> Do not check kvm_eventfds_enabled() when KVM is disabled since it
>>> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
>>> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
>>> qtest or TCG mode.
>>>
>>> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
>>> when KVM is disabled.
>>>
>>> I have tested that virtio-scsi-ccw works under tcg both with and without
>>> iothread.
>>>
>>> This patch fixes qemu-iotests 068, which was accidentally merged early
>>> despite the dependency on ioeventfd.
>>>
>>> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> cut'n'paste mistake of adding Stefans signoff?
>>
>> Otherwise it looks good.
> I just want to mark that this patch is related with the former one from Stefan.
> Is that ok to add this sign-off? thanks!

No, sign-off indicates who passes the patch along for integration, so only Stefan
is allowed to add that - if he actually takes the patch. It is very important to 
not mangle the sign-off-chain as it is actually used to track how a patch moved from
the developer into the tree.

You can give credit to Stefan in the patch description - e.g. by saying in the patch
description something like 

based on a similar patch from Stefan Hajnoczi - commit c324fd0a39c (" virtio-pci: use 
ioeventfd even when KVM is disabled)



>>> ---
>>>   hw/s390x/virtio-ccw.c | 2 +-
>>>   target/s390x/kvm.c    | 3 +++
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>>> index 90d37cb9ff..35896eb007 100644
>>> --- a/hw/s390x/virtio-ccw.c
>>> +++ b/hw/s390x/virtio-ccw.c
>>> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>>>           sch->cssid, sch->ssid, sch->schid, sch->devno,
>>>           ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>>>
>>> -    if (!kvm_eventfds_enabled()) {
>>> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
>>>           dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>>>       }
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index a3d00196f4..c37f9c3b9e 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>>           .addr = sch,
>>>           .len = 8,
>>>       };
>>> +    if (!kvm_enabled()) {
>>> +        return 0;
>>> +    }
>>>       if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>>>           return -ENOSYS;
>>>       }
>>>
> 

Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/7/3 16:21, Christian Borntraeger 写道:
> On 07/03/2017 10:08 AM, QingFeng Hao wrote:
>>
>> 在 2017/7/3 15:41, Christian Borntraeger 写道:
>>> On 07/03/2017 09:38 AM, QingFeng Hao wrote:
>>>> Do not check kvm_eventfds_enabled() when KVM is disabled since it
>>>> always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
>>>> ("memory: emulate ioeventfd") it has been possible to use ioeventfds in
>>>> qtest or TCG mode.
>>>>
>>>> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
>>>> when KVM is disabled.
>>>>
>>>> I have tested that virtio-scsi-ccw works under tcg both with and without
>>>> iothread.
>>>>
>>>> This patch fixes qemu-iotests 068, which was accidentally merged early
>>>> despite the dependency on ioeventfd.
>>>>
>>>> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> cut'n'paste mistake of adding Stefans signoff?
>>>
>>> Otherwise it looks good.
>> I just want to mark that this patch is related with the former one from Stefan.
>> Is that ok to add this sign-off? thanks!
> No, sign-off indicates who passes the patch along for integration, so only Stefan
> is allowed to add that - if he actually takes the patch. It is very important to
> not mangle the sign-off-chain as it is actually used to track how a patch moved from
> the developer into the tree.
>
> You can give credit to Stefan in the patch description - e.g. by saying in the patch
> description something like
>
> based on a similar patch from Stefan Hajnoczi - commit c324fd0a39c (" virtio-pci: use
> ioeventfd even when KVM is disabled)
Thanks for your good explanation and I'll change the commit message.
>
>
>
>>>> ---
>>>>    hw/s390x/virtio-ccw.c | 2 +-
>>>>    target/s390x/kvm.c    | 3 +++
>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>>>> index 90d37cb9ff..35896eb007 100644
>>>> --- a/hw/s390x/virtio-ccw.c
>>>> +++ b/hw/s390x/virtio-ccw.c
>>>> @@ -711,7 +711,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>>>>            sch->cssid, sch->ssid, sch->schid, sch->devno,
>>>>            ccw_dev->devno.valid ? "user-configured" : "auto-configured");
>>>>
>>>> -    if (!kvm_eventfds_enabled()) {
>>>> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
>>>>            dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>>>>        }
>>>>
>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>> index a3d00196f4..c37f9c3b9e 100644
>>>> --- a/target/s390x/kvm.c
>>>> +++ b/target/s390x/kvm.c
>>>> @@ -2220,6 +2220,9 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>>>            .addr = sch,
>>>>            .len = 8,
>>>>        };
>>>> +    if (!kvm_enabled()) {
>>>> +        return 0;
>>>> +    }
>>>>        if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>>>>            return -ENOSYS;
>>>>        }
>>>>

-- 
Regards
QingFeng Hao

Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Cornelia Huck]

On Mon,  3 Jul 2017 09:38:36 +0200
QingFeng Hao <haoqf@linux.vnet.ibm.com> wrote:

> Do not check kvm_eventfds_enabled() when KVM is disabled since it
> always returns 0.  Since commit
> 8c56c1a592b5092d91da8d8943c17777d6462a6f ("memory: emulate
> ioeventfd") it has been possible to use ioeventfds in qtest or TCG
> mode.
> 
> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
> when KVM is disabled.
> 
> I have tested that virtio-scsi-ccw works under tcg both with and
> without iothread.
> 
> This patch fixes qemu-iotests 068, which was accidentally merged early
> despite the dependency on ioeventfd.
> 
> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/s390x/virtio-ccw.c | 2 +-
>  target/s390x/kvm.c    | 3 +++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 90d37cb9ff..35896eb007 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -711,7 +711,7 @@ static void
> virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
> sch->cssid, sch->ssid, sch->schid, sch->devno, ccw_dev->devno.valid ?
> "user-configured" : "auto-configured"); 
> -    if (!kvm_eventfds_enabled()) {
> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
>          dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>      }
>  
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a3d00196f4..c37f9c3b9e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2220,6 +2220,9 @@ int
> kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t
> sch, .addr = sch, .len = 8,
>      };
> +    if (!kvm_enabled()) {
> +        return 0;
> +    }

I'd prefer if you moved the kvm_enabled() check into
s390_assign_subch_ioeventfd().

>      if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>          return -ENOSYS;
>      }

Re: [Qemu-devel] [PATCH 1/1] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/7/3 19:48, Cornelia Huck 写道:
> On Mon,  3 Jul 2017 09:38:36 +0200
> QingFeng Hao <haoqf@linux.vnet.ibm.com> wrote:
>
>> Do not check kvm_eventfds_enabled() when KVM is disabled since it
>> always returns 0.  Since commit
>> 8c56c1a592b5092d91da8d8943c17777d6462a6f ("memory: emulate
>> ioeventfd") it has been possible to use ioeventfds in qtest or TCG
>> mode.
>>
>> This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
>> when KVM is disabled.
>>
>> I have tested that virtio-scsi-ccw works under tcg both with and
>> without iothread.
>>
>> This patch fixes qemu-iotests 068, which was accidentally merged early
>> despite the dependency on ioeventfd.
>>
>> Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>   hw/s390x/virtio-ccw.c | 2 +-
>>   target/s390x/kvm.c    | 3 +++
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index 90d37cb9ff..35896eb007 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -711,7 +711,7 @@ static void
>> virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>> sch->cssid, sch->ssid, sch->schid, sch->devno, ccw_dev->devno.valid ?
>> "user-configured" : "auto-configured");
>> -    if (!kvm_eventfds_enabled()) {
>> +    if (kvm_enabled() && !kvm_eventfds_enabled()) {
>>           dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
>>       }
>>   
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index a3d00196f4..c37f9c3b9e 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -2220,6 +2220,9 @@ int
>> kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t
>> sch, .addr = sch, .len = 8,
>>       };
>> +    if (!kvm_enabled()) {
>> +        return 0;
>> +    }
> I'd prefer if you moved the kvm_enabled() check into
> s390_assign_subch_ioeventfd().
Thanks and I'll change it just as Christian's comment.
>
>>       if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) {
>>           return -ENOSYS;
>>       }

-- 
Regards
QingFeng Hao