[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] [PATCH 4/4] ccw: allow WRITE_STATUS to fail
On Wed, 4 Feb 2015 00:12:06 +0100 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Fri, Oct 31, 2014 at 03:26:19PM +0100, Cornelia Huck wrote: > > We want to be able to reject a status (e.g. FEATURES_OK if the device > > can't work with the features negotiated). > > I guess the reason it's needed is because ccw doesn't have > READ_STATUS? Yes, because we need to introduce channel commands for things that pci/mmio just can do out of the box by reading/writing some memory area. And allowing failures is the least invasive way to get the effects we want. > > > > The easiest way to do that > > is to allow the device to fail the WRITE_STATUS command. > > > > VIRTIO-116 > > > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > diff --git a/content.tex b/content.tex > > index 38ae49e..490823e 100644 > > --- a/content.tex > > +++ b/content.tex > > @@ -2627,6 +2627,21 @@ The driver changes the status of a device via the > > CCW_CMD_WRITE_STATUS command, which transmits an 8 bit status > > value. > > > > +The device is allowed to reject a status: For example, it might fail to accept > > +the FEATURES_OK status bit during device initialization. > > I kind of dislike it that we are adding this new concept of > "rejecting a status". The generic spec says: > > The device SHOULD accept any valid subset > of features the driver accepts, otherwise it MUST fail to set the > FEATURES_OK \field{device status} bit when the driver writes it. > > so devices accept features, not status, I think applying concept of > accept/reject to status is confusing. It basically comes from the concept of "rejecting a ccw with unit check". > Let's say that device fails to set status to requested value. > For example: > > > > As described in <cross reference> devices sometimes fail to set > the \field{device status} to value written by the driver. No objection from me, it basically ends up meaning the same thing. > > > + > > +\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 > > add: setting device failed to set the status and that > > > +the status > > +field > > +retained its previous value. > > + > > +\devicenormative{\paragraph}{Communicating Status Information}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Communicating Status Information} > > + > > > +If the driver attempts to set a status that the device does not accept, > > instead: > If the device fails to set the \field{device status} field > to the value written by driver > > > > + the > > +device MUST leave the status field unchanged and MUST post a unit check > > +with command reject. > > + > > \subsubsection{Handling Device Features}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Handling Device Features} > > > > Feature bits are arranged in an array of 32 bit values, making Your suggested changes look OK to me. I'll prepare an updated patch.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]