Re: [PATCH 06/10] s390/cio: add basic protected virtualization support

From: Cornelia Huck <cohuck@redhat.com>
To: Halil Pasic <pasic@linux.ibm.com>
Cc: kvm@vger.kernel.org, linux-s390@vger.kernel.org,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Sebastian Ott <sebott@linux.ibm.com>,
	virtualization@lists.linux-foundation.org,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Christoph Hellwig <hch@infradead.org>,
	Thomas Huth <thuth@redhat.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Viktor Mihajlovski <mihajlov@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	Farhan Ali <alifm@linux.ibm.com>,
	Eric Farman <farman@linux.ibm.com>
Subject: Re: [PATCH 06/10] s390/cio: add basic protected virtualization support
Date: Mon, 20 May 2019 15:43:46 +0200	[thread overview]
Message-ID: <20190520154346.4f95ab3a.cohuck@redhat.com> (raw)
In-Reply-To: <20190520143411.15130af3.pasic@linux.ibm.com>

On Mon, 20 May 2019 14:34:11 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:

> On Mon, 20 May 2019 12:21:43 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > On Sat, 18 May 2019 20:11:00 +0200
> > Halil Pasic <pasic@linux.ibm.com> wrote:
> >   
> > > On Thu, 16 May 2019 08:29:28 +0200
> > > Cornelia Huck <cohuck@redhat.com> wrote:
> > >   
> > > > On Wed, 15 May 2019 22:51:58 +0200
> > > > Halil Pasic <pasic@linux.ibm.com> wrote:  

> > > > > A side note: making the subchannel device 'own' the DMA stuff of a
> > > > > ccw device (something that was discussed in the RFC thread) is tricky
> > > > > because the ccw device may outlive the subchannel (all that orphan
> > > > > stuff).    
> > > > 
> > > > Yes, that's... eww. Not really a problem for virtio-ccw devices (which
> > > > do not support the disconnected state), but can we make DMA and the
> > > > subchannel moving play nice with each other at all?
> > > >     
> > > 
> > > I don't quite understand the question. This series does not have any
> > > problems with that AFAIU. Can you please clarify?  
> > 
> > Wait, weren't you saying that there actually is a problem?
> >  
> 
> No, what I tried to say is: if we tried to make all the dma mem belong to
> the subchannel device, we would have a problem. It appeared as a
> tempting opportunity for consolidation, but I decided to not do it.

Ok, that makes sense.

> 
> > We seem to have the following situation:
> > - the device per se is represented by the ccw device
> > - the subchannel is the means of communication, and dma is tied to the
> >   (I/O ?) subchannel  
> 
> It is not. When for example a virtio-ccw device talks to the device
> using a channel program, the dma mem hosting the channel program belongs
> to the ccw device and not to the subchannel.
> 
> In fact everything but the stuff in io_priv->dma_area belongs to the ccw
> device.

Normal machine check handling hopefully should cover this one, then.

> 
> > - the machine check handling code may move a ccw device to a different
> >   subchannel, or even to a fake subchannel (orphanage handling)
> >   
> 
> Right!
> 
> > The moving won't happen with virtio-ccw devices (as they do not support
> > the disconnected state, which is a prereq for being moved around), but
> > at a glance, this looks like it is worth some more thought.
> > 
> > - Are all (I/O) subchannels using e.g. the same dma size? (TBH, that
> >   question sounds a bit silly: that should be a property belonging to
> >   the ccw device, shouldn't it?)
> > - What dma properties does the fake subchannel have? (Probably none, as
> >   its only purpose is to serve as a parent for otherwise parentless
> >   disconnected ccw devices, and is therefore not involved in any I/O.)
> > - There needs to be some kind of handling in the machine check code, I
> >   guess? We would probably need a different allocation if we end up at
> >   a different subchannel?
> >   
> 
> Basically nothing changes with mem ownership, except that some bits are
> dma memory now. Should I provide a more detailed answer to the
> questions above?

No real need, I simply did not understand your initial remark correctly.

> 
> > I think we can assume that the dma size is at most 31 bits (since that
> > is what the common I/O layer needs); but can we also assume that it
> > will always be at least 31 bits?
> >   
> 
> You mean dma_mas by dma size?

Whatever it is called :) IIUC, we need to go with 31 bit for any
channel I/O related structures; I was mainly wondering whether any
devices need a lower limit for some of the memory they use. I would be
surprised if they did, but you never know :)