Re: [PATCH v1] virtio-mmio: Specify wait needed in driver during reset

> > Changes since v0:

> >

> > Introduce a new feature bit, VIRTIO_F_MMIO_RESET_WAIT, which controls whether

> > a driver polls

> > for reset completion or not.

> Nit: changelog should go into a "---" section, next to the diffstat (git

> format-patch should give you the correct base to edit.)

Thanks for your review. Will take care of that in next version.

> > +indicated by a read of \field{Status} returning 0. Such a device MUST also

> > fail to accept

> > +FEATURES_OK bit if driver does not negotiate VIRTIO_F_MMIO_RESET_WAIT.

> So, this basically means that an older driver that does not know the new

> feature bit will not work with devices offering this bit, even if it did

> poll? This seems acceptable, I just wanted to spell it out.

Yes I believe that's the desired result. Device has no way to know if driver

will poll for reset completion or not unless it accepts the feature.

Are you suggesting we explicitly put in some text in spec to that effect?

> > +reset of such a device that offers VIRTIO_F_MMIO_RESET_WAIT, driver MUST poll

> > for

> s/driver/the driver/

> But maybe use "if VIRTIO_F_MMIO_RESET_WAIT has been negotiated" instead?

> This is implied by the statements above, but the deciding factor here is

> that the feature has been negotiated, and not that it has been offered.

Sure sounds good.

> > +reset completion indicated by a read of \field{Status} returning 0.

> > +

> > The driver MUST ignore a device with \field{MagicValue} which is not

> > 0x74726976,

> > although it MAY report an error.

> >

> > @@ -6672,6 +6679,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved

> > Feature Bits}

> > transport specific.

> > For more details about driver notifications over PCI see \ref{sec:Virtio

> > Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And

> > Device Operation / Available Buffer Notifications}.

> >

> > + \item[VIRTIO_F_MMIO_RESET_WAIT(40)] This feature could be offered by a

> > device

> > + using MMIO transport and indicates that the driver needs to poll for reset

> "This feature is specific to the MMIO transport and indicates that the

> device requires the driver to poll..."

> ?

> > + completion indicated by a read of \field{Status} returning 0.

> > +

> > \end{description}

> >

> > \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}

> > @@ -6708,6 +6719,8 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved

> > Feature Bits}

> >

> > A driver SHOULD accept VIRTIO_F_NOTIF_CONFIG_DATA if it is offered.

> >

> > +A driver SHOULD accept VIRTIO_F_MMIO_RESET_WAIT if it is offered.

> > +

> But this was specified as MUST above, wasn't it?

I guess I followed the convention of using SHOULD for other bits such as

VIRTIO_F_NOTIF_CONFIG_DATA in that section, but yes MUST sounds better.

I will post the next version shortly with recommended changes.

- vatsa

---

Qualcomm Innovation Center, Inc. is submitting the attached "feedback" as a

non-member to the virtio-dev mailing list for consideration and inclusion.

virtio-dev message