[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] [PATCH v2] virtio: Improve queue_reset polarity to match to default reset state
On Wed, Apr 27, 2022 at 04:15:05PM +0000, Parav Pandit wrote: > > > From: Parav Pandit > > Sent: Wednesday, April 27, 2022 11:58 AM > > > > But this is so basic. > > It's hard to gaze at this spec for coming years and the code to see, Hey > > sometimes 0 means disabled, sometime 0 means still enabled, sometime 1 > > means enabled, and sometimes 1 means now disabled... > > And maintain those weird code in device side and extra state bits burning > > some expensive chip resource. > > > > Is removing from 1.2 is equal delay to get is fixed in 1.3? > > If yes, I make humble request to fix this and have errata. > > Some of the professional standard bodies release the spec and short after > > that errata/ratification follows the release that resolve such small issues. > > May be time for virtio spec to take this opportunity now and be bit agile on it. > > Your call. :) > > > I think further, it appears a real bug that requires so special handling in the device for next years to carry. > > Imagine this sequence. > 1. A virtio device is handed over to guest VM > 2. A virtio device started queue_reset sequence, > So register values are: > queue_enable = 1, q_reset=1 > 2. guest VM poled the q_reset register. > Queue_enable = 1, q_reset = 0 (because device is doing the resetting the queue) > 3. HV suspend the VM and queried the VQ state > VQ state returned > q_enable=1, q_reset = 0. > > HV doesn't know what q_reset=0 mean here, is it 0 because it was never reset? > Or it is 0 because GVM Started the reset, but reset didn't finish? > > When this virtio device is restored on the other side, HV and device doesn't know how to deal with this. > > A WA that all devices will implement is, not returning 0, in step_2, but return say q_reset = 0xa to indicate that its other than 1 and other than 0. > But hey, the destination side needs to treat this special 0xa and convert to internal q_yet_busy stage. > > And this answer Jason and myself why queue_enable shouldn't be overloaded for this busy_wait register. > Hence queue_reset register seems the right choice with the fix. > > Starting to think of workaround even before the spec release is not elegant. > Lets please fix this, bug effect is expanding beyond the primary virtio device itself. Thanks Parav. Updating the github issue with more motivation would be a good idea. -- MST
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]