Re: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised

[Julien Grall <julien@xxxxxxx>]

Hi David,

On 23/03/2020 10:55, David Woodhouse wrote:
> On Mon, 2020-03-23 at 09:34 +0000, Julien Grall wrote:
> > For liveupdate, we will need a way to initialize a page but mark it as
> > already inuse (i.e in the same state as they would be if allocated
> > normally).
> I am unconvinced of the veracity of this claim.
>
> We don't want to turn specific details of the current Xen buddy
> allocator part into of the implicit ABI of live update. That goes for
> the power-of-two zone boundaries, amongst other things.
Why would you to do that? Marking the page as already used is no 
different to "PGC_state_unitialized" except the "struct page_info" and 
the internal of the buddy allocator would be properly setup for start 
rather than at free.
> What if Xen receives LU state in which *all* pages in a given zone are
> marked as already in use? That's one of the cases in which we *really*
> want to pass through init_heap_pages() instead of just
> free_heap_pages(), in order to allocate the zone data structures for
> the first pages that get freed into that zone.
>
> What if Xen starts to exclude more pages, like the exclusion at zero?
>
> What if new Xen wants to exclude an additional page due to a hardware
> erratum? It can't take it away from existing domains (especially if
> there are assigned PCI devices) but it could be part of the vetting in
> init_heap_pages(), for example.
I don't think it would be safe to continue to run a guest using pages 
that were excluded for an HW erratum. It would be safer to not restart 
the domain (or replace the page) in the target Xen if that's hapenning.
> My intent for PGC_state_uninitialised was to mark pages that haven't
> been through init_heap_pages(), whatever init_heap_pages() does in the
> current version of Xen.
>
> The pages which are "already in use" because they're inherited through
> LU state should be in PGC_state_uninitialised, shouldn't they?
I think using "PGC_state_unitialised" for preserved page is an abuse. I 
understand this is existing in other part of Xen (particularly on x86), 
but I would rather not try to add more.
The PGC_state_unitialised may work for the current allocator because 
most of the fields are not going to be used after allocation. But it may 
not hold for any new allocator (I know the embedded folks are working on 
a new one).
> Perhaps if there's a need for a helper, it could be a companion
> function to init_heap_pages() which would return a boolean saying,
> "nah, I didn't want to do anything to this page anyway", which could
> short-circuit it into the PGC_state_inuse state. But I'm not sure I see
> the need for such an optimisation.
I don't view it as an optimisation but as a way to avoid spreading the 
current misbehavior.
Cheers,

--
Julien Grall