arm64: mm: set ZONE_DMA size based on early IORT scan

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 10 lines · Ard Biesheuvel

arch/arm64/mm/init.c +2 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Nitpick: can we add this prototype to include/linux/acpi_iort.h?

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 202

Quoted 7 lines

arch/arm64/mm/init.c +6 -0

+
+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);

I think we should initialise zone_dma_bits slightly earlier via arm64_memblock_init(). We'll eventually have reserve_crashkernel() called before this and it will make use of arm64_dma_phys_limit for "low" reservations:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

Quoted 28 lines · Catalin Marinas

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 10 lines · Ard Biesheuvel

arch/arm64/mm/init.c +2 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Nitpick: can we add this prototype to include/linux/acpi_iort.h?

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 202

Quoted 7 lines

arch/arm64/mm/init.c +6 -0

+
+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);

I think we should initialise zone_dma_bits slightly earlier via arm64_memblock_init(). We'll eventually have reserve_crashkernel() called before this and it will make use of arm64_dma_phys_limit for "low" reservations:

Quoted 2 lines

https://lore.kernel.org/linux-arm-kernel/[email protected]/

We don't have access to the ACPI tables yet at that point.

Quoted 31 lines · Ard Biesheuvel

Quoted 29 lines · Catalin Marinas

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 10 lines · Ard Biesheuvel

arch/arm64/mm/init.c +2 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Nitpick: can we add this prototype to include/linux/acpi_iort.h?

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 202

Quoted 7 lines

arch/arm64/mm/init.c +6 -0

+
+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);

I think we should initialise zone_dma_bits slightly earlier via arm64_memblock_init(). We'll eventually have reserve_crashkernel() called before this and it will make use of arm64_dma_phys_limit for "low" reservations:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

Quoted 1 line

We don't have access to the ACPI tables yet at that point.

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

Quoted 32 lines · Ard Biesheuvel

Quoted 31 lines · Ard Biesheuvel

Quoted 29 lines · Catalin Marinas

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 10 lines · Ard Biesheuvel

arch/arm64/mm/init.c +2 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Nitpick: can we add this prototype to include/linux/acpi_iort.h?

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 202

Quoted 7 lines

arch/arm64/mm/init.c +6 -0

+
+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);

I think we should initialise zone_dma_bits slightly earlier via arm64_memblock_init(). We'll eventually have reserve_crashkernel() called before this and it will make use of arm64_dma_phys_limit for "low" reservations:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

Quoted 8 lines

Quoted 1 line

We don't have access to the ACPI tables yet at that point.

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

I thought the crashkernel needs some additional reserved RAM as well to be able to run. It should not touch the original kernel's memory as it usually needs to dump it.

Quoted 42 lines · Catalin Marinas

Quoted 32 lines · Ard Biesheuvel

Quoted 31 lines · Ard Biesheuvel

Quoted 29 lines · Catalin Marinas

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 10 lines · Ard Biesheuvel

arch/arm64/mm/init.c +2 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Nitpick: can we add this prototype to include/linux/acpi_iort.h?

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 202

Quoted 7 lines

arch/arm64/mm/init.c +6 -0

+
+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);

I think we should initialise zone_dma_bits slightly earlier via arm64_memblock_init(). We'll eventually have reserve_crashkernel() called before this and it will make use of arm64_dma_phys_limit for "low" reservations:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

Quoted 8 lines

Quoted 1 line

We don't have access to the ACPI tables yet at that point.

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

Quoted 3 lines

I thought the crashkernel needs some additional reserved RAM as well to be able to run. It should not touch the original kernel's memory as it usually needs to dump it.

Looking at the code, it is definitely allocating memory for the kernel itself (as it refers to the 2 MB alignment requirement), and given that we used to require the kernel to be at the base of the linear region to even be able to access all of memory, I suspect that we might be able to relax this requirement. Not sure what that means for the userland tools, though.

Quoted 47 lines · Ard Biesheuvel

Quoted 42 lines · Catalin Marinas

Quoted 32 lines · Ard Biesheuvel

Quoted 31 lines · Ard Biesheuvel

Quoted 29 lines · Catalin Marinas

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 10 lines · Ard Biesheuvel

arch/arm64/mm/init.c +2 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Nitpick: can we add this prototype to include/linux/acpi_iort.h?

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 202

Quoted 7 lines

arch/arm64/mm/init.c +6 -0

+
+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);

I think we should initialise zone_dma_bits slightly earlier via arm64_memblock_init(). We'll eventually have reserve_crashkernel() called before this and it will make use of arm64_dma_phys_limit for "low" reservations:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

Quoted 8 lines

Quoted 1 line

We don't have access to the ACPI tables yet at that point.

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

Quoted 3 lines

I thought the crashkernel needs some additional reserved RAM as well to be able to run. It should not touch the original kernel's memory as it usually needs to dump it.

Quoted 6 lines

Looking at the code, it is definitely allocating memory for the kernel itself (as it refers to the 2 MB alignment requirement), and given that we used to require the kernel to be at the base of the linear region to even be able to access all of memory, I suspect that we might be able to relax this requirement. Not sure what that means for the userland tools, though.

The 2MB is an interpretation of booting.txt that the DRAM must start at this alignment (not sure what we do these days, in lots of configurations we just use 4K pages for the linear map).

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

[...]

Quoted 8 lines

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Yes, the crash kernel doesn't need to be loaded in the low memory. But some low memory needs to end up in its perceived System RAM. That's what Chen is trying to do with this series:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

It reserves the normal crashkernel memory at some high address range with a small block (currently proposed as 256MB similar to x86) in the "low" range.

This "low" range for arm64 currently means below 1GB but it's only RPi4 that needs it this low, all other platforms are fine with the full low 32-bit range.

If it's not doable in a nice way, we'll just leave with this permanent 1GB ZONE_DMA and hope we won't get platforms requiring an even smaller one. There's also the option of ignoring kdump on RPi4, make ZONE_DMA depend on !CRASH_DUMP and the "low" reservations can use the full 32-bit range since the kdump kernel won't need <30-bit addresses.

Quoted 33 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

[...]

Quoted 8 lines

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Yes, the crash kernel doesn't need to be loaded in the low memory. But some low memory needs to end up in its perceived System RAM. That's what Chen is trying to do with this series:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

It reserves the normal crashkernel memory at some high address range with a small block (currently proposed as 256MB similar to x86) in the "low" range.

This "low" range for arm64 currently means below 1GB but it's only RPi4 that needs it this low, all other platforms are fine with the full low 32-bit range.

Quoted 5 lines

If it's not doable in a nice way, we'll just leave with this permanent 1GB ZONE_DMA and hope we won't get platforms requiring an even smaller one. There's also the option of ignoring kdump on RPi4, make ZONE_DMA depend on !CRASH_DUMP and the "low" reservations can use the full 32-bit range since the kdump kernel won't need <30-bit addresses.

Are you aware of any reason why we cannot defer the call to reserve_crashkernel() to the start of bootmem_init()? That way, we have access to the unflattened DT as well as the IORT, and so we can tweak the zone limits based on the h/w description, but before allocating the crashkernel.

Quoted 40 lines · Ard Biesheuvel

Quoted 38 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

[...]

Quoted 8 lines

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Yes, the crash kernel doesn't need to be loaded in the low memory. But some low memory needs to end up in its perceived System RAM. That's what Chen is trying to do with this series:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

It reserves the normal crashkernel memory at some high address range with a small block (currently proposed as 256MB similar to x86) in the "low" range.

This "low" range for arm64 currently means below 1GB but it's only RPi4 that needs it this low, all other platforms are fine with the full low 32-bit range.

If it's not doable in a nice way, we'll just leave with this permanent 1GB ZONE_DMA and hope we won't get platforms requiring an even smaller one. There's also the option of ignoring kdump on RPi4, make ZONE_DMA depend on !CRASH_DUMP and the "low" reservations can use the full 32-bit range since the kdump kernel won't need <30-bit addresses.

Quoted 5 lines

Are you aware of any reason why we cannot defer the call to reserve_crashkernel() to the start of bootmem_init()? That way, we have access to the unflattened DT as well as the IORT, and so we can tweak the zone limits based on the h/w description, but before allocating the crashkernel.

Not really, as long as memblock_reserve/alloc() still works.

Quoted 47 lines · Catalin Marinas

Quoted 45 lines · Ard Biesheuvel

Quoted 33 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

[...]

Quoted 8 lines

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Yes, the crash kernel doesn't need to be loaded in the low memory. But some low memory needs to end up in its perceived System RAM. That's what Chen is trying to do with this series:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

It reserves the normal crashkernel memory at some high address range with a small block (currently proposed as 256MB similar to x86) in the "low" range.

This "low" range for arm64 currently means below 1GB but it's only RPi4 that needs it this low, all other platforms are fine with the full low 32-bit range.

Quoted 5 lines

If it's not doable in a nice way, we'll just leave with this permanent 1GB ZONE_DMA and hope we won't get platforms requiring an even smaller one. There's also the option of ignoring kdump on RPi4, make ZONE_DMA depend on !CRASH_DUMP and the "low" reservations can use the full 32-bit range since the kdump kernel won't need <30-bit addresses.

Are you aware of any reason why we cannot defer the call to reserve_crashkernel() to the start of bootmem_init()? That way, we have access to the unflattened DT as well as the IORT, and so we can tweak the zone limits based on the h/w description, but before allocating the crashkernel.

Quoted 1 line

Not really, as long as memblock_reserve/alloc() still works.

I had a look at this myself, and IIUC we're free to call reserve_crashkernel() anytime as long as it's before memblock_free_all().

So, should I add a patch in my series taking care of that? or you'd rather take care of it yourselves?

Quoted 47 lines · Catalin Marinas

Quoted 45 lines · Ard Biesheuvel

Quoted 33 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

[...]

Quoted 8 lines

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Yes, the crash kernel doesn't need to be loaded in the low memory. But some low memory needs to end up in its perceived System RAM. That's what Chen is trying to do with this series:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

It reserves the normal crashkernel memory at some high address range with a small block (currently proposed as 256MB similar to x86) in the "low" range.

This "low" range for arm64 currently means below 1GB but it's only RPi4 that needs it this low, all other platforms are fine with the full low 32-bit range.

Quoted 5 lines

If it's not doable in a nice way, we'll just leave with this permanent 1GB ZONE_DMA and hope we won't get platforms requiring an even smaller one. There's also the option of ignoring kdump on RPi4, make ZONE_DMA depend on !CRASH_DUMP and the "low" reservations can use the full 32-bit range since the kdump kernel won't need <30-bit addresses.

Are you aware of any reason why we cannot defer the call to reserve_crashkernel() to the start of bootmem_init()? That way, we have access to the unflattened DT as well as the IORT, and so we can tweak the zone limits based on the h/w description, but before allocating the crashkernel.

Quoted 1 line

Not really, as long as memblock_reserve/alloc() still works.

I had a look at this myself, and IIUC we're free to call reserve_crashkernel() anytime as long as it's before memblock_free_all().

So, should I add a patch in my series taking care of that? or you'd rather take care of it yourselves?

Quoted 53 lines · Nicolas Saenz Julienne

Quoted 41 lines · Catalin Marinas

Quoted 40 lines · Ard Biesheuvel

Quoted 38 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Catalin Marinas

Quoted 7 lines

Quoted 7 lines · Ard Biesheuvel

Quoted 6 lines

Also, could someone give an executive summary of why it matters where the crashkernel is loaded? As far as I can tell, reserve_crashkernel() only allocates memory for the kernel's executable image itself, which can usually be loaded anywhere in memory. I could see how a crashkernel might need some DMA'able memory if it needs to use the hardware, but I don't think that is what is going on here.

[...]

Quoted 8 lines

Quoted 2 lines

However, the crashkernel=... range is meant for sufficiently large reservation to be able to run the kdump kernel, not just load the image.

Sure. But I was referring to the requirement that it is loaded low in memory. Unless I am misunderstanding something, all we need for the crashkernel to be able to operate is some ZONE_DMA memory in case it is needed by the hardware, and beyond that, it could happily live anywhere in memory.

Yes, the crash kernel doesn't need to be loaded in the low memory. But some low memory needs to end up in its perceived System RAM. That's what Chen is trying to do with this series:

https://lore.kernel.org/linux-arm-kernel/[email protected]/

It reserves the normal crashkernel memory at some high address range with a small block (currently proposed as 256MB similar to x86) in the "low" range.

This "low" range for arm64 currently means below 1GB but it's only RPi4 that needs it this low, all other platforms are fine with the full low 32-bit range.

If it's not doable in a nice way, we'll just leave with this permanent 1GB ZONE_DMA and hope we won't get platforms requiring an even smaller one. There's also the option of ignoring kdump on RPi4, make ZONE_DMA depend on !CRASH_DUMP and the "low" reservations can use the full 32-bit range since the kdump kernel won't need <30-bit addresses.

Quoted 7 lines

Quoted 5 lines

Are you aware of any reason why we cannot defer the call to reserve_crashkernel() to the start of bootmem_init()? That way, we have access to the unflattened DT as well as the IORT, and so we can tweak the zone limits based on the h/w description, but before allocating the crashkernel.

Not really, as long as memblock_reserve/alloc() still works.

I had a look at this myself, and IIUC we're free to call reserve_crashkernel() anytime as long as it's before memblock_free_all().

Quoted 3 lines

So, should I add a patch in my series taking care of that? or you'd rather take care of it yourselves?

Would you mind adopting this patch, and insert it into your series where appropriate? (after dropping the Documentation/ change, and moving the prototype declaration into linux/acpi_iort.h?) Then, you can also include moving the reserve_crashkernel() into bootmem_init().

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms incorporating masters that can address less than 32 bits of DMA, in particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has peripherals that can only address up to 1 GB (and its PCIe host bridge can only access the bottom 3 GB)

Instructing the DMA layer about these limitations is straight-forward, even though we had to fix some issues regarding memory limits set in the IORT for named components, and regarding the handling of ACPI _DMA methods. However, the DMA layer also needs to be able to allocate memory that is guaranteed to meet those DMA constraints, for bounce buffering as well as allocating the backing for consistent mappings.

This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately, it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and potentially in other places where allocations cannot cross zone boundaries. Therefore, we should avoid having two separate DMA zones when possible.

So let's do an early scan of the IORT, and only create the ZONE_DMA if we encounter any devices that need it. This puts the burden on the firmware to describe such limitations in the IORT, which may be redundant (and less precise) if _DMA methods are also being provided. However, it should be noted that this situation is highly unusual for arm64 ACPI machines. Also, the DMA subsystem still gives precedence to the _DMA method if implemented, and so we will not lose the ability to perform streaming DMA outside the ZONE_DMA if the _DMA method permits it.

Cc Jeremy Linton <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Nicolas Saenz Julienne <[email protected]>

Cc Rob Herring <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Robin Murphy <[email protected]>

Cc Hanjun Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Anshuman Khandual <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
This is related to the discussion in

https://lore.kernel.org/linux-arm-kernel/[email protected]/

 Documentation/arm64/arm-acpi.rst |  7 +++
 arch/arm64/mm/init.c             |  8 +++
 drivers/acpi/arm64/iort.c        | 51 ++++++++++++++++++++
 3 files changed, 66 insertions(+)

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

arch/arm64/mm/init.c +5 -0

+		zone_dma_bits = min(zone_dma_bits,
+				    acpi_iort_get_zone_dma_size());
+		arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+	}
+
 	max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32

drivers/acpi/arm64/iort.c +32 -0

index ec782e4a0fe4..c3db44896e49 100644
@@ -1722,3 +1722,54 @@ void __init acpi_iort_init(void)
 
 	iort_init_platform_devices();
 }
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+	struct acpi_table_iort *iort;
+	struct acpi_iort_node *node, *end;
+	acpi_status status;
+	u8 limit = 32;
+	int i;
+
+	if (acpi_disabled)
+		return limit;
+
+	status = acpi_get_table(ACPI_SIG_IORT, 0,
+				(struct acpi_table_header **)&iort);
+	if (ACPI_FAILURE(status))
+		return limit;
+
+	node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+	end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+	for (i = 0; i < iort->node_count; i++) {
+		if (node >= end)
+			break;
+
+		switch (node->type) {
+			struct acpi_iort_named_component *ncomp;
+			struct acpi_iort_root_complex *rc;

drivers/acpi/arm64/iort.c +9 -0

+				limit = min(limit, rc->memory_address_limit);
+			break;
+		}
+		node = ACPI_ADD_PTR(struct acpi_iort_node, node, node->length);
+	}
+	acpi_put_table(&iort->header);
+	return limit;
+}
+#endif

Signature · 1 line

2.17.1

Quoted 42 lines · Ard Biesheuvel

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms incorporating masters that can address less than 32 bits of DMA, in particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has peripherals that can only address up to 1 GB (and its PCIe host bridge can only access the bottom 3 GB)

Instructing the DMA layer about these limitations is straight-forward, even though we had to fix some issues regarding memory limits set in the IORT for named components, and regarding the handling of ACPI _DMA methods. However, the DMA layer also needs to be able to allocate memory that is guaranteed to meet those DMA constraints, for bounce buffering as well as allocating the backing for consistent mappings.

This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately, it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and potentially in other places where allocations cannot cross zone boundaries. Therefore, we should avoid having two separate DMA zones when possible.

So let's do an early scan of the IORT, and only create the ZONE_DMA if we encounter any devices that need it. This puts the burden on the firmware to describe such limitations in the IORT, which may be redundant (and less precise) if _DMA methods are also being provided. However, it should be noted that this situation is highly unusual for arm64 ACPI machines. Also, the DMA subsystem still gives precedence to the _DMA method if implemented, and so we will not lose the ability to perform streaming DMA outside the ZONE_DMA if the _DMA method permits it.

Cc Jeremy Linton <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Nicolas Saenz Julienne <[email protected]>

Cc Rob Herring <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Robin Murphy <[email protected]>

Cc Hanjun Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Anshuman Khandual <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
This is related to the discussion in

Quoted 6 lines

https://lore.kernel.org/linux-arm-kernel/[email protected]/

 Documentation/arm64/arm-acpi.rst |  7 +++
 arch/arm64/mm/init.c             |  8 +++
 drivers/acpi/arm64/iort.c        | 51 ++++++++++++++++++++
 3 files changed, 66 insertions(+)

Thanks for putting it together so promptly.

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Quoted 9 lines

Documentation/arm64/arm-acpi.rst +0 -0

 ACPI Detection
 --------------

arch/arm64/mm/init.c +0 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 3 lines

arch/arm64/mm/init.c +2 -0

 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Yes as Catalin asked please add a declaration in IORT headers.

Quoted 53 lines

arch/arm64/mm/init.c +5 -0

+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32

drivers/acpi/arm64/iort.c +37 -0

index ec782e4a0fe4..c3db44896e49 100644
@@ -1722,3 +1722,54 @@ void __init acpi_iort_init(void)
 
      iort_init_platform_devices();
 }
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+     struct acpi_table_iort *iort;
+     struct acpi_iort_node *node, *end;
+     acpi_status status;
+     u8 limit = 32;
+     int i;
+
+     if (acpi_disabled)
+             return limit;
+
+     status = acpi_get_table(ACPI_SIG_IORT, 0,
+                             (struct acpi_table_header **)&iort);
+     if (ACPI_FAILURE(status))
+             return limit;
+
+     node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+     end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+     for (i = 0; i < iort->node_count; i++) {
+             if (node >= end)
+                     break;
+
+             switch (node->type) {
+                     struct acpi_iort_named_component *ncomp;
+                     struct acpi_iort_root_complex *rc;
+
+             case ACPI_IORT_NODE_NAMED_COMPONENT:
+                     ncomp = (struct acpi_iort_named_component *)node->node_data;
+                     if (ncomp->memory_address_limit)
+                             limit = min(limit, ncomp->memory_address_limit);

drivers/acpi/arm64/iort.c void __init acpi_iort_init(void) · line 1766

Quoted 5 lines

drivers/acpi/arm64/iort.c +5 -0

+                     break;
+
+             case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+                     rc = (struct acpi_iort_root_complex *)node->node_data;
+                     if (rc->memory_address_limit);

You need a node->revision check here otherwise we may end up dereferencing junk. AKA ACPI versioning in all its glory.

Thanks,
Lorenzo

Quoted 12 lines

drivers/acpi/arm64/iort.c +9 -1

+                             limit = min(limit, rc->memory_address_limit);
+                     break;
+             }
+             node = ACPI_ADD_PTR(struct acpi_iort_node, node, node->length);
+     }
+     acpi_put_table(&iort->header);
+     return limit;
+}
+#endif
--

2.17.1

Quoted 72 lines · Lorenzo Pieralisi

Quoted 42 lines · Ard Biesheuvel

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms incorporating masters that can address less than 32 bits of DMA, in particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has peripherals that can only address up to 1 GB (and its PCIe host bridge can only access the bottom 3 GB)

Instructing the DMA layer about these limitations is straight-forward, even though we had to fix some issues regarding memory limits set in the IORT for named components, and regarding the handling of ACPI _DMA methods. However, the DMA layer also needs to be able to allocate memory that is guaranteed to meet those DMA constraints, for bounce buffering as well as allocating the backing for consistent mappings.

This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately, it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and potentially in other places where allocations cannot cross zone boundaries. Therefore, we should avoid having two separate DMA zones when possible.

So let's do an early scan of the IORT, and only create the ZONE_DMA if we encounter any devices that need it. This puts the burden on the firmware to describe such limitations in the IORT, which may be redundant (and less precise) if _DMA methods are also being provided. However, it should be noted that this situation is highly unusual for arm64 ACPI machines. Also, the DMA subsystem still gives precedence to the _DMA method if implemented, and so we will not lose the ability to perform streaming DMA outside the ZONE_DMA if the _DMA method permits it.

Cc Jeremy Linton <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Nicolas Saenz Julienne <[email protected]>

Cc Rob Herring <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Robin Murphy <[email protected]>

Cc Hanjun Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Anshuman Khandual <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
This is related to the discussion in

Quoted 6 lines

https://lore.kernel.org/linux-arm-kernel/[email protected]/

 Documentation/arm64/arm-acpi.rst |  7 +++
 arch/arm64/mm/init.c             |  8 +++
 drivers/acpi/arm64/iort.c        | 51 ++++++++++++++++++++
 3 files changed, 66 insertions(+)

Thanks for putting it together so promptly.

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

Quoted 6 lines

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

Quoted 9 lines

Quoted 9 lines

Documentation/arm64/arm-acpi.rst +0 -0

 ACPI Detection
 --------------

arch/arm64/mm/init.c +0 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 

Quoted 6 lines

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 3 lines

arch/arm64/mm/init.c +2 -0

 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Yes as Catalin asked please add a declaration in IORT headers.

Ack.

Quoted 53 lines

Quoted 53 lines

arch/arm64/mm/init.c +5 -0

+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32

drivers/acpi/arm64/iort.c +37 -0

index ec782e4a0fe4..c3db44896e49 100644
@@ -1722,3 +1722,54 @@ void __init acpi_iort_init(void)
 
      iort_init_platform_devices();
 }
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+     struct acpi_table_iort *iort;
+     struct acpi_iort_node *node, *end;
+     acpi_status status;
+     u8 limit = 32;
+     int i;
+
+     if (acpi_disabled)
+             return limit;
+
+     status = acpi_get_table(ACPI_SIG_IORT, 0,
+                             (struct acpi_table_header **)&iort);
+     if (ACPI_FAILURE(status))
+             return limit;
+
+     node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+     end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+     for (i = 0; i < iort->node_count; i++) {
+             if (node >= end)
+                     break;
+
+             switch (node->type) {
+                     struct acpi_iort_named_component *ncomp;
+                     struct acpi_iort_root_complex *rc;
+
+             case ACPI_IORT_NODE_NAMED_COMPONENT:
+                     ncomp = (struct acpi_iort_named_component *)node->node_data;
+                     if (ncomp->memory_address_limit)
+                             limit = min(limit, ncomp->memory_address_limit);

Quoted 9 lines

drivers/acpi/arm64/iort.c void __init acpi_iort_init(void) · line 1766

Quoted 5 lines

drivers/acpi/arm64/iort.c +5 -0

+                     break;
+
+             case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+                     rc = (struct acpi_iort_root_complex *)node->node_data;
+                     if (rc->memory_address_limit);

You need a node->revision check here otherwise we may end up dereferencing junk. AKA ACPI versioning in all its glory.

The address limit field was there since the beginning, and DEN0049B defines its value as 0x0, so I don't think we need to check anything here.

Quoted 171 lines · Ard Biesheuvel

Quoted 72 lines · Lorenzo Pieralisi

Quoted 42 lines · Ard Biesheuvel

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms incorporating masters that can address less than 32 bits of DMA, in particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has peripherals that can only address up to 1 GB (and its PCIe host bridge can only access the bottom 3 GB)

Instructing the DMA layer about these limitations is straight-forward, even though we had to fix some issues regarding memory limits set in the IORT for named components, and regarding the handling of ACPI _DMA methods. However, the DMA layer also needs to be able to allocate memory that is guaranteed to meet those DMA constraints, for bounce buffering as well as allocating the backing for consistent mappings.

This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately, it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes problems with kdump, and potentially in other places where allocations cannot cross zone boundaries. Therefore, we should avoid having two separate DMA zones when possible.

So let's do an early scan of the IORT, and only create the ZONE_DMA if we encounter any devices that need it. This puts the burden on the firmware to describe such limitations in the IORT, which may be redundant (and less precise) if _DMA methods are also being provided. However, it should be noted that this situation is highly unusual for arm64 ACPI machines. Also, the DMA subsystem still gives precedence to the _DMA method if implemented, and so we will not lose the ability to perform streaming DMA outside the ZONE_DMA if the _DMA method permits it.

Cc Jeremy Linton <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Cc Nicolas Saenz Julienne <[email protected]>

Cc Rob Herring <[email protected]>

Cc Christoph Hellwig <[email protected]>

Cc Robin Murphy <[email protected]>

Cc Hanjun Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Cc Anshuman Khandual <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
This is related to the discussion in

Quoted 6 lines

https://lore.kernel.org/linux-arm-kernel/[email protected]/

 Documentation/arm64/arm-acpi.rst |  7 +++
 arch/arm64/mm/init.c             |  8 +++
 drivers/acpi/arm64/iort.c        | 51 ++++++++++++++++++++
 3 files changed, 66 insertions(+)

Thanks for putting it together so promptly.

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

Quoted 6 lines

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

Quoted 9 lines

Quoted 9 lines

Documentation/arm64/arm-acpi.rst +0 -0

 ACPI Detection
 --------------

arch/arm64/mm/init.c +0 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 

Quoted 6 lines

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 3 lines

arch/arm64/mm/init.c +2 -0

 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Yes as Catalin asked please add a declaration in IORT headers.

Ack.

Quoted 62 lines

Quoted 48 lines

arch/arm64/mm/init.c +5 -0

+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32

drivers/acpi/arm64/iort.c +32 -0

index ec782e4a0fe4..c3db44896e49 100644
@@ -1722,3 +1722,54 @@ void __init acpi_iort_init(void)
 
      iort_init_platform_devices();
 }
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+     struct acpi_table_iort *iort;
+     struct acpi_iort_node *node, *end;
+     acpi_status status;
+     u8 limit = 32;
+     int i;
+
+     if (acpi_disabled)
+             return limit;
+
+     status = acpi_get_table(ACPI_SIG_IORT, 0,
+                             (struct acpi_table_header **)&iort);
+     if (ACPI_FAILURE(status))
+             return limit;
+
+     node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+     end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+     for (i = 0; i < iort->node_count; i++) {
+             if (node >= end)
+                     break;
+
+             switch (node->type) {
+                     struct acpi_iort_named_component *ncomp;
+                     struct acpi_iort_root_complex *rc;

drivers/acpi/arm64/iort.c void __init acpi_iort_init(void) · line 1766

Quoted 10 lines

drivers/acpi/arm64/iort.c +10 -0

+
+             case ACPI_IORT_NODE_NAMED_COMPONENT:
+                     ncomp = (struct acpi_iort_named_component *)node->node_data;
+                     if (ncomp->memory_address_limit)
+                             limit = min(limit, ncomp->memory_address_limit);
+                     break;
+
+             case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+                     rc = (struct acpi_iort_root_complex *)node->node_data;
+                     if (rc->memory_address_limit);

You need a node->revision check here otherwise we may end up dereferencing junk. AKA ACPI versioning in all its glory.

Quoted 4 lines

The address limit field was there since the beginning, and DEN0049B defines its value as 0x0, so I don't think we need to check anything here.

I meant here that the NC node revision is defined as 0x0 in DEN0049B

Quoted 19 lines

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

Quoted 6 lines

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

Quoted 9 lines

Quoted 9 lines

Documentation/arm64/arm-acpi.rst +0 -0

 ACPI Detection
 --------------

arch/arm64/mm/init.c +0 -0

index f0599ae73b8d..829fa63c3d72 100644
@@ -191,6 +191,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
      unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 

Quoted 6 lines

arch/arm64/mm/init.c static void __init zone_sizes_init(unsigned long min, unsigned long max) · line 195

Quoted 3 lines

arch/arm64/mm/init.c +2 -0

 #ifdef CONFIG_ZONE_DMA
+     if (IS_ENABLED(CONFIG_ACPI)) {
+             extern unsigned int acpi_iort_get_zone_dma_size(void);

Yes as Catalin asked please add a declaration in IORT headers.

Ack.

Quoted 53 lines

Quoted 53 lines

arch/arm64/mm/init.c +5 -0

+             zone_dma_bits = min(zone_dma_bits,
+                                 acpi_iort_get_zone_dma_size());
+             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
+     }
+
      max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32

drivers/acpi/arm64/iort.c +37 -0

index ec782e4a0fe4..c3db44896e49 100644
@@ -1722,3 +1722,54 @@ void __init acpi_iort_init(void)
 
      iort_init_platform_devices();
 }
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+     struct acpi_table_iort *iort;
+     struct acpi_iort_node *node, *end;
+     acpi_status status;
+     u8 limit = 32;
+     int i;
+
+     if (acpi_disabled)
+             return limit;
+
+     status = acpi_get_table(ACPI_SIG_IORT, 0,
+                             (struct acpi_table_header **)&iort);
+     if (ACPI_FAILURE(status))
+             return limit;
+
+     node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+     end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+     for (i = 0; i < iort->node_count; i++) {
+             if (node >= end)
+                     break;
+
+             switch (node->type) {
+                     struct acpi_iort_named_component *ncomp;
+                     struct acpi_iort_root_complex *rc;
+
+             case ACPI_IORT_NODE_NAMED_COMPONENT:
+                     ncomp = (struct acpi_iort_named_component *)node->node_data;
+                     if (ncomp->memory_address_limit)
+                             limit = min(limit, ncomp->memory_address_limit);

Quoted 9 lines

drivers/acpi/arm64/iort.c void __init acpi_iort_init(void) · line 1766

Quoted 5 lines

drivers/acpi/arm64/iort.c +5 -0

+                     break;
+
+             case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+                     rc = (struct acpi_iort_root_complex *)node->node_data;
+                     if (rc->memory_address_limit);

You need a node->revision check here otherwise we may end up dereferencing junk. AKA ACPI versioning in all its glory.

The address limit field was there since the beginning, and DEN0049B defines its value as 0x0, so I don't think we need to check anything here.

Quoted 15 lines

Thanks,
Lorenzo

Quoted 12 lines

drivers/acpi/arm64/iort.c +9 -1

+                             limit = min(limit, rc->memory_address_limit);
+                     break;
+             }
+             node = ACPI_ADD_PTR(struct acpi_iort_node, node, node->length);
+     }
+     acpi_put_table(&iort->header);
+     return limit;
+}
+#endif
--

2.17.1

On Tue, Oct 13, 2020 at 01:22:32PM +0200, Ard Biesheuvel wrote:

[...]

Quoted 26 lines

Quoted 19 lines

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

Quoted 6 lines

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Quoted 6 lines

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

I think we do - if we don't we should not add this documentation either.

ACPI on ARM64 software stack is based on standardized HW requirements. The sheer fact that we need to work around a HW deficiency shows that either this platform should have never been booted with ACPI or the _HW_ design guidelines (BSA) are not tight enough.

Please note that as you may have understood I asked if we can implement a workaround in IORT because that's information that must be there regardless (and an OEM ID match in arch code - though pragmatic - defeats the whole purpose), I don't think we should tell Linux kernel developers how firmware must be written to work around blatantly non-compliant systems.

Quoted 45 lines · Lorenzo Pieralisi

On Tue, Oct 13, 2020 at 01:22:32PM +0200, Ard Biesheuvel wrote:

[...]

Quoted 26 lines

Quoted 19 lines

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
  in some environments other tables may be needed (e.g., any of the APEI
  tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

Quoted 6 lines

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Quoted 6 lines

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

I think we do - if we don't we should not add this documentation either.

ACPI on ARM64 software stack is based on standardized HW requirements. The sheer fact that we need to work around a HW deficiency shows that either this platform should have never been booted with ACPI or the _HW_ design guidelines (BSA) are not tight enough.

Quoted 7 lines

Please note that as you may have understood I asked if we can implement a workaround in IORT because that's information that must be there regardless (and an OEM ID match in arch code - though pragmatic - defeats the whole purpose), I don't think we should tell Linux kernel developers how firmware must be written to work around blatantly non-compliant systems.

This is not about systems being compliant or not, unless there is a requirement somewhere that I missed that all masters in the system must be able to access at least 32 bits of DMA.

The problem here is that Linux/arm64 cannot deal with fully compliant systems that communicate their [permitted] DMA limitations via a _DMA method if this limitation happens to be that the address limit < 32 bits. The DMA subsystem can deal with this fine, only the default DMA zone sizing policy creates an internal issue where the DMA subsystem is not able to allocate memory that matches the DMA constraints.

So the 'correct' fix here would be to rework the memory allocator so it can deal with arbitrary DMA limits at allocation time, so that any limit returned by a _DMA method can be adhered to on the fly.

However, we all agree that the Raspberry Pi4 is not worth that effort, and that in the general case, SoCs with such limitations, even if they are compliant per the spec, are not worth the trouble of complicating this even more. So as a compromise, I think it is perfectly reasonable to require that systems that have such limitations communicate them via the IORT, which we can parse early, regardless of whether _DMA methods exist as well, and whether they return the same information.

So this is not a requirement on arm64 ACPI systems in general. It is a requirement that expresses that we, as arm64 contributors/[co-]maintainers, are willing to cater for such systems if they implement their firmware in a particular way.

Quoted 45 lines · Lorenzo Pieralisi

On Tue, Oct 13, 2020 at 01:22:32PM +0200, Ard Biesheuvel wrote:

[...]

Quoted 26 lines

Quoted 19 lines

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

Quoted 6 lines

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

I would not add this paragraph to arm-acpi.rst, yet.

Quoted 6 lines

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

I think we do - if we don't we should not add this documentation either.

ACPI on ARM64 software stack is based on standardized HW requirements. The sheer fact that we need to work around a HW deficiency shows that either this platform should have never been booted with ACPI or the _HW_ design guidelines (BSA) are not tight enough.

Quoted 7 lines

Please note that as you may have understood I asked if we can implement a workaround in IORT because that's information that must be there regardless (and an OEM ID match in arch code - though pragmatic - defeats the whole purpose), I don't think we should tell Linux kernel developers how firmware must be written to work around blatantly non-compliant systems.

This is not about systems being compliant or not, unless there is a requirement somewhere that I missed that all masters in the system must be able to access at least 32 bits of DMA.

The problem here is that Linux/arm64 cannot deal with fully compliant systems that communicate their [permitted] DMA limitations via a _DMA method if this limitation happens to be that the address limit < 32 bits. The DMA subsystem can deal with this fine, only the default DMA zone sizing policy creates an internal issue where the DMA subsystem is not able to allocate memory that matches the DMA constraints.

So the 'correct' fix here would be to rework the memory allocator so it can deal with arbitrary DMA limits at allocation time, so that any limit returned by a _DMA method can be adhered to on the fly.

However, we all agree that the Raspberry Pi4 is not worth that effort, and that in the general case, SoCs with such limitations, even if they are compliant per the spec, are not worth the trouble of complicating this even more. So as a compromise, I think it is perfectly reasonable to require that systems that have such limitations communicate them via the IORT, which we can parse early, regardless of whether _DMA methods exist as well, and whether they return the same information.

So this is not a requirement on arm64 ACPI systems in general. It is a requirement that expresses that we, as arm64 contributors/[co-]maintainers, are willing to cater for such systems if they implement their firmware in a particular way.

Quoted 49 lines · Ard Biesheuvel

Quoted 46 lines · Lorenzo Pieralisi

Quoted 23 lines · Ard Biesheuvel

Quoted 23 lines

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

Quoted 8 lines

Quoted 1 line

I would not add this paragraph to arm-acpi.rst, yet.

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

I think we do - if we don't we should not add this documentation either.

ACPI on ARM64 software stack is based on standardized HW requirements. The sheer fact that we need to work around a HW deficiency shows that either this platform should have never been booted with ACPI or the _HW_ design guidelines (BSA) are not tight enough.

Please note that as you may have understood I asked if we can implement a workaround in IORT because that's information that must be there regardless (and an OEM ID match in arch code - though pragmatic - defeats the whole purpose), I don't think we should tell Linux kernel developers how firmware must be written to work around blatantly non-compliant systems.

Quoted 3 lines

This is not about systems being compliant or not, unless there is a requirement somewhere that I missed that all masters in the system must be able to access at least 32 bits of DMA.

I think there is in the SBSA (4.1.3 Memory Map) but regardless, this is clearly a design bug, that's not a feature.

Quoted 19 lines

The problem here is that Linux/arm64 cannot deal with fully compliant systems that communicate their [permitted] DMA limitations via a _DMA method if this limitation happens to be that the address limit < 32 bits. The DMA subsystem can deal with this fine, only the default DMA zone sizing policy creates an internal issue where the DMA subsystem is not able to allocate memory that matches the DMA constraints.

So the 'correct' fix here would be to rework the memory allocator so it can deal with arbitrary DMA limits at allocation time, so that any limit returned by a _DMA method can be adhered to on the fly.

However, we all agree that the Raspberry Pi4 is not worth that effort, and that in the general case, SoCs with such limitations, even if they are compliant per the spec, are not worth the trouble of complicating this even more. So as a compromise, I think it is perfectly reasonable to require that systems that have such limitations communicate them via the IORT, which we can parse early, regardless of whether _DMA methods exist as well, and whether they return the same information.

Quoted 4 lines

So this is not a requirement on arm64 ACPI systems in general. It is a requirement that expresses that we, as arm64 contributors/[co-]maintainers, are willing to cater for such systems if they implement their firmware in a particular way.

I don't think they should implement their firmware in any particular way, that's my point, I don't want them to in the first place.

To start with there is no spec I am aware of that defines when/how to use _DMA vs IORT address limits, maybe we should spell that out better somewhere and that's useful regardless.

My point is: either this workaround works with firmware written with guidelines valid for all arm64 systems (not as a special case: add an IORT table because we can't parse _DMA to workaround DMA address range shenanigans) or I am not willing to merge it - I prefer to add an OEM ID quirk and show what we are forced to do to make this work.

Hi Lorenzo,

Quoted 50 lines · Lorenzo Pieralisi

Quoted 49 lines · Ard Biesheuvel

Quoted 46 lines · Lorenzo Pieralisi

Quoted 23 lines · Ard Biesheuvel

Quoted 23 lines

Quoted 5 lines

Documentation/arm64/arm-acpi.rst +0 -0

index 47ecb9930dde..947f5b5c45ef 100644

Documentation/arm64/arm-acpi.rst devices available. This list of tables is not meant to be all inclusive; · line 214

Quoted 10 lines

Documentation/arm64/arm-acpi.rst +7 -0

@@ -205,6 +205,13 @@ devices available.  This list of tables is not meant to be all inclusive;
 in some environments other tables may be needed (e.g., any of the APEI
 tables from section 18) to support specific functionality.
 
+It is assumed that all DMA capable devices in the system are able to
+access the lowest 4 GB of system memory. If this is not the case, an
+IORT describing those limitations is mandatory, even if an IORT is not
+otherwise necessary to describe the I/O topology, and regardless of
+whether _DMA methods are used to describe the DMA limitations more
+precisely. Once the system has booted, _DMA methods will take precedence
+over DMA addressing limits described in the IORT.

If this is a boot requirement it must be in ARM's official documentation, first, not the kernel one.

I understand this is an urgent (well - no comments on why bootstrapping ACPI on Raspberry PI4 is causing all this fuss, honestly) fix but that's not a reason to rush through these guidelines.

Quoted 8 lines

Quoted 1 line

I would not add this paragraph to arm-acpi.rst, yet.

Which documentation? ACPI compliance by itself is not sufficient for a system to be able to boot Linux/arm64, which is why we documented the requirements for ACPI boot on Linux/arm64 in this file. I don't think we need endorsement from ARM to decide that odd platforms like this need to abide by some additional rules if they want to boot in ACPI mode.

I think we do - if we don't we should not add this documentation either.

ACPI on ARM64 software stack is based on standardized HW requirements. The sheer fact that we need to work around a HW deficiency shows that either this platform should have never been booted with ACPI or the _HW_ design guidelines (BSA) are not tight enough.

Please note that as you may have understood I asked if we can implement a workaround in IORT because that's information that must be there regardless (and an OEM ID match in arch code - though pragmatic - defeats the whole purpose), I don't think we should tell Linux kernel developers how firmware must be written to work around blatantly non-compliant systems.

Quoted 6 lines

Quoted 3 lines

This is not about systems being compliant or not, unless there is a requirement somewhere that I missed that all masters in the system must be able to access at least 32 bits of DMA.

I think there is in the SBSA (4.1.3 Memory Map) but regardless, this is clearly a design bug, that's not a feature.

I think in revision D of the SBSA 6.0 this was moved to 3.1.3. Anyway, I guess you are referring to:

All Non-secure on-chip masters in a base server system that are expected to be under the control of the operating system or hypervisor must be capable of addressing all of the Non-secure address space.

IIUC, this rules out 32-bit devices as well. Even if we look at the less strict BSA (https://developer.arm.com/documentation/den0094/latest), the requirements are the same.

So we can consider the bug either in the *BSA specs or in hardware. If at least the 32-bit devices are acceptable, the specs should probably be updated. Otherwise we just state that Linux has slightly different requirements than *BSA (more relaxed or tightened in various areas) and we describe them somewhere under Documentation/arm64/.

Quoted 24 lines

Quoted 23 lines

The problem here is that Linux/arm64 cannot deal with fully compliant systems that communicate their [permitted] DMA limitations via a _DMA method if this limitation happens to be that the address limit < 32 bits. The DMA subsystem can deal with this fine, only the default DMA zone sizing policy creates an internal issue where the DMA subsystem is not able to allocate memory that matches the DMA constraints.

So the 'correct' fix here would be to rework the memory allocator so it can deal with arbitrary DMA limits at allocation time, so that any limit returned by a _DMA method can be adhered to on the fly.

However, we all agree that the Raspberry Pi4 is not worth that effort, and that in the general case, SoCs with such limitations, even if they are compliant per the spec, are not worth the trouble of complicating this even more. So as a compromise, I think it is perfectly reasonable to require that systems that have such limitations communicate them via the IORT, which we can parse early, regardless of whether _DMA methods exist as well, and whether they return the same information.

So this is not a requirement on arm64 ACPI systems in general. It is a requirement that expresses that we, as arm64 contributors/[co-]maintainers, are willing to cater for such systems if they implement their firmware in a particular way.

Quoted 2 lines

I don't think they should implement their firmware in any particular way, that's my point, I don't want them to in the first place.

I haven't checked the *BBR specs, do they say anything about _DMA methods or IORT tables for devices that can't access the full memory?

Quoted 3 lines

To start with there is no spec I am aware of that defines when/how to use _DMA vs IORT address limits, maybe we should spell that out better somewhere and that's useful regardless.

I guess this answers my question above.

Quoted 5 lines

My point is: either this workaround works with firmware written with guidelines valid for all arm64 systems (not as a special case: add an IORT table because we can't parse _DMA to workaround DMA address range shenanigans) or I am not willing to merge it - I prefer to add an OEM ID quirk and show what we are forced to do to make this work.

If the guidelines don't say anything about <=32-bit device masks, we can make up our requirements for Linux. Currently it happens to work on some kernel versions because we have a ZONE_DMA of 1GB but prior to this change it wouldn't have worked. We might as well disable ZONE_DMA and ZONE_DMA32 altogether because the BSA specs state that there are no such limited devices, but some SoCs would no longer boot.

I think all this depends on whether 32-bit devices are also special and need quirks. If they do, can we add both 32-bit and <32-bit devices in the same "special" pot and require IORT tables? I agree it's not nice to ignore the _DMA method alternative during zone setup but we could still check the latter at run-time at warn if smaller than the minimum ZONE_DMA available.