linux-kernel - Re: [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190514144210.GA14625@lst.de>
Date:   Tue, 14 May 2019 16:42:10 +0200
From:   Christoph Hellwig <hch@....de>
To:     laurentiu.tudor@....com
Cc:     hch@....de, stern@...land.harvard.edu, gregkh@...uxfoundation.org,
        linux-usb@...r.kernel.org, marex@...x.de, leoyang.li@....com,
        linux-kernel@...r.kernel.org, robin.murphy@....com
Subject: Re: [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local
 memory

> @@ -136,6 +137,10 @@ void *hcd_buffer_alloc(
>  		if (size <= pool_max[i])
>  			return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
>  	}
> +
> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
> +		return gen_pool_dma_alloc(hcd->localmem_pool, size, dma);

I think this check needs to be before the above code to use the dma
pools, as we should always use the HCD local memory.  Probably all the
way up just below the size == 0 check, that way we can also remove the
other HCD_LOCAL_MEM check.

> @@ -165,5 +170,10 @@ void hcd_buffer_free(
>  			return;
>  		}
>  	}
> -	dma_free_coherent(hcd->self.sysdev, size, addr, dma);
> +
> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
> +		gen_pool_free(hcd->localmem_pool, (unsigned long)addr,
> +			      size);
> +	else
> +		dma_free_coherent(hcd->self.sysdev, size, addr, dma);

Same here.

> @@ -505,8 +506,15 @@ static int ohci_init (struct ohci_hcd *ohci)
>  	timer_setup(&ohci->io_watchdog, io_watchdog_func, 0);
>  	ohci->prev_frame_no = IO_WATCHDOG_OFF;
>  
> -	ohci->hcca = dma_alloc_coherent (hcd->self.controller,
> -			sizeof(*ohci->hcca), &ohci->hcca_dma, GFP_KERNEL);
> +	if (hcd->driver->flags & HCD_LOCAL_MEM)
> +		ohci->hcca = gen_pool_dma_alloc(hcd->localmem_pool,
> +						sizeof(*ohci->hcca),
> +						&ohci->hcca_dma);
> +	else
> +		ohci->hcca = dma_alloc_coherent(hcd->self.controller,
> +						sizeof(*ohci->hcca),
> +						&ohci->hcca_dma,
> +						GFP_KERNEL);

I wonder if we could just use hcd_buffer_alloc/free here, althought
that would require them to be exported.  I'll leave that decision to
the relevant maintainers, though.

Except for this the series looks exactly what I had envisioned to
get rid of the device local dma_declare_coherent use case, thanks!