All of lore.kernel.org
 help / color / mirror / Atom feed
From: rusty@rustcorp.com.au (Rusty Russell)
To: linux-arm-kernel@lists.infradead.org
Subject: scatterlist: sg_set_buf() argument must be in linear mapping (sha1: ac4e97abce9b80c020e7113325f49e58b7b15e3f)
Date: Tue, 23 Jul 2013 12:00:30 +0930	[thread overview]
Message-ID: <87ppua9feh.fsf@rustcorp.com.au> (raw)
In-Reply-To: <51ED2C74.7050502@monstr.eu>

Michal Simek <monstr@monstr.eu> writes:
> + Ohad
>
> On 07/22/2013 03:47 AM, Rusty Russell wrote:
>> Michal Simek <monstr@monstr.eu> writes:
>>> Hi Rusty and Jens,
>>>
>>> I am getting problem with your patch which you have added to the kernel.
>>> The problem is with my arm zynq remoteproc driver where
>>> I use dma_declare_coherent_memory() to specify memory for remoteproc
>>> which is ioremap to the vmalloc area.
>>>
>>> Based on that buf addr is not inside ram even this coherent memory is in ram.
>>> That's why virt_addr_valid(buf) is failing.
>> 
>> But sg_set_buf() calls virt_to_page(buf).  So does that work?
>> 
>> I would think calling sg_set_page() directly would be what you want
>> here...
>
> Let me take some code from virtio_rpmsg_bus.c to show that problematic part.
>
> bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
> 				RPMSG_TOTAL_BUF_SPACE,
> 				&vrp->bufs_dma, GFP_KERNEL);
> vrp->rbufs = bufs_va;
> for (i = 0; i < RPMSG_NUM_BUFS / 2; i++) {
> 	struct scatterlist sg;
> 	void *cpu_addr = vrp->rbufs + i * RPMSG_BUF_SIZE;
>
> 	sg_init_one(&sg, cpu_addr, RPMSG_BUF_SIZE);
> }

Hmm.  Looking at arch/arm/include/asm/memory.h:

#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
#define virt_addr_valid(kaddr)	((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory)

pfn_to_page in ARM seems to be the asm-generic one, which depends on
CONFIG_SPARSEMEM/CONFIG_DISCONTIGMEM/CONFIG_FLATMEM etc.

Perhaps virt_addr_valid() is wrong for your config?  It's pretty clear
that you shouldn't call virt_to_page() on something for which
!virt_addr_valid().

Cheers,
Rusty.

WARNING: multiple messages have this Message-ID (diff)
From: Rusty Russell <rusty@rustcorp.com.au>
To: monstr@monstr.eu
Cc: Jens Axboe <axboe@kernel.dk>, LKML <linux-kernel@vger.kernel.org>,
	Ohad Ben-Cohen <ohad@wizery.com>,
	Russell King <linux@arm.linux.org.uk>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: scatterlist: sg_set_buf() argument must be in linear mapping (sha1: ac4e97abce9b80c020e7113325f49e58b7b15e3f)
Date: Tue, 23 Jul 2013 12:00:30 +0930	[thread overview]
Message-ID: <87ppua9feh.fsf@rustcorp.com.au> (raw)
In-Reply-To: <51ED2C74.7050502@monstr.eu>

Michal Simek <monstr@monstr.eu> writes:
> + Ohad
>
> On 07/22/2013 03:47 AM, Rusty Russell wrote:
>> Michal Simek <monstr@monstr.eu> writes:
>>> Hi Rusty and Jens,
>>>
>>> I am getting problem with your patch which you have added to the kernel.
>>> The problem is with my arm zynq remoteproc driver where
>>> I use dma_declare_coherent_memory() to specify memory for remoteproc
>>> which is ioremap to the vmalloc area.
>>>
>>> Based on that buf addr is not inside ram even this coherent memory is in ram.
>>> That's why virt_addr_valid(buf) is failing.
>> 
>> But sg_set_buf() calls virt_to_page(buf).  So does that work?
>> 
>> I would think calling sg_set_page() directly would be what you want
>> here...
>
> Let me take some code from virtio_rpmsg_bus.c to show that problematic part.
>
> bufs_va = dma_alloc_coherent(vdev->dev.parent->parent,
> 				RPMSG_TOTAL_BUF_SPACE,
> 				&vrp->bufs_dma, GFP_KERNEL);
> vrp->rbufs = bufs_va;
> for (i = 0; i < RPMSG_NUM_BUFS / 2; i++) {
> 	struct scatterlist sg;
> 	void *cpu_addr = vrp->rbufs + i * RPMSG_BUF_SIZE;
>
> 	sg_init_one(&sg, cpu_addr, RPMSG_BUF_SIZE);
> }

Hmm.  Looking at arch/arm/include/asm/memory.h:

#define virt_to_page(kaddr)	pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
#define virt_addr_valid(kaddr)	((unsigned long)(kaddr) >= PAGE_OFFSET && (unsigned long)(kaddr) < (unsigned long)high_memory)

pfn_to_page in ARM seems to be the asm-generic one, which depends on
CONFIG_SPARSEMEM/CONFIG_DISCONTIGMEM/CONFIG_FLATMEM etc.

Perhaps virt_addr_valid() is wrong for your config?  It's pretty clear
that you shouldn't call virt_to_page() on something for which
!virt_addr_valid().

Cheers,
Rusty.

  reply	other threads:[~2013-07-23  2:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-17  7:48 scatterlist: sg_set_buf() argument must be in linear mapping (sha1: ac4e97abce9b80c020e7113325f49e58b7b15e3f) Michal Simek
2013-07-22  1:47 ` Rusty Russell
2013-07-22 12:58   ` Michal Simek
2013-07-23  2:30     ` Rusty Russell [this message]
2013-07-23  2:30       ` Rusty Russell
2013-07-23 10:37       ` Russell King - ARM Linux
2013-07-23 10:37         ` Russell King - ARM Linux
2013-07-23 12:12         ` Michal Simek
2013-07-23 12:12           ` Michal Simek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ppua9feh.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.