[Qemu-devel] virtio-blk broken after system reset

[Qemu-devel] virtio-blk broken after system reset

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

Hi,

both after hard and guest-initiated reset, something is seriously broken
with virtio block devices. If I reset my Linux guest while still in
grub, the bios will simply fail to read from the disk after the reboot. If I
reset after Linux touched the device, qemu terminates:

Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
#1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
#2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
#3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
#4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
#5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
#6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369

This is with current qemu.git head, haven't tried older versions. Known bug?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] virtio-blk broken after system reset

[Stefan Hajnoczi]

On Fri, Nov 12, 2010 at 10:02 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Hi,
>
> both after hard and guest-initiated reset, something is seriously broken
> with virtio block devices. If I reset my Linux guest while still in
> grub, the bios will simply fail to read from the disk after the reboot. If I
> reset after Linux touched the device, qemu terminates:
>
> Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
> (gdb) bt
> #0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
> #1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
> #2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
> #3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
> #4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
> #5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
> #6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369
>
> This is with current qemu.git head, haven't tried older versions. Known bug?

This is a known issue.  Gleb has posted a SeaBIOS fix:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg45849.html

Currently the patch only appears in SeaBIOS master.  Gleb and Kevin
have discussed putting it into 0.6.1.2 stable (see linked thread).
QEMU should then pick that release up.

Stefan

Re: [Qemu-devel] virtio-blk broken after system reset

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

Am 13.11.2010 08:49, Stefan Hajnoczi wrote:
> On Fri, Nov 12, 2010 at 10:02 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Hi,
>>
>> both after hard and guest-initiated reset, something is seriously broken
>> with virtio block devices. If I reset my Linux guest while still in
>> grub, the bios will simply fail to read from the disk after the reboot. If I
>> reset after Linux touched the device, qemu terminates:
>>
>> Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>> (gdb) bt
>> #0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>> #1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
>> #2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
>> #3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
>> #4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
>> #5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
>> #6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369
>>
>> This is with current qemu.git head, haven't tried older versions. Known bug?
> 
> This is a known issue.  Gleb has posted a SeaBIOS fix:
> 
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg45849.html
> 
> Currently the patch only appears in SeaBIOS master.  Gleb and Kevin
> have discussed putting it into 0.6.1.2 stable (see linked thread).
> QEMU should then pick that release up.

Ah, good.

And what about the guest-triggerable qemu exit above?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] virtio-blk broken after system reset

[Michael Tokarev]

13.11.2010 10:51, Jan Kiszka wrote:
> Am 13.11.2010 08:49, Stefan Hajnoczi wrote:
>> On Fri, Nov 12, 2010 at 10:02 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>> Hi,
>>>
>>> both after hard and guest-initiated reset, something is seriously broken
>>> with virtio block devices. If I reset my Linux guest while still in
>>> grub, the bios will simply fail to read from the disk after the reboot. If I
>>> reset after Linux touched the device, qemu terminates:
>>>
>>> Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>> (gdb) bt
>>> #0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>> #1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
>>> #2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
>>> #3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
>>> #4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
>>> #5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
>>> #6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369
>>>
[]
> And what about the guest-triggerable qemu exit above?

There are _lots_ of guest-triggerable qemu exits out there.

static int virtqueue_num_heads(VirtQueue *vq, unsigned int idx)
{
    uint16_t num_heads = vring_avail_idx(vq) - idx;

    /* Check it isn't doing very strange things with descriptor numbers. */
    if (num_heads > vq->vring.num) {
        fprintf(stderr, "Guest moved used index from %u to %u",
                idx, vring_avail_idx(vq));
        exit(1);
    }

    return num_heads;
}

This is done when guest behaves insanely (or qemu thinks it does).
On a real hw similar behavour most likely will lead to a system
lockup, qemu just exits.

Why it is trying to print things to stderr is a different
matter, it should be using a proper error-reporting routine,
but this is a different story.

Speaking of the bios bug, I included the fix to debian qemu-kvm
package quite some time ago (19 Aug 2010), it is included since
0.12.5+dfsg-2 debian release.  This one:

http://git.debian.org/?p=collab-maint/qemu-kvm.git;a=commit;h=5533a3e87fd19f35a580c8178ce59da72708c63a

/mjt

Re: [Qemu-devel] virtio-blk broken after system reset

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2524 bytes --]

Am 13.11.2010 11:01, Michael Tokarev wrote:
> 13.11.2010 10:51, Jan Kiszka wrote:
>> Am 13.11.2010 08:49, Stefan Hajnoczi wrote:
>>> On Fri, Nov 12, 2010 at 10:02 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>> Hi,
>>>>
>>>> both after hard and guest-initiated reset, something is seriously broken
>>>> with virtio block devices. If I reset my Linux guest while still in
>>>> grub, the bios will simply fail to read from the disk after the reboot. If I
>>>> reset after Linux touched the device, qemu terminates:
>>>>
>>>> Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>>> (gdb) bt
>>>> #0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>>> #1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
>>>> #2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
>>>> #3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
>>>> #4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
>>>> #5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
>>>> #6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369
>>>>
> []
>> And what about the guest-triggerable qemu exit above?
> 
> There are _lots_ of guest-triggerable qemu exits out there.
> 
> static int virtqueue_num_heads(VirtQueue *vq, unsigned int idx)
> {
>     uint16_t num_heads = vring_avail_idx(vq) - idx;
> 
>     /* Check it isn't doing very strange things with descriptor numbers. */
>     if (num_heads > vq->vring.num) {
>         fprintf(stderr, "Guest moved used index from %u to %u",
>                 idx, vring_avail_idx(vq));
>         exit(1);
>     }
> 
>     return num_heads;
> }
> 
> This is done when guest behaves insanely (or qemu thinks it does).
> On a real hw similar behavour most likely will lead to a system
> lockup, qemu just exits.

There is also real hw out there that goes into an error state if it's
misprogrammed.

I think we have to remove all those premature exits. They also prevent
handing the device inside the guest to an untrusted driver (relevant
once we have IOMMU emulation).

> 
> Why it is trying to print things to stderr is a different
> matter, it should be using a proper error-reporting routine,
> but this is a different story.

Jep. Even worse: the above message is not dumped to the console as the
stream isn't flushed on exit.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] virtio-blk broken after system reset

[Stefan Hajnoczi]

On Sat, Nov 13, 2010 at 10:09 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Am 13.11.2010 11:01, Michael Tokarev wrote:
>> 13.11.2010 10:51, Jan Kiszka wrote:
>>> Am 13.11.2010 08:49, Stefan Hajnoczi wrote:
>>>> On Fri, Nov 12, 2010 at 10:02 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>> Hi,
>>>>>
>>>>> both after hard and guest-initiated reset, something is seriously broken
>>>>> with virtio block devices. If I reset my Linux guest while still in
>>>>> grub, the bios will simply fail to read from the disk after the reboot. If I
>>>>> reset after Linux touched the device, qemu terminates:
>>>>>
>>>>> Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>>>> (gdb) bt
>>>>> #0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>>>> #1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
>>>>> #2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
>>>>> #3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
>>>>> #4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
>>>>> #5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
>>>>> #6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369
>>>>>
>> []
>>> And what about the guest-triggerable qemu exit above?
>>
>> There are _lots_ of guest-triggerable qemu exits out there.
>>
>> static int virtqueue_num_heads(VirtQueue *vq, unsigned int idx)
>> {
>>     uint16_t num_heads = vring_avail_idx(vq) - idx;
>>
>>     /* Check it isn't doing very strange things with descriptor numbers. */
>>     if (num_heads > vq->vring.num) {
>>         fprintf(stderr, "Guest moved used index from %u to %u",
>>                 idx, vring_avail_idx(vq));
>>         exit(1);
>>     }
>>
>>     return num_heads;
>> }
>>
>> This is done when guest behaves insanely (or qemu thinks it does).
>> On a real hw similar behavour most likely will lead to a system
>> lockup, qemu just exits.
>
> There is also real hw out there that goes into an error state if it's
> misprogrammed.
>
> I think we have to remove all those premature exits. They also prevent
> handing the device inside the guest to an untrusted driver (relevant
> once we have IOMMU emulation).

Interesting point about IOMMU.

>> Why it is trying to print things to stderr is a different
>> matter, it should be using a proper error-reporting routine,
>> but this is a different story.
>
> Jep. Even worse: the above message is not dumped to the console as the
> stream isn't flushed on exit.

stderr is normally unbuffered.  Are you running via libvirt?

Stefan

Re: [Qemu-devel] virtio-blk broken after system reset

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

Am 13.11.2010 11:54, Stefan Hajnoczi wrote:
> On Sat, Nov 13, 2010 at 10:09 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Am 13.11.2010 11:01, Michael Tokarev wrote:
>>> Why it is trying to print things to stderr is a different
>>> matter, it should be using a proper error-reporting routine,
>>> but this is a different story.
>>
>> Jep. Even worse: the above message is not dumped to the console as the
>> stream isn't flushed on exit.
> 
> stderr is normally unbuffered.  Are you running via libvirt?

No, I was wrong. I probably missed the output as it does not issue a
newline.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [Qemu-devel] virtio-blk broken after system reset

[Kevin Wolf]

Am 13.11.2010 11:09, schrieb Jan Kiszka:
> Am 13.11.2010 11:01, Michael Tokarev wrote:
>> 13.11.2010 10:51, Jan Kiszka wrote:
>>> Am 13.11.2010 08:49, Stefan Hajnoczi wrote:
>>>> On Fri, Nov 12, 2010 at 10:02 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>> Hi,
>>>>>
>>>>> both after hard and guest-initiated reset, something is seriously broken
>>>>> with virtio block devices. If I reset my Linux guest while still in
>>>>> grub, the bios will simply fail to read from the disk after the reboot. If I
>>>>> reset after Linux touched the device, qemu terminates:
>>>>>
>>>>> Breakpoint 1, 0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>>>> (gdb) bt
>>>>> #0  0x00007ffff4b945b0 in _exit () from /lib64/libc.so.6
>>>>> #1  0x00007ffff4b2948d in __run_exit_handlers () from /lib64/libc.so.6
>>>>> #2  0x00007ffff4b29535 in exit () from /lib64/libc.so.6
>>>>> #3  0x0000000000568da3 in virtqueue_num_heads (vq=0x17040e0, idx=0) at /data/qemu/hw/virtio.c:258
>>>>> #4  0x0000000000569511 in virtqueue_pop (vq=0x17040e0, elem=0x17cea58) at /data/qemu/hw/virtio.c:388
>>>>> #5  0x0000000000419e31 in virtio_blk_get_request (s=0x1704010) at /data/qemu/hw/virtio-blk.c:132
>>>>> #6  virtio_blk_handle_output (vdev=0x1704010, vq=<value optimized out>) at /data/qemu/hw/virtio-blk.c:369
>>>>>
>> []
>>> And what about the guest-triggerable qemu exit above?
>>
>> There are _lots_ of guest-triggerable qemu exits out there.
>>
>> static int virtqueue_num_heads(VirtQueue *vq, unsigned int idx)
>> {
>>     uint16_t num_heads = vring_avail_idx(vq) - idx;
>>
>>     /* Check it isn't doing very strange things with descriptor numbers. */
>>     if (num_heads > vq->vring.num) {
>>         fprintf(stderr, "Guest moved used index from %u to %u",
>>                 idx, vring_avail_idx(vq));
>>         exit(1);
>>     }
>>
>>     return num_heads;
>> }
>>
>> This is done when guest behaves insanely (or qemu thinks it does).
>> On a real hw similar behavour most likely will lead to a system
>> lockup, qemu just exits.
> 
> There is also real hw out there that goes into an error state if it's
> misprogrammed.
> 
> I think we have to remove all those premature exits. They also prevent
> handing the device inside the guest to an untrusted driver (relevant
> once we have IOMMU emulation).

I agree, those exits are bugs. There are a lot of them in virtio code.
At some point, someone should go through the whole virtio code and fix
them all.

Kevin

Re: [Qemu-devel] virtio-blk broken after system reset

[Anthony Liguori]

On 11/13/2010 04:09 AM, Jan Kiszka wrote:
>
> There is also real hw out there that goes into an error state if it's
> misprogrammed.
>
> I think we have to remove all those premature exits. They also prevent
> handing the device inside the guest to an untrusted driver (relevant
> once we have IOMMU emulation).
>    

I think the key to achieving this is to isolate the device within QEMU.

IOW, have all fd callbacks, bottom halves, etc. tagged with a device 
context.  Have a mechanism that raises an error on a device that can 
then be used to stop issuing any type of callback to the device until reset.

Obviously, we can fix some of these by just simple code refactoring.

Regards,

Anthony Liguori

>> Why it is trying to print things to stderr is a different
>> matter, it should be using a proper error-reporting routine,
>> but this is a different story.
>>      
> Jep. Even worse: the above message is not dumped to the console as the
> stream isn't flushed on exit.
>
> Jan
>
>