Re: [PATCH v3 02/18] x86/mm: purge unneeded destroy_perdomain_mapping()

[George Dunlap <gwd@xxxxxxxxxxxxxx>]

On Thu, Oct 8, 2026 at 9:31 AM Roger Pau Monné <roger@xxxxxxxxxxxxxx> wrote:
>
> On Thu, Oct 08, 2026 at 09:55:46AM +0200, Jan Beulich wrote:
> > On 07.10.2026 12:40, George Dunlap wrote:
> > > --- a/xen/arch/x86/x86_64/mm.c
> > > +++ b/xen/arch/x86/x86_64/mm.c
> > > @@ -737,6 +737,11 @@ int setup_compat_arg_xlat(struct vcpu *v)
> > >                                      NULL, NIL(struct page_info *));
> > >  }
> > >
> > > +/*
> > > + * Besides vCPU teardown, which free_perdomain_mappings() would cover, 
> > > this
> > > + * serves switch_compat()'s undo path: the domain lives on as a 64-bit 
> > > one
> > > + * there, so the translation area has to go right away.
> > > + */
> >
> > Is it really "has to"? The xlat area is simply unused for 64-bit guests,
> > so it would be merely "needlessly occupying resources" if we deferred
> > freeing until domain destruction.
>
> I wondered the same, but pv_vcpu_destroy() won't free the xlat are if
> is_pv_32bit_vcpu() returns false, which would be the case if
> switch_compat() fails.  All this could be adjusted, but it seemed more
> churn than benefit.

But free_perdomain_mappings() would still free them; so as Jan says,
it's just about hanging on to unnecessary resources.

I was really on the fence about the comment at all; but I think it
should probably go.  There's little chance of someone removing it
later without checking, and even if they did, the only result would be
a few pages held unnecessarily; in the common case that would be
almost immediately.  (It's theoretically possible for a toolstack to
keep a 64-bit domain around after the 32-bit domain creation failed,
but it would be very strange.)

 -George