On Mon, May 20, 2019 at 5:13 PM Christoph Hellwig <h...@infradead.org> wrote: > > > void __init parse_dtb(unsigned int hartid, void *dtb) > > { > > - if (early_init_dt_scan(__va(dtb))) > > + dtb = (void *)fix_to_virt(FIX_FDT) + ((uintptr_t)dtb & ~PAGE_MASK); > > + if (early_init_dt_scan(dtb)) > > FYI, parse_dtb in mainline now lost the hartid argument and takes a > phys_addr_t for the dtb address.
Okay, this patch is based on 5.1 kernel. I guess I will have to rebase it anyway. > > That being said I find the above way to magic. So we take the fixmap > address and then only the offset from something passed as a pointer? > This just looks very weird. The way FIX_FDT is defined to add to my > confusion, which might partially be due to not understanding fixmaps > very well. But it seems like at very least we should set up an > actual kernel pointer for the dtb in setup_vm based on what that > gets passed and stop passing any arguments to parse_dtb to keep > that magic in one place. And possibly add some comment. I agree with your suggestion. I will setup early_dtb_ptr in setup_vm() and use it here. FYI, the fixmap virtual address range is not covered by linear va-to-pa translation (i.e. __va() and __pa() cannot be used). The mapping granularity of fixmap is always PAGE_SIZE hence add offset to fix_to_virt(FIX_FDT). > > > +#if MAX_EARLY_MAPPING_SIZE < PGDIR_SIZE > > It seems MAX_EARLY_MAPPING_SIZE is defined to a fix constant, > why do we need these conditionals? Sure, I will remove the conditional. It's totally redundant. I forgot to remove it previously. Regards, Anup