On Mon, 12 Oct 2015 15:34:18 +0300
"Michael S. Tsirkin" <> wrote:
> On Mon, Oct 12, 2015 at 01:44:33PM +0200, Cornelia Huck wrote:
> > On Sun, 11 Oct 2015 18:41:52 +0300
> > "Michael S. Tsirkin" <> wrote:
> >
> > > On Fri, Sep 11, 2015 at 03:52:27PM +0200, Cornelia Huck wrote:
> > > > \drivernormative{\paragraph}{Communicating Status Information}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Communicating Status Information}
> > > >
> > > > If the device posts a unit check with command reject in response to the
> > > > CCW_CMD_WRITE_STATUS command, the driver MUST assume that the device failed
> > > > to set the status and the \field{status} field retained its previous value.
> > > >
> > > > +If at least revision 2 has been negotiated, the driver MUST use the
> > >
> > > Why is this MUST here?
> >
> > The idea was that the device can rely on the driver having fetched the
> > new value and not needing to accommodate drivers that are not aware of
> > recent status changes.
> >
> > If we consider it the driver's responsibility to keep abreast of
> > changes (so that the device does not really need to care whether the
> > driver fetches all changes), we can change it to SHOULD.
>
> I this so - see below:
>
>
> > >
> > > > +CCW_CMD_READ_STATUS command to retrieve the \field{status} field after
> > > > +a configuration change interrupt has been retrieved.
> > >
> > > received?
> > > Maybe better "after a configuration change is detected".
> >
> > Hm... although I'm not sure how a driver may detect a configuration
> > change other than through an interrupt.
>
> It reads the indicator, isn't this right?
Hm, yes, it might poll the configuration changed indicator.
Although I'm wondering: While 2.1.2 talks about "sending" a change
notification (which one might interpret as making the interrupt
pending), 3.2.3 explicitly talks about delivering/triggering an
interrupt. Should the wording in 3.2.3 be changed to match the wording
in 2.1.2, or is this really picking at words?
>
> > > What if interrupts are disabled?
> > >
> > > > It also SHOULD use
> > > > +the CCW_CMD_READ_STATUS command to verify the \field{status} field after it
> > > > +issued the CCW_CMD_WRITE_STATUS command.
> > >
> > > Seems to conflict with above logic which says
> > > assume it retained previous value.
> >
> > There are two parts to that:
> > - WRITE_STATUS failed -> status field is unchanged
> > - WRITE_STATUS did not fail -> status field has been modified; but use
> > READ_STATUS to verify that the device did not modify it itself
> >
> > I was thinking of the logic "write status with FEATURES_OK, then read
> > it to check whether the device kept FEATURES_OK". But it's probably ok
> > to keep that logic inside the generic sections; I'll remove this
> > sentence.
>
> My question is generally this: what's wrong with
> keeping the current logic where we require FEATURES_OK to fail
> if device doesn't set valid features?
>
> The result is that CCW_CMD_READ_STATUS is merely there as an option: for
> fatal errors or for drivers that don't want to maintain the status
> internally.
We still have NEEDS_RESET - the driver needs to be able to retrieve
this when the configuration changed indicator is set.
> > I'll prepare a v2.
>
