[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH] virtio: Improve queue_reset polarity to match to default reset state
> From: Michael S. Tsirkin <mst@redhat.com> > Sent: Monday, April 25, 2022 7:11 PM > [..] > That's a weird way to describe this. I guess the logic in the spec is simply a > register bit that is set to != 1 when reset starts and set to 1 when reset > completes. > You propose reversing its polarity. Fair enough. > I will drop this part from the commit message as example clarifies it enough without device side code. > > I do think it's inelegant that the value is different when reset is through vq > reset (1) and through device reset (0). > > If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 > > to -\field{queue_reset} to reset the queue, it MUST verify that the > > queue -has been reset by reading back \field{queue_reset} and ensuring > > that it -is 1. The driver MAY re-enable the queue by writing a 1 to > > +\field{queue_reset} to reset the queue, driver MUST verify that the > > +queue has been reset by reading back \field{queue_reset} until device > returns a value of 0. > > +Device continue to report value of 1 for the \field{queue_reset} > > +until device finishes > > this MUST should move to conformance chapter BTW. In this proposal it is covered in the section 4.1.4.3.2 " Driver Requirements: Common configuration structure layout". This section is listed in 7.2 " Clause 2: PCI Driver Conformance". To me it appears that this is covered in the conformance. What am I missing? > > > +the queue reset request. When the device completes the queue reset, > > +\field{queue_reset} and \field{queue_enable} set to zero, > > set-> "are set" > Will fix. > > indicating > > +that specified queue is ready to be enabled again. The driver MAY > > +re-enable the queue by writing a 1 to > > a 1 -> 1 Will fix. > > > \field{queue_enable} after ensuring that the other virtqueue fields > > have > > the other -> other (not your fault) > Should this be pre-patch? Or small enough editorial change to be part of this? > > been set up correctly. The driver MAY set driver-writeable queue > > configuration values to different values than those that were used before > the queue reset. > > (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue > Reset}). > > > > + > > drop this pls. My bad. Will fix it. > > ccw will have to be changed too. My weak area. Didn't find the vq reset section in below 3 commits. a4ce81a virtio: mmio support virtqueue reset 12998e7 virtio: pci support virtqueue reset 3b5378d virtio: introduce virtqueue reset as basic facility In a previous email by Cornelia, she replied " For ccw, we have not yet added queue reset;". So, can we please do this later in a some a non_bug_fix patch? I will revise v1 for the comments that I can fix. Based on v1 review, we can split patches and improve further.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]