OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: RE: RE: [virtio-dev] queue_reset register polarity to improve


> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 27, 2022 4:30 PM
> 
> On Wed, Apr 27, 2022 at 04:28:43PM +0800, Xuan Zhuo wrote:
> > On Tue, 26 Apr 2022 12:00:24 +0000, Parav Pandit <parav@nvidia.com>
> wrote:
> > >
> > > > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > Sent: Sunday, April 24, 2022 3:29 AM
> > > > >
> > > > > A recently defined queue_reset register has a little weird
> > > > > definition that
> > > > we should improve.
> > > > > When driver initiate queue reset, it writes queue_reset = 1.
> > > > > When device is busy resetting the queue, on this driver request,
> > > > > it is
> > > > expected to return queue_reset=0.
> > > > > Once queue reset is completed it is expected to return queue_reset
> = 1.
> > > > > (Polarity changed twice to same value as what was driver set).
> > > > > See more
> > > > below.
> > > > >
> > > > > So state wise,
> > > > > # q_enable, q_reset represents :
> > > > > a) 0,0 -> device init time value
> > > > > b) 1,0 -> vq is enabled and working
> > > > > c) 1,1 -> vq is enabled, driver initiated reset
> > > > > d) 1,0 -> vq is enabled, but device is busy doing the reset
> > > > > (conflicting definition with above #b )
> > > > > e) 0,1 -> vq reset is complete in the device and VQ is now
> > > > > disabled (again conflict with #a above )
> > > > >
> > > > > Instead, I think we should have below better, consistent
> > > > > definition, no
> > > > matter how queue reset occurs (init time or later).
> > > > >
> > > > > q_enable, q_reset
> > > > > A) 0, 0 -> default, device init time
> > > > > B) 1, 0 -> driver has enabled vq
> > > > > C) 1, 1 -> driver started q reset
> > > > > D) 1, 1 -> q_reset stays 1 until device is busy resetting vq
> > > > > (communicating that its working on resetting, consistent with
> > > > > #C)
> > > > > E) 0, 0 -> q_reset by device is completed, q got disabled (now
> > > > > matches the state same as device init time #A)
> > > >
> > > > Seems to me to be two different designs, I don't see any
> > > > particular value in the conflict mentioned here, and under what
> > > > circumstances would it cause any trouble?
> > > >
> > > queue reset state doesn't go back same register value at the end of
> queue reset operation as what it was initial device reset operation.
> > > And also, the special weird handling needed due to this on device side.
> > > The rest of the commit message text deleted describes the discrepancies
> and also above in point #E.
> > >
> > > > The design here is similar to many scenarios, such as device reset.
> > > >
> > > Taking device reset example is not that great. As this is something exists
> in the code way before the spec 1.x was written.
> > >
> > > In current form, device tells driver that queue is reset = 0) during initial
> time.
> > > And when done using register method, it says queue reset is complete by
> saying queue_reset = 1.
> > >
> > > This doesn't match with existing device reset semantics anyway.
> > >
> > > No matter how to reset the device, the end result of device_status = 0,
> when reset is completed.
> > >
> > > > So if you want to change to your design, I want to know if there
> > > > are other reasons.
> > > Those are described in the rest of the commit message and above.
> > > Its very simple, queue reset to go back to same state as what it was
> during device reset time, i.e. 0.
> >
> > OK, I have no problem.
> >
> > Thanks for pointing this out.
> >
> > Hi, Michael, what's our plan now? Will it be merged into 1.2?
> >
> > Should my linux code be implemented based on this new specification?
> >
> >
> > Thanks.
> 
> So given the original commenter also feels we should make the change,
> maybe the best way forward is redoing the 1.2 draft before the public review
> starts. The proposal has not been finalized yet though, whether that is
> practical would depend on when that is ready.

I am preparing v3 to address yours and Cornelia's comments.


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]