Messages in this thread | | | Date | Sun, 11 Jun 2023 07:27:08 +0800 | From | Woody Zhang <> | Subject | Re: [PATCH] riscv: reserve DTB before possible memblock allocation |
| |
On Sat, Jun 10, 2023 at 04:41:17PM +0100, Conor Dooley wrote: >On Thu, Jun 08, 2023 at 09:49:44AM +0200, Alexandre Ghiti wrote: >> On Wed, Jun 7, 2023 at 8:17 PM Conor Dooley <conor@kernel.org> wrote: >> > >> > +CC Alex, you should take a look at this patch. >> > >> > On Wed, Jun 07, 2023 at 09:35:19PM +0800, Woody Zhang wrote: >> > > It's possible that early_init_fdt_scan_reserved_mem() allocates memory >> > > from memblock for dynamic reserved memory in `/reserved-memory` node. >> > > Any fixed reservation must be done before that to avoid potential >> > > conflicts. >> > > >> > > Reserve the DTB in memblock just after early scanning it. >> > >> > The rationale makes sense to me, I am just wondering what compelling >> > reason there is to move it away from the memblock_reserve()s for the >> > initd and vmlinux? Moving it above early_init_fdt_scan_reserved_mem() >> > should be the sufficient minimum & would keep things together. > >> Thanks Conor. >> >> So the patch looks good to me. >> >> But I find this fragile: >> >> - we do not check memblock_reserve() return value to make sure the >> reservation really happened (and quickly looking at the code, I'm not >> even sure it returns an error if the region was already allocated). >> - we have to make sure no memblock allocation happens before setup_bootmem(). >> - we also have to check that no fixed memblock_reserve() happens after. >> >> The last 2 points may sound natural, but we'll have to take great care >> when adding some code around here. I'm working on an "early boot >> document" and I'll add something about that, but a runtime thing would >> be way better IMO. >> >> You can add: >> >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> > >btw Alex/Woody, what is the appropriate Fixes tag here?
In ef69d2559fe9 ("riscv: Move early dtb mapping into the fixmap region"), Alex move early_init_fdt_scan_reserved_mem to setup_bootmem() to prevent memory allocations before of reservations. But it should not be put before DTB reservation.
Woody
| |