mbox series

[kvmtool,00/13] Implement reset of virtio devices

Message ID 1543921726-54571-1-git-send-email-julien.thierry@arm.com (mailing list archive)
Headers show
Series Implement reset of virtio devices | expand

Message

Julien Thierry Dec. 4, 2018, 11:08 a.m. UTC
Hi,

This series was developped by Jean-Philippe and is needed for a series
I'll be posting shortly after to load firmwares on arm kvmtool.

Currently, when a guest tries to reset a device, a lot of ressources
aren't reset (threads keep running, virtio queue keep their state, etc).

When the guest only does the reset to initialize the device and there
were no previous users, there is no noticeable issue. But when a guest
has a firmare + Linux, if the firmware uses a virtio device, Linux will
fail to probe that device.

This series aim to properly reset the virtio resources when the guests
requests it.

Reset of net vhost is unsupported for now.

Patch 1 is a bug fix on ioeventfd
Patch 2-6 provide the core support so devices can implement their reset
Patch 7-13 implements the reset for the various virtio devices

Thanks,

Julien

-->

Jean-Philippe Brucker (13):
  ioeventfd: Fix removal of ioeventfd
  virtio: Implement notify_status
  virtio: Add get_vq_count() callback
  virtio: Add get_vq() callback
  virtio: Add exit_vq() callback
  virtio: Add reset() callback
  net/uip: Add exit function
  virtio/net: Clean virtqueue state
  virtio/net: Implement device and virtqueue reset
  virtio/blk: Reset virtqueue
  threadpool: Add cancel() function
  virtio/p9: Implement reset
  virtio/console: Implement reset

 include/kvm/threadpool.h  |   2 +
 include/kvm/uip.h         |   6 ++
 include/kvm/virtio-mmio.h |   1 +
 include/kvm/virtio-pci.h  |   1 +
 include/kvm/virtio.h      |  30 ++++++-
 ioeventfd.c               |   6 +-
 net/uip/core.c            |  17 ++++
 net/uip/dhcp.c            |   6 ++
 net/uip/tcp.c             |  54 ++++++++++---
 net/uip/udp.c             |  41 ++++++++--
 util/threadpool.c         |  25 +++++-
 virtio/9p.c               |  33 +++++++-
 virtio/balloon.c          |  17 +++-
 virtio/blk.c              |  84 ++++++++++++++------
 virtio/console.c          |  49 ++++++++----
 virtio/core.c             |  42 ++++++++++
 virtio/mmio.c             |  52 +++++++++----
 virtio/net.c              | 195 +++++++++++++++++++++++++++++++++-------------
 virtio/pci.c              |  49 ++++++++----
 virtio/rng.c              |  12 ++-
 virtio/scsi.c             |  17 +++-
 21 files changed, 582 insertions(+), 157 deletions(-)

--
1.9.1

Comments

Gerd Hoffmann Dec. 5, 2018, 8:21 a.m. UTC | #1
On Tue, Dec 04, 2018 at 11:08:33AM +0000, Julien Thierry wrote:
> Hi,
> 
> This series was developped by Jean-Philippe and is needed for a series
> I'll be posting shortly after to load firmwares on arm kvmtool.
> 
> Currently, when a guest tries to reset a device, a lot of ressources
> aren't reset (threads keep running, virtio queue keep their state, etc).
> 
> When the guest only does the reset to initialize the device and there
> were no previous users, there is no noticeable issue. But when a guest
> has a firmare + Linux, if the firmware uses a virtio device, Linux will
> fail to probe that device.
> 
> This series aim to properly reset the virtio resources when the guests
> requests it.
> 
> Reset of net vhost is unsupported for now.
> 
> Patch 1 is a bug fix on ioeventfd
> Patch 2-6 provide the core support so devices can implement their reset
> Patch 7-13 implements the reset for the various virtio devices

Fails to build:

kraxel@sirius ~/projects/kvmtool (virtio)# make 
Makefile:323: Skipping optional libraries: vncserver
  CC       virtio/core.o
In file included from virtio/core.c:8:0:
virtio/core.c: In function ‘virtio_notify_status’:
include/kvm/virtio.h:37:3: error: ‘VIRTIO_CONFIG_S_NEEDS_RESET’
undeclared (first use in this function)
   VIRTIO_CONFIG_S_NEEDS_RESET | \
   ^
virtio/core.c:232:19: note: in expansion of macro ‘VIRTIO_CONFIG_S_MASK’
  vdev->status &= ~VIRTIO_CONFIG_S_MASK;

Yes, it is an older machine (rhel-7), VIRTIO_CONFIG_S_NEEDS_RESET isn't
in the system headers (IIRC this bit was added with virtio-1.0).

cheers,
  Gerd
Julien Thierry Dec. 5, 2018, 9:10 a.m. UTC | #2
On 05/12/18 08:21, Gerd Hoffmann wrote:
> On Tue, Dec 04, 2018 at 11:08:33AM +0000, Julien Thierry wrote:
>> Hi,
>>
>> This series was developped by Jean-Philippe and is needed for a series
>> I'll be posting shortly after to load firmwares on arm kvmtool.
>>
>> Currently, when a guest tries to reset a device, a lot of ressources
>> aren't reset (threads keep running, virtio queue keep their state, etc).
>>
>> When the guest only does the reset to initialize the device and there
>> were no previous users, there is no noticeable issue. But when a guest
>> has a firmare + Linux, if the firmware uses a virtio device, Linux will
>> fail to probe that device.
>>
>> This series aim to properly reset the virtio resources when the guests
>> requests it.
>>
>> Reset of net vhost is unsupported for now.
>>
>> Patch 1 is a bug fix on ioeventfd
>> Patch 2-6 provide the core support so devices can implement their reset
>> Patch 7-13 implements the reset for the various virtio devices
> 
> Fails to build:
> 
> kraxel@sirius ~/projects/kvmtool (virtio)# make 
> Makefile:323: Skipping optional libraries: vncserver
>   CC       virtio/core.o
> In file included from virtio/core.c:8:0:
> virtio/core.c: In function ‘virtio_notify_status’:
> include/kvm/virtio.h:37:3: error: ‘VIRTIO_CONFIG_S_NEEDS_RESET’
> undeclared (first use in this function)
>    VIRTIO_CONFIG_S_NEEDS_RESET | \
>    ^
> virtio/core.c:232:19: note: in expansion of macro ‘VIRTIO_CONFIG_S_MASK’
>   vdev->status &= ~VIRTIO_CONFIG_S_MASK;
> 
> Yes, it is an older machine (rhel-7), VIRTIO_CONFIG_S_NEEDS_RESET isn't
> in the system headers (IIRC this bit was added with virtio-1.0).
> 

Thanks for reporting the issue. I think this series was initially
developed with the intent of supporting virtio-1.0 but then reset and
virtio-1.0 were split into two series.

I think here I can just remove that bit from the config mask while
virtio-1.0 is not supported, and things should work.

Thanks,
Gerd Hoffmann Dec. 5, 2018, 10:09 a.m. UTC | #3
On Wed, Dec 05, 2018 at 09:10:33AM +0000, Julien Thierry wrote:
> 
> 
> On 05/12/18 08:21, Gerd Hoffmann wrote:
> > On Tue, Dec 04, 2018 at 11:08:33AM +0000, Julien Thierry wrote:
> >> Hi,
> >>
> >> This series was developped by Jean-Philippe and is needed for a series
> >> I'll be posting shortly after to load firmwares on arm kvmtool.
> >>
> >> Currently, when a guest tries to reset a device, a lot of ressources
> >> aren't reset (threads keep running, virtio queue keep their state, etc).
> >>
> >> When the guest only does the reset to initialize the device and there
> >> were no previous users, there is no noticeable issue. But when a guest
> >> has a firmare + Linux, if the firmware uses a virtio device, Linux will
> >> fail to probe that device.
> >>
> >> This series aim to properly reset the virtio resources when the guests
> >> requests it.
> >>
> >> Reset of net vhost is unsupported for now.
> >>
> >> Patch 1 is a bug fix on ioeventfd
> >> Patch 2-6 provide the core support so devices can implement their reset
> >> Patch 7-13 implements the reset for the various virtio devices
> > 
> > Fails to build:
> > 
> > kraxel@sirius ~/projects/kvmtool (virtio)# make 
> > Makefile:323: Skipping optional libraries: vncserver
> >   CC       virtio/core.o
> > In file included from virtio/core.c:8:0:
> > virtio/core.c: In function ‘virtio_notify_status’:
> > include/kvm/virtio.h:37:3: error: ‘VIRTIO_CONFIG_S_NEEDS_RESET’
> > undeclared (first use in this function)
> >    VIRTIO_CONFIG_S_NEEDS_RESET | \
> >    ^
> > virtio/core.c:232:19: note: in expansion of macro ‘VIRTIO_CONFIG_S_MASK’
> >   vdev->status &= ~VIRTIO_CONFIG_S_MASK;
> > 
> > Yes, it is an older machine (rhel-7), VIRTIO_CONFIG_S_NEEDS_RESET isn't
> > in the system headers (IIRC this bit was added with virtio-1.0).
> > 
> 
> Thanks for reporting the issue. I think this series was initially
> developed with the intent of supporting virtio-1.0 but then reset and
> virtio-1.0 were split into two series.
> 
> I think here I can just remove that bit from the config mask while
> virtio-1.0 is not supported, and things should work.

Fixed that using ...

+#ifndef VIRTIO_CONFIG_S_NEEDS_RESET
+# define VIRTIO_CONFIG_S_NEEDS_RESET 0x40
+#endif

... in include/kvm/virtio.h

With that in place firmware boot works for x86 too.

Well, to be exact, sort of starts working.  There is still the problem
that kvmtool tweaks the linux kernel command line to compensate for
incomplete emulation.  So when booting a kernel from the disk image via
firmware and boot loader you have to apply the same tweaks to the boot
loader config.  At minimum pci=conf1 is required, otherwise the kernel
doesn't find any pci devices ...

cheers,
  Gerd