[PATCH] linux/x86-64: allow kernel init memory to be freed

[PATCH] linux/x86-64: allow kernel init memory to be freed

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: head-2007-02-08/arch/x86_64/mm/init-xen.c
===================================================================
--- head-2007-02-08.orig/arch/x86_64/mm/init-xen.c	2007-02-27 15:00:16.000000000 +0100
+++ head-2007-02-08/arch/x86_64/mm/init-xen.c	2007-02-27 15:00:42.000000000 +0100
@@ -102,14 +102,11 @@ static void __meminit early_make_page_re
 		BUG();
 }
 
-void make_page_readonly(void *va, unsigned int feature)
+static void __make_page_readonly(void *va)
 {
 	pgd_t *pgd; pud_t *pud; pmd_t *pmd; pte_t pte, *ptep;
 	unsigned long addr = (unsigned long) va;
 
-	if (xen_feature(feature))
-		return;
-
 	pgd = pgd_offset_k(addr);
 	pud = pud_offset(pgd, addr);
 	pmd = pmd_offset(pud, addr);
@@ -120,17 +117,14 @@ void make_page_readonly(void *va, unsign
 		xen_l1_entry_update(ptep, pte); /* fallback */
 
 	if ((addr >= VMALLOC_START) && (addr < VMALLOC_END))
-		make_page_readonly(__va(pte_pfn(pte) << PAGE_SHIFT), feature);
+		__make_page_readonly(__va(pte_pfn(pte) << PAGE_SHIFT));
 }
 
-void make_page_writable(void *va, unsigned int feature)
+static void __make_page_writable(void *va)
 {
 	pgd_t *pgd; pud_t *pud; pmd_t *pmd; pte_t pte, *ptep;
 	unsigned long addr = (unsigned long) va;
 
-	if (xen_feature(feature))
-		return;
-
 	pgd = pgd_offset_k(addr);
 	pud = pud_offset(pgd, addr);
 	pmd = pmd_offset(pud, addr);
@@ -141,7 +135,19 @@ void make_page_writable(void *va, unsign
 		xen_l1_entry_update(ptep, pte); /* fallback */
 
 	if ((addr >= VMALLOC_START) && (addr < VMALLOC_END))
-		make_page_writable(__va(pte_pfn(pte) << PAGE_SHIFT), feature);
+		__make_page_writable(__va(pte_pfn(pte) << PAGE_SHIFT));
+}
+
+void make_page_readonly(void *va, unsigned int feature)
+{
+	if (!xen_feature(feature))
+		__make_page_readonly(va);
+}
+
+void make_page_writable(void *va, unsigned int feature)
+{
+	if (!xen_feature(feature))
+		__make_page_writable(va);
 }
 
 void make_pages_readonly(void *va, unsigned nr, unsigned int feature)
@@ -150,7 +156,7 @@ void make_pages_readonly(void *va, unsig
 		return;
 
 	while (nr-- != 0) {
-		make_page_readonly(va, feature);
+		__make_page_readonly(va);
 		va = (void*)((unsigned long)va + PAGE_SIZE);
 	}
 }
@@ -161,7 +167,7 @@ void make_pages_writable(void *va, unsig
 		return;
 
 	while (nr-- != 0) {
-		make_page_writable(va, feature);
+		__make_page_writable(va);
 		va = (void*)((unsigned long)va + PAGE_SIZE);
 	}
 }
@@ -1028,11 +1034,6 @@ void free_init_pages(char *what, unsigne
 
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
 {
-#ifdef __DO_LATER__
-	/*
-	 * Some pages can be pinned, but some are not. Unpinning such pages 
-	 * triggers BUG(). 
-	 */
 	unsigned long addr;
 
 	if (begin >= end)
@@ -1044,25 +1045,20 @@ void free_init_pages(char *what, unsigne
 		init_page_count(virt_to_page(addr));
 		memset((void *)(addr & ~(PAGE_SIZE-1)),
 		       POISON_FREE_INITMEM, PAGE_SIZE); 
-		make_page_writable(
-			__va(__pa(addr)), XENFEAT_writable_page_tables);
-		/*
-		 * Make pages from __PAGE_OFFSET address as well
-		 */
-		make_page_writable(
-			(void *)addr, XENFEAT_writable_page_tables);
+		if (addr >= __START_KERNEL_map) {
+			/* make_readonly() reports all kernel addresses. */
+			__make_page_writable(__va(__pa(addr)));
+			__make_page_readonly((void *)addr);
+		}
 		free_page(addr);
 		totalram_pages++;
 	}
-#endif
 }
 
 void free_initmem(void)
 {
-#ifdef __DO_LATER__
 	memset(__initdata_begin, POISON_FREE_INITDATA,
 		__initdata_end - __initdata_begin);
-#endif
 	free_init_pages("unused kernel memory",
 			(unsigned long)(&__init_begin),
 			(unsigned long)(&__init_end));

Re: [PATCH] linux/x86-64: allow kernel init memory to be freed

[Keir Fraser]

On 2/3/07 11:04, "Jan Beulich" <jbeulich@novell.com> wrote:

> +  if (addr >= __START_KERNEL_map) {
> +   /* make_readonly() reports all kernel addresses. */
> +   __make_page_writable(__va(__pa(addr)));
> +   __make_page_readonly((void *)addr);
> +  }

I'm confused by this:
 1. Why does the write-protection need to be changed unconditionally, or
even at all? Is there anything write-protected in the init sections?
 2. Is it safe to keep init mappings above START_KERNEL_map at all, even
read-only? I'd have thought we'd be in trouble if the balloon driver manages
to allocate those pages and tries to free them to Xen. Perhaps they should
be blown away entirely?

 -- Keir

Re: [PATCH] linux/x86-64: allow kernel init memory to be freed

[Jan Beulich]

>>> Keir Fraser <keir@xensource.com> 02.03.07 16:23 >>>
>On 2/3/07 11:04, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>> +  if (addr >= __START_KERNEL_map) {
>> +   /* make_readonly() reports all kernel addresses. */
>> +   __make_page_writable(__va(__pa(addr)));
>> +   __make_page_readonly((void *)addr);
>> +  }
>
>I'm confused by this:
> 1. Why does the write-protection need to be changed unconditionally, or
>even at all? Is there anything write-protected in the init sections?

The mappings in the direct map area must be writeable and, as the comment
says, make_readonly() forces all kernel space mappings to be set up as
read-only during boot. Thus this does *not* depend on
XENFEAT_writable_page_tables, unless make_readonly() is changed.

> 2. Is it safe to keep init mappings above START_KERNEL_map at all, even
>read-only? I'd have thought we'd be in trouble if the balloon driver manages
>to allocate those pages and tries to free them to Xen. Perhaps they should
>be blown away entirely?

Yes, ultimately I wanted them to go away entirely, likewise in native. That is
why I wanted to push this change through mainline rather than directly into
Xen (it also is somewhat more involved as I [mis-]use change_page_attr_addr()
here instead of creating a new function to do this zapping, and as I also at
once extend the range mark_rodata_ro() handles).
But indeed, I didn't consider the balloon driver here, which would - as I
understand it now - erroneously think it freed such pages when Xen really
didn't due to a remaining reference.
So - would you be okay with taking the full-blown patch (probably not, as it
touches another file not currently in the sparse tree), or should I create a
__make_page_inaccessible() function along the lines of
__make_page_readonly()/__make_page_writable()?

Jan

Re: [PATCH] linux/x86-64: allow kernel init memory to be freed

[Keir Fraser]

On 2/3/07 16:01, "Jan Beulich" <jbeulich@novell.com> wrote:

> The mappings in the direct map area must be writeable and, as the comment
> says, make_readonly() forces all kernel space mappings to be set up as
> read-only during boot. Thus this does *not* depend on
> XENFEAT_writable_page_tables, unless make_readonly() is changed.

Ok, understood.

> So - would you be okay with taking the full-blown patch (probably not, as it
> touches another file not currently in the sparse tree), or should I create a
> __make_page_inaccessible() function along the lines of
> __make_page_readonly()/__make_page_writable()?

Yeah, I'll go with __make_page_inaccessible(). :-)

 -- Keir

Re: [PATCH] linux/x86-64: allow kernel init memory to be freed

[Jan Beulich]

>> So - would you be okay with taking the full-blown patch (probably not, as it
>> touches another file not currently in the sparse tree), or should I create a
>> __make_page_inaccessible() function along the lines of
>> __make_page_readonly()/__make_page_writable()?
>
>Yeah, I'll go with __make_page_inaccessible(). :-)

I decided against an extra function, after realizing that the vmalloc area
handling part is useless for this case, and thus the code is more reasonable
to be put in explicitly. And also because I hope to get this code out again
via a mainline change. Here's the updated patch.

Subject: allow kernel init memory to be freed

Signed-off-by: Jan Beulich <jbeulich@novell.com>

Index: head-2007-02-08/arch/x86_64/mm/init-xen.c
===================================================================
--- head-2007-02-08.orig/arch/x86_64/mm/init-xen.c	2007-02-27 15:00:16.000000000 +0100
+++ head-2007-02-08/arch/x86_64/mm/init-xen.c	2007-02-27 15:00:42.000000000 +0100
@@ -102,14 +102,11 @@ static void __meminit early_make_page_re
 		BUG();
 }
 
-void make_page_readonly(void *va, unsigned int feature)
+static void __make_page_readonly(void *va)
 {
 	pgd_t *pgd; pud_t *pud; pmd_t *pmd; pte_t pte, *ptep;
 	unsigned long addr = (unsigned long) va;
 
-	if (xen_feature(feature))
-		return;
-
 	pgd = pgd_offset_k(addr);
 	pud = pud_offset(pgd, addr);
 	pmd = pmd_offset(pud, addr);
@@ -120,17 +117,14 @@ void make_page_readonly(void *va, unsign
 		xen_l1_entry_update(ptep, pte); /* fallback */
 
 	if ((addr >= VMALLOC_START) && (addr < VMALLOC_END))
-		make_page_readonly(__va(pte_pfn(pte) << PAGE_SHIFT), feature);
+		__make_page_readonly(__va(pte_pfn(pte) << PAGE_SHIFT));
 }
 
-void make_page_writable(void *va, unsigned int feature)
+static void __make_page_writable(void *va)
 {
 	pgd_t *pgd; pud_t *pud; pmd_t *pmd; pte_t pte, *ptep;
 	unsigned long addr = (unsigned long) va;
 
-	if (xen_feature(feature))
-		return;
-
 	pgd = pgd_offset_k(addr);
 	pud = pud_offset(pgd, addr);
 	pmd = pmd_offset(pud, addr);
@@ -141,7 +135,19 @@ void make_page_writable(void *va, unsign
 		xen_l1_entry_update(ptep, pte); /* fallback */
 
 	if ((addr >= VMALLOC_START) && (addr < VMALLOC_END))
-		make_page_writable(__va(pte_pfn(pte) << PAGE_SHIFT), feature);
+		__make_page_writable(__va(pte_pfn(pte) << PAGE_SHIFT));
+}
+
+void make_page_readonly(void *va, unsigned int feature)
+{
+	if (!xen_feature(feature))
+		__make_page_readonly(va);
+}
+
+void make_page_writable(void *va, unsigned int feature)
+{
+	if (!xen_feature(feature))
+		__make_page_writable(va);
 }
 
 void make_pages_readonly(void *va, unsigned nr, unsigned int feature)
@@ -150,7 +156,7 @@ void make_pages_readonly(void *va, unsig
 		return;
 
 	while (nr-- != 0) {
-		make_page_readonly(va, feature);
+		__make_page_readonly(va);
 		va = (void*)((unsigned long)va + PAGE_SIZE);
 	}
 }
@@ -161,7 +167,7 @@ void make_pages_writable(void *va, unsig
 		return;
 
 	while (nr-- != 0) {
-		make_page_writable(va, feature);
+		__make_page_writable(va);
 		va = (void*)((unsigned long)va + PAGE_SIZE);
 	}
 }
@@ -1028,11 +1034,6 @@ void free_init_pages(char *what, unsigne
 
 void free_init_pages(char *what, unsigned long begin, unsigned long end)
 {
-#ifdef __DO_LATER__
-	/*
-	 * Some pages can be pinned, but some are not. Unpinning such pages 
-	 * triggers BUG(). 
-	 */
 	unsigned long addr;
 
 	if (begin >= end)
@@ -1044,25 +1045,27 @@ void free_init_pages(char *what, unsigne
 		init_page_count(virt_to_page(addr));
 		memset((void *)(addr & ~(PAGE_SIZE-1)),
 		       POISON_FREE_INITMEM, PAGE_SIZE); 
-		make_page_writable(
-			__va(__pa(addr)), XENFEAT_writable_page_tables);
-		/*
-		 * Make pages from __PAGE_OFFSET address as well
-		 */
-		make_page_writable(
-			(void *)addr, XENFEAT_writable_page_tables);
+		if (addr >= __START_KERNEL_map) {
+			/* make_readonly() reports all kernel addresses. */
+			__make_page_writable(__va(__pa(addr)));
+			if (HYPERVISOR_update_va_mapping(addr, __pte(0), 0)) {
+				pgd_t *pgd = pgd_offset_k(addr);
+				pud_t *pud = pud_offset(pgd, addr);
+				pmd_t *pmd = pmd_offset(pud, addr);
+				pte_t *pte = pte_offset_kernel(pmd, addr);
+
+				xen_l1_entry_update(pte, __pte(0)); /* fallback */
+			}
+		}
 		free_page(addr);
 		totalram_pages++;
 	}
-#endif
 }
 
 void free_initmem(void)
 {
-#ifdef __DO_LATER__
 	memset(__initdata_begin, POISON_FREE_INITDATA,
 		__initdata_end - __initdata_begin);
-#endif
 	free_init_pages("unused kernel memory",
 			(unsigned long)(&__init_begin),
 			(unsigned long)(&__init_end));