On 22.11.2022 21:25, Julien Grall wrote: > 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 <jbeul...@suse.com> 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.
There could be final checking right before unpausing a domain for the first time. >> 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. Sure - I didn't suggest to leave that information out of domain_create. > 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. I agree with the latter - that would at best be a bodge. And yes, I was thinking of having a separate domctl for that purpose. Jan
