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

[Julien Grall <julien@xxxxxxx>]

Hi Bertrand,

On 03/01/2023 12:34, Bertrand Marquis wrote:
> Hi,
>
> Sorry for the delay but I have very limited access to my mails right now.
>
> > On 9 Dec 2022, at 23:11, Stefano Stabellini <sstabellini@xxxxxxxxxx 
> > <mailto:sstabellini@xxxxxxxxxx>> wrote:
> > 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 <mailto: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:
> How option 1 would work for dom0less ?
Xen would call the function directly. Effectively, this would the same 
as all the other "hypercalls" we are using to build a dom0less domain.
> Option 2 would make safety more challenging but not impossible (we have 
> a lot of other use cases where we cannot map everything on boot).
> I would vote for option 2 as I think we will not certify gicv2 and it is 
> not adding an other hyper call.
It sounds like option 2 is the preferred way for now. Henry, can you 
have a look?
Cheers,

--
Julien Grall