Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi Jan, On 20/05/2021 10:27, Jan Beulich wrote: On 20.05.2021 10:59, Julien Grall wrote: On 20/05/2021 09:40, Penny Zheng wrote: 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. I'm having trouble seeing why it can't be addressed right away: It can be solved right away. Dom0less domUs don't officially know they are running on Xen (they could bruteforce it though), so I think it would be fine to merge without it for a tech preview. Such guests can balloon in only after they've ballooned out some pages, and such balloon-in requests would be satisfied from the same static memory that is associated by the guest anyway. This would require some bookeeping to know the page was used previously. But nothing very challenging though. Cheers, -- Julien Grall
Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
On 20.05.2021 10:59, Julien Grall wrote: > On 20/05/2021 09:40, Penny Zheng wrote: >> 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. I'm having trouble seeing why it can't be addressed right away: Such guests can balloon in only after they've ballooned out some pages, and such balloon-in requests would be satisfied from the same static memory that is associated by the guest anyway. Jan
Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
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 ; xen-devel@lists.xenproject.org;
sstabell...@kernel.org
Cc: Bertrand Marquis ; Wei Chen
; nd
Subject: RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi Julien
-Original Message-
From: Julien Grall
Sent: Thursday, May 20, 2021 3:46 AM
To: Penny Zheng ; xen-devel@lists.xenproject.org;
sstabell...@kernel.org
Cc: Bertrand Marquis ; Wei Chen
; nd
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
Sent: Tuesday, May 18, 2021 5:46 PM
To: Penny Zheng ;
xen-devel@lists.xenproject.org; sstabell...@kernel.org
Cc: Bertrand Marquis ; Wei Chen
; nd
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
---
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
RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi julien
> -Original Message-
> From: Penny Zheng
> Sent: Thursday, May 20, 2021 2:20 PM
> To: Julien Grall ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> Subject: RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
>
> Hi Julien
>
> > -Original Message-
> > From: Julien Grall
> > Sent: Thursday, May 20, 2021 3:46 AM
> > To: Penny Zheng ; xen-devel@lists.xenproject.org;
> > sstabell...@kernel.org
> > Cc: Bertrand Marquis ; Wei Chen
> > ; nd
> > 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
> > >> Sent: Tuesday, May 18, 2021 5:46 PM
> > >> To: Penny Zheng ;
> > >> xen-devel@lists.xenproject.org; sstabell...@kernel.org
> > >> Cc: Bertrand Marquis ; Wei Chen
> > >> ; nd
> > >> 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
> > >>> ---
> > >>>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 hea
Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
On 19.05.2021 21:49, Julien Grall wrote:
> On 19/05/2021 10:49, Jan Beulich wrote:
>> On 19.05.2021 05:16, Penny Zheng wrote:
From: Julien Grall
Sent: Tuesday, May 18, 2021 5:46 PM
On 18/05/2021 06:21, Penny Zheng wrote:
> --- 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.
>>>
>>> And when rebooting domain on static allocation, all these reserved pages
>>> could
>>> not go back to heap when freeing them. So I am considering to use one
>>> global
>>> `struct page_info*[DOMID]` value to store.
>>
>> Except such a separate array will consume quite a bit of space for
>> no real gain: v.free has 32 bits of padding space right now on
>> Arm64, so there's room for a domid_t there already. Even on Arm32
>> this could be arranged for, as I doubt "order" needs to be 32 bits
>> wide.
>
> I agree we shouldn't need 32-bit to cover the "order". Although, I would
> like to see any user reading the field before introducing it.
Of course, but I thought the plan was to mark static pages with their
designated domid, which would happen prior to domain creation. The
reader of the field would then naturally appear during domain creation.
Jan
RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Thursday, May 20, 2021 3:46 AM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> 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
> >> Sent: Tuesday, May 18, 2021 5:46 PM
> >> To: Penny Zheng ;
> >> xen-devel@lists.xenproject.org; sstabell...@kernel.org
> >> Cc: Bertrand Marquis ; Wei Chen
> >> ; nd
> >> 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
> >>> ---
> >>>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 i
Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi Jan,
On 19/05/2021 10:49, Jan Beulich wrote:
On 19.05.2021 05:16, Penny Zheng wrote:
From: Julien Grall
Sent: Tuesday, May 18, 2021 5:46 PM
On 18/05/2021 06:21, Penny Zheng wrote:
--- 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.
And when rebooting domain on static allocation, all these reserved pages could
not go back to heap when freeing them. So I am considering to use one global
`struct page_info*[DOMID]` value to store.
Except such a separate array will consume quite a bit of space for
no real gain: v.free has 32 bits of padding space right now on
Arm64, so there's room for a domid_t there already. Even on Arm32
this could be arranged for, as I doubt "order" needs to be 32 bits
wide.
I agree we shouldn't need 32-bit to cover the "order". Although, I would
like to see any user reading the field before introducing it.
Cheers,
--
Julien Grall
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
Sent: Tuesday, May 18, 2021 5:46 PM
To: Penny Zheng ; xen-devel@lists.xenproject.org;
sstabell...@kernel.org
Cc: Bertrand Marquis ; Wei Chen
; nd
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
---
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?
And when rebooting domain on static allocation, all these reserved pages could
not go back to heap when freeing them. So I am considering to use one global
`struct page_info*[DOMID]` value to store.
As Jan suggested, when domain get rebooted, struct domain will not exist anymore.
But I think DOMID info could last.
You would need to make sure the domid to be reserved. But I am not
entirely convinced this is necessary here.
When recreating the domain, you need a way to know its configuration.
Mostly likely this will come from the Device-Tree. At which point, you
can also find the static region from there.
Cheers,
[1] <7ab73cb0-39d5-f8bf-660f-b3d77f324...@xen.org>
--
Julien Grall
Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
On 19.05.2021 05:16, Penny Zheng wrote:
>> From: Julien Grall
>> Sent: Tuesday, May 18, 2021 5:46 PM
>>
>> On 18/05/2021 06:21, Penny Zheng wrote:
>>> --- 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.
>
> And when rebooting domain on static allocation, all these reserved pages could
> not go back to heap when freeing them. So I am considering to use one global
> `struct page_info*[DOMID]` value to store.
Except such a separate array will consume quite a bit of space for
no real gain: v.free has 32 bits of padding space right now on
Arm64, so there's room for a domid_t there already. Even on Arm32
this could be arranged for, as I doubt "order" needs to be 32 bits
wide.
Jan
RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Tuesday, May 18, 2021 5:46 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> 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
> > ---
> > 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.
And when rebooting domain on static allocation, all these reserved pages could
not go back to heap when freeing them. So I am considering to use one global
`struct page_info*[DOMID]` value to store.
As Jan suggested, when domain get rebooted, struct domain will not exist
anymore.
But I think DOMID info could last.
> > };
> >
> > #define PG_shift(idx) (BITS_PER_LONG - (idx))
> > @@ -108,6 +116,9 @@ struct page_info
> > /* Page is Xen heap? */
> > #define _PGC_xen_heap PG_shift(2)
> > #define PGC_xen_heap PG_mask(1, 2)
> > + /* Page is reserved, referring static memory */
>
> I would drop the second part of the sentence because the flag could be used
> for other purpose. One example is reserved memory when Live Updating.
>
Sure, I will drop it.
> > +#define _PGC_reserved PG_shift(3)
> > +#define PGC_reserved PG_mask(1, 3)
> > /* ... */
> > /* Page is broken? */
> > #define _PGC_broken PG_shift(7)
> > @@ -161,6 +172,9 @@ extern unsigned long xenheap_base_pdx;
> > #define page_get_owner(_p)(_p)->v.inuse.domain
> > #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
> >
> > +#define page_get_reserved_owner(_p)(_p)->reserved.domain
> > +#define page_set_reserved_owner(_p,_d) ((_p)->reserved.domain = (_d))
> > +
> > #define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma
> >
> > #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
> >
>
> Cheers,
>
> --
> Julien Grall
Cheers,
Penny Zheng
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
---
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.
};
#define PG_shift(idx) (BITS_PER_LONG - (idx))
@@ -108,6 +116,9 @@ struct page_info
/* Page is Xen heap? */
#define _PGC_xen_heap PG_shift(2)
#define PGC_xen_heap PG_mask(1, 2)
+ /* Page is reserved, referring static memory */
I would drop the second part of the sentence because the flag could be
used for other purpose. One example is reserved memory when Live Updating.
+#define _PGC_reserved PG_shift(3)
+#define PGC_reserved PG_mask(1, 3)
/* ... */
/* Page is broken? */
#define _PGC_broken PG_shift(7)
@@ -161,6 +172,9 @@ extern unsigned long xenheap_base_pdx;
#define page_get_owner(_p)(_p)->v.inuse.domain
#define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
+#define page_get_reserved_owner(_p)(_p)->reserved.domain
+#define page_set_reserved_owner(_p,_d) ((_p)->reserved.domain = (_d))
+
#define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma
#define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
Cheers,
--
Julien Grall
