On Mon, 27 May 2019 14:30:14 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
> On Mon, 27 May 2019 12:38:02 +0200
> Cornelia Huck <cohuck@xxxxxxxxxx> wrote:
>
> > On Thu, 23 May 2019 18:22:04 +0200
> > Michael Mueller <mimu@xxxxxxxxxxxxx> wrote:
> >
> > > From: Halil Pasic <pasic@xxxxxxxxxxxxx>
> > >
> > > As virtio-ccw devices are channel devices, we need to use the dma area
> > > for any communication with the hypervisor.
> > >
> > > It handles neither QDIO in the common code, nor any device type specific
> > > stuff (like channel programs constructed by the DASD driver).
> > >
> > > An interesting side effect is that virtio structures are now going to
> > > get allocated in 31 bit addressable storage.
> > >
> > > Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
> >
> > [Side note: you really should add your s-o-b if you send someone else's
> > patches... if Halil ends up committing them, it's fine, though.]
> >
> > > ---
> > > arch/s390/include/asm/ccwdev.h | 4 +++
> > > drivers/s390/cio/ccwreq.c | 9 +++---
> > > drivers/s390/cio/device.c | 64 +++++++++++++++++++++++++++++++++-------
> > > drivers/s390/cio/device_fsm.c | 53 ++++++++++++++++++++-------------
> > > drivers/s390/cio/device_id.c | 20 +++++++------
> > > drivers/s390/cio/device_ops.c | 21 +++++++++++--
> > > drivers/s390/cio/device_pgid.c | 22 +++++++-------
> > > drivers/s390/cio/device_status.c | 24 +++++++--------
> > > drivers/s390/cio/io_sch.h | 20 +++++++++----
> > > drivers/s390/virtio/virtio_ccw.c | 10 -------
> > > 10 files changed, 164 insertions(+), 83 deletions(-)
> > >
> >
> > (...)
> >
> > > @@ -1593,20 +1622,31 @@ struct ccw_device * __init ccw_device_create_console(struct ccw_driver *drv)
> > > return ERR_CAST(sch);
> > >
> > > io_priv = kzalloc(sizeof(*io_priv), GFP_KERNEL | GFP_DMA);
> > > - if (!io_priv) {
> > > - put_device(&sch->dev);
> > > - return ERR_PTR(-ENOMEM);
> > > - }
> > > + if (!io_priv)
> > > + goto err_priv;
> > > + io_priv->dma_area = dma_alloc_coherent(&sch->dev,
> > > + sizeof(*io_priv->dma_area),
> > > + &io_priv->dma_area_dma, GFP_KERNEL);
> >
> > Even though we'll only end up here for 3215 or 3270 consoles, this sent
> > me looking.
> >
> > This code is invoked via console_init(). A few lines down in
> > start_kernel(), we have
> >
> > /*
> > * This needs to be called before any devices perform DMA
> > * operations that might use the SWIOTLB bounce buffers. It will
> > * mark the bounce buffers as decrypted so that their usage will
> > * not cause "plain-text" data to be decrypted when accessed.
> > */
> > mem_encrypt_init();
> >
> > So, I'm wondering if creating the console device interacts in any way
> > with the memory encryption interface?
>
> I do things a bit different than x86: the SWIOTLB stuff is set up in
> mem_init(). So I think we should be fine. If there is a down-side to
> calling swiotlb_update_mem_attributes() earlier, honestly I'm
> not sure.
Neither am I; do any of the folks who looked at the swiotlb patch have
an idea?
>
> >
> > [Does basic recognition work if you start a protected virt guest with a
> > 3270 console? I realize that the console is unlikely to work, but that
> > should at least exercise this code path.]
>
> I've already had some thoughts along these lines and slapped
> -device x-terminal3270,chardev=char_0,devno=fe.0.000a,id=terminal_0 \
> on my qemu command line. The ccw device does show up in the guest...
>
> Device Subchan. DevType CU Type Use PIM PAM POM CHPIDs
> ----------------------------------------------------------------------
> 0.0.0000 0.0.0000 0000/00 3832/01 yes 80 80 ff 00000000 00000000
> 0.0.000a 0.0.0001 0000/00 3270/00 80 80 ff 01000000 00000000
> 0.0.0002 0.0.0002 0000/00 3832/09 yes 80 80 ff 00000000 00000000
> 0.0.0300 0.0.0003 0000/00 3832/02 yes 80 80 ff 00000000 00000000
> 0.0.0301 0.0.0004 0000/00 3832/02 yes 80 80 ff 00000000 00000000
>
> But I would not call it a comprehensive test...
If you only add the device, it will show up as a normal ccw device in
the guest; i.e. device recognition is done at the same time as for the
other ccw devices. Still good to see that nothing breaks there :)
To actually make the guest use the 3270 as its console, I guess you
need to explicitly force it (see
https://wiki.qemu.org/Features/3270#Using_3270_as_the_console)...
actually starting the console will almost certainly fail; but you can
at least check whether device recognition in the console path works.
>
> Mimu, do we have something more elaborate with regards to this?
I don't think we need extensive testing here; just checking that the
sequence is not fundamentally broken.
