qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Claudio Fontana <cfontana@suse.de>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	Marcel Apfelbaum <marcel@redhat.com>
Subject: Re: virtio: why no full reset on virtio_set_status 0 ?
Date: Mon, 01 Aug 2022 09:44:55 +0100	[thread overview]
Message-ID: <87fsigpcza.fsf@linaro.org> (raw)
In-Reply-To: <8b008e30-15df-91cd-c891-b7707324abe1@suse.de>


Claudio Fontana <cfontana@suse.de> writes:

> On 7/29/22 16:00, Claudio Fontana wrote:
>> On 7/29/22 15:21, Alex Bennée wrote:
>>>
>>> Claudio Fontana <cfontana@suse.de> writes:
>>>
>>>> On 7/29/22 12:13, Michael S. Tsirkin wrote:
>>>>> On Fri, Jul 29, 2022 at 11:46:05AM +0200, Claudio Fontana wrote:
>>>>>>>> @@ -2025,7 +2031,6 @@ void virtio_reset(void *opaque)
>>>>>>>>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>>>>>>>>      int i;
>>>>>>>>  
>>>>>>>> -    virtio_set_status(vdev, 0);
>>>>>>>>      if (current_cpu) {
>>>>>>>>          /* Guest initiated reset */
>>>>>>>>          vdev->device_endian = virtio_current_cpu_endian();
>>>>>>>> -- 
>>>>>>>> 2.26.2
>>>>>>>
>>>>>>> As you say this is incomplete ... bout could you share a bit more
>>>>>>> of what issue does this address?
>>>>>>>
>>>>>>
>>>>>> Hi, the problem I am trying to address is a segfault in OVS/dpdk that looks like this:
>>>>>
>>>>> Sorry I was not clear. What I mean is, you don't yet know why does removing
>>>>> virtio_set_status call here prevent the crash in ovs, do you?
>>>>>
>>>>
>>>> I have no idea. Trying to collect logs to figure things out, but as
>>>> mentioned the logs easily hide the issue.
>>>> Likely there is just more to study here.
>>>
>>> Given the OVS is going off on a NULL ptr deref could it just be it's not
>>> handling the disabling/reenabling of the virtqueues as you pause and
>>> restart properly? I could certainly imagine a backend jumping the gun to
>>> read a queue going very wrong if the current queue state is disabled.
>>>
>> 
>> In this case both the ovs buf_addr and buf_iova are NULL, which is a
>> nice case as they are more detectable,
>> however I also have segfaults where the addresses are just garbage.
>> 
>> I wonder whether it's possible that given the fact that the guest is
>> going away without notification (SIGKILL),
>> as the guest driver resets the device and communicates with QEMU,
>> QEMU adapts the state without notifying ovs,
>> so ovs happily tries to dequeue data from memory that isn't there. But I am just guessing.
>> 
>> I am still studying the qemu vhost user side and ovs/dpdk side to
>> try to understand how this whole thing works.
>> 
>> Thanks,
>> 
>> CLaudio
>> 
>
> I am pursuing this as a DPDK library issue.
>
> It would be cool to have ovs, dpdk and vhost-user with the default
> test-pmd application somehow hooked up in a basic test
> in one of these projects..

I agree although it's hard to marshal multiple projects into a known
working state that isn't too brittle for CI purposes. The existing
qos-test testing doesn't really exercise any more than the initial setup
and register reading of the VirtIO device.

For example we have a number of non network standalone vhost-user
backends in rust-vmm which would be nice to plumb in somehow.

>
> Thanks,
>
> Claudio


-- 
Alex Bennée


  reply	other threads:[~2022-08-01  8:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 10:51 virtio: why no full reset on virtio_set_status 0 ? Claudio Fontana
2022-07-27 15:32 ` Michael S. Tsirkin
2022-07-28  1:27   ` Jason Wang
2022-07-28  7:16     ` Claudio Fontana
2022-07-28  7:43     ` Claudio Fontana
2022-07-28  9:09       ` Claudio Fontana
2022-07-28 10:24         ` Cornelia Huck
2022-07-31 20:38           ` Claudio Fontana
2022-07-28 13:39         ` Michael S. Tsirkin
2022-07-29  9:46           ` Claudio Fontana
2022-07-29 10:13             ` Michael S. Tsirkin
2022-07-29 10:19               ` Claudio Fontana
2022-07-29 13:21                 ` Alex Bennée
2022-07-29 14:00                   ` Claudio Fontana
2022-07-31 20:42                     ` Claudio Fontana
2022-08-01  8:44                       ` Alex Bennée [this message]
2022-07-28 11:41       ` Michael S. Tsirkin
2022-07-27 16:17 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fsigpcza.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=cfontana@suse.de \
    --cc=jasowang@redhat.com \
    --cc=marcel@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).