* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type [not found] <a2a6bf12-529c-cb20-c69f-106d34e1204a@st.com> @ 2018-01-30 10:11 ` Benjamin GAIGNARD 2018-01-30 13:25 ` Arnd Bergmann 0 siblings, 1 reply; 7+ messages in thread From: Benjamin GAIGNARD @ 2018-01-30 10:11 UTC (permalink / raw) To: Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> ; David Brown, Arnd Bergmann Cc: linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Gaignard On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: > Hello Andy,David, + Arnd I have the same issue on drm-misc-next. Does Arnaud's fix make sense or should we update/change the way of how we compile the kernel ? Regards, Benjamin > > > I'm facing a compilation error using COMPILE_TEST config, > could you crosscheck the issue. > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git > commit: b2cd1df66037e7c4697c7e40496bf7e4a5e16a2d :Linux 4.15-rc7 > > reproduce: > --------- > #apt-get install gcc-arm-linux-gnueabi > #apt-get install gcc-arm-linux-gnueabihf > > apply .config (in attachment) > > #make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- -j4 > > > error: > ----- > CC drivers/firmware/qcom_scm.o > drivers/firmware/qcom_scm.c: In function ‘qcom_scm_assign_mem’: > drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of > ‘dma_alloc_coherent’ from incompatible pointer type > [-Werror=incompatible-pointer-types] > ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_phys, GFP_KERNEL); > ^ > In file included from drivers/firmware/qcom_scm.c:21:0: > ./include/linux/dma-mapping.h:549:21: note: expected ‘dma_addr_t * {aka > long long unsigned int *}’ but argument is of type ‘phys_addr_t * {aka > unsigned int *}’ > static inline void *dma_alloc_coherent(struct device *dev, size_t size, > ^ > cc1: some warnings being treated as errors > scripts/Makefile.build:310: recipe for target > 'drivers/firmware/qcom_scm.o' failed > make[2]: *** [drivers/firmware/qcom_scm.o] Error 1 > > patch to fix compilation issue: > ------------------------------ > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c > index af4c752..8dfbe61 100644 > --- a/drivers/firmware/qcom_scm.c > +++ b/drivers/firmware/qcom_scm.c > @@ -448,7 +448,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t > mem_sz, > struct qcom_scm_mem_map_info *mem_to_map; > phys_addr_t mem_to_map_phys; > phys_addr_t dest_phys; > - phys_addr_t ptr_phys; > + dma_addr_t ptr_phys; > size_t mem_to_map_sz; > size_t dest_sz; > size_t src_sz; > > Thanks, > Arnaud ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type 2018-01-30 10:11 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type Benjamin GAIGNARD @ 2018-01-30 13:25 ` Arnd Bergmann 2018-01-30 14:00 ` Benjamin Gaignard 2018-02-02 23:29 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? " Bjorn Andersson 0 siblings, 2 replies; 7+ messages in thread From: Arnd Bergmann @ 2018-01-30 13:25 UTC (permalink / raw) To: Benjamin GAIGNARD Cc: Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> , David Brown, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Gaignard On Tue, Jan 30, 2018 at 11:11 AM, Benjamin GAIGNARD <benjamin.gaignard@st.com> wrote: > > On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: >> Hello Andy,David, > + Arnd > > I have the same issue on drm-misc-next. > Does Arnaud's fix make sense or should we update/change the way of how > we compile the kernel ? We've hit a couple of bugs with qcom drivers confusing physical addresses and DMA addresses in the past, usually the drivers were buggy in some form, and tried to use dma_alloc_coherent() to get a buffer that gets passed into a firmware interface taking a physical address, which is of course completely wrong. >> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >> index af4c752..8dfbe61 100644 >> --- a/drivers/firmware/qcom_scm.c >> +++ b/drivers/firmware/qcom_scm.c >> @@ -448,7 +448,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t >> mem_sz, >> struct qcom_scm_mem_map_info *mem_to_map; >> phys_addr_t mem_to_map_phys; >> phys_addr_t dest_phys; >> - phys_addr_t ptr_phys; >> + dma_addr_t ptr_phys; >> size_t mem_to_map_sz; >> size_t dest_sz; >> size_t src_sz; This would be bad: you can basically never have a 'dma_addr_t ptr_phys': it can be exactly one of 'dma address', 'physical address' or a pointer, this claims that the struct member is all three of them. The proper fix here is to stop using dma_alloc_coherent. Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type 2018-01-30 13:25 ` Arnd Bergmann @ 2018-01-30 14:00 ` Benjamin Gaignard 2018-01-30 14:17 ` Arnd Bergmann 2018-02-02 23:29 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? " Bjorn Andersson 1 sibling, 1 reply; 7+ messages in thread From: Benjamin Gaignard @ 2018-01-30 14:00 UTC (permalink / raw) To: Arnd Bergmann Cc: Benjamin GAIGNARD, Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> , David Brown, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org 2018-01-30 14:25 GMT+01:00 Arnd Bergmann <arnd@arndb.de>: > On Tue, Jan 30, 2018 at 11:11 AM, Benjamin GAIGNARD > <benjamin.gaignard@st.com> wrote: >> >> On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: >>> Hello Andy,David, >> + Arnd >> >> I have the same issue on drm-misc-next. >> Does Arnaud's fix make sense or should we update/change the way of how >> we compile the kernel ? > > We've hit a couple of bugs with qcom drivers confusing physical addresses > and DMA addresses in the past, usually the drivers were buggy in > some form, and tried to use dma_alloc_coherent() to get a buffer > that gets passed into a firmware interface taking a physical address, > which is of course completely wrong. > >>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >>> index af4c752..8dfbe61 100644 >>> --- a/drivers/firmware/qcom_scm.c >>> +++ b/drivers/firmware/qcom_scm.c >>> @@ -448,7 +448,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t >>> mem_sz, >>> struct qcom_scm_mem_map_info *mem_to_map; >>> phys_addr_t mem_to_map_phys; >>> phys_addr_t dest_phys; >>> - phys_addr_t ptr_phys; >>> + dma_addr_t ptr_phys; >>> size_t mem_to_map_sz; >>> size_t dest_sz; >>> size_t src_sz; > > This would be bad: you can basically never have a 'dma_addr_t ptr_phys': it can > be exactly one of 'dma address', 'physical address' or a pointer, > this claims that the > struct member is all three of them. > > The proper fix here is to stop using dma_alloc_coherent. Okay but that doesn't explain why we are the only ones to get an issue while the parameter doesn't match function prototype > > Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type 2018-01-30 14:00 ` Benjamin Gaignard @ 2018-01-30 14:17 ` Arnd Bergmann 0 siblings, 0 replies; 7+ messages in thread From: Arnd Bergmann @ 2018-01-30 14:17 UTC (permalink / raw) To: Benjamin Gaignard Cc: Benjamin GAIGNARD, Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> , David Brown, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org On Tue, Jan 30, 2018 at 3:00 PM, Benjamin Gaignard <benjamin.gaignard@linaro.org> wrote: > 2018-01-30 14:25 GMT+01:00 Arnd Bergmann <arnd@arndb.de>: >> On Tue, Jan 30, 2018 at 11:11 AM, Benjamin GAIGNARD >> <benjamin.gaignard@st.com> wrote: >>> >>> On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: >>>> Hello Andy,David, >>> + Arnd >>> >>> I have the same issue on drm-misc-next. >>> Does Arnaud's fix make sense or should we update/change the way of how >>> we compile the kernel ? >> >> We've hit a couple of bugs with qcom drivers confusing physical addresses >> and DMA addresses in the past, usually the drivers were buggy in >> some form, and tried to use dma_alloc_coherent() to get a buffer >> that gets passed into a firmware interface taking a physical address, >> which is of course completely wrong. >> >>>> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c >>>> index af4c752..8dfbe61 100644 >>>> --- a/drivers/firmware/qcom_scm.c >>>> +++ b/drivers/firmware/qcom_scm.c >>>> @@ -448,7 +448,7 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t >>>> mem_sz, >>>> struct qcom_scm_mem_map_info *mem_to_map; >>>> phys_addr_t mem_to_map_phys; >>>> phys_addr_t dest_phys; >>>> - phys_addr_t ptr_phys; >>>> + dma_addr_t ptr_phys; >>>> size_t mem_to_map_sz; >>>> size_t dest_sz; >>>> size_t src_sz; >> >> This would be bad: you can basically never have a 'dma_addr_t ptr_phys': it can >> be exactly one of 'dma address', 'physical address' or a pointer, >> this claims that the >> struct member is all three of them. >> >> The proper fix here is to stop using dma_alloc_coherent. > > Okay but that doesn't explain why we are the only ones to get an issue > while the parameter > doesn't match function prototype For almost all configurations, dma_addr_t and phys_addr_t are the same width, and gcc treats them as compatible. I usually get the warning during randconfig builds, but you seem to have started with a configuration like this. Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? from incompatible pointer type 2018-01-30 13:25 ` Arnd Bergmann 2018-01-30 14:00 ` Benjamin Gaignard @ 2018-02-02 23:29 ` Bjorn Andersson 2018-02-04 10:01 ` Benjamin Gaignard 2018-02-05 16:34 ` Arnd Bergmann 1 sibling, 2 replies; 7+ messages in thread From: Bjorn Andersson @ 2018-02-02 23:29 UTC (permalink / raw) To: Arnd Bergmann Cc: Benjamin GAIGNARD, Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> , David Brown, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Gaignard On Tue 30 Jan 05:25 PST 2018, Arnd Bergmann wrote: > On Tue, Jan 30, 2018 at 11:11 AM, Benjamin GAIGNARD > <benjamin.gaignard@st.com> wrote: > > > > On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: > >> Hello Andy,David, > > + Arnd > > > > I have the same issue on drm-misc-next. > > Does Arnaud's fix make sense or should we update/change the way of how > > we compile the kernel ? > > We've hit a couple of bugs with qcom drivers confusing physical addresses > and DMA addresses in the past, usually the drivers were buggy in > some form, and tried to use dma_alloc_coherent() to get a buffer > that gets passed into a firmware interface taking a physical address, > which is of course completely wrong. > Thanks Arnd, for once again using the words "bug" and "completely wrong" when referring to something that obviously works just fine... The solution you introduced for venus and adreno relies on static reservations of system ram, which isn't pretty, but more importantly isn't viable for the qcom_scm driver. So, how do I dynamically allocate a chunk of coherent memory? Preferably with the possibility of unmapping it temporarily from Linux while passing the buffer into the trusted environment (as any accesses during the operation might cause access violations). Regards, Bjorn ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? from incompatible pointer type 2018-02-02 23:29 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? " Bjorn Andersson @ 2018-02-04 10:01 ` Benjamin Gaignard 2018-02-05 16:34 ` Arnd Bergmann 1 sibling, 0 replies; 7+ messages in thread From: Benjamin Gaignard @ 2018-02-04 10:01 UTC (permalink / raw) To: Bjorn Andersson Cc: Arnd Bergmann, Benjamin GAIGNARD, Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> , David Brown, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org 2018-02-03 0:29 GMT+01:00 Bjorn Andersson <bjorn.andersson@linaro.org>: > On Tue 30 Jan 05:25 PST 2018, Arnd Bergmann wrote: > >> On Tue, Jan 30, 2018 at 11:11 AM, Benjamin GAIGNARD >> <benjamin.gaignard@st.com> wrote: >> > >> > On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: >> >> Hello Andy,David, >> > + Arnd >> > >> > I have the same issue on drm-misc-next. >> > Does Arnaud's fix make sense or should we update/change the way of how >> > we compile the kernel ? >> >> We've hit a couple of bugs with qcom drivers confusing physical addresses >> and DMA addresses in the past, usually the drivers were buggy in >> some form, and tried to use dma_alloc_coherent() to get a buffer >> that gets passed into a firmware interface taking a physical address, >> which is of course completely wrong. >> > > Thanks Arnd, for once again using the words "bug" and "completely wrong" > when referring to something that obviously works just fine... > I can't judge if using dma_alloc_coherent is correct or not here but, obviously, the type of the third parameter doesn't match dma_alloc_coherent prototype. May you consider to fix that ? > > The solution you introduced for venus and adreno relies on static > reservations of system ram, which isn't pretty, but more importantly > isn't viable for the qcom_scm driver. > > > So, how do I dynamically allocate a chunk of coherent memory? > > Preferably with the possibility of unmapping it temporarily from Linux > while passing the buffer into the trusted environment (as any accesses > during the operation might cause access violations). > > Regards, > Bjorn ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? from incompatible pointer type 2018-02-02 23:29 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? " Bjorn Andersson 2018-02-04 10:01 ` Benjamin Gaignard @ 2018-02-05 16:34 ` Arnd Bergmann 1 sibling, 0 replies; 7+ messages in thread From: Arnd Bergmann @ 2018-02-05 16:34 UTC (permalink / raw) To: Bjorn Andersson Cc: Benjamin GAIGNARD, Arnaud POULIQUEN, Andy Gross <andy.gross@linaro.org> , David Brown, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Benjamin Gaignard On Sat, Feb 3, 2018 at 12:29 AM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > On Tue 30 Jan 05:25 PST 2018, Arnd Bergmann wrote: > >> On Tue, Jan 30, 2018 at 11:11 AM, Benjamin GAIGNARD >> <benjamin.gaignard@st.com> wrote: >> > >> > On 01/12/2018 05:11 PM, Arnaud Pouliquen wrote: >> >> Hello Andy,David, >> > + Arnd >> > >> > I have the same issue on drm-misc-next. >> > Does Arnaud's fix make sense or should we update/change the way of how >> > we compile the kernel ? >> >> We've hit a couple of bugs with qcom drivers confusing physical addresses >> and DMA addresses in the past, usually the drivers were buggy in >> some form, and tried to use dma_alloc_coherent() to get a buffer >> that gets passed into a firmware interface taking a physical address, >> which is of course completely wrong. >> > > Thanks Arnd, for once again using the words "bug" and "completely wrong" > when referring to something that obviously works just fine... Sorry, you are right my choice of words was uncalled for. I was getting carried away after seeing Arnaud's suggestion for changing the variable type but not the name, and got carried away. > The solution you introduced for venus and adreno relies on static > reservations of system ram, which isn't pretty, but more importantly > isn't viable for the qcom_scm driver. > > > So, how do I dynamically allocate a chunk of coherent memory? > > Preferably with the possibility of unmapping it temporarily from Linux > while passing the buffer into the trusted environment (as any accesses > during the operation might cause access violations). Well, there is a fundamental problem here that 'coherent memory' as such does not exist. The DMA mapping interface is meant to guarantee coherency between a pair of bus masters, i.e. a CPU and a device, in a way that is a specific to that pair, without the driver having to worry about the details. What the qcom_scm driver does here is to approximate the behavior it needs by calling the dma mapping interfaces. It happens to work some of the time, but it's really something else as we are talking to firmware here and it doesn't have the same semantics that the dma mapping code provides. I think what we want here is to come up with a new allocator that matches the requirements of SCM and that doesn't use dma_addr_t. For allocating memory that can be unmapped, I think the low-level CMA allocation can work, which would also make it independent of the device we are allocating for. Can you say what exactly the requirements are on the memory buffer? I assume you don't really want uncached memory, but instead want a buffer that gets flushed to physical memory before you pass it down so you don't get a writeback to memory that is inaccessible by the OS, correct? Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-02-05 16:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <a2a6bf12-529c-cb20-c69f-106d34e1204a@st.com>
2018-01-30 10:11 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ‘dma_alloc_coherent’ from incompatible pointer type Benjamin GAIGNARD
2018-01-30 13:25 ` Arnd Bergmann
2018-01-30 14:00 ` Benjamin Gaignard
2018-01-30 14:17 ` Arnd Bergmann
2018-02-02 23:29 ` Compilation error report for: drivers/firmware/qcom_scm.c:469:47: error: passing argument 3 of ?dma_alloc_coherent? " Bjorn Andersson
2018-02-04 10:01 ` Benjamin Gaignard
2018-02-05 16:34 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox