On Fri, 26 Apr 2019, Halil Pasic wrote:> @@ -224,6 +228,9 @@ struct subchannel *css_alloc_subchannel(struct subchannel_id schid, > INIT_WORK(&sch->todo_work, css_sch_todo); > sch->dev.release = &css_subchannel_release; > device_initialize(&sch->dev); > + sch->dma_mask = css_dev_dma_mask; > + sch->dev.dma_mask = &sch->dma_mask; > + sch->dev.coherent_dma_mask = sch->dma_mask;Could we do: sch->dev.dma_mask = &sch->dev.coherent_dma_mask; sch->dev.coherent_dma_mask = css_dev_dma_mask; ?> +#define POOL_INIT_PAGES 1 > +static struct gen_pool *cio_dma_pool; > +/* Currently cio supports only a single css */ > +#define CIO_DMA_GFP (GFP_KERNEL | __GFP_ZERO)__GFP_ZERO has no meaning with the dma api (since all implementations do an implicit zero initialization) but let's keep it for the sake of documentation. We need GFP_DMA here (which will return addresses < 2G on s390)!> +void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size) > +{ > + if (!cpu_addr) > + return; > + memset(cpu_addr, 0, size);Hm, normally I'd do the memset during alloc not during free - but maybe this makes more sense here with your usecase in mind.> @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > unregister_reboot_notifier(&css_reboot_notifier); > goto out_unregister; > } > + cio_dma_pool_init();This is too late for early devices (ccw console!).
On Wed, 8 May 2019 15:18:10 +0200 (CEST) Sebastian Ott <sebott at linux.ibm.com> wrote:> > +void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size) > > +{ > > + if (!cpu_addr) > > + return; > > + memset(cpu_addr, 0, size); > > Hm, normally I'd do the memset during alloc not during free - but maybe > this makes more sense here with your usecase in mind.I allocate the backing as zeroed, and zero the memory before putting it back to the pool. So the stuff in the pool is always zeroed.> > > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > > unregister_reboot_notifier(&css_reboot_notifier); > > goto out_unregister; > > } > > + cio_dma_pool_init(); > > This is too late for early devices (ccw console!).You have already raised concern about this last time (thanks). I think, I've addressed this issue: tje cio_dma_pool is only used by the airq stuff. I don't think the ccw console needs it. Please have an other look at patch #6, and explain your concern in more detail if it persists. (@Mimu: What do you think, am I wrong here?) Thanks for having a look! Regards, Halil
On Wed, 8 May 2019, Halil Pasic wrote:> On Wed, 8 May 2019 15:18:10 +0200 (CEST) > Sebastian Ott <sebott at linux.ibm.com> wrote: > > > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > > > unregister_reboot_notifier(&css_reboot_notifier); > > > goto out_unregister; > > > } > > > + cio_dma_pool_init(); > > > > This is too late for early devices (ccw console!). > > You have already raised concern about this last time (thanks). I think, > I've addressed this issue: tje cio_dma_pool is only used by the airq > stuff. I don't think the ccw console needs it. Please have an other look > at patch #6, and explain your concern in more detail if it persists.Oh, you don't use the global pool for that - yes that should work.
On Wed, 8 May 2019 23:22:10 +0200 Halil Pasic <pasic at linux.ibm.com> wrote:> On Wed, 8 May 2019 15:18:10 +0200 (CEST) > Sebastian Ott <sebott at linux.ibm.com> wrote:> > > @@ -1063,6 +1163,7 @@ static int __init css_bus_init(void) > > > unregister_reboot_notifier(&css_reboot_notifier); > > > goto out_unregister; > > > } > > > + cio_dma_pool_init(); > > > > This is too late for early devices (ccw console!). > > You have already raised concern about this last time (thanks). I think, > I've addressed this issue: tje cio_dma_pool is only used by the airq > stuff. I don't think the ccw console needs it. Please have an other look > at patch #6, and explain your concern in more detail if it persists.What about changing the naming/adding comments here, so that (1) folks aren't confused by the same thing in the future and (2) folks don't try to use that pool for something needed for the early ccw consoles?
On Wed, 8 May 2019 15:18:10 +0200 (CEST) Sebastian Ott <sebott at linux.ibm.com> wrote:> > @@ -224,6 +228,9 @@ struct subchannel *css_alloc_subchannel(struct subchannel_id schid, > > INIT_WORK(&sch->todo_work, css_sch_todo); > > sch->dev.release = &css_subchannel_release; > > device_initialize(&sch->dev); > > + sch->dma_mask = css_dev_dma_mask; > > + sch->dev.dma_mask = &sch->dma_mask; > > + sch->dev.coherent_dma_mask = sch->dma_mask; > > Could we do: > sch->dev.dma_mask = &sch->dev.coherent_dma_mask; > sch->dev.coherent_dma_mask = css_dev_dma_mask; > ?Looks like a good idea to me. We will do it for all 3 (sch, ccw and css). Thanks! Regards, Halil