From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 References: <1632883462.8908377-1-xuanzhuo@linux.alibaba.com> <87tui349zu.fsf@redhat.com> In-Reply-To: <87tui349zu.fsf@redhat.com> From: Jason Wang Date: Thu, 30 Sep 2021 09:18:10 +0800 Message-ID: Subject: Re: [PATCH v4 2/3] virtio: pci support virtqueue reset Content-Type: text/plain; charset="UTF-8" To: Cornelia Huck Cc: Xuan Zhuo , Virtio-Dev List-ID: On Thu, Sep 30, 2021 at 12:33 AM Cornelia Huck wrote: > > On Wed, Sep 29 2021, Xuan Zhuo wrote: > > > On Tue, 28 Sep 2021 12:20:46 +0200, Cornelia Huck wrote: > >> On Tue, Sep 28 2021, Xuan Zhuo wrote: > >> > +The device MUST reset the queue when 1 is written to \field{queue_reset}, and > >> > +present a 1 in \field{queue_reset} after the queue has been reset, until the > >> > +driver re-enables the queue via \field{queue_enable}. (see \ref{sec:Virtqueues / Virtqueue Reset}). > >> > >> "...or the device is reset." ? > >> > > > > The device MUST reset the queue when 1 is written to \field{queue_reset}, and > > present a 1 in \field{queue_reset} after the queue has been reset, until the > > driver re-enables the queue via \field{queue_enable} or the device is reset. (see \ref{sec:Virtqueues / Virtqueue Reset}). > > > > Is that so? Doesn't it feel necessary? > > Is queue_reset supposed to persist across device reset? That feels a bit > odd to me. I think it's better to follow e.g the pci "status". Driver writes 1 to queue_reset. And the device notified the completion by presenting 0. This looks easier to be dealt with during device reset. > > > > > > >> Maybe also > >> > >> "The device MAY change the value of \field{queue_size} if the queue has > >> been reset." ? > >> > >> Should it always set that field to the currently maximum supported queue > >> size (assuming that can change dynamically)? Do we need some kind of > >> synchronization for those changes? > > > > When the queue is reset, all states of this queue MUST be modified to the > > initial value. For example, queue_size MUST be reset to the maximum value > > supported by the device. Because in the last reset queue or the entire device > > reset process, the driver will modify the queue_size of the device so that its > > value may be less than the maximum value. > > I think the question is whether the device may choose a different > initial maximum value once the queue has been reset. If it does change > the value, there may be a race where the driver has reset the queue, > read queue_reset back, read the max queue size, and the device only then > changing the max queue size. Maybe I'm overthinking this. I think the driver should do the exact same steps as the device initialization. Not sure it's worth mentioning here. Thanks >