Re: [PATCH v2 04/17] kexec: Add KHO parsing support

[Alexander Graf]

On 01.01.24 04:33, Stanislav Kinsburskii wrote:
> On Fri, Dec 22, 2023 at 07:35:54PM +0000, Alexander Graf wrote:
> > +/**
> > + * kho_reserve_previous_mem - Adds all memory reservations into memblocks
> > + * and moves us out of the scratch only phase. Must be called after page tables
> > + * are initialized and memblock_allow_resize().
> > + */
> > +void __init kho_reserve_previous_mem(void)
> > +{
> > +     void *mem_virt = __va(mem_phys);
> > +     int off, err;
> > +
> > +     if (!handover_phys || !mem_phys)
> > +             return;
> > +
> > +     /*
> > +      * We reached here because we are running inside a working linear map
> > +      * that allows us to resize memblocks dynamically. Use the chance and
> > +      * populate the global fdt pointer
> > +      */
> > +     fdt = __va(handover_phys);
> > +
> > +     off = fdt_path_offset(fdt, "/");
> > +     if (off < 0) {
> > +             fdt = NULL;
> > +             return;
> > +     }
> > +
> > +     err = fdt_node_check_compatible(fdt, off, "kho-v1");
> > +     if (err) {
> > +             pr_warn("KHO has invalid compatible, disabling.");
> It looks like KHO preserved regions won't be reserved in this case.
> Should KHO DT state be destroyed here to prevent KHO memory regions
> reuse upon rollback?


Good catch. I'll set fdt to NULL in that case in v3.


> > +
> > +void __init kho_populate(phys_addr_t handover_dt_phys, phys_addr_t scratch_phys,
> > +                      u64 scratch_len, phys_addr_t mem_cache_phys,
> > +                      u64 mem_cache_len)
> > +{
> > +     void *handover_dt;
> > +
> > +     /* Determine the real size of the DT */
> > +     handover_dt = early_memremap(handover_dt_phys, sizeof(struct fdt_header));
> > +     if (!handover_dt) {
> > +             pr_warn("setup: failed to memremap kexec FDT (0x%llx)\n", handover_dt_phys);
> > +             return;
> > +     }
> > +
> > +     if (fdt_check_header(handover_dt)) {
> > +             pr_warn("setup: kexec handover FDT is invalid (0x%llx)\n", handover_dt_phys);
> > +             early_memunmap(handover_dt, PAGE_SIZE);
> > +             return;
> > +     }
> > +
> > +     handover_len = fdt_totalsize(handover_dt);
> > +     handover_phys = handover_dt_phys;
> > +
> > +     /* Reserve the DT so we can still access it in late boot */
> > +     memblock_reserve(handover_phys, handover_len);
> > +
> > +     /* Reserve the mem cache so we can still access it later */
> > +     memblock_reserve(mem_cache_phys, mem_cache_len);
> > +
> > +     /*
> > +      * We pass a safe contiguous block of memory to use for early boot purporses from
> > +      * the previous kernel so that we can resize the memblock array as needed.
> > +      */
> > +     memblock_add(scratch_phys, scratch_len);
> > +
> > +     if (WARN_ON(memblock_mark_scratch(scratch_phys, scratch_len))) {
> > +             pr_err("Kexec failed to mark the scratch region. Disabling KHO.");
> > +             handover_len = 0;
> > +             handover_phys = 0;
> Same question here: doesn't all the KHO state gets invalid in case of any
> restoration error?


It does, which is what the error case here does, no? Or are you 
referring to the fact that we're not unrolling the memblock 
reservations? If we can't mark the scratch region, I'd rather leave 
everything else alone. It means the scratch region is in a hole, which 
should never happen.


Alex




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879