Re: [PATCH 03/10] xen/arm: introduce PGC_reserved

[Julien Grall <julien@xxxxxxx>]

On 20/05/2021 09:40, Penny Zheng wrote:
> Hi julien
Hi Penny,

> > -----Original Message-----
> > From: Penny Zheng
> > Sent: Thursday, May 20, 2021 2:20 PM
> > To: Julien Grall <julien@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > sstabellini@xxxxxxxxxx
> > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
> > <Wei.Chen@xxxxxxx>; nd <nd@xxxxxxx>
> > Subject: RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
> >
> > Hi Julien
> >
> > > -----Original Message-----
> > > From: Julien Grall <julien@xxxxxxx>
> > > Sent: Thursday, May 20, 2021 3:46 AM
> > > To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > > sstabellini@xxxxxxxxxx
> > > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
> > > <Wei.Chen@xxxxxxx>; nd <nd@xxxxxxx>
> > > Subject: Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
> > >
> > >
> > >
> > > On 19/05/2021 04:16, Penny Zheng wrote:
> > > > Hi Julien
> > > Hi Penny,
> > >
> > > > > -----Original Message-----
> > > > > From: Julien Grall <julien@xxxxxxx>
> > > > > Sent: Tuesday, May 18, 2021 5:46 PM
> > > > > To: Penny Zheng <Penny.Zheng@xxxxxxx>;
> > > > > xen-devel@xxxxxxxxxxxxxxxxxxxx; sstabellini@xxxxxxxxxx
> > > > > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; Wei Chen
> > > > > <Wei.Chen@xxxxxxx>; nd <nd@xxxxxxx>
> > > > > Subject: Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
> > > > >
> > > > >
> > > > >
> > > > > On 18/05/2021 06:21, Penny Zheng wrote:
> > > > > > In order to differentiate pages of static memory, from those
> > > > > > allocated from heap, this patch introduces a new page flag
> > > > > > PGC_reserved
> > > to tell.
> > > > > > New struct reserved in struct page_info is to describe reserved
> > > > > > page info, that is, which specific domain this page is reserved
> > > > > > to. > Helper page_get_reserved_owner and page_set_reserved_owner
> > > > > > are designated to get/set reserved page's owner.
> > > > > >
> > > > > > Struct domain is enlarged to more than PAGE_SIZE, due to
> > > > > > newly-imported struct reserved in struct page_info.
> > > > > struct domain may embed a pointer to a struct page_info but never
> > > > > directly embed the structure. So can you clarify what you mean?
> > > > >
> > > > > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > > > > > ---
> > > > > >     xen/include/asm-arm/mm.h | 16 +++++++++++++++-
> > > > > >     1 file changed, 15 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> > > > > index
> > > > > > 0b7de3102e..d8922fd5db 100644
> > > > > > --- a/xen/include/asm-arm/mm.h
> > > > > > +++ b/xen/include/asm-arm/mm.h
> > > > > > @@ -88,7 +88,15 @@ struct page_info
> > > > > >              */
> > > > > >             u32 tlbflush_timestamp;
> > > > > >         };
> > > > > > -    u64 pad;
> > > > > > +
> > > > > > +    /* Page is reserved. */
> > > > > > +    struct {
> > > > > > +        /*
> > > > > > +         * Reserved Owner of this page,
> > > > > > +         * if this page is reserved to a specific domain.
> > > > > > +         */
> > > > > > +        struct domain *domain;
> > > > > > +    } reserved;
> > > > > The space in page_info is quite tight, so I would like to avoid
> > > > > introducing new fields unless we can't get away from it.
> > > > >
> > > > > In this case, it is not clear why we need to differentiate the "Owner"
> > > > > vs the "Reserved Owner". It might be clearer if this change is
> > > > > folded in the first user of the field.
> > > > >
> > > > > As an aside, for 32-bit Arm, you need to add a 4-byte padding.
> > > > Yeah, I may delete this change. I imported this change as
> > > > considering the functionality of rebooting domain on static allocation.
> > > >
> > > > A little more discussion on rebooting domain on static allocation.
> > > > Considering the major user cases for domain on static allocation are
> > > > system has a total pre-defined, static behavior all the time. No
> > > > domain allocation on runtime, while there still exists domain rebooting.
> > > Hmmm... With this series it is still possible to allocate memory at
> > > runtime outside of the static allocation (see my comment on the design
> > document [1]).
> > > So is it meant to be complete?
> > I'm guessing that the "allocate memory at runtime outside of the static
> > allocation" is referring to XEN heap on static allocation, that is, users pre-
> > defining heap in device tree configuration to let the whole system more static
> > and predictable.
> >
> > And I've replied you in the design there, sorry for the late reply. Save your 
> > time,
> > and I’ll paste here:
> >
> > "Right now, on AArch64, all RAM, except reserved memory, will be finally
> > given to buddy allocator as heap,  like you said, guest RAM for normal domain
> > will be allocated from there, xmalloc eventually is get memory from there, etc.
> > So we want to refine the heap here, not iterating through `bootinfo.mem` to
> > set up XEN heap, but like iterating `bootinfo. reserved_heap` to set up XEN
> > heap.
> >
> > On ARM32, xen heap and domain heap are separately mapped, which is more
> > complicated here. That's why I only talking about implementing these features
> > on AArch64 as first step."
> >
> >   Above implementation will be delivered through another patch Serie. This
> > patch Serie Is only focusing on Domain on Static Allocation.
> Oh, Second thought on this.
> And I think you are referring to balloon in/out here, hmm, also, like
Yes I am referring to balloon in/out.

> I replied there:
> "For issues on ballooning out or in, it is not supported here.
So long you are not using the solution in prod then you are fine (see 
below)... But then we should make clear this feature is a tech preview.
> Domain on Static Allocation and 1:1 direct-map are all based on
> dom0-less right now, so no PV, grant table, event channel, etc, considered.
>
> Right now, it only supports device got passthrough into the guest."
So we are not creating the hypervisor node in the DT for dom0less domU. 
However, the hypercalls are still accessible by a domU if it really
wants to use them.

Therefore, a guest can easily mess up with your static configuration and 
predictability.
IMHO, this is a must to solve before "static memory" can be used in 
production.
Cheers,

--
Julien Grall