Re: [PATCH v1 02/13] xen/arm: introduce a special domain DOMID_SHARED

[Julien Grall <julien.grall@xxxxxxxxx>]

Hi Penny,

Sorry for the formatting.

On Fri, 15 Apr 2022 at 09:53, Penny Zheng <Penny.Zheng@xxxxxxx> wrote:

Hi jan

> -----
> > +#ifdef CONFIG_STATIC_SHM
> > +static void __init setup_shared_domain(void) {
> > +    /*
> > +     * Initialise our DOMID_SHARED domain.
> > +     * This domain owns statically shared pages when owner domain is not
> > +     * explicitly defined.
> > +     */
> > +    dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap);
> > +    if ( IS_ERR(dom_shared) )
> > +        panic("Failed to create d[SHARED]: %ld\n",
> > +PTR_ERR(dom_shared));
>
> I don't think this should be a panic - the system ought to be able to come up
> fine, just without actually using this domain. After all this is an optional
> feature which may not actually be used.
>
> Also, along the lines of what Stefano has said, this setting up of the domain
> would also better live next to where the other special domains are set up. And
> even if it was to remain here, ...
>

The reason why I place the setting up here is that DOMID_SHARED needs to map
pre-configured static shared memory in its p2m table, so it must be set up
after system P2M initialization(setup_virt_paging()). setup_system_domains()
is called before system P2M initialization on xen/arch/arm/setup.c, which
can't meet the requirement.

AFAIU, DOM_SHARED (or whatever you want to call it) will never run and the GFN will always be equal to the MFN. So it sounds to me that creating a P2M is a bit over the top. Instead, we should have a special case in get_page_from_gfn like we did for DOM_XEN.

Cheers,

--

Julien Grall