Re: [virtio] [PATCH] virtio-ccw: add adapter interrupt support

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Tue, 15 Oct 2013 14:13:58 +1030
Rusty Russell <rusty@au1.ibm.com> wrote:

> Cornelia Huck <cornelia.huck@de.ibm.com> writes:
> > Specify a new channel command to set up two-stage indicators as needed
> > for adapter interrupts and describe how adapter interrupts are used
> > for notifications.
> 
> Since this presumably isn't in the current implementation, should there
> be a legacy section talking about what happens if it's not supported?
> Or is that implied and I missed it?

As a device will reject any channel command it doesn't know, we should
be fine. I'll add a legacy section though; a transitional driver should
just fall back to classic indicators.

> 
> Nice to see that CCW is getting some benefit from the pain of v1.0 :)
> 
> > VIRTIO-43
> >
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  virtio-v1.0-wd01-part1-specification.txt |  100 ++++++++++++++++++++++++++++--
> >  1 file changed, 96 insertions(+), 4 deletions(-)
> >
> > diff --git a/virtio-v1.0-wd01-part1-specification.txt b/virtio-v1.0-wd01-part1-specification.txt
> > index 3ba7f52..ae646db 100644
> > --- a/virtio-v1.0-wd01-part1-specification.txt
> > +++ b/virtio-v1.0-wd01-part1-specification.txt
> > @@ -1545,6 +1545,7 @@ virtio:
> >  #define CCW_CMD_VDEV_RESET 0x33
> >  #define CCW_CMD_SET_IND 0x43
> >  #define CCW_CMD_SET_CONF_IND 0x53
> > +#define CCW_CMD_SET_IND_ADAPTER 0x73
> >  #define CCW_CMD_READ_FEAT 0x12
> >  #define CCW_CMD_WRITE_FEAT 0x11
> >  #define CCW_CMD_READ_CONF 0x22
> > @@ -1724,15 +1725,74 @@ In both cases, the complete configuration space is transmitted.
> >  2.3.3.2.6. Setting Up Indicators
> >  --------------------------------
> >  
> > +In order to set up the indicator bits for host->guest notification,
> > +the driver uses different channel commands depending on whether it
> > +wishes to use traditional I/O interrupts tied to a subchannel or
> > +adapter I/O interrupts. For any given device, the two mechanisms
> > +are mutually exclusive.
> > +
> > +For the configuration change indicators, the same mechanism is used
> > +in both cases.
> > +
> > +2.3.3.2.6.1. Setting Up Classic Queue Indicators
> > +------------------------------------------------
> > +
> > +Indicators for notification via classic I/O interrupts are contained
> > +in a 64 bit value per virtio-ccw proxy device.
> > +
> >  To communicate the location of the indicator bits for host->guest
> >  notification, the guest uses the CCW_CMD_SET_IND command,
> >  pointing to a location containing the guest address of the
> >  indicators in a 64 bit value.
> >  
> > -For the indicator bits used in the configuration change
> > -host->guest notification, the CCW_CMD_SET_CONF_IND command is
> > -used analogously.
> > +If the driver has already set up two-staged queue indicators via the
> > +CCW_CMD_SET_IND_ADAPTER command, the device MUST post a unit check
> > +with command reject to any subsequent CCW_CMD_SET_IND command.
> > +
> > +2.3.3.2.6.2. Setting Up Configuration Change Indicators
> > +-------------------------------------------------------
> > +
> > +Indicators for configuration change host->guest notification are
> > +contained in a 64 bit value per virtio-ccw proxy device.
> > +
> > +To communicate the location of the indicator bits used in the
> > +configuration change host->guest notification, the driver issues the
> > +CCW_CMD_SET_CONF_IND command, pointing to a location containing the
> > +guest address of the indicators in a 64 bit value.
> > +
> > +2.3.3.2.6.3. Setting Up Two-Stage Queue Indicators
> > +--------------------------------------------------
> > +
> > +Indicators for notification via adapter I/O interrupts consist of
> > +two stages:
> > +- a summary indicator byte covering the virtqueues for one or more
> > +  virtio-ccw proxy devices
> > +- a set of contigous indicator bits for the virtqueues for a
> > +  virtio-ccw proxy device
> > +
> > +To communicate the location of the summary and queue indicator bits,
> > +the driver uses the CCW_CMD_SET_IND_ADAPTER command with the following
> > +payload:
> > +
> > +struct virtio_thinint_area {
> > +       __u64 summary_indicator;
> > +       __u64 indicator;
> > +       __u64 bit_nr;
> > +       __u8 isc;
> > +};
> 
> Hmm... you __attribute__((packed)) the other structures; I assume
> that's wanted here too?

I'll add it.

> 
> Cheers,
> Rusty.