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: [virtio-dev] [PATCH v2] virtio: Improve queue_reset polarity to match to default reset state


> 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.


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