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

[Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>]

* Cornelia Huck <cornelia.huck@de.ibm.com> [2017-05-08 12:50:40 +0200]:

> 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;
Aha, here is the origination of my former understanding.

> but I think the virtual/non-virtual distinction actually makes more
> sense.
Agreed. And since we do not have legacy problem that stops us from doing
this, I will fix according to your comments.

> 
> - 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.
Nod.

> 
> - 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 :)
Nod.

> 
> - 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...)
Ok.

-- 
Dong Jia Shi