Re: [PATCH 08/15] x86/PVH: split pvh_setup_acpi()

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Wed, Oct 14, 2026 at 11:42:57AM +0200, Jan Beulich wrote:
> On 14.10.2026 11:31, Roger Pau Monné wrote:
> > On Tue, Oct 13, 2026 at 06:04:37PM +0200, Jan Beulich wrote:
> >> On 13.10.2026 17:52, Roger Pau Monné wrote:
> >>> On Mon, Oct 12, 2026 at 10:23:16AM +0200, Jan Beulich wrote:
> >>>> All E820 manipulations want to happen before the initrd is placed, or 
> >>>> else
> >>>> pvh_steal_ram() may steal from the tail part of it. As a second step,
> >>>> split placing ACPI data from actually populating it.
> >>>
> >>> Would it be less intrusive to duplicate the memory map (like it's done
> >>> in a change ahead of this one), and subtract the kernel and initrd
> >>> region from that memory map, then use it as the basis for
> >>> pvh_steal_ram()?
> >>>
> >>> We could adjust/replace pvh_add_mem_range() so it allows changing
> >>> regions from RAM to other types.  That would seem to be less churn
> >>> overall.  Maybe you did already consider this and found further
> >>> problem with this approach that I'm failing to see?
> >>
> >> No, I didn't try that route. In fact despite the order now this patch
> >> (and maybe also the VMX one) pre-dates the E820-copying one. The one
> >> aspect I consider problematic with the suggested alternative is that
> >> then two E820 maps need updating: The one with kernel (not sure about
> >> initrd; see below) subtracted out, and the one we actually mean to hand
> >> to Dom0.
> >>
> >> As to overall churn: I'm not sure which one would cause less. Certainly
> >> the patch here would remain largely unaltered, as the splitting is
> >> going to be necessary no matter what.
> > 
> > I'm possibly missing something, but the splitting won't be strictly
> > necessary?
> > 
> > We could do:
> > 
> >  * Populate p2m
> >  * Place kernel
> >  * Place ACPI (also VMX metadata)
> >  * Place initrd
> 
> The goal is to directly map the initrd. That direct-mapping (or at least
> the determination of where it is to go) needs to happen before populating
> the (rest of) the P2M (as otherwise this would populate the respective
> part of the P2M). Else we'll continue to see the problem that Marek
> reported. That is:

Bah, yes, sorry, I keep forgetting we must not populate the initrd
region because we are directly mapping it.  I find it tedious to have
to first do all the allocations and then do a second call to actually
copy the data, but alas, I don't see another way to deal with this.

>  * Place kernel
>  * Place ACPI (maybe VMX metadata)
>  * Place initrd (possibly direct-mapping it)
>  * Populate P2M
>  * Load kernel and initrd
>  * Fill ACPI (maybe VMX metadata)
> 
> (Of course the order of the latter two items doesn't matter much.)
> 
> Even if we didn't direct-map the initrd along with placing it, we'd
> still need to tell populate_p2m() to skip that region. I.e. the split
> between placing and loading/filling would remain.

Hm, yes, the placement of the initrd blocks populating the p2m, and
cannot be decided until we have positioned the ACPI stuff.

I would really like to find a way to avoid this split, but I don't
think it's possible.

Thanks, Roger.