Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in arch_domain_create()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 9 Dec 2022, Julien Grall wrote:
> Hi Henry,
> 
> On 08/12/2022 03:06, Henry Wang wrote:
> > I am trying to work on the follow-up improvements about the Arm P2M code,
> > and while trying to address the comment below, I noticed there was an
> > unfinished
> > discussion between me and Julien which I would like to continue and here
> > opinions from all of you (if possible).
> > 
> > > -----Original Message-----
> > > From: Julien Grall <julien@xxxxxxx>
> > > Subject: Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in
> > > arch_domain_create()
> > > > > I also noticed that relinquish_p2m_mapping() is not called. This
> > > > > should
> > > > > be fine for us because arch_domain_create() should never create a
> > > > > mapping that requires p2m_put_l3_page() to be called.
> > 
> > For the background of the discussion, this was about the failure path of
> > arch_domain_create(), where we only call p2m_final_teardown() which does
> > not call relinquish_p2m_mapping()...
> So all this mess with the P2M is necessary because we are mapping the GICv2
> CPU interface in arch_domain_create(). I think we should consider to defer the
> mapping to later.
> 
> Other than it makes the code simpler, it also means we could also the P2M root
> on the same numa node the domain is going to run (those information are passed
> later on).
> 
> There is a couple of options:
>  1. Introduce a new hypercall to finish the initialization of a domain (e.g.
> allocating the P2M and map the GICv2 CPU interface). This option was briefly
> discussed with Jan (see [2])./
>  2. Allocate the P2M when populate the P2M pool and defer the GICv2 CPU
> interface mapping until the first access (similar to how with deal with MMIO
> access for ACPI).
> 
> I find the second option is neater but it has the drawback that it requires on
> more trap to the hypervisor and we can't report any mapping failure (which
> should not happen if the P2M was correctly sized). So I am leaning towards
> option 2.
> 
> Any opinions?

Option 1 is not great due to the extra hypercall. But I worry that
option 2 might make things harder for safety because the
mapping/initialization becomes "dynamic". I don't know if this is a
valid concern.

I would love to hear Bertrand's thoughts about it. Putting him in To: