Re: [PATCH v7 05/13] s390x/css: realize css_create_sch

Cornelia Huck
2017-05-08T10:50:40Z

On Mon, 8 May 2017 13:18:22 +0800
Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote:

> * Cornelia Huck <cornelia.huck@de.ibm.com> [2017-05-05 14:11:53 +0200]:
>
> > On Fri, 5 May 2017 04:03:44 +0200
> > Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote:

> > > -SubchDev *css_create_virtual_sch(CssDevId bus_id, Error **errp)
> > > +SubchDev *css_create_sch(CssDevId bus_id, bool is_virtio, bool squash_mcss,
> > > + Error **errp)
> > > {
> > > uint16_t schid = 0;
> > > SubchDev *sch;
> > >
> > > if (bus_id.valid) {
> > > - /* Enforce use of virtual cssid. */
> > > - if (bus_id.cssid != VIRTUAL_CSSID) {
> > > - error_setg(errp, "cssid %hhx not valid for virtual devices",
> > > - bus_id.cssid);
> > > + if (is_virtio != (bus_id.cssid == VIRTUAL_CSSID)) {
> > > + error_setg(errp, "cssid %hhx not valid for %s devices",
> > > + bus_id.cssid,
> > > + (is_virtio ? "virtio" : "non-virtio"));
> >
> > This reminds me of something else: virtual 3270 devices will use the
> > virtual channel subsystem by default. I think this is fine: Even though
> > they are not virtio devices, they are fully virtual constructs, and it
> > does not make sense to artificially shove them into another css.
> VIRTUAL CSS (0xFE) is reserved for virtio devices only, no? This is what
> I understood... So we should not put any non-virtio devices into CSS
> 0xFE.
>
> If my understanding is not right before, I agree that we put non-virtio
> (e.g. 3270) devices into CSS 0xFE, and ...
>
> >
> > But this means the distinction should be virtual vs. non-virtual and
> > not virtio vs. non-virtio, and the squashing is only applicable to
> > non-virtual devices. Mainly a naming thing.
> ... do the following modifications here:
> s/virtio/virtual
> s/non-virtio/non-virtual
> s/is_virtio/is_virtual

I realize that I wanted to treat css 0xfe as virtio-only initially; but
I think the virtual/non-virtual distinction actually makes more sense.

- For devices that don't have a hardware equivalent at all (like
virtio-ccw), it's clear that they should be in the virtual css.

- For devices that are always fully emulated (because there's no device
that could be passthroughed), I'd argue that they should be treated as
fully virtual as well. This means 3270, and would include things like a
card punch should anyone feel an urge to emulate that one :)

- An emulation of a device that could also be passthroughed is a bit of
a grey area. One could argue to put it either with the virtual devices
or with the non-virtual ones. But I think we can ignore that for now
(until someone decides that a dasd emulation is a thing qemu urgently
needs...)