Re: [PATCH v2 3/8] s390/cio: add basic protected virtualization support

[Michael Mueller <mimu@xxxxxxxxxxxxx>]

On 25.05.19 11:44, Sebastian Ott wrote:
> On Thu, 23 May 2019, Michael Mueller wrote:
> >   static struct ccw_device * io_subchannel_allocate_dev(struct subchannel *sch)
> >   {
> >   	struct ccw_device *cdev;
> > +	struct gen_pool *dma_pool;
> >   
> >   	cdev  = kzalloc(sizeof(*cdev), GFP_KERNEL);
> > -	if (cdev) {
> > -		cdev->private = kzalloc(sizeof(struct ccw_device_private),
> > -					GFP_KERNEL | GFP_DMA);
> > -		if (cdev->private)
> > -			return cdev;
> > -	}
> > +	if (!cdev)
> > +		goto err_cdev;
> > +	cdev->private = kzalloc(sizeof(struct ccw_device_private),
> > +				GFP_KERNEL | GFP_DMA);
> > +	if (!cdev->private)
> > +		goto err_priv;
> > +	cdev->dev.coherent_dma_mask = sch->dev.coherent_dma_mask;
> > +	cdev->dev.dma_mask = &cdev->dev.coherent_dma_mask;
> > +	dma_pool = cio_gp_dma_create(&cdev->dev, 1);
>
> This can return NULL. gen_pool_alloc will panic in this case.
> [...]

yep, will handled in next version

> > +err_dma_area:
> > +		kfree(io_priv);

one tab gone

> Indentation.
>
> > +err_priv:
> > +	put_device(&sch->dev);
> > +	return ERR_PTR(-ENOMEM);
> >   }
> [...]
> >   void ccw_device_update_sense_data(struct ccw_device *cdev)
> >   {
> >   	memset(&cdev->id, 0, sizeof(cdev->id));
> > -	cdev->id.cu_type   = cdev->private->senseid.cu_type;
> > -	cdev->id.cu_model  = cdev->private->senseid.cu_model;
> > -	cdev->id.dev_type  = cdev->private->senseid.dev_type;
> > -	cdev->id.dev_model = cdev->private->senseid.dev_model;
> > +	cdev->id.cu_type   =
> > +		cdev->private->dma_area->senseid.cu_type;
> > +	cdev->id.cu_model  =
> > +		cdev->private->dma_area->senseid.cu_model;
> > +	cdev->id.dev_type  =
> > +		cdev->private->dma_area->senseid.dev_type;
> > +	cdev->id.dev_model =
> > +		cdev->private->dma_area->senseid.dev_model;
>
> These fit into one line.

yep, surprisingly below 80 characters

> > +/**
> > + * Allocate zeroed dma coherent 31 bit addressable memory using
> > + * the subchannels dma pool. Maximal size of allocation supported
> > + * is PAGE_SIZE.
> > + */
> drivers/s390/cio/device_ops.c:708: warning: Function parameter or member 'cdev' not described in 'ccw_device_dma_zalloc'
> drivers/s390/cio/device_ops.c:708: warning: Function parameter or member 'size' not described in 'ccw_device_dma_zalloc'

changing comment open token

> Reviewed-by: Sebastian Ott <sebott@xxxxxxxxxxxxx>

Thanks!


Michael