Re: [PATCH 04/10] xen/arm: static memory initialization
On 20.05.2021 11:04, Penny Zheng wrote: > Hi Jan > >> -Original Message- >> From: Jan Beulich >> Sent: Tuesday, May 18, 2021 6:43 PM >> To: Penny Zheng >> Cc: Bertrand Marquis ; Wei Chen >> ; nd ; xen-devel@lists.xenproject.org; >> sstabell...@kernel.org; jul...@xen.org >> Subject: Re: [PATCH 04/10] xen/arm: static memory initialization >> >> On 18.05.2021 11:51, Penny Zheng wrote: >>>> From: Jan Beulich >>>> Sent: Tuesday, May 18, 2021 3:16 PM >>>> >>>> On 18.05.2021 07:21, Penny Zheng wrote: >>>>> This patch introduces static memory initialization, during system >>>>> RAM boot >>>> up. >>>>> >>>>> New func init_staticmem_pages is the equivalent of init_heap_pages, >>>>> responsible for static memory initialization. >>>>> >>>>> Helper func free_staticmem_pages is the equivalent of >>>>> free_heap_pages, to free nr_pfns pages of static memory. >>>>> For each page, it includes the following steps: >>>>> 1. change page state from in-use(also initialization state) to free >>>>> state and grant PGC_reserved. >>>>> 2. set its owner NULL and make sure this page is not a guest frame >>>>> any more >>>> >>>> But isn't the goal (as per the previous patch) to associate such >>>> pages with a _specific_ domain? >>>> >>> >>> Free_staticmem_pages are alike free_heap_pages, are not used only for >> initialization. >>> Freeing used pages to unused are also included. >>> Here, setting its owner NULL is to set owner in used field NULL. >> >> I'm afraid I still don't understand. >> > > When initializing heap, xen is using free_heap_pages to do the initialization. > And also when normal domain get destroyed/rebooted, xen is using > free_domheap_pages, > which calls free_heap_pages to free the pages. > > So here, since free_staticmem_pages is the equivalent of the free_heap_pages > for static > memory, I'm also considering both two scenarios here. And if it is domain get > destroyed/rebooted, > Page state is in inuse state(PGC_inuse), and the page_info.v.inuse.domain is > holding the > domain owner. > When freeing then, we need to switch the page state to free state(PGC_free) > and set its inuse owner to NULL. Perhaps my confusion comes from your earlier outline missing 3. re-associate the page with the domain (as represented in free pages) The property of "designated for Dom" should never go away, if I understand the overall proposal correctly. Jan
RE: [PATCH 04/10] xen/arm: static memory initialization
Hi Jan > -Original Message- > From: Jan Beulich > Sent: Tuesday, May 18, 2021 6:43 PM > To: Penny Zheng > Cc: Bertrand Marquis ; Wei Chen > ; nd ; xen-devel@lists.xenproject.org; > sstabell...@kernel.org; jul...@xen.org > Subject: Re: [PATCH 04/10] xen/arm: static memory initialization > > On 18.05.2021 11:51, Penny Zheng wrote: > >> From: Jan Beulich > >> Sent: Tuesday, May 18, 2021 3:16 PM > >> > >> On 18.05.2021 07:21, Penny Zheng wrote: > >>> This patch introduces static memory initialization, during system > >>> RAM boot > >> up. > >>> > >>> New func init_staticmem_pages is the equivalent of init_heap_pages, > >>> responsible for static memory initialization. > >>> > >>> Helper func free_staticmem_pages is the equivalent of > >>> free_heap_pages, to free nr_pfns pages of static memory. > >>> For each page, it includes the following steps: > >>> 1. change page state from in-use(also initialization state) to free > >>> state and grant PGC_reserved. > >>> 2. set its owner NULL and make sure this page is not a guest frame > >>> any more > >> > >> But isn't the goal (as per the previous patch) to associate such > >> pages with a _specific_ domain? > >> > > > > Free_staticmem_pages are alike free_heap_pages, are not used only for > initialization. > > Freeing used pages to unused are also included. > > Here, setting its owner NULL is to set owner in used field NULL. > > I'm afraid I still don't understand. > When initializing heap, xen is using free_heap_pages to do the initialization. And also when normal domain get destroyed/rebooted, xen is using free_domheap_pages, which calls free_heap_pages to free the pages. So here, since free_staticmem_pages is the equivalent of the free_heap_pages for static memory, I'm also considering both two scenarios here. And if it is domain get destroyed/rebooted, Page state is in inuse state(PGC_inuse), and the page_info.v.inuse.domain is holding the domain owner. When freeing then, we need to switch the page state to free state(PGC_free) and set its inuse owner to NULL. I'll clarify it more clearly in commit message. > > Still, I need to add more explanation here. > > Yes please. > > >>> @@ -1512,6 +1515,49 @@ static void free_heap_pages( > >>> spin_unlock(_lock); > >>> } > >>> > >>> +/* Equivalent of free_heap_pages to free nr_pfns pages of static > >>> +memory. */ static void free_staticmem_pages(struct page_info *pg, > >> unsigned long nr_pfns, > >>> + bool need_scrub) > >> > >> Right now this function gets called only from an __init one. Unless > >> it is intended to gain further callers, it should be marked __init itself > >> then. > >> Otherwise it should be made sure that other architectures don't > >> include this (dead there) code. > >> > > > > Sure, I'll add __init. Thx. > > Didn't I see a 2nd call to the function later in the series? That one doesn't > permit the function to be __init, iirc. > > Jan Cheers Penny
RE: [PATCH 04/10] xen/arm: static memory initialization
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Tuesday, May 18, 2021 6:01 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> Subject: Re: [PATCH 04/10] xen/arm: static memory initialization
>
> Hi Penny,
>
> On 18/05/2021 06:21, Penny Zheng wrote:
> > This patch introduces static memory initialization, during system RAM boot
> up.
> >
> > New func init_staticmem_pages is the equivalent of init_heap_pages,
> > responsible for static memory initialization.
> >
> > Helper func free_staticmem_pages is the equivalent of free_heap_pages,
> > to free nr_pfns pages of static memory.
> > For each page, it includes the following steps:
> > 1. change page state from in-use(also initialization state) to free
> > state and grant PGC_reserved.
> > 2. set its owner NULL and make sure this page is not a guest frame any
> > more 3. follow the same cache coherency policy in free_heap_pages 4.
> > scrub the page in need
> >
> > Signed-off-by: Penny Zheng
> > ---
> > xen/arch/arm/setup.c| 2 ++
> > xen/common/page_alloc.c | 70
> +
> > xen/include/xen/mm.h| 3 ++
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index
> > 444dbbd676..f80162c478 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -818,6 +818,8 @@ static void __init setup_mm(void)
> >
> > setup_frametable_mappings(ram_start, ram_end);
> > max_page = PFN_DOWN(ram_end);
> > +
> > +init_staticmem_pages();
> > }
> > #endif
> >
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > ace6333c18..58b53c6ac2 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -150,6 +150,9 @@
> > #define p2m_pod_offline_or_broken_hit(pg) 0
> > #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> > #endif
> > +#ifdef CONFIG_ARM_64
> > +#include
> > +#endif
> >
> > /*
> >* Comma-separated list of hexadecimal page numbers containing bad
> bytes.
> > @@ -1512,6 +1515,49 @@ static void free_heap_pages(
> > spin_unlock(_lock);
> > }
> >
> > +/* Equivalent of free_heap_pages to free nr_pfns pages of static
> > +memory. */ static void free_staticmem_pages(struct page_info *pg,
> > +unsigned long nr_pfns,
>
> This function is dealing with MFNs, so the second parameter should be called
> nr_mfns.
>
Agreed, thx.
> > + bool need_scrub) {
> > +mfn_t mfn = page_to_mfn(pg);
> > +int i;
> > +
> > +for ( i = 0; i < nr_pfns; i++ )
> > +{
> > +switch ( pg[i].count_info & PGC_state )
> > +{
> > +case PGC_state_inuse:
> > +BUG_ON(pg[i].count_info & PGC_broken);
> > +/* Make it free and reserved. */
> > +pg[i].count_info = PGC_state_free | PGC_reserved;
> > +break;
> > +
> > +default:
> > +printk(XENLOG_ERR
> > + "Page state shall be only in PGC_state_inuse. "
> > + "pg[%u] MFN %"PRI_mfn" count_info=%#lx
> tlbflush_timestamp=%#x.\n",
> > + i, mfn_x(mfn) + i,
> > + pg[i].count_info,
> > + pg[i].tlbflush_timestamp);
> > +BUG();
> > +}
> > +
> > +/*
> > + * Follow the same cache coherence scheme in free_heap_pages.
> > + * If a page has no owner it will need no safety TLB flush.
> > + */
> > +pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
> > +if ( pg[i].u.free.need_tlbflush )
> > +page_set_tlbflush_timestamp([i]);
> > +
> > +/* This page is not a guest frame any more. */
> > +page_set_owner([i], NULL);
> > +set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
>
> The code looks quite similar to free_heap_pages(). Could we possibly create
> an helper which can be called from both?
>
Ok, I will extract the common code here and there.
> > +
> > +if ( need_scrub )
> > +scrub_one_page([i]);
>
> So the scrubbing will be synchronous. Is it what we want?
>
> You also seem to miss the flush the call to flush_page_to_ram().
>
Yea
Re: [PATCH 04/10] xen/arm: static memory initialization
On 18.05.2021 11:51, Penny Zheng wrote: >> From: Jan Beulich >> Sent: Tuesday, May 18, 2021 3:16 PM >> >> On 18.05.2021 07:21, Penny Zheng wrote: >>> This patch introduces static memory initialization, during system RAM boot >> up. >>> >>> New func init_staticmem_pages is the equivalent of init_heap_pages, >>> responsible for static memory initialization. >>> >>> Helper func free_staticmem_pages is the equivalent of free_heap_pages, >>> to free nr_pfns pages of static memory. >>> For each page, it includes the following steps: >>> 1. change page state from in-use(also initialization state) to free >>> state and grant PGC_reserved. >>> 2. set its owner NULL and make sure this page is not a guest frame any >>> more >> >> But isn't the goal (as per the previous patch) to associate such pages with a >> _specific_ domain? >> > > Free_staticmem_pages are alike free_heap_pages, are not used only for > initialization. > Freeing used pages to unused are also included. > Here, setting its owner NULL is to set owner in used field NULL. I'm afraid I still don't understand. > Still, I need to add more explanation here. Yes please. >>> @@ -1512,6 +1515,49 @@ static void free_heap_pages( >>> spin_unlock(_lock); >>> } >>> >>> +/* Equivalent of free_heap_pages to free nr_pfns pages of static >>> +memory. */ static void free_staticmem_pages(struct page_info *pg, >> unsigned long nr_pfns, >>> + bool need_scrub) >> >> Right now this function gets called only from an __init one. Unless it is >> intended to gain further callers, it should be marked __init itself then. >> Otherwise it should be made sure that other architectures don't include this >> (dead there) code. >> > > Sure, I'll add __init. Thx. Didn't I see a 2nd call to the function later in the series? That one doesn't permit the function to be __init, iirc. Jan
Re: [PATCH 04/10] xen/arm: static memory initialization
On 18/05/2021 11:00, Julien Grall wrote:
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
This patch introduces static memory initialization, during system RAM
boot up.
New func init_staticmem_pages is the equivalent of init_heap_pages,
responsible
for static memory initialization.
Helper func free_staticmem_pages is the equivalent of free_heap_pages,
to free
nr_pfns pages of static memory.
For each page, it includes the following steps:
1. change page state from in-use(also initialization state) to free
state and
grant PGC_reserved.
2. set its owner NULL and make sure this page is not a guest frame any
more
3. follow the same cache coherency policy in free_heap_pages
4. scrub the page in need
Signed-off-by: Penny Zheng
---
xen/arch/arm/setup.c | 2 ++
xen/common/page_alloc.c | 70 +
xen/include/xen/mm.h | 3 ++
3 files changed, 75 insertions(+)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 444dbbd676..f80162c478 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -818,6 +818,8 @@ static void __init setup_mm(void)
setup_frametable_mappings(ram_start, ram_end);
max_page = PFN_DOWN(ram_end);
+
+ init_staticmem_pages();
}
#endif
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ace6333c18..58b53c6ac2 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -150,6 +150,9 @@
#define p2m_pod_offline_or_broken_hit(pg) 0
#define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
#endif
+#ifdef CONFIG_ARM_64
+#include
+#endif
/*
* Comma-separated list of hexadecimal page numbers containing bad
bytes.
@@ -1512,6 +1515,49 @@ static void free_heap_pages(
spin_unlock(_lock);
}
+/* Equivalent of free_heap_pages to free nr_pfns pages of static
memory. */
+static void free_staticmem_pages(struct page_info *pg, unsigned long
nr_pfns,
This function is dealing with MFNs, so the second parameter should be
called nr_mfns.
+ bool need_scrub)
+{
+ mfn_t mfn = page_to_mfn(pg);
+ int i;
+
+ for ( i = 0; i < nr_pfns; i++ )
+ {
+ switch ( pg[i].count_info & PGC_state )
+ {
+ case PGC_state_inuse:
+ BUG_ON(pg[i].count_info & PGC_broken);
+ /* Make it free and reserved. */
+ pg[i].count_info = PGC_state_free | PGC_reserved;
+ break;
+
+ default:
+ printk(XENLOG_ERR
+ "Page state shall be only in PGC_state_inuse. "
+ "pg[%u] MFN %"PRI_mfn" count_info=%#lx
tlbflush_timestamp=%#x.\n",
+ i, mfn_x(mfn) + i,
+ pg[i].count_info,
+ pg[i].tlbflush_timestamp);
+ BUG();
+ }
+
+ /*
+ * Follow the same cache coherence scheme in free_heap_pages.
+ * If a page has no owner it will need no safety TLB flush.
+ */
+ pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
+ if ( pg[i].u.free.need_tlbflush )
+ page_set_tlbflush_timestamp([i]);
+
+ /* This page is not a guest frame any more. */
+ page_set_owner([i], NULL);
+ set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
The code looks quite similar to free_heap_pages(). Could we possibly
create an helper which can be called from both?
+
+ if ( need_scrub )
+ scrub_one_page([i]);
So the scrubbing will be synchronous. Is it what we want?
You also seem to miss the flush the call to flush_page_to_ram().
H... Sorry I looked at the wrong function. This is not necessary for
the free part.
+ }
+}
/*
* Following rules applied for page offline:
@@ -1828,6 +1874,30 @@ static void init_heap_pages(
}
}
+/* Equivalent of init_heap_pages to do static memory initialization */
+void __init init_staticmem_pages(void)
+{
+ int bank;
+
+ /*
+ * TODO: Considering NUMA-support scenario.
+ */
+ for ( bank = 0 ; bank < bootinfo.static_mem.nr_banks; bank++ )
bootinfo is arm specific, so this code should live in arch/arm rather
than common/.
+ {
+ paddr_t bank_start = bootinfo.static_mem.bank[bank].start;
+ paddr_t bank_size = bootinfo.static_mem.bank[bank].size;
+ paddr_t bank_end = bank_start + bank_size;
+
+ bank_start = round_pgup(bank_start);
+ bank_end = round_pgdown(bank_end);
+ if ( bank_end <= bank_start )
+ return;
+
+ free_staticmem_pages(maddr_to_page(bank_start),
+ (bank_end - bank_start) >> PAGE_SHIFT,
false);
+ }
+}
+
static unsigned long avail_heap_pages(
unsigned int zone_lo, unsigned int zone_hi, unsigned int node)
{
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 667f9dac83..8b1a2207b2 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,6 +85,9 @@ bool scrub_free_pages(void);
} while ( false
Re: [PATCH 04/10] xen/arm: static memory initialization
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
This patch introduces static memory initialization, during system RAM boot up.
New func init_staticmem_pages is the equivalent of init_heap_pages, responsible
for static memory initialization.
Helper func free_staticmem_pages is the equivalent of free_heap_pages, to free
nr_pfns pages of static memory.
For each page, it includes the following steps:
1. change page state from in-use(also initialization state) to free state and
grant PGC_reserved.
2. set its owner NULL and make sure this page is not a guest frame any more
3. follow the same cache coherency policy in free_heap_pages
4. scrub the page in need
Signed-off-by: Penny Zheng
---
xen/arch/arm/setup.c| 2 ++
xen/common/page_alloc.c | 70 +
xen/include/xen/mm.h| 3 ++
3 files changed, 75 insertions(+)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 444dbbd676..f80162c478 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -818,6 +818,8 @@ static void __init setup_mm(void)
setup_frametable_mappings(ram_start, ram_end);
max_page = PFN_DOWN(ram_end);
+
+init_staticmem_pages();
}
#endif
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ace6333c18..58b53c6ac2 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -150,6 +150,9 @@
#define p2m_pod_offline_or_broken_hit(pg) 0
#define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
#endif
+#ifdef CONFIG_ARM_64
+#include
+#endif
/*
* Comma-separated list of hexadecimal page numbers containing bad bytes.
@@ -1512,6 +1515,49 @@ static void free_heap_pages(
spin_unlock(_lock);
}
+/* Equivalent of free_heap_pages to free nr_pfns pages of static memory. */
+static void free_staticmem_pages(struct page_info *pg, unsigned long nr_pfns,
This function is dealing with MFNs, so the second parameter should be
called nr_mfns.
+ bool need_scrub)
+{
+mfn_t mfn = page_to_mfn(pg);
+int i;
+
+for ( i = 0; i < nr_pfns; i++ )
+{
+switch ( pg[i].count_info & PGC_state )
+{
+case PGC_state_inuse:
+BUG_ON(pg[i].count_info & PGC_broken);
+/* Make it free and reserved. */
+pg[i].count_info = PGC_state_free | PGC_reserved;
+break;
+
+default:
+printk(XENLOG_ERR
+ "Page state shall be only in PGC_state_inuse. "
+ "pg[%u] MFN %"PRI_mfn" count_info=%#lx
tlbflush_timestamp=%#x.\n",
+ i, mfn_x(mfn) + i,
+ pg[i].count_info,
+ pg[i].tlbflush_timestamp);
+BUG();
+}
+
+/*
+ * Follow the same cache coherence scheme in free_heap_pages.
+ * If a page has no owner it will need no safety TLB flush.
+ */
+pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
+if ( pg[i].u.free.need_tlbflush )
+page_set_tlbflush_timestamp([i]);
+
+/* This page is not a guest frame any more. */
+page_set_owner([i], NULL);
+set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
The code looks quite similar to free_heap_pages(). Could we possibly
create an helper which can be called from both?
+
+if ( need_scrub )
+scrub_one_page([i]);
So the scrubbing will be synchronous. Is it what we want?
You also seem to miss the flush the call to flush_page_to_ram().
+}
+}
/*
* Following rules applied for page offline:
@@ -1828,6 +1874,30 @@ static void init_heap_pages(
}
}
+/* Equivalent of init_heap_pages to do static memory initialization */
+void __init init_staticmem_pages(void)
+{
+int bank;
+
+/*
+ * TODO: Considering NUMA-support scenario.
+ */
+for ( bank = 0 ; bank < bootinfo.static_mem.nr_banks; bank++ )
bootinfo is arm specific, so this code should live in arch/arm rather
than common/.
+{
+paddr_t bank_start = bootinfo.static_mem.bank[bank].start;
+paddr_t bank_size = bootinfo.static_mem.bank[bank].size;
+paddr_t bank_end = bank_start + bank_size;
+
+bank_start = round_pgup(bank_start);
+bank_end = round_pgdown(bank_end);
+if ( bank_end <= bank_start )
+return;
+
+free_staticmem_pages(maddr_to_page(bank_start),
+(bank_end - bank_start) >> PAGE_SHIFT, false);
+}
+}
+
static unsigned long avail_heap_pages(
unsigned int zone_lo, unsigned int zone_hi, unsigned int node)
{
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 667f9dac83..8b1a2207b2 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,6 +85,9 @@ bool scrub_free_pages(void);
} while ( false )
#define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
+/* Static Memory */
+void init_staticmem_pages(void);
+
/* Map
RE: [PATCH 04/10] xen/arm: static memory initialization
Hi Jan
> -Original Message-
> From: Jan Beulich
> Sent: Tuesday, May 18, 2021 3:16 PM
> To: Penny Zheng
> Cc: Bertrand Marquis ; Wei Chen
> ; nd ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jul...@xen.org
> Subject: Re: [PATCH 04/10] xen/arm: static memory initialization
>
> On 18.05.2021 07:21, Penny Zheng wrote:
> > This patch introduces static memory initialization, during system RAM boot
> up.
> >
> > New func init_staticmem_pages is the equivalent of init_heap_pages,
> > responsible for static memory initialization.
> >
> > Helper func free_staticmem_pages is the equivalent of free_heap_pages,
> > to free nr_pfns pages of static memory.
> > For each page, it includes the following steps:
> > 1. change page state from in-use(also initialization state) to free
> > state and grant PGC_reserved.
> > 2. set its owner NULL and make sure this page is not a guest frame any
> > more
>
> But isn't the goal (as per the previous patch) to associate such pages with a
> _specific_ domain?
>
Free_staticmem_pages are alike free_heap_pages, are not used only for
initialization.
Freeing used pages to unused are also included.
Here, setting its owner NULL is to set owner in used field NULL.
Still, I need to add more explanation here.
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -150,6 +150,9 @@
> > #define p2m_pod_offline_or_broken_hit(pg) 0 #define
> > p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL) #endif
> > +#ifdef CONFIG_ARM_64
> > +#include
> > +#endif
>
> Whatever it is that's needed from this header suggests the code won't build
> for other architectures. I think init_staticmem_pages() in its current shape
> shouldn't live in this (common) file.
>
Yes, I should include them all both under one specific config, maybe like
CONFIG_STATIC_MEM, and this config is arm-specific.
> > @@ -1512,6 +1515,49 @@ static void free_heap_pages(
> > spin_unlock(_lock);
> > }
> >
> > +/* Equivalent of free_heap_pages to free nr_pfns pages of static
> > +memory. */ static void free_staticmem_pages(struct page_info *pg,
> unsigned long nr_pfns,
> > + bool need_scrub)
>
> Right now this function gets called only from an __init one. Unless it is
> intended to gain further callers, it should be marked __init itself then.
> Otherwise it should be made sure that other architectures don't include this
> (dead there) code.
>
Sure, I'll add __init. Thx.
> > +{
> > +mfn_t mfn = page_to_mfn(pg);
> > +int i;
>
> This type doesn't fit nr_pfns'es.
>
Sure, nr_mfns is better in also many other places.
> Jan
Re: [PATCH 04/10] xen/arm: static memory initialization
On 18.05.2021 07:21, Penny Zheng wrote:
> This patch introduces static memory initialization, during system RAM boot up.
>
> New func init_staticmem_pages is the equivalent of init_heap_pages,
> responsible
> for static memory initialization.
>
> Helper func free_staticmem_pages is the equivalent of free_heap_pages, to free
> nr_pfns pages of static memory.
> For each page, it includes the following steps:
> 1. change page state from in-use(also initialization state) to free state and
> grant PGC_reserved.
> 2. set its owner NULL and make sure this page is not a guest frame any more
But isn't the goal (as per the previous patch) to associate such pages
with a _specific_ domain?
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -150,6 +150,9 @@
> #define p2m_pod_offline_or_broken_hit(pg) 0
> #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> #endif
> +#ifdef CONFIG_ARM_64
> +#include
> +#endif
Whatever it is that's needed from this header suggests the code won't
build for other architectures. I think init_staticmem_pages() in its
current shape shouldn't live in this (common) file.
> @@ -1512,6 +1515,49 @@ static void free_heap_pages(
> spin_unlock(_lock);
> }
>
> +/* Equivalent of free_heap_pages to free nr_pfns pages of static memory. */
> +static void free_staticmem_pages(struct page_info *pg, unsigned long nr_pfns,
> + bool need_scrub)
Right now this function gets called only from an __init one. Unless
it is intended to gain further callers, it should be marked __init
itself then. Otherwise it should be made sure that other
architectures don't include this (dead there) code.
> +{
> +mfn_t mfn = page_to_mfn(pg);
> +int i;
This type doesn't fit nr_pfns'es.
Jan
[PATCH 04/10] xen/arm: static memory initialization
This patch introduces static memory initialization, during system RAM boot up.
New func init_staticmem_pages is the equivalent of init_heap_pages, responsible
for static memory initialization.
Helper func free_staticmem_pages is the equivalent of free_heap_pages, to free
nr_pfns pages of static memory.
For each page, it includes the following steps:
1. change page state from in-use(also initialization state) to free state and
grant PGC_reserved.
2. set its owner NULL and make sure this page is not a guest frame any more
3. follow the same cache coherency policy in free_heap_pages
4. scrub the page in need
Signed-off-by: Penny Zheng
---
xen/arch/arm/setup.c| 2 ++
xen/common/page_alloc.c | 70 +
xen/include/xen/mm.h| 3 ++
3 files changed, 75 insertions(+)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 444dbbd676..f80162c478 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -818,6 +818,8 @@ static void __init setup_mm(void)
setup_frametable_mappings(ram_start, ram_end);
max_page = PFN_DOWN(ram_end);
+
+init_staticmem_pages();
}
#endif
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ace6333c18..58b53c6ac2 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -150,6 +150,9 @@
#define p2m_pod_offline_or_broken_hit(pg) 0
#define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
#endif
+#ifdef CONFIG_ARM_64
+#include
+#endif
/*
* Comma-separated list of hexadecimal page numbers containing bad bytes.
@@ -1512,6 +1515,49 @@ static void free_heap_pages(
spin_unlock(_lock);
}
+/* Equivalent of free_heap_pages to free nr_pfns pages of static memory. */
+static void free_staticmem_pages(struct page_info *pg, unsigned long nr_pfns,
+ bool need_scrub)
+{
+mfn_t mfn = page_to_mfn(pg);
+int i;
+
+for ( i = 0; i < nr_pfns; i++ )
+{
+switch ( pg[i].count_info & PGC_state )
+{
+case PGC_state_inuse:
+BUG_ON(pg[i].count_info & PGC_broken);
+/* Make it free and reserved. */
+pg[i].count_info = PGC_state_free | PGC_reserved;
+break;
+
+default:
+printk(XENLOG_ERR
+ "Page state shall be only in PGC_state_inuse. "
+ "pg[%u] MFN %"PRI_mfn" count_info=%#lx
tlbflush_timestamp=%#x.\n",
+ i, mfn_x(mfn) + i,
+ pg[i].count_info,
+ pg[i].tlbflush_timestamp);
+BUG();
+}
+
+/*
+ * Follow the same cache coherence scheme in free_heap_pages.
+ * If a page has no owner it will need no safety TLB flush.
+ */
+pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
+if ( pg[i].u.free.need_tlbflush )
+page_set_tlbflush_timestamp([i]);
+
+/* This page is not a guest frame any more. */
+page_set_owner([i], NULL);
+set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
+
+if ( need_scrub )
+scrub_one_page([i]);
+}
+}
/*
* Following rules applied for page offline:
@@ -1828,6 +1874,30 @@ static void init_heap_pages(
}
}
+/* Equivalent of init_heap_pages to do static memory initialization */
+void __init init_staticmem_pages(void)
+{
+int bank;
+
+/*
+ * TODO: Considering NUMA-support scenario.
+ */
+for ( bank = 0 ; bank < bootinfo.static_mem.nr_banks; bank++ )
+{
+paddr_t bank_start = bootinfo.static_mem.bank[bank].start;
+paddr_t bank_size = bootinfo.static_mem.bank[bank].size;
+paddr_t bank_end = bank_start + bank_size;
+
+bank_start = round_pgup(bank_start);
+bank_end = round_pgdown(bank_end);
+if ( bank_end <= bank_start )
+return;
+
+free_staticmem_pages(maddr_to_page(bank_start),
+(bank_end - bank_start) >> PAGE_SHIFT, false);
+}
+}
+
static unsigned long avail_heap_pages(
unsigned int zone_lo, unsigned int zone_hi, unsigned int node)
{
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 667f9dac83..8b1a2207b2 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,6 +85,9 @@ bool scrub_free_pages(void);
} while ( false )
#define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
+/* Static Memory */
+void init_staticmem_pages(void);
+
/* Map machine page range in Xen virtual address space. */
int map_pages_to_xen(
unsigned long virt,
--
2.25.1
