Re: Memory corruption with CONFIG_SWIOTLB_DYNAMIC=y

From: Halil Pasic
Date: Mon Nov 06 2023 - 05:09:06 EST


On Fri, 3 Nov 2023 21:50:53 +0100
Petr Tesařík <petr@xxxxxxxxxxx> wrote:

> >[...]
> > In our case min_align_mask == 0 and a) is thus not applicable, because b) and
> > c) we end up with iotlb_align_mask = 0x800. And because orig_add & 0x800 ==
> > 0x800 but pool->start & 0x800 == 0 and the slot at index i is skipped over. The
> > slot 0 is skipped over because it is page aligned, when !!((1UL << PAGE_SHIFT)
> > & orig_addr)
>
> Wait. These mask values can quickly become confusing. Do you mean
> iotlb_align_mask == 0xfff?

I mean iotlb_align_mask == 0x800. Because of
iotlb_align_mask &= ~(IO_TLB_SIZE - 1);
in line 994 masks away the 0x7FF part
(https://github.com/torvalds/linux/blob/d2f51b3516dade79269ff45eae2a7668ae711b25/kernel/dma/swiotlb.c#L994C2-L994C41)
what remains of 0xfff (when PAGE_SHIFT == 12). My idea was to write
0x800 differently with a reference to PAGE_SHIFT, because for a
larger PAGE_SHIFT we end up with a different pattern and thus
requirement, but didn't really think it through properly because
even (1UL << (PAGE_SHIFT- 1)) (which is for PAGE_SHIFT == 12
0x800) does not tell the full story. Because all bits in the
interval [PAGE_SHIFT,IO_TLB_SHIFT) matter and not just the most
significant one (for PAGE_SHIFT == 12 and IO_TLB_SHIFT == 1 there is
just one).

Shame on me! Sorry for the confusion!

Regards,
Halil