| Message ID | fee6eaa0-8132-b3ce-bb3e-0a716eb72064@de.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 3 Jul 2017 12:20:03 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 07/03/2017 10:51 AM, QingFeng Hao wrote: > > > 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; > > + } > > thinking more about it. wouldnt it be better to do something like this instead > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 058ddad..cc47831 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -1240,7 +1240,11 @@ static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier, > uint32_t sch_id, int vq, > bool assign) > { > - return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); > + if (kvm_enabled()) { > + return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); > + } else { > + return 0; > + } > } > > static inline void s390_crypto_reset(void) Agreed. > > > FWIW, it seems that we (s390) do not have a functional equivalent function as commit > 8c56c1a592b5092d91da8d8943c17777d6462a6f ("memory: emulate ioeventfd") , so we will > not use the iothreads. Should not be a problem, I think. We may want to cook up an equivalent, though. > > > > if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) { > > return -ENOSYS; > > } > > > >
在 2017/7/3 18:20, Christian Borntraeger 写道: > On 07/03/2017 10:51 AM, QingFeng Hao wrote: >> This patch is based on a similar patch from Stefan Hajnoczi - >> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled) >> >> 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> >> --- >> 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; >> + } > thinking more about it. wouldnt it be better to do something like this instead > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 058ddad..cc47831 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -1240,7 +1240,11 @@ static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier, > uint32_t sch_id, int vq, > bool assign) > { > - return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); > + if (kvm_enabled()) { > + return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); > + } else { > + return 0; > + } > } Thanks. It makes sense. I'll change it. > static inline void s390_crypto_reset(void) > > > FWIW, it seems that we (s390) do not have a functional equivalent function as commit > 8c56c1a592b5092d91da8d8943c17777d6462a6f ("memory: emulate ioeventfd") , so we will > not use the iothreads. Ok, but might s390 have skipped the iothread arguments and passed the test, so we could still keep this test case? > >> if (!kvm_check_extension(kvm_state, KVM_CAP_IOEVENTFD)) { >> return -ENOSYS; >> } >>
On 07/04/2017 05:41 AM, QingFeng Hao wrote: > > > 在 2017/7/3 18:20, Christian Borntraeger 写道: >> On 07/03/2017 10:51 AM, QingFeng Hao wrote: >>> This patch is based on a similar patch from Stefan Hajnoczi - >>> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled) >>> >>> 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> >>> --- >>> 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; >>> + } >> thinking more about it. wouldnt it be better to do something like this instead >> >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >> index 058ddad..cc47831 100644 >> --- a/target/s390x/cpu.h >> +++ b/target/s390x/cpu.h >> @@ -1240,7 +1240,11 @@ static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier, >> uint32_t sch_id, int vq, >> bool assign) >> { >> - return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); >> + if (kvm_enabled()) { >> + return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); >> + } else { >> + return 0; >> + } >> } > Thanks. It makes sense. I'll change it. >> static inline void s390_crypto_reset(void) >> >> >> FWIW, it seems that we (s390) do not have a functional equivalent function as commit >> 8c56c1a592b5092d91da8d8943c17777d6462a6f ("memory: emulate ioeventfd") , so we will >> not use the iothreads. > Ok, but might s390 have skipped the iothread arguments and passed the test, so we could > still keep this test case? Yes, lets just fix the test case. We can implement emulated ioeventfds later.
在 2017/7/4 15:06, Christian Borntraeger 写道: > On 07/04/2017 05:41 AM, QingFeng Hao wrote: >> >> 在 2017/7/3 18:20, Christian Borntraeger 写道: >>> On 07/03/2017 10:51 AM, QingFeng Hao wrote: >>>> This patch is based on a similar patch from Stefan Hajnoczi - >>>> commit c324fd0a39c (" virtio-pci: use ioeventfd even when KVM is disabled) >>>> >>>> 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> >>>> --- >>>> 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; >>>> + } >>> thinking more about it. wouldnt it be better to do something like this instead >>> >>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >>> index 058ddad..cc47831 100644 >>> --- a/target/s390x/cpu.h >>> +++ b/target/s390x/cpu.h >>> @@ -1240,7 +1240,11 @@ static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier, >>> uint32_t sch_id, int vq, >>> bool assign) >>> { >>> - return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); >>> + if (kvm_enabled()) { >>> + return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); >>> + } else { >>> + return 0; >>> + } >>> } >> Thanks. It makes sense. I'll change it. >>> static inline void s390_crypto_reset(void) >>> >>> >>> FWIW, it seems that we (s390) do not have a functional equivalent function as commit >>> 8c56c1a592b5092d91da8d8943c17777d6462a6f ("memory: emulate ioeventfd") , so we will >>> not use the iothreads. >> Ok, but might s390 have skipped the iothread arguments and passed the test, so we could >> still keep this test case? > Yes, lets just fix the test case. We can implement emulated ioeventfds later. Fine, thanks! >
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 058ddad..cc47831 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -1240,7 +1240,11 @@ static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id, int vq, bool assign) { - return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); + if (kvm_enabled()) { + return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign); + } else { + return 0; + } } static inline void s390_crypto_reset(void)