Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Thu, Jun 24, 2021 at 11:58:57PM +0800, Claire Chang wrote: > On Thu, Jun 24, 2021 at 11:56 PM Konrad Rzeszutek Wilk > wrote: > > > > On Thu, Jun 24, 2021 at 10:10:51AM -0400, Qian Cai wrote: > > > > > > > > > On 6/24/2021 7:48 AM, Will Deacon wrote: > > > > Ok, diff below which attempts to tackle the offset issue I mentioned as > > > > well. Qian Cai -- please can you try with these changes? > > > > > > This works fine. > > > > Cool. Let me squash this patch in #6 and rebase the rest of them. > > > > Claire, could you check the devel/for-linus-5.14 say by end of today to > > double check that I didn't mess anything up please? > > I just submitted v15 here > (https://lore.kernel.org/patchwork/cover/1451322/) in case it's > helpful. Oh! Nice! > I'll double check of course. Thanks for the efforts! I ended up using your patch #6 and #7. Please double-check.
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Thu, Jun 24, 2021 at 11:56 PM Konrad Rzeszutek Wilk
wrote:
>
> On Thu, Jun 24, 2021 at 10:10:51AM -0400, Qian Cai wrote:
> >
> >
> > On 6/24/2021 7:48 AM, Will Deacon wrote:
> > > Ok, diff below which attempts to tackle the offset issue I mentioned as
> > > well. Qian Cai -- please can you try with these changes?
> >
> > This works fine.
>
> Cool. Let me squash this patch in #6 and rebase the rest of them.
>
> Claire, could you check the devel/for-linus-5.14 say by end of today to
> double check that I didn't mess anything up please?
I just submitted v15 here
(https://lore.kernel.org/patchwork/cover/1451322/) in case it's
helpful.
I'll double check of course. Thanks for the efforts!
>
> Will,
>
> Thank you for generating the fix! I am going to run it on x86 and Xen
> to make sure all is good (granted last time I ran devel/for-linus-5.14
> on that setup I didn't see any errors so I need to double check
> I didn't do something silly like run a wrong kernel).
>
>
> >
> > >
> > > Will
> > >
> > > --->8
> > >
> > > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> > > index 175b6c113ed8..39284ff2a6cd 100644
> > > --- a/include/linux/swiotlb.h
> > > +++ b/include/linux/swiotlb.h
> > > @@ -116,7 +116,9 @@ static inline bool is_swiotlb_buffer(struct device
> > > *dev, phys_addr_t paddr)
> > >
> > > static inline bool is_swiotlb_force_bounce(struct device *dev)
> > > {
> > > - return dev->dma_io_tlb_mem->force_bounce;
> > > + struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> > > +
> > > + return mem && mem->force_bounce;
> > > }
> > >
> > > void __init swiotlb_exit(void);
> > > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > > index 44be8258e27b..0ffbaae9fba2 100644
> > > --- a/kernel/dma/swiotlb.c
> > > +++ b/kernel/dma/swiotlb.c
> > > @@ -449,6 +449,7 @@ static int swiotlb_find_slots(struct device *dev,
> > > phys_addr_t orig_addr,
> > > dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
> > > unsigned int nslots = nr_slots(alloc_size), stride;
> > > unsigned int index, wrap, count = 0, i;
> > > + unsigned int offset = swiotlb_align_offset(dev, orig_addr);
> > > unsigned long flags;
> > >
> > > BUG_ON(!nslots);
> > > @@ -497,7 +498,7 @@ static int swiotlb_find_slots(struct device *dev,
> > > phys_addr_t orig_addr,
> > > for (i = index; i < index + nslots; i++) {
> > > mem->slots[i].list = 0;
> > > mem->slots[i].alloc_size =
> > > - alloc_size - ((i - index) << IO_TLB_SHIFT);
> > > + alloc_size - (offset + ((i - index) <<
> > > IO_TLB_SHIFT));
> > > }
> > > for (i = index - 1;
> > > io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
> > >
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Thu, Jun 24, 2021 at 10:10:51AM -0400, Qian Cai wrote:
>
>
> On 6/24/2021 7:48 AM, Will Deacon wrote:
> > Ok, diff below which attempts to tackle the offset issue I mentioned as
> > well. Qian Cai -- please can you try with these changes?
>
> This works fine.
Cool. Let me squash this patch in #6 and rebase the rest of them.
Claire, could you check the devel/for-linus-5.14 say by end of today to
double check that I didn't mess anything up please?
Will,
Thank you for generating the fix! I am going to run it on x86 and Xen
to make sure all is good (granted last time I ran devel/for-linus-5.14
on that setup I didn't see any errors so I need to double check
I didn't do something silly like run a wrong kernel).
>
> >
> > Will
> >
> > --->8
> >
> > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> > index 175b6c113ed8..39284ff2a6cd 100644
> > --- a/include/linux/swiotlb.h
> > +++ b/include/linux/swiotlb.h
> > @@ -116,7 +116,9 @@ static inline bool is_swiotlb_buffer(struct device
> > *dev, phys_addr_t paddr)
> >
> > static inline bool is_swiotlb_force_bounce(struct device *dev)
> > {
> > - return dev->dma_io_tlb_mem->force_bounce;
> > + struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> > +
> > + return mem && mem->force_bounce;
> > }
> >
> > void __init swiotlb_exit(void);
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index 44be8258e27b..0ffbaae9fba2 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -449,6 +449,7 @@ static int swiotlb_find_slots(struct device *dev,
> > phys_addr_t orig_addr,
> > dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
> > unsigned int nslots = nr_slots(alloc_size), stride;
> > unsigned int index, wrap, count = 0, i;
> > + unsigned int offset = swiotlb_align_offset(dev, orig_addr);
> > unsigned long flags;
> >
> > BUG_ON(!nslots);
> > @@ -497,7 +498,7 @@ static int swiotlb_find_slots(struct device *dev,
> > phys_addr_t orig_addr,
> > for (i = index; i < index + nslots; i++) {
> > mem->slots[i].list = 0;
> > mem->slots[i].alloc_size =
> > - alloc_size - ((i - index) << IO_TLB_SHIFT);
> > + alloc_size - (offset + ((i - index) <<
> > IO_TLB_SHIFT));
> > }
> > for (i = index - 1;
> > io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
> >
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On 6/24/2021 7:48 AM, Will Deacon wrote:
> Ok, diff below which attempts to tackle the offset issue I mentioned as
> well. Qian Cai -- please can you try with these changes?
This works fine.
>
> Will
>
> --->8
>
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 175b6c113ed8..39284ff2a6cd 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -116,7 +116,9 @@ static inline bool is_swiotlb_buffer(struct device *dev,
> phys_addr_t paddr)
>
> static inline bool is_swiotlb_force_bounce(struct device *dev)
> {
> - return dev->dma_io_tlb_mem->force_bounce;
> + struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> +
> + return mem && mem->force_bounce;
> }
>
> void __init swiotlb_exit(void);
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 44be8258e27b..0ffbaae9fba2 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -449,6 +449,7 @@ static int swiotlb_find_slots(struct device *dev,
> phys_addr_t orig_addr,
> dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
> unsigned int nslots = nr_slots(alloc_size), stride;
> unsigned int index, wrap, count = 0, i;
> + unsigned int offset = swiotlb_align_offset(dev, orig_addr);
> unsigned long flags;
>
> BUG_ON(!nslots);
> @@ -497,7 +498,7 @@ static int swiotlb_find_slots(struct device *dev,
> phys_addr_t orig_addr,
> for (i = index; i < index + nslots; i++) {
> mem->slots[i].list = 0;
> mem->slots[i].alloc_size =
> - alloc_size - ((i - index) << IO_TLB_SHIFT);
> + alloc_size - (offset + ((i - index) << IO_TLB_SHIFT));
> }
> for (i = index - 1;
> io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
>
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Thu, Jun 24, 2021 at 12:34:09PM +0100, Robin Murphy wrote:
> On 2021-06-24 12:18, Will Deacon wrote:
> > On Thu, Jun 24, 2021 at 12:14:39PM +0100, Robin Murphy wrote:
> > > On 2021-06-24 07:05, Claire Chang wrote:
> > > > On Thu, Jun 24, 2021 at 1:43 PM Christoph Hellwig wrote:
> > > > >
> > > > > On Wed, Jun 23, 2021 at 02:44:34PM -0400, Qian Cai wrote:
> > > > > > is_swiotlb_force_bounce at
> > > > > > /usr/src/linux-next/./include/linux/swiotlb.h:119
> > > > > >
> > > > > > is_swiotlb_force_bounce() was the new function introduced in this
> > > > > > patch here.
> > > > > >
> > > > > > +static inline bool is_swiotlb_force_bounce(struct device *dev)
> > > > > > +{
> > > > > > + return dev->dma_io_tlb_mem->force_bounce;
> > > > > > +}
> > > > >
> > > > > To me the crash looks like dev->dma_io_tlb_mem is NULL. Can you
> > > > > turn this into :
> > > > >
> > > > > return dev->dma_io_tlb_mem &&
> > > > > dev->dma_io_tlb_mem->force_bounce;
> > > > >
> > > > > for a quick debug check?
> > > >
> > > > I just realized that dma_io_tlb_mem might be NULL like Christoph
> > > > pointed out since swiotlb might not get initialized.
> > > > However, `Unable to handle kernel paging request at virtual address
> > > > dfff800e` looks more like the address is garbage rather than
> > > > NULL?
> > > > I wonder if that's because dev->dma_io_tlb_mem is not assigned
> > > > properly (which means device_initialize is not called?).
> > >
> > > What also looks odd is that the base "address" 0xdfff8000 is held
> > > in
> > > a couple of registers, but the offset 0xe looks too small to match up to
> > > any
> > > relevant structure member in that dereference chain :/
> >
> > FWIW, I've managed to trigger a NULL dereference locally when swiotlb hasn't
> > been initialised but we dereference 'dev->dma_io_tlb_mem', so I think
> > Christoph's suggestion is needed regardless.
>
> Ack to that - for SWIOTLB_NO_FORCE, io_tlb_default_mem will remain NULL. The
> massive jump in KernelCI baseline failures as of yesterday looks like every
> arm64 machine with less than 4GB of RAM blowing up...
Ok, diff below which attempts to tackle the offset issue I mentioned as
well. Qian Cai -- please can you try with these changes?
Will
--->8
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 175b6c113ed8..39284ff2a6cd 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -116,7 +116,9 @@ static inline bool is_swiotlb_buffer(struct device *dev,
phys_addr_t paddr)
static inline bool is_swiotlb_force_bounce(struct device *dev)
{
- return dev->dma_io_tlb_mem->force_bounce;
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+
+ return mem && mem->force_bounce;
}
void __init swiotlb_exit(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 44be8258e27b..0ffbaae9fba2 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -449,6 +449,7 @@ static int swiotlb_find_slots(struct device *dev,
phys_addr_t orig_addr,
dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
unsigned int nslots = nr_slots(alloc_size), stride;
unsigned int index, wrap, count = 0, i;
+ unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned long flags;
BUG_ON(!nslots);
@@ -497,7 +498,7 @@ static int swiotlb_find_slots(struct device *dev,
phys_addr_t orig_addr,
for (i = index; i < index + nslots; i++) {
mem->slots[i].list = 0;
mem->slots[i].alloc_size =
- alloc_size - ((i - index) << IO_TLB_SHIFT);
+ alloc_size - (offset + ((i - index) << IO_TLB_SHIFT));
}
for (i = index - 1;
io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On 2021-06-24 12:18, Will Deacon wrote:
On Thu, Jun 24, 2021 at 12:14:39PM +0100, Robin Murphy wrote:
On 2021-06-24 07:05, Claire Chang wrote:
On Thu, Jun 24, 2021 at 1:43 PM Christoph Hellwig wrote:
On Wed, Jun 23, 2021 at 02:44:34PM -0400, Qian Cai wrote:
is_swiotlb_force_bounce at /usr/src/linux-next/./include/linux/swiotlb.h:119
is_swiotlb_force_bounce() was the new function introduced in this patch here.
+static inline bool is_swiotlb_force_bounce(struct device *dev)
+{
+ return dev->dma_io_tlb_mem->force_bounce;
+}
To me the crash looks like dev->dma_io_tlb_mem is NULL. Can you
turn this into :
return dev->dma_io_tlb_mem && dev->dma_io_tlb_mem->force_bounce;
for a quick debug check?
I just realized that dma_io_tlb_mem might be NULL like Christoph
pointed out since swiotlb might not get initialized.
However, `Unable to handle kernel paging request at virtual address
dfff800e` looks more like the address is garbage rather than
NULL?
I wonder if that's because dev->dma_io_tlb_mem is not assigned
properly (which means device_initialize is not called?).
What also looks odd is that the base "address" 0xdfff8000 is held in
a couple of registers, but the offset 0xe looks too small to match up to any
relevant structure member in that dereference chain :/
FWIW, I've managed to trigger a NULL dereference locally when swiotlb hasn't
been initialised but we dereference 'dev->dma_io_tlb_mem', so I think
Christoph's suggestion is needed regardless.
Ack to that - for SWIOTLB_NO_FORCE, io_tlb_default_mem will remain NULL.
The massive jump in KernelCI baseline failures as of yesterday looks
like every arm64 machine with less than 4GB of RAM blowing up...
Robin.
But I agree that it won't help
with the issue reported by Qian Cai.
Qian Cai: please can you share your .config and your command line?
Thanks,
Will
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Thu, Jun 24, 2021 at 12:14:39PM +0100, Robin Murphy wrote:
> On 2021-06-24 07:05, Claire Chang wrote:
> > On Thu, Jun 24, 2021 at 1:43 PM Christoph Hellwig wrote:
> > >
> > > On Wed, Jun 23, 2021 at 02:44:34PM -0400, Qian Cai wrote:
> > > > is_swiotlb_force_bounce at
> > > > /usr/src/linux-next/./include/linux/swiotlb.h:119
> > > >
> > > > is_swiotlb_force_bounce() was the new function introduced in this patch
> > > > here.
> > > >
> > > > +static inline bool is_swiotlb_force_bounce(struct device *dev)
> > > > +{
> > > > + return dev->dma_io_tlb_mem->force_bounce;
> > > > +}
> > >
> > > To me the crash looks like dev->dma_io_tlb_mem is NULL. Can you
> > > turn this into :
> > >
> > > return dev->dma_io_tlb_mem && dev->dma_io_tlb_mem->force_bounce;
> > >
> > > for a quick debug check?
> >
> > I just realized that dma_io_tlb_mem might be NULL like Christoph
> > pointed out since swiotlb might not get initialized.
> > However, `Unable to handle kernel paging request at virtual address
> > dfff800e` looks more like the address is garbage rather than
> > NULL?
> > I wonder if that's because dev->dma_io_tlb_mem is not assigned
> > properly (which means device_initialize is not called?).
>
> What also looks odd is that the base "address" 0xdfff8000 is held in
> a couple of registers, but the offset 0xe looks too small to match up to any
> relevant structure member in that dereference chain :/
FWIW, I've managed to trigger a NULL dereference locally when swiotlb hasn't
been initialised but we dereference 'dev->dma_io_tlb_mem', so I think
Christoph's suggestion is needed regardless. But I agree that it won't help
with the issue reported by Qian Cai.
Qian Cai: please can you share your .config and your command line?
Thanks,
Will
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On 2021-06-24 07:05, Claire Chang wrote:
On Thu, Jun 24, 2021 at 1:43 PM Christoph Hellwig wrote:
On Wed, Jun 23, 2021 at 02:44:34PM -0400, Qian Cai wrote:
is_swiotlb_force_bounce at /usr/src/linux-next/./include/linux/swiotlb.h:119
is_swiotlb_force_bounce() was the new function introduced in this patch here.
+static inline bool is_swiotlb_force_bounce(struct device *dev)
+{
+ return dev->dma_io_tlb_mem->force_bounce;
+}
To me the crash looks like dev->dma_io_tlb_mem is NULL. Can you
turn this into :
return dev->dma_io_tlb_mem && dev->dma_io_tlb_mem->force_bounce;
for a quick debug check?
I just realized that dma_io_tlb_mem might be NULL like Christoph
pointed out since swiotlb might not get initialized.
However, `Unable to handle kernel paging request at virtual address
dfff800e` looks more like the address is garbage rather than
NULL?
I wonder if that's because dev->dma_io_tlb_mem is not assigned
properly (which means device_initialize is not called?).
What also looks odd is that the base "address" 0xdfff8000 is
held in a couple of registers, but the offset 0xe looks too small to
match up to any relevant structure member in that dereference chain :/
Robin.
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Thu, Jun 24, 2021 at 1:43 PM Christoph Hellwig wrote:
>
> On Wed, Jun 23, 2021 at 02:44:34PM -0400, Qian Cai wrote:
> > is_swiotlb_force_bounce at /usr/src/linux-next/./include/linux/swiotlb.h:119
> >
> > is_swiotlb_force_bounce() was the new function introduced in this patch
> > here.
> >
> > +static inline bool is_swiotlb_force_bounce(struct device *dev)
> > +{
> > + return dev->dma_io_tlb_mem->force_bounce;
> > +}
>
> To me the crash looks like dev->dma_io_tlb_mem is NULL. Can you
> turn this into :
>
> return dev->dma_io_tlb_mem && dev->dma_io_tlb_mem->force_bounce;
>
> for a quick debug check?
I just realized that dma_io_tlb_mem might be NULL like Christoph
pointed out since swiotlb might not get initialized.
However, `Unable to handle kernel paging request at virtual address
dfff800e` looks more like the address is garbage rather than
NULL?
I wonder if that's because dev->dma_io_tlb_mem is not assigned
properly (which means device_initialize is not called?).
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Wed, Jun 23, 2021 at 02:44:34PM -0400, Qian Cai wrote:
> is_swiotlb_force_bounce at /usr/src/linux-next/./include/linux/swiotlb.h:119
>
> is_swiotlb_force_bounce() was the new function introduced in this patch here.
>
> +static inline bool is_swiotlb_force_bounce(struct device *dev)
> +{
> + return dev->dma_io_tlb_mem->force_bounce;
> +}
To me the crash looks like dev->dma_io_tlb_mem is NULL. Can you
turn this into :
return dev->dma_io_tlb_mem && dev->dma_io_tlb_mem->force_bounce;
for a quick debug check?
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On 6/18/2021 11:40 PM, Claire Chang wrote: > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and > use it to determine whether to bounce the data or not. This will be > useful later to allow for different pools. > > Signed-off-by: Claire Chang > Reviewed-by: Christoph Hellwig > Tested-by: Stefano Stabellini > Tested-by: Will Deacon > Acked-by: Stefano Stabellini Reverting the rest of the series up to this patch fixed a boot crash with NVMe on today's linux-next. [ 22.286574][T7] Unable to handle kernel paging request at virtual address dfff800e [ 22.295225][T7] Mem abort info: [ 22.298743][T7] ESR = 0x9604 [ 22.302496][T7] EC = 0x25: DABT (current EL), IL = 32 bits [ 22.308525][T7] SET = 0, FnV = 0 [ 22.312274][T7] EA = 0, S1PTW = 0 [ 22.316131][T7] FSC = 0x04: level 0 translation fault [ 22.321704][T7] Data abort info: [ 22.325278][T7] ISV = 0, ISS = 0x0004 [ 22.329840][T7] CM = 0, WnR = 0 [ 22.333503][T7] [dfff800e] address between user and kernel address ranges [ 22.338543][ T256] igb 0006:01:00.0: Intel(R) Gigabit Ethernet Network Connection [ 22.341400][T7] Internal error: Oops: 9604 [#1] SMP [ 22.348915][ T256] igb 0006:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:38:d5:09:c8:83 [ 22.354458][T7] Modules linked in: igb(+) i2c_algo_bit nvme mlx5_core(+) i2c_core nvme_core firmware_class [ 22.362512][ T256] igb 0006:01:00.0: eth0: PBA No: G69016-004 [ 22.372287][T7] CPU: 13 PID: 7 Comm: kworker/u64:0 Not tainted 5.13.0-rc7-next-20210623+ #47 [ 22.372293][T7] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020 [ 22.372298][T7] Workqueue: nvme-reset-wq nvme_reset_work [nvme] [ 22.378145][ T256] igb 0006:01:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s) [ 22.386901][T7] [ 22.386905][T7] pstate: 1005 (nzcV daif -PAN -UAO -TCO BTYPE=--) [ 22.386910][T7] pc : dma_direct_map_sg+0x304/0x8f0 is_swiotlb_force_bounce at /usr/src/linux-next/./include/linux/swiotlb.h:119 (inlined by) dma_direct_map_page at /usr/src/linux-next/kernel/dma/direct.h:90 (inlined by) dma_direct_map_sg at /usr/src/linux-next/kernel/dma/direct.c:428 [ 22.386919][T7] lr : dma_map_sg_attrs+0x6c/0x118 [ 22.386924][T7] sp : 80001dc8eac0 [ 22.386926][T7] x29: 80001dc8eac0 x28: 199e70b0 x27: [ 22.386935][T7] x26: 000847ee7000 x25: 80001158e570 x24: 0002 [ 22.386943][T7] x23: dfff8000 x22: 0100 x21: 199e7460 [ 22.386951][T7] x20: 199e7488 x19: 0001 x18: 10062670 [ 22.386955][ T253] Unable to handle kernel paging request at virtual address dfff800e [ 22.386958][T7] x17: 8000109f6a90 x16: 8000109e1b4c x15: 89303420 [ 22.386965][ T253] Mem abort info: [ 22.386967][T7] x14: 0001 x13: 80001158e000 [ 22.386970][ T253] ESR = 0x9604 [ 22.386972][T7] x12: 1fffe00108fdce01 [ 22.386975][ T253] EC = 0x25: DABT (current EL), IL = 32 bits [ 22.386976][T7] x11: 1fffe00108fdce03 x10: 000847ee700c x9 : 0004 [ 22.386981][ T253] SET = 0, FnV = 0 [ 22.386983][T7] [ 22.386985][T7] x8 : 73b91d72 [ 22.386986][ T253] EA = 0, S1PTW = 0 [ 22.386987][T7] x7 : x6 : 000e [ 22.386990][ T253] FSC = 0x04: level 0 translation fault [ 22.386992][T7] [ 22.386994][T7] x5 : dfff8000 [ 22.386995][ T253] Data abort info: [ 22.386997][T7] x4 : 0008c7ede000 [ 22.386999][ T253] ISV = 0, ISS = 0x0004 [ 22.386999][T7] x3 : 0008c7ede000 [ 22.387003][T7] x2 : 1000 [ 22.387003][ T253] CM = 0, WnR = 0 [ 22.387006][T7] x1 : x0 : 0071 [ 22.387008][ T253] [dfff800e] address between user and kernel address ranges [ 22.387011][T7] [ 22.387013][T7] Call trace: [ 22.387016][T7] dma_direct_map_sg+0x304/0x8f0 [ 22.387022][T7] dma_map_sg_attrs+0x6c/0x118 [ 22.387026][T7] nvme_map_data+0x2ec/0x21d8 [nvme] [ 22.387040][T7] nvme_queue_rq+0x274/0x3f0 [nvme] [ 22.387052][T7] blk_mq_dispatch_rq_list+0x2ec/0x18a0 [ 22.387060][T7] __blk_mq_sched_dispatch_requests+0x2a0/0x3e8 [ 22.387065][T7] blk_mq_sched_dispatch_requests+0xa4/0x100 [ 22.387070][T7] __blk_mq_run_hw_queue+0x148/0x1d8 [ 22.387075][T7] __blk_mq_delay_run_hw_queue+0x3f8/0x730 [ 22.414539][ T269] igb 0006:01:00.0 enP6p1s0: renamed from eth0 [ 22.418957][T7] blk_mq_run_hw_queue+0x148/0x248 [ 22.418969][T7] blk_mq_sched_insert_request+0x2a4/0x330 [ 22.418975][T7] blk_execute_rq_nowait+0xc8/0x118 [ 22.418981][T7] blk_execute_rq+0xd4/0x188 [ 22.453203][ T255]
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On 6/23/2021 2:37 PM, Will Deacon wrote:
> On Wed, Jun 23, 2021 at 12:39:29PM -0400, Qian Cai wrote:
>>
>>
>> On 6/18/2021 11:40 PM, Claire Chang wrote:
>>> Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and
>>> use it to determine whether to bounce the data or not. This will be
>>> useful later to allow for different pools.
>>>
>>> Signed-off-by: Claire Chang
>>> Reviewed-by: Christoph Hellwig
>>> Tested-by: Stefano Stabellini
>>> Tested-by: Will Deacon
>>> Acked-by: Stefano Stabellini
>>
>> Reverting the rest of the series up to this patch fixed a boot crash with
>> NVMe on today's linux-next.
>
> Hmm, so that makes patch 7 the suspicious one, right?
Will, no. It is rather patch #6 (this patch). Only the patch from #6 to #12
were reverted to fix the issue. Also, looking at this offset of the crash,
pc : dma_direct_map_sg+0x304/0x8f0
is_swiotlb_force_bounce at /usr/src/linux-next/./include/linux/swiotlb.h:119
is_swiotlb_force_bounce() was the new function introduced in this patch here.
+static inline bool is_swiotlb_force_bounce(struct device *dev)
+{
+ return dev->dma_io_tlb_mem->force_bounce;
+}
>
> Looking at that one more closely, it looks like swiotlb_find_slots() takes
> 'alloc_size + offset' as its 'alloc_size' parameter from
> swiotlb_tbl_map_single() and initialises 'mem->slots[i].alloc_size' based
> on 'alloc_size + offset', which looks like a change in behaviour from the
> old code, which didn't include the offset there.
>
> swiotlb_release_slots() then adds the offset back on afaict, so we end up
> accounting for it twice and possibly unmap more than we're supposed to?
>
> Will
>
Re: [PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
On Wed, Jun 23, 2021 at 12:39:29PM -0400, Qian Cai wrote: > > > On 6/18/2021 11:40 PM, Claire Chang wrote: > > Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and > > use it to determine whether to bounce the data or not. This will be > > useful later to allow for different pools. > > > > Signed-off-by: Claire Chang > > Reviewed-by: Christoph Hellwig > > Tested-by: Stefano Stabellini > > Tested-by: Will Deacon > > Acked-by: Stefano Stabellini > > Reverting the rest of the series up to this patch fixed a boot crash with > NVMe on today's linux-next. Hmm, so that makes patch 7 the suspicious one, right? Looking at that one more closely, it looks like swiotlb_find_slots() takes 'alloc_size + offset' as its 'alloc_size' parameter from swiotlb_tbl_map_single() and initialises 'mem->slots[i].alloc_size' based on 'alloc_size + offset', which looks like a change in behaviour from the old code, which didn't include the offset there. swiotlb_release_slots() then adds the offset back on afaict, so we end up accounting for it twice and possibly unmap more than we're supposed to? Will
[PATCH v14 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
Propagate the swiotlb_force into io_tlb_default_mem->force_bounce and
use it to determine whether to bounce the data or not. This will be
useful later to allow for different pools.
Signed-off-by: Claire Chang
Reviewed-by: Christoph Hellwig
Tested-by: Stefano Stabellini
Tested-by: Will Deacon
Acked-by: Stefano Stabellini
---
drivers/xen/swiotlb-xen.c | 2 +-
include/linux/swiotlb.h | 11 +++
kernel/dma/direct.c | 2 +-
kernel/dma/direct.h | 2 +-
kernel/dma/swiotlb.c | 4
5 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 0c6ed09f8513..4730a146fa35 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -369,7 +369,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev,
struct page *page,
if (dma_capable(dev, dev_addr, size, true) &&
!range_straddles_page_boundary(phys, size) &&
!xen_arch_need_swiotlb(dev, phys, dev_addr) &&
- swiotlb_force != SWIOTLB_FORCE)
+ !is_swiotlb_force_bounce(dev))
goto done;
/*
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index dd1c30a83058..8d8855c77d9a 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -84,6 +84,7 @@ extern enum swiotlb_force swiotlb_force;
* unmap calls.
* @debugfs: The dentry to debugfs.
* @late_alloc:%true if allocated using the page allocator
+ * @force_bounce: %true if swiotlb bouncing is forced
*/
struct io_tlb_mem {
phys_addr_t start;
@@ -94,6 +95,7 @@ struct io_tlb_mem {
spinlock_t lock;
struct dentry *debugfs;
bool late_alloc;
+ bool force_bounce;
struct io_tlb_slot {
phys_addr_t orig_addr;
size_t alloc_size;
@@ -109,6 +111,11 @@ static inline bool is_swiotlb_buffer(struct device *dev,
phys_addr_t paddr)
return mem && paddr >= mem->start && paddr < mem->end;
}
+static inline bool is_swiotlb_force_bounce(struct device *dev)
+{
+ return dev->dma_io_tlb_mem->force_bounce;
+}
+
void __init swiotlb_exit(void);
unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
@@ -120,6 +127,10 @@ static inline bool is_swiotlb_buffer(struct device *dev,
phys_addr_t paddr)
{
return false;
}
+static inline bool is_swiotlb_force_bounce(struct device *dev)
+{
+ return false;
+}
static inline void swiotlb_exit(void)
{
}
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 7a88c34d0867..a92465b4eb12 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -496,7 +496,7 @@ size_t dma_direct_max_mapping_size(struct device *dev)
{
/* If SWIOTLB is active, use its maximum mapping size */
if (is_swiotlb_active(dev) &&
- (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
+ (dma_addressing_limited(dev) || is_swiotlb_force_bounce(dev)))
return swiotlb_max_mapping_size(dev);
return SIZE_MAX;
}
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index 13e9e7158d94..4632b0f4f72e 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -87,7 +87,7 @@ static inline dma_addr_t dma_direct_map_page(struct device
*dev,
phys_addr_t phys = page_to_phys(page) + offset;
dma_addr_t dma_addr = phys_to_dma(dev, phys);
- if (unlikely(swiotlb_force == SWIOTLB_FORCE))
+ if (is_swiotlb_force_bounce(dev))
return swiotlb_map(dev, phys, size, dir, attrs);
if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 8a120f42340b..0d294bbf274c 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -179,6 +179,10 @@ static void swiotlb_init_io_tlb_mem(struct io_tlb_mem
*mem, phys_addr_t start,
mem->end = mem->start + bytes;
mem->index = 0;
mem->late_alloc = late_alloc;
+
+ if (swiotlb_force == SWIOTLB_FORCE)
+ mem->force_bounce = true;
+
spin_lock_init(>lock);
for (i = 0; i < mem->nslabs; i++) {
mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
--
2.32.0.288.g62a8d224e6-goog
