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

[Pratyush Yadav <ptyadav@amazon.de>]

Hi,

On Wed, Jan 17 2024, Alexander Graf wrote:

> When we have a KHO kexec, we get a device tree, mem cache and scratch
> region to populate the state of the system. Provide helper functions
> that allow architecture code to easily handle memory reservations based
> on them and give device drivers visibility into the KHO DT and memory
> reservations so they can recover their own state.
>
> Signed-off-by: Alexander Graf <graf@amazon.com>
>
> ---
>
[...]
> +/**
> + * kho_return_mem - Notify the kernel that initially reserved memory is no
> + * longer needed. When the last consumer of a page returns their mem, kho
> + * returns the page to the buddy allocator as free page.
> + */
> +void kho_return_mem(const struct kho_mem *mem)
> +{
> +	uint64_t start_pfn, end_pfn, pfn;
> +
> +	start_pfn = PFN_DOWN(mem->addr);
> +	end_pfn = PFN_UP(mem->addr + mem->len);
> +
> +	for (pfn = start_pfn; pfn < end_pfn; pfn++)
> +		kho_return_pfn(pfn);
> +}
> +EXPORT_SYMBOL_GPL(kho_return_mem);
> +
> +static void kho_claim_pfn(ulong pfn)
> +{
> +	struct page *page = pfn_to_page(pfn);
> +
> +	WARN_ON(!page);
> +	if (WARN_ON(page_count(page) != 1))
> +		pr_err("Claimed non kho pfn %lx", pfn);

You do sanity checks but then never actually change anything on the
page. kho_claim_mem()'s documentation says: "This function removes the
reserved state for all pages that the mem spans". So this function
should at the very least call ClearPageReserved().

Also, checking the page count is a very rough heuristic. There can be
other non-KHO pages with page count == 1. Do you think it would make
more sense to use one of the private pageflags bits to mark a page
KHO-owned? If not, shouldn't you at least also check if the page is
reserved?

> +}
> +
> +/**
> + * kho_claim_mem - Notify the kernel that a handed over memory range is now in
> + * use by a kernel subsystem and considered an allocated page. This function
> + * removes the reserved state for all pages that the mem spans.
> + */
> +void *kho_claim_mem(const struct kho_mem *mem)
> +{
> +	u64 start_pfn, end_pfn, pfn;
> +	void *va = __va(mem->addr);
> +
> +	start_pfn = PFN_DOWN(mem->addr);
> +	end_pfn = PFN_UP(mem->addr + mem->len);
> +
> +	for (pfn = start_pfn; pfn < end_pfn; pfn++)
> +		kho_claim_pfn(pfn);
> +
> +	return va;
> +}
> +EXPORT_SYMBOL_GPL(kho_claim_mem);
> +
[...]

--
Regards,
Pratyush Yadav



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