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


On Thu, Apr 28, 2022 at 12:00 PM Parav Pandit <parav@nvidia.com> wrote:
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > Sent: Wednesday, April 27, 2022 11:41 PM
> >
> >
> > Hypervisor needs to trap the access to q_reset, otherwise there will be a
> > race anyhow. So it knows the device is being reset and only after the device
> > rest is finished, it can suspend a VM.
> >
> The state can come from the device directly without the trap via HV to device direct access.

So this complicates the implementation of the hardware:

1) it needs to track a state like busy resetting which seems unnecessary
2) in the case of nesting, we need to trap anyhow

>
> >
> > > 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?
> >
> >
> > If the device can change any register value (e.g q_reset), it's not stopped.
> > We can't do the migration anyhow. No?
> The state is in the device, so depending on the device type it may be able to store its internal state.
>
> >
> >
> > >
> > > 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.
> >
> >
> > Well, we've already had busy waiting register that is device_status.
> >
> > """
> >
> > After writing 0 todevice_status, the driver MUST wait for a read
> > ofdevice_statusto return 0 before reinitializing the device.
> >
> I think yes, we can overload q_enable busy wait to register like device_status.
> Q_enable turns zero only when queue is disabled, till that time driver polls for it and gets 1.
>
> In this approach, external query entity will not be able to see ongoing queue reset.

Yes, this can simplify a lot, there's no much value in migrating
ongoing queue rest.

> This may be ok because device is anyway maintaining its internal state regardless.
>
> queue_reset removal has far more editorial changes.
> Before I update the v4 to incorporate, please discuss/sync with Michael or others what is best course of action from spec timing perspective.
>  I am ok either way to draft either by
> (a) removal of queue_reset register and allow queue_disable by replacing RING_RESET definition in v4
> Or
> (b) fixing the polarity as done in v3

Ok.

Thanks



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