It may be useful to disable the SWIOTLB completely for testing or when a
platform is known not to have any DRAM addressing limitations what so
ever.
Signed-off-by: Florian Fainelli <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 1 +
include/linux/swiotlb.h | 1 +
kernel/dma/swiotlb.c | 9 +++++++++
3 files changed, 11 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 04545725f187..b0223e48921e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5278,6 +5278,7 @@
force -- force using of bounce buffers even if they
wouldn't be automatically used by the kernel
noforce -- Never use bounce buffers (for debugging)
+ off -- Completely disable SWIOTLB
switches= [HW,M68k]
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 5857a937c637..23f86243defe 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -15,6 +15,7 @@ enum swiotlb_force {
SWIOTLB_NORMAL, /* Default - depending on HW DMA mask etc. */
SWIOTLB_FORCE, /* swiotlb=force */
SWIOTLB_NO_FORCE, /* swiotlb=noforce */
+ SWIOTLB_OFF, /* swiotlb=off */
};
/*
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c10e855a03bc..d7a4a789c7d3 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -126,6 +126,8 @@ setup_io_tlb_npages(char *str)
} else if (!strcmp(str, "noforce")) {
swiotlb_force = SWIOTLB_NO_FORCE;
io_tlb_nslabs = 1;
+ } else if (!strcmp(str, "off")) {
+ swiotlb_force = SWIOTLB_OFF;
}
return 0;
@@ -229,6 +231,9 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
unsigned long i, bytes;
size_t alloc_size;
+ if (swiotlb_force == SWIOTLB_OFF)
+ return 0;
+
bytes = nslabs << IO_TLB_SHIFT;
io_tlb_nslabs = nslabs;
@@ -284,6 +289,9 @@ swiotlb_init(int verbose)
unsigned char *vstart;
unsigned long bytes;
+ if (swiotlb_force == SWIOTLB_OFF)
+ goto out;
+
if (!io_tlb_nslabs) {
io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
@@ -302,6 +310,7 @@ swiotlb_init(int verbose)
io_tlb_start = 0;
}
pr_warn("Cannot allocate buffer");
+out:
no_iotlb_memory = true;
}
--
2.25.1
On 3/18/2021 12:18 PM, Florian Fainelli wrote:
> It may be useful to disable the SWIOTLB completely for testing or when a
> platform is known not to have any DRAM addressing limitations what so
> ever.
>
> Signed-off-by: Florian Fainelli <[email protected]>
Christoph, in addition to this change, how would you feel if we
qualified the swiotlb_init() in arch/arm/mm/init.c with a:
if (memblock_end_of_DRAM() >= SZ_4G)
swiotlb_init(1)
right now this is made unconditional whenever ARM_LPAE is enabled which
is the case for the platforms I maintain (ARCH_BRCMSTB) however we do
not really need a SWIOTLB so long as the largest DRAM physical address
does not exceed 4GB AFAICT.
Thanks!
> ---
> Documentation/admin-guide/kernel-parameters.txt | 1 +
> include/linux/swiotlb.h | 1 +
> kernel/dma/swiotlb.c | 9 +++++++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 04545725f187..b0223e48921e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5278,6 +5278,7 @@
> force -- force using of bounce buffers even if they
> wouldn't be automatically used by the kernel
> noforce -- Never use bounce buffers (for debugging)
> + off -- Completely disable SWIOTLB
>
> switches= [HW,M68k]
>
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 5857a937c637..23f86243defe 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -15,6 +15,7 @@ enum swiotlb_force {
> SWIOTLB_NORMAL, /* Default - depending on HW DMA mask etc. */
> SWIOTLB_FORCE, /* swiotlb=force */
> SWIOTLB_NO_FORCE, /* swiotlb=noforce */
> + SWIOTLB_OFF, /* swiotlb=off */
> };
>
> /*
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index c10e855a03bc..d7a4a789c7d3 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -126,6 +126,8 @@ setup_io_tlb_npages(char *str)
> } else if (!strcmp(str, "noforce")) {
> swiotlb_force = SWIOTLB_NO_FORCE;
> io_tlb_nslabs = 1;
> + } else if (!strcmp(str, "off")) {
> + swiotlb_force = SWIOTLB_OFF;
> }
>
> return 0;
> @@ -229,6 +231,9 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
> unsigned long i, bytes;
> size_t alloc_size;
>
> + if (swiotlb_force == SWIOTLB_OFF)
> + return 0;
> +
> bytes = nslabs << IO_TLB_SHIFT;
>
> io_tlb_nslabs = nslabs;
> @@ -284,6 +289,9 @@ swiotlb_init(int verbose)
> unsigned char *vstart;
> unsigned long bytes;
>
> + if (swiotlb_force == SWIOTLB_OFF)
> + goto out;
> +
> if (!io_tlb_nslabs) {
> io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
> io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> @@ -302,6 +310,7 @@ swiotlb_init(int verbose)
> io_tlb_start = 0;
> }
> pr_warn("Cannot allocate buffer");
> +out:
> no_iotlb_memory = true;
> }
>
>
--
Florian
On 2021-03-18 19:22, Florian Fainelli wrote:
>
>
> On 3/18/2021 12:18 PM, Florian Fainelli wrote:
>> It may be useful to disable the SWIOTLB completely for testing or when a
>> platform is known not to have any DRAM addressing limitations what so
>> ever.
Isn't that what "swiotlb=noforce" is for? If you're confident that we've
really ironed out *all* the awkward corners that used to blow up if
various internal bits were left uninitialised, then it would make sense
to just tweak the implementation of what we already have.
I wouldn't necessarily disagree with adding "off" as an additional alias
for "noforce", though, since it does come across as a bit wacky for
general use.
>> Signed-off-by: Florian Fainelli <[email protected]>
>
> Christoph, in addition to this change, how would you feel if we
> qualified the swiotlb_init() in arch/arm/mm/init.c with a:
>
>
> if (memblock_end_of_DRAM() >= SZ_4G)
> swiotlb_init(1)
Modulo "swiotlb=force", of course ;)
Robin.
> right now this is made unconditional whenever ARM_LPAE is enabled which
> is the case for the platforms I maintain (ARCH_BRCMSTB) however we do
> not really need a SWIOTLB so long as the largest DRAM physical address
> does not exceed 4GB AFAICT.
>
> Thanks!
>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 1 +
>> include/linux/swiotlb.h | 1 +
>> kernel/dma/swiotlb.c | 9 +++++++++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 04545725f187..b0223e48921e 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5278,6 +5278,7 @@
>> force -- force using of bounce buffers even if they
>> wouldn't be automatically used by the kernel
>> noforce -- Never use bounce buffers (for debugging)
>> + off -- Completely disable SWIOTLB
>>
>> switches= [HW,M68k]
>>
>> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
>> index 5857a937c637..23f86243defe 100644
>> --- a/include/linux/swiotlb.h
>> +++ b/include/linux/swiotlb.h
>> @@ -15,6 +15,7 @@ enum swiotlb_force {
>> SWIOTLB_NORMAL, /* Default - depending on HW DMA mask etc. */
>> SWIOTLB_FORCE, /* swiotlb=force */
>> SWIOTLB_NO_FORCE, /* swiotlb=noforce */
>> + SWIOTLB_OFF, /* swiotlb=off */
>> };
>>
>> /*
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index c10e855a03bc..d7a4a789c7d3 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>> @@ -126,6 +126,8 @@ setup_io_tlb_npages(char *str)
>> } else if (!strcmp(str, "noforce")) {
>> swiotlb_force = SWIOTLB_NO_FORCE;
>> io_tlb_nslabs = 1;
>> + } else if (!strcmp(str, "off")) {
>> + swiotlb_force = SWIOTLB_OFF;
>> }
>>
>> return 0;
>> @@ -229,6 +231,9 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
>> unsigned long i, bytes;
>> size_t alloc_size;
>>
>> + if (swiotlb_force == SWIOTLB_OFF)
>> + return 0;
>> +
>> bytes = nslabs << IO_TLB_SHIFT;
>>
>> io_tlb_nslabs = nslabs;
>> @@ -284,6 +289,9 @@ swiotlb_init(int verbose)
>> unsigned char *vstart;
>> unsigned long bytes;
>>
>> + if (swiotlb_force == SWIOTLB_OFF)
>> + goto out;
>> +
>> if (!io_tlb_nslabs) {
>> io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
>> io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
>> @@ -302,6 +310,7 @@ swiotlb_init(int verbose)
>> io_tlb_start = 0;
>> }
>> pr_warn("Cannot allocate buffer");
>> +out:
>> no_iotlb_memory = true;
>> }
>>
>>
>
On 3/18/2021 12:34 PM, Robin Murphy wrote:
> On 2021-03-18 19:22, Florian Fainelli wrote:
>>
>>
>> On 3/18/2021 12:18 PM, Florian Fainelli wrote:
>>> It may be useful to disable the SWIOTLB completely for testing or when a
>>> platform is known not to have any DRAM addressing limitations what so
>>> ever.
>
> Isn't that what "swiotlb=noforce" is for? If you're confident that we've
> really ironed out *all* the awkward corners that used to blow up if
> various internal bits were left uninitialised, then it would make sense
> to just tweak the implementation of what we already have.
swiotlb=noforce does prevent dma_direct_map_page() from resorting to the
swiotlb, however what I am also after is reclaiming these 64MB of
default SWIOTLB bounce buffering memory because my systems run with
large amounts of reserved memory into ZONE_MOVABLE and everything in
ZONE_NORMAL is precious at that point.
>
> I wouldn't necessarily disagree with adding "off" as an additional alias
> for "noforce", though, since it does come across as a bit wacky for
> general use.
>
>>> Signed-off-by: Florian Fainelli <[email protected]>
>>
>> Christoph, in addition to this change, how would you feel if we
>> qualified the swiotlb_init() in arch/arm/mm/init.c with a:
>>
>>
>> if (memblock_end_of_DRAM() >= SZ_4G)
>> swiotlb_init(1)
>
> Modulo "swiotlb=force", of course ;)
Indeed, we would need to handle that case as well. Does it sound
reasonable to do that to you as well?
--
Florian
On 2021-03-18 19:43, Florian Fainelli wrote:
>
>
> On 3/18/2021 12:34 PM, Robin Murphy wrote:
>> On 2021-03-18 19:22, Florian Fainelli wrote:
>>>
>>>
>>> On 3/18/2021 12:18 PM, Florian Fainelli wrote:
>>>> It may be useful to disable the SWIOTLB completely for testing or when a
>>>> platform is known not to have any DRAM addressing limitations what so
>>>> ever.
>>
>> Isn't that what "swiotlb=noforce" is for? If you're confident that we've
>> really ironed out *all* the awkward corners that used to blow up if
>> various internal bits were left uninitialised, then it would make sense
>> to just tweak the implementation of what we already have.
>
> swiotlb=noforce does prevent dma_direct_map_page() from resorting to the
> swiotlb, however what I am also after is reclaiming these 64MB of
> default SWIOTLB bounce buffering memory because my systems run with
> large amounts of reserved memory into ZONE_MOVABLE and everything in
> ZONE_NORMAL is precious at that point.
It also forces io_tlb_nslabs to the minimum, so it should be claiming
considerably less than 64MB. IIRC the original proposal *did* skip
initialisation completely, but that turned up the aforementioned issues.
>> I wouldn't necessarily disagree with adding "off" as an additional alias
>> for "noforce", though, since it does come across as a bit wacky for
>> general use.
>>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>
>>> Christoph, in addition to this change, how would you feel if we
>>> qualified the swiotlb_init() in arch/arm/mm/init.c with a:
>>>
>>>
>>> if (memblock_end_of_DRAM() >= SZ_4G)
>>> swiotlb_init(1)
>>
>> Modulo "swiotlb=force", of course ;)
>
> Indeed, we would need to handle that case as well. Does it sound
> reasonable to do that to you as well?
I wouldn't like it done to me personally, but for arm64, observe what
mem_init() in arch/arm64/mm/init.c already does.
Robin.
On 3/18/2021 12:53 PM, Robin Murphy wrote:
> On 2021-03-18 19:43, Florian Fainelli wrote:
>>
>>
>> On 3/18/2021 12:34 PM, Robin Murphy wrote:
>>> On 2021-03-18 19:22, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 3/18/2021 12:18 PM, Florian Fainelli wrote:
>>>>> It may be useful to disable the SWIOTLB completely for testing or
>>>>> when a
>>>>> platform is known not to have any DRAM addressing limitations what so
>>>>> ever.
>>>
>>> Isn't that what "swiotlb=noforce" is for? If you're confident that we've
>>> really ironed out *all* the awkward corners that used to blow up if
>>> various internal bits were left uninitialised, then it would make sense
>>> to just tweak the implementation of what we already have.
>>
>> swiotlb=noforce does prevent dma_direct_map_page() from resorting to the
>> swiotlb, however what I am also after is reclaiming these 64MB of
>> default SWIOTLB bounce buffering memory because my systems run with
>> large amounts of reserved memory into ZONE_MOVABLE and everything in
>> ZONE_NORMAL is precious at that point.
>
> It also forces io_tlb_nslabs to the minimum, so it should be claiming
> considerably less than 64MB. IIRC the original proposal *did* skip
> initialisation completely, but that turned up the aforementioned issues.
AFAICT in that case we will have iotlb_n_slabs will set to 1, which will
still make us allocate io_tlb_n_slabs << IO_TLB_SHIFT bytes in
swiotlb_init(), which still gives us 64MB.
>
>>> I wouldn't necessarily disagree with adding "off" as an additional alias
>>> for "noforce", though, since it does come across as a bit wacky for
>>> general use.
>>>
>>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>>
>>>> Christoph, in addition to this change, how would you feel if we
>>>> qualified the swiotlb_init() in arch/arm/mm/init.c with a:
>>>>
>>>>
>>>> if (memblock_end_of_DRAM() >= SZ_4G)
>>>> swiotlb_init(1)
>>>
>>> Modulo "swiotlb=force", of course ;)
>>
>> Indeed, we would need to handle that case as well. Does it sound
>> reasonable to do that to you as well?
>
> I wouldn't like it done to me personally, but for arm64, observe what
> mem_init() in arch/arm64/mm/init.c already does.
>
> Robin.
--
Florian
On 2021-03-18 21:31, Florian Fainelli wrote:
>
>
> On 3/18/2021 12:53 PM, Robin Murphy wrote:
>> On 2021-03-18 19:43, Florian Fainelli wrote:
>>>
>>>
>>> On 3/18/2021 12:34 PM, Robin Murphy wrote:
>>>> On 2021-03-18 19:22, Florian Fainelli wrote:
>>>>>
>>>>>
>>>>> On 3/18/2021 12:18 PM, Florian Fainelli wrote:
>>>>>> It may be useful to disable the SWIOTLB completely for testing or
>>>>>> when a
>>>>>> platform is known not to have any DRAM addressing limitations what so
>>>>>> ever.
>>>>
>>>> Isn't that what "swiotlb=noforce" is for? If you're confident that we've
>>>> really ironed out *all* the awkward corners that used to blow up if
>>>> various internal bits were left uninitialised, then it would make sense
>>>> to just tweak the implementation of what we already have.
>>>
>>> swiotlb=noforce does prevent dma_direct_map_page() from resorting to the
>>> swiotlb, however what I am also after is reclaiming these 64MB of
>>> default SWIOTLB bounce buffering memory because my systems run with
>>> large amounts of reserved memory into ZONE_MOVABLE and everything in
>>> ZONE_NORMAL is precious at that point.
>>
>> It also forces io_tlb_nslabs to the minimum, so it should be claiming
>> considerably less than 64MB. IIRC the original proposal *did* skip
>> initialisation completely, but that turned up the aforementioned issues.
>
> AFAICT in that case we will have iotlb_n_slabs will set to 1, which will
> still make us allocate io_tlb_n_slabs << IO_TLB_SHIFT bytes in
> swiotlb_init(), which still gives us 64MB.
Eh? When did 2KB become 64MB? IO_TLB_SHIFT is 11, so that's at most one
page in anyone's money...
>>>> I wouldn't necessarily disagree with adding "off" as an additional alias
>>>> for "noforce", though, since it does come across as a bit wacky for
>>>> general use.
>>>>
>>>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>>>
>>>>> Christoph, in addition to this change, how would you feel if we
>>>>> qualified the swiotlb_init() in arch/arm/mm/init.c with a:
>>>>>
>>>>>
>>>>> if (memblock_end_of_DRAM() >= SZ_4G)
>>>>> swiotlb_init(1)
>>>>
>>>> Modulo "swiotlb=force", of course ;)
>>>
>>> Indeed, we would need to handle that case as well. Does it sound
>>> reasonable to do that to you as well?
>>
>> I wouldn't like it done to me personally, but for arm64, observe what
>> mem_init() in arch/arm64/mm/init.c already does.
In fact I should have looked more closely at that myself - checking
debugfs on my 4GB arm64 board actually shows io_tlb_nslabs = 0, and
indeed we are bypassing initialisation completely and (ab)using
SWIOTLB_NO_FORCE to cover it up, so I guess it probably *is* safe now
for the noforce option to do the same for itself and save even that one
page.
Robin.
On 3/18/2021 4:35 PM, Robin Murphy wrote:
> On 2021-03-18 21:31, Florian Fainelli wrote:
>>
>>
>> On 3/18/2021 12:53 PM, Robin Murphy wrote:
>>> On 2021-03-18 19:43, Florian Fainelli wrote:
>>>>
>>>>
>>>> On 3/18/2021 12:34 PM, Robin Murphy wrote:
>>>>> On 2021-03-18 19:22, Florian Fainelli wrote:
>>>>>>
>>>>>>
>>>>>> On 3/18/2021 12:18 PM, Florian Fainelli wrote:
>>>>>>> It may be useful to disable the SWIOTLB completely for testing or
>>>>>>> when a
>>>>>>> platform is known not to have any DRAM addressing limitations
>>>>>>> what so
>>>>>>> ever.
>>>>>
>>>>> Isn't that what "swiotlb=noforce" is for? If you're confident that
>>>>> we've
>>>>> really ironed out *all* the awkward corners that used to blow up if
>>>>> various internal bits were left uninitialised, then it would make
>>>>> sense
>>>>> to just tweak the implementation of what we already have.
>>>>
>>>> swiotlb=noforce does prevent dma_direct_map_page() from resorting to
>>>> the
>>>> swiotlb, however what I am also after is reclaiming these 64MB of
>>>> default SWIOTLB bounce buffering memory because my systems run with
>>>> large amounts of reserved memory into ZONE_MOVABLE and everything in
>>>> ZONE_NORMAL is precious at that point.
>>>
>>> It also forces io_tlb_nslabs to the minimum, so it should be claiming
>>> considerably less than 64MB. IIRC the original proposal *did* skip
>>> initialisation completely, but that turned up the aforementioned issues.
>>
>> AFAICT in that case we will have iotlb_n_slabs will set to 1, which will
>> still make us allocate io_tlb_n_slabs << IO_TLB_SHIFT bytes in
>> swiotlb_init(), which still gives us 64MB.
>
> Eh? When did 2KB become 64MB? IO_TLB_SHIFT is 11, so that's at most one
> page in anyone's money...
Yes, sorry incorrect shift applied here. Still, and I believe this is
what you mean below, architecture code setting swiotlb_force =
SWIOTLB_NO_FORCE does not result in not allocating the SWIOTLB, because
io_tlb_nslabs is still left set to 0 so swiotlb_init() will proceed with
allocating the default size.
>
>>>>> I wouldn't necessarily disagree with adding "off" as an additional
>>>>> alias
>>>>> for "noforce", though, since it does come across as a bit wacky for
>>>>> general use.
>>>>>
>>>>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>>>>
>>>>>> Christoph, in addition to this change, how would you feel if we
>>>>>> qualified the swiotlb_init() in arch/arm/mm/init.c with a:
>>>>>>
>>>>>>
>>>>>> if (memblock_end_of_DRAM() >= SZ_4G)
>>>>>> swiotlb_init(1)
>>>>>
>>>>> Modulo "swiotlb=force", of course ;)
>>>>
>>>> Indeed, we would need to handle that case as well. Does it sound
>>>> reasonable to do that to you as well?
>>>
>>> I wouldn't like it done to me personally, but for arm64, observe what
>>> mem_init() in arch/arm64/mm/init.c already does.
>
> In fact I should have looked more closely at that myself - checking
> debugfs on my 4GB arm64 board actually shows io_tlb_nslabs = 0, and
> indeed we are bypassing initialisation completely and (ab)using
> SWIOTLB_NO_FORCE to cover it up, so I guess it probably *is* safe now
> for the noforce option to do the same for itself and save even that one
> page.
OK, I can submit a patch that does that. 5.12-rc3 works correctly for me
here as well and only allocates SWIOTLB when needed which in our case is
either:
- we have DRAM at PA >= 4GB
- we have limited peripherals (Raspberry Pi 4 derivative) that can only
address the lower 1GB
Now let's see if we can get ARM 32-bit to match :)
--
Florian
> >
> > In fact I should have looked more closely at that myself - checking
> > debugfs on my 4GB arm64 board actually shows io_tlb_nslabs = 0, and
> > indeed we are bypassing initialisation completely and (ab)using
> > SWIOTLB_NO_FORCE to cover it up, so I guess it probably *is* safe now
> > for the noforce option to do the same for itself and save even that one
> > page.
>
> OK, I can submit a patch that does that. 5.12-rc3 works correctly for me
> here as well and only allocates SWIOTLB when needed which in our case is
> either:
>
> - we have DRAM at PA >= 4GB
> - we have limited peripherals (Raspberry Pi 4 derivative) that can only
> address the lower 1GB
>
> Now let's see if we can get ARM 32-bit to match :)
Whatever patch you come up with, if it is against SWIOTLB please base it on top of
devel/for-linus-5.12 in https://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb.git/
Thx
> --
> Florian
When SWIOTLB_NO_FORCE is used, there should really be no allocations of
io_tlb_nslabs to occur since we are not going to use those slabs. If a
platform was somehow setting swiotlb_no_force and a later call to
swiotlb_init() was to be made we would still be proceeding with
allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
was set on the kernel command line we would have only allocated 2KB.
This would be inconsistent and the point of initializing io_tlb_nslabs
to 1, was to avoid hitting the test for io_tlb_nslabs being 0/not
initialized.
Signed-off-by: Florian Fainelli <[email protected]>
---
kernel/dma/swiotlb.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index c10e855a03bc..526c8321b76f 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -121,12 +121,10 @@ setup_io_tlb_npages(char *str)
}
if (*str == ',')
++str;
- if (!strcmp(str, "force")) {
+ if (!strcmp(str, "force"))
swiotlb_force = SWIOTLB_FORCE;
- } else if (!strcmp(str, "noforce")) {
+ else if (!strcmp(str, "noforce"))
swiotlb_force = SWIOTLB_NO_FORCE;
- io_tlb_nslabs = 1;
- }
return 0;
}
@@ -284,6 +282,9 @@ swiotlb_init(int verbose)
unsigned char *vstart;
unsigned long bytes;
+ if (swiotlb_force == SWIOTLB_NO_FORCE)
+ goto out;
+
if (!io_tlb_nslabs) {
io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
@@ -302,6 +303,7 @@ swiotlb_init(int verbose)
io_tlb_start = 0;
}
pr_warn("Cannot allocate buffer");
+out:
no_iotlb_memory = true;
}
--
2.25.1
On Thu, Mar 18, 2021 at 09:00:54PM -0700, Florian Fainelli wrote:
> When SWIOTLB_NO_FORCE is used, there should really be no allocations of
> io_tlb_nslabs to occur since we are not going to use those slabs. If a
> platform was somehow setting swiotlb_no_force and a later call to
> swiotlb_init() was to be made we would still be proceeding with
> allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
> was set on the kernel command line we would have only allocated 2KB.
>
> This would be inconsistent and the point of initializing io_tlb_nslabs
> to 1, was to avoid hitting the test for io_tlb_nslabs being 0/not
> initialized.
Could you rebase this on devel/for-linus-5.13 in
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb.git
please?
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> kernel/dma/swiotlb.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index c10e855a03bc..526c8321b76f 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -121,12 +121,10 @@ setup_io_tlb_npages(char *str)
> }
> if (*str == ',')
> ++str;
> - if (!strcmp(str, "force")) {
> + if (!strcmp(str, "force"))
> swiotlb_force = SWIOTLB_FORCE;
> - } else if (!strcmp(str, "noforce")) {
> + else if (!strcmp(str, "noforce"))
> swiotlb_force = SWIOTLB_NO_FORCE;
> - io_tlb_nslabs = 1;
> - }
>
> return 0;
> }
> @@ -284,6 +282,9 @@ swiotlb_init(int verbose)
> unsigned char *vstart;
> unsigned long bytes;
>
> + if (swiotlb_force == SWIOTLB_NO_FORCE)
> + goto out;
> +
> if (!io_tlb_nslabs) {
> io_tlb_nslabs = (default_size >> IO_TLB_SHIFT);
> io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> @@ -302,6 +303,7 @@ swiotlb_init(int verbose)
> io_tlb_start = 0;
> }
> pr_warn("Cannot allocate buffer");
> +out:
> no_iotlb_memory = true;
> }
>
> --
> 2.25.1
>
When SWIOTLB_NO_FORCE is used, there should really be no allocations of
default_nslabs to occur since we are not going to use those slabs. If a
platform was somehow setting swiotlb_no_force and a later call to
swiotlb_init() was to be made we would still be proceeding with
allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
was set on the kernel command line we would have only allocated 2KB.
This would be inconsistent and the point of initializing default_nslabs
to 1, was intended to allocate the minimum amount of memory possible, so
simply remove that minimal allocation period.
Signed-off-by: Florian Fainelli <[email protected]>
---
Changes in v2:
- rebased against devel/for-linus-5.13
- updated commit message to reflect variable names
kernel/dma/swiotlb.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 539c76beb52e..d20002a61546 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -83,12 +83,10 @@ setup_io_tlb_npages(char *str)
}
if (*str == ',')
++str;
- if (!strcmp(str, "force")) {
+ if (!strcmp(str, "force"))
swiotlb_force = SWIOTLB_FORCE;
- } else if (!strcmp(str, "noforce")) {
+ else if (!strcmp(str, "noforce"))
swiotlb_force = SWIOTLB_NO_FORCE;
- default_nslabs = 1;
- }
return 0;
}
@@ -211,6 +209,9 @@ swiotlb_init(int verbose)
size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
void *tlb;
+ if (swiotlb_force == SWIOTLB_NO_FORCE)
+ return;
+
/* Get IO TLB memory from the low pages */
tlb = memblock_alloc_low(bytes, PAGE_SIZE);
if (!tlb)
--
2.25.1
On Sat, Mar 20, 2021 at 08:37:40PM -0700, Florian Fainelli wrote:
> - if (!strcmp(str, "force")) {
> + if (!strcmp(str, "force"))
> swiotlb_force = SWIOTLB_FORCE;
> - } else if (!strcmp(str, "noforce")) {
> + else if (!strcmp(str, "noforce"))
> swiotlb_force = SWIOTLB_NO_FORCE;
> - default_nslabs = 1;
> - }
>
> return 0;
> }
> @@ -211,6 +209,9 @@ swiotlb_init(int verbose)
> size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
> void *tlb;
>
> + if (swiotlb_force == SWIOTLB_NO_FORCE)
> + return;
We'll also need this in the other callers of swiotlb_init_with_tbl
and swiotlb_late_init_with_tbl.
I actually had a plan to mostly kill them, but that can better
way until more support for multiple io_tlb structures is merged.
When SWIOTLB_NO_FORCE is used, there should really be no allocations of
default_nslabs to occur since we are not going to use those slabs. If a
platform was somehow setting swiotlb_no_force and a later call to
swiotlb_init() was to be made we would still be proceeding with
allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
was set on the kernel command line we would have only allocated 2KB.
This would be inconsistent and the point of initializing default_nslabs
to 1, was intended to allocate the minimum amount of memory possible, so
simply remove that minimal allocation period.
Signed-off-by: Florian Fainelli <[email protected]>
---
Changes in v3:
- patch all call sites that can allocate SWIOTLB memory
Changes in v2:
- rebased against devel/for-linus-5.13
- updated commit message to reflect variable names
kernel/dma/swiotlb.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 539c76beb52e..0a5b6f7e75bc 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -83,12 +83,10 @@ setup_io_tlb_npages(char *str)
}
if (*str == ',')
++str;
- if (!strcmp(str, "force")) {
+ if (!strcmp(str, "force"))
swiotlb_force = SWIOTLB_FORCE;
- } else if (!strcmp(str, "noforce")) {
+ else if (!strcmp(str, "noforce"))
swiotlb_force = SWIOTLB_NO_FORCE;
- default_nslabs = 1;
- }
return 0;
}
@@ -174,6 +172,9 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
struct io_tlb_mem *mem;
size_t alloc_size;
+ if (swiotlb_force == SWIOTLB_NO_FORCE)
+ return 0;
+
/* protect against double initialization */
if (WARN_ON_ONCE(io_tlb_default_mem))
return -ENOMEM;
@@ -211,6 +212,9 @@ swiotlb_init(int verbose)
size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
void *tlb;
+ if (swiotlb_force == SWIOTLB_NO_FORCE)
+ return;
+
/* Get IO TLB memory from the low pages */
tlb = memblock_alloc_low(bytes, PAGE_SIZE);
if (!tlb)
@@ -240,6 +244,9 @@ swiotlb_late_init_with_default_size(size_t default_size)
unsigned int order;
int rc = 0;
+ if (swiotlb_force == SWIOTLB_NO_FORCE)
+ return 0;
+
/*
* Get IO TLB memory from the low pages
*/
@@ -276,6 +283,9 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
struct io_tlb_mem *mem;
+ if (swiotlb_force == SWIOTLB_NO_FORCE)
+ return 0;
+
/* protect against double initialization */
if (WARN_ON_ONCE(io_tlb_default_mem))
return -ENOMEM;
--
2.25.1
On Mon, Mar 22, 2021 at 06:53:49PM -0700, Florian Fainelli wrote:
> When SWIOTLB_NO_FORCE is used, there should really be no allocations of
> default_nslabs to occur since we are not going to use those slabs. If a
> platform was somehow setting swiotlb_no_force and a later call to
> swiotlb_init() was to be made we would still be proceeding with
> allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
> was set on the kernel command line we would have only allocated 2KB.
>
> This would be inconsistent and the point of initializing default_nslabs
> to 1, was intended to allocate the minimum amount of memory possible, so
> simply remove that minimal allocation period.
>
> Signed-off-by: Florian Fainelli <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On 3/24/2021 1:42 AM, Christoph Hellwig wrote:
> On Mon, Mar 22, 2021 at 06:53:49PM -0700, Florian Fainelli wrote:
>> When SWIOTLB_NO_FORCE is used, there should really be no allocations of
>> default_nslabs to occur since we are not going to use those slabs. If a
>> platform was somehow setting swiotlb_no_force and a later call to
>> swiotlb_init() was to be made we would still be proceeding with
>> allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
>> was set on the kernel command line we would have only allocated 2KB.
>>
>> This would be inconsistent and the point of initializing default_nslabs
>> to 1, was intended to allocate the minimum amount of memory possible, so
>> simply remove that minimal allocation period.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>
> Looks good,
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
Thanks! Konrad, can you apply this patch to your for-linus-5.13 branch
if you are also happy with it?
--
Florian
On Thu, Apr 08, 2021 at 08:13:07PM -0700, Florian Fainelli wrote:
>
>
> On 3/24/2021 1:42 AM, Christoph Hellwig wrote:
> > On Mon, Mar 22, 2021 at 06:53:49PM -0700, Florian Fainelli wrote:
> >> When SWIOTLB_NO_FORCE is used, there should really be no allocations of
> >> default_nslabs to occur since we are not going to use those slabs. If a
> >> platform was somehow setting swiotlb_no_force and a later call to
> >> swiotlb_init() was to be made we would still be proceeding with
> >> allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
> >> was set on the kernel command line we would have only allocated 2KB.
> >>
> >> This would be inconsistent and the point of initializing default_nslabs
> >> to 1, was intended to allocate the minimum amount of memory possible, so
> >> simply remove that minimal allocation period.
> >>
> >> Signed-off-by: Florian Fainelli <[email protected]>
> >
> > Looks good,
> >
> > Reviewed-by: Christoph Hellwig <[email protected]>
> >
>
> Thanks! Konrad, can you apply this patch to your for-linus-5.13 branch
> if you are also happy with it?
It should be now visible?
> --
> Florian
On 4/9/2021 12:32 PM, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 08, 2021 at 08:13:07PM -0700, Florian Fainelli wrote:
>>
>>
>> On 3/24/2021 1:42 AM, Christoph Hellwig wrote:
>>> On Mon, Mar 22, 2021 at 06:53:49PM -0700, Florian Fainelli wrote:
>>>> When SWIOTLB_NO_FORCE is used, there should really be no allocations of
>>>> default_nslabs to occur since we are not going to use those slabs. If a
>>>> platform was somehow setting swiotlb_no_force and a later call to
>>>> swiotlb_init() was to be made we would still be proceeding with
>>>> allocating the default SWIOTLB size (64MB), whereas if swiotlb=noforce
>>>> was set on the kernel command line we would have only allocated 2KB.
>>>>
>>>> This would be inconsistent and the point of initializing default_nslabs
>>>> to 1, was intended to allocate the minimum amount of memory possible, so
>>>> simply remove that minimal allocation period.
>>>>
>>>> Signed-off-by: Florian Fainelli <[email protected]>
>>>
>>> Looks good,
>>>
>>> Reviewed-by: Christoph Hellwig <[email protected]>
>>>
>>
>> Thanks! Konrad, can you apply this patch to your for-linus-5.13 branch
>> if you are also happy with it?
>
> It should be now visible?
Not seeing it here:
https://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb.git/log/?h=devel/for-linus-5.13
--
Florian