Re: [RFC PATCH 04/12] s390/cio: introduce cio DMA pool

[Halil Pasic <pasic@linux.ibm.com>]

On Wed, 10 Apr 2019 18:07:19 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 10 Apr 2019 17:31:48 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Tue, 9 Apr 2019 19:14:53 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> > 
> > [..]
> > 
> > > > > At this point, you're always going via the css0 device. I'm
> > > > > wondering whether you should pass in the cssid here and use
> > > > > css_by_id(cssid) to make this future proof. But then, I'm not
> > > > > quite clear from which context this will be called.      
> > > > 
> > > > As said before I don't see MCSS-E coming. Regarding the client code,
> > > > please check out the rest of the series. Especially patch 6. 
> > > > 
> > > > From my perspective it would be at this stage saner to make it more
> > > > obvious that only one css is supported (at the moment), than to
> > > > implement MCSS-E, or to make this (kind of) MCSS-E ready.    
> > > 
> > > I disagree. I think there's value in keeping the interfaces clean
> > > (within reasonable bounds, of course.) Even if there is no
> > > implementation of MCSS-E other than QEMU... it is probably a good idea
> > > to spend some brain cycles to make this conceptually clean.  
> > 
> > AFAIU Linux currently does not support MCSS-E. I don't have the
> > bandwidth to implement MCSS-E support in the kernel.
> > 
> > I fully agree for external interfaces should be MCSS-E conform, so
> > should we ever decide to implement we don't have to overcome self-made
> > obstacles.
> > 
> > Kernel internal stuff however, IMHO, should avoid carrying a ballast of
> > an 20%-30% implemented MCSS-E support. I see no benefit.
> > 
> > But I don't insist. If you have a good idea how to make this more MCSS-E
> > conform, you are welcome. In think something like this is best done on
> > top of a series that provides a basic working solution. Especially if the
> > conceptually clean thing is conceptually or code-wise more complex than
> > the basic solution. If you have something in mind that is simpler and
> > more lightweight than what I did here, I would be more than happy to make
> > that happen.
> 
> I haven't asked for adding complete support for MCSS-E, just to keep it
> in the back of our minds...

I will, I promise ;)!

> 
> Back to the issue at hand. You're currently hard-wiring this to the
> css0 device. My question is: Does it make sense to have a per-css area?

What do you mean by area?

> From my limited perspective, I think it does (more css images would
> mean more devices, and just adding smaller areas per-css is probably
> easier than adding one big area scaling with the number of css images).

Currently all devices are hooked up to css0. This is also hard-wired:

int css_register_subchannel(struct subchannel *sch)                             
{                                                                               
        int ret;                                                                
                                                                                
        /* Initialize the subchannel structure */                               
        sch->dev.parent = &channel_subsystems[0]->device;

I think the hard-wiring the DMA properties of the CSS to
channel_subsystems[0] is consistent with prior art.

> In that case, what about tacking the area to the actual css object,
> instead of having the global variable? Should be an easy change, and
> feels cleaner.
> 

I already got rid of the global variable. Instead we have an accessor
now.

My problem with a gen_pool per ccs approach is the airq aiv vectors.
Those vectors are currently shared between devices (at least for
virtio-ccw), and those vectors must be allocated as shared (DMA) memory.
Please take another look at patch #6. Would you like 
airq_iv_create(unsigned long bits, unsigned long flags) to take
a struct channel_subsystem argument?

Regards,
Halil