On Fri, May 02, 2025 at 11:48:54AM -0700, Dave Hansen wrote:
> On 5/1/25 15:54, Changyuan Lyu wrote:
> > +/*
> > + * If KHO is active, only process its scratch areas to ensure we are not
> > + * stepping onto preserved memory.
> > + */
> > +#ifdef CONFIG_KEXEC_HANDOVER
> > +static bool process_kho_entries(unsigned long minimum, unsigned long
> > image_size)
> > +{
>
> I thought we agreed to rework this to unconditionally define the
> kho_scratch structures so the #ifdef can go away?
It's either #ifdef or double casting and my understanding was that your
preference was to get rid of the double casting.
> > + struct kho_scratch *kho_scratch;
> > + struct setup_data *ptr;
> > + int i, nr_areas = 0;
> > +
> > + ptr = (struct setup_data *)boot_params_ptr->hdr.setup_data;
> > + while (ptr) {
> > + if (ptr->type == SETUP_KEXEC_KHO) {
> > + struct kho_data *kho = (struct kho_data *)ptr->data;
> > +
> > + kho_scratch = (void *)kho->scratch_addr;
> > + nr_areas = kho->scratch_size / sizeof(*kho_scratch);
> > +
> > + break;
> > + }
> > +
> > + ptr = (struct setup_data *)ptr->next;
> > + }
> > +
> > + if (!nr_areas)
> > + return false;
> > +
> > + for (i = 0; i < nr_areas; i++) {
> > + struct kho_scratch *area = &kho_scratch[i];
> > + struct mem_vector region = {
> > + .start = area->addr,
> > + .size = area->size,
> > + };
> > +
> > + if (process_mem_region(®ion, minimum, image_size))
> > + break;
> > + }
> > +
> > + return true;
> > +}
> > +#else
> > +static inline bool process_kho_entries(unsigned long minimum,
> > + unsigned long image_size)
> > +{
> > + return false;
> > +}
> > +#endif
> > +
> > static unsigned long find_random_phys_addr(unsigned long minimum,
> > unsigned long image_size)
> > {
> > @@ -775,7 +824,8 @@ static unsigned long find_random_phys_addr(unsigned
> > long minimum,
> > return 0;
> > }
> >
> > - if (!process_efi_entries(minimum, image_size))
> > + if (!process_kho_entries(minimum, image_size) &&
> > + !process_efi_entries(minimum, image_size))
> > process_e820_entries(minimum, image_size);
> >
> > phys_addr = slots_fetch_random();
>
> I made a comment about this in the last round, making this the second
> thing that I've noticed that was not addressed.
>
> Could you please go back through the last round of comments before you
> repost these?
I presumed that changelog covers it. We'll add a comment here for the next
posting.
> Just to be clear: these are making progress, but they're not OK from the
> x86 side yet.
--
Sincerely yours,
Mike.
