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 hch@lst.de 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 dfff80000000000e` 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" 0xdfff800000000000 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 &&

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 &&