Re: [PATCH v3 2/9] xen/arm: add cache coloring initialization for domains

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 21/11/2022 16:40, Jan Beulich wrote:
> On 21.11.2022 17:23, Carlo Nonato wrote:
> > On Mon, Nov 21, 2022 at 4:14 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > > On 21.11.2022 15:50, Carlo Nonato wrote:
> > > > I want to ask you some questions about this patch because in the previous
> > > > version me and Julien have discussed how cache colors should be passed in
> > > > domain creation. You should be able to read that discussion, anyway here is
> > > > a link to it
> > > >
> > > > https://marc.info/?l=xen-devel&m=166151802002263
> > > I've looked at the first few entries, without finding an answer to ...
> > >
> > > > In short, using struct xen_arch_domainconfig works fine only when domctl
> > > > hypercall is issued. That struct contains a XEN_GUEST_HANDLE so it
> > > > should point to guest memory and must not be used when creating a domain
> > > > from Xen itself (i.e. dom0 or dom0less domains). The easy way to go is then
> > > > changing the domain_create() signature to require also a color array and its
> > > > length to be passed in for these latter cases.
> > > > Are you ok with that? See below for more comments.
> > > ... my immediate question: Does supplying the colors necessarily need to
> > > done right at domain creation? Wouldn't it suffice to be done before first
> > > allocating memory to the new domain, i.e. via a separate domctl (and then
> > > for Xen-created domains via a separate Xen-internal function, which the
> > > new domctl handling would also call)? Or do colors also affect the
> > > allocation of struct domain itself (and pointers hanging off of it)?
> > This would be really good. The only problem I can see is the p2m allocation
> > which is done during domain creation. With the current set of patches it
> > results in a "Failed to allocate P2M pages" since we want to have p2m tables
> > allocated with the same color of the domain and a null page is returned
> > because we have no colors.
> Hmm, I see. It would seem to me that this p2m init really is happening
> too early. Ideally domain_create would merely mean creating a largely
> empty container, with stuff being populated subsequently as necessary.
The vGIC is not optional. So to me it sounds wrong to defer the decision 
to after the domain is created.
It is not clear to me how you would check that mandatory components have 
been properly initialized.
> But I guess this is too much of a re-work to be done in the context
> here, plus of course I may be overlooking something which actually
> makes it necessary for domain creation to be structured the way it is
> right now. (Imo the reason for the early minimal population of the p2m,
> added only quite recently, wasn't a good one, and the vGIC init would
> better be deferred. Yet once again I may lack details on why that's not
> possible.)
See above for the theoritical part. For the practice part, we need to 
know the vGIC version at domain creation because it impact the maximum 
of vCPU we can expose.
It is also not very clear where this could be initialized. Are you 
suggesting to add an extra mandatory hypercall? FAOD, I don't think 
p2m_set_allocation() would be the right place.
Cheers,

--
Julien Grall