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: <f4fc0151-5e49-77fd-7736-37cdfe57c268@nxp.com>
Date:   Wed, 15 May 2019 09:57:12 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Christoph Hellwig <hch@....de>
CC:     "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "marex@...x.de" <marex@...x.de>, Leo Li <leoyang.li@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robin.murphy@....com" <robin.murphy@....com>
Subject: Re: [RFC PATCH v2 1/3] USB: use genalloc for USB HCs with local
 memory

On 14.05.2019 17:42, Christoph Hellwig wrote:
>> @@ -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.

Alright.

>> @@ -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.

Ok.

>> @@ -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!

Glad I could help. On the remoteproc_virtio.c case, I had a cursory look 
and found out that the dma_declare_coherent_memory() usage was 
introduced quite recently, by this patch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=086d08725d34c6b3333db710344ae9c4fdafb2d5

---
Best Regards, Laurentiu