Re: [PATCH 5/8] x86, brk: Make extend_brk() available with va/pa.

[H. Peter Anvin]

Global symbols are inaccessible in physical mode.

This is incidentally yet another example of "PV/weird platform violence", since in their absence it would be trivial to work around this by using segmentation.

Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
>Tang Chen <tangchen@xxxxxxxxxxxxxx> wrote:
>>We are going to do acpi_initrd_override() at very early time:
>>
>>On 32bit: do it in head_32.S, before paging is enabled. In this case,
>>we can
>>          access initrd with physical address without page tables.
>>
>>On 64bit: do it in head_64.c, after paging is enabled but before
>direct
>>mapping
>>          is setup.
>>
>>   On 64bit, we have an early page fault handler to help to access
>data
>>     with direct mapping page tables. So it is easy to do in
>head_64.c.
>>
>>And we need to allocate memory to store override tables. At such an
>>early time,
>>no memory allocator works. So we can only use BRK.
>>
>>As mentioned above, on 32bit before paging is enabled, we have to
>>access variables
>>with pa. So introduce a "bool is_phys" parameter to extend_brk(), and
>>convert va
>>to pa is it is true.
>
>Could you do it differently? Meaning have a global symbol
>(paging_enabled) which will be used by most of the functions you
>changed in this patch and the next ones? It would naturally be enabled
>when paging is on and __va addresses can be used. 
>
>That could also be used in the printk case to do a BUG_ON before paging
>is enabled on 32bit. Or perhaps use a different code path to deal with
>using __pa address. 
>
>? 
>>
>>Signed-off-by: Tang Chen <tangchen@xxxxxxxxxxxxxx>
>>---
>> arch/x86/include/asm/dmi.h   |    2 +-
>> arch/x86/include/asm/setup.h |    2 +-
>> arch/x86/kernel/setup.c      |   20 ++++++++++++++------
>> arch/x86/mm/init.c           |    2 +-
>> arch/x86/xen/enlighten.c     |    2 +-
>> arch/x86/xen/mmu.c           |    6 +++---
>> arch/x86/xen/p2m.c           |   27 ++++++++++++++-------------
>> drivers/acpi/osl.c           |    2 +-
>> 8 files changed, 36 insertions(+), 27 deletions(-)
>>
>>diff --git a/arch/x86/include/asm/dmi.h b/arch/x86/include/asm/dmi.h
>>index fd8f9e2..3b51d81 100644
>>--- a/arch/x86/include/asm/dmi.h
>>+++ b/arch/x86/include/asm/dmi.h
>>@@ -9,7 +9,7 @@
>> 
>> static __always_inline __init void *dmi_alloc(unsigned len)
>> {
>>-	return extend_brk(len, sizeof(int));
>>+	return extend_brk(len, sizeof(int), false);
>> }
>> 
>> /* Use early IO mappings for DMI because it's initialized early */
>>diff --git a/arch/x86/include/asm/setup.h
>>b/arch/x86/include/asm/setup.h
>>index 4f71d48..96d00da 100644
>>--- a/arch/x86/include/asm/setup.h
>>+++ b/arch/x86/include/asm/setup.h
>>@@ -75,7 +75,7 @@ extern struct boot_params boot_params;
>> 
>> /* exceedingly early brk-like allocator */
>> extern unsigned long _brk_end;
>>-void *extend_brk(size_t size, size_t align);
>>+void *extend_brk(size_t size, size_t align, bool is_phys);
>> 
>> /*
>>  * Reserve space in the brk section.  The name must be unique within
>>diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>>index 51fcd5d..a189909 100644
>>--- a/arch/x86/kernel/setup.c
>>+++ b/arch/x86/kernel/setup.c
>>@@ -259,19 +259,27 @@ static inline void __init copy_edd(void)
>> }
>> #endif
>> 
>>-void * __init extend_brk(size_t size, size_t align)
>>+void * __init extend_brk(size_t size, size_t align, bool is_phys)
>> {
>> 	size_t mask = align - 1;
>> 	void *ret;
>>+	unsigned long *brk_start, *brk_end, *brk_limit;
>> 
>>-	BUG_ON(_brk_start == 0);
>>+	brk_start = is_phys ? (unsigned long *)__pa_nodebug(&_brk_start) :
>>+			      (unsigned long *)&_brk_start;
>>+	brk_end = is_phys ? (unsigned long *)__pa_nodebug(&_brk_end) :
>>+			    (unsigned long *)&_brk_end;
>>+	brk_limit = is_phys ? (unsigned long *)__pa_nodebug(__brk_limit) :
>>+			      (unsigned long *)__brk_limit;
>>+
>>+	BUG_ON(*brk_start == 0);
>> 	BUG_ON(align & mask);
>> 
>>-	_brk_end = (_brk_end + mask) & ~mask;
>>-	BUG_ON((char *)(_brk_end + size) > __brk_limit);
>>+	*brk_end = (*brk_end + mask) & ~mask;
>>+	BUG_ON((char *)(*brk_end + size) > brk_limit);
>> 
>>-	ret = (void *)_brk_end;
>>-	_brk_end += size;
>>+	ret = (void *)(*brk_end);
>>+	*brk_end += size;
>> 
>> 	memset(ret, 0, size);
>> 
>>diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
>>index 2ec29ac..189a9e2 100644
>>--- a/arch/x86/mm/init.c
>>+++ b/arch/x86/mm/init.c
>>@@ -86,7 +86,7 @@ void  __init early_alloc_pgt_buf(void)
>> 	unsigned long tables = INIT_PGT_BUF_SIZE;
>> 	phys_addr_t base;
>> 
>>-	base = __pa(extend_brk(tables, PAGE_SIZE));
>>+	base = __pa(extend_brk(tables, PAGE_SIZE, false));
>> 
>> 	pgt_buf_start = base >> PAGE_SHIFT;
>> 	pgt_buf_end = pgt_buf_start;
>>diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>>index 193097e..2d5a34f 100644
>>--- a/arch/x86/xen/enlighten.c
>>+++ b/arch/x86/xen/enlighten.c
>>@@ -1629,7 +1629,7 @@ void __ref xen_hvm_init_shared_info(void)
>> 
>> 	if (!shared_info_page)
>> 		shared_info_page = (struct shared_info *)
>>-			extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+			extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 	xatp.domid = DOMID_SELF;
>> 	xatp.idx = 0;
>> 	xatp.space = XENMAPSPACE_shared_info;
>>diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
>>index fdc3ba2..573bc50 100644
>>--- a/arch/x86/xen/mmu.c
>>+++ b/arch/x86/xen/mmu.c
>>@@ -1768,7 +1768,7 @@ static void __init xen_map_identity_early(pmd_t
>>*pmd, unsigned long max_pfn)
>> 	unsigned long pfn;
>> 
>> 	level1_ident_pgt = extend_brk(sizeof(pte_t) * LEVEL1_IDENT_ENTRIES,
>>-				      PAGE_SIZE);
>>+				      PAGE_SIZE, false);
>> 
>> 	ident_pte = 0;
>> 	pfn = 0;
>>@@ -1980,7 +1980,7 @@ static void __init xen_write_cr3_init(unsigned
>>long cr3)
>> 	 * swapper_pg_dir.
>> 	 */
>> 	swapper_kernel_pmd =
>>-		extend_brk(sizeof(pmd_t) * PTRS_PER_PMD, PAGE_SIZE);
>>+		extend_brk(sizeof(pmd_t) * PTRS_PER_PMD, PAGE_SIZE, false);
>> 	copy_page(swapper_kernel_pmd, initial_kernel_pmd);
>> 	swapper_pg_dir[KERNEL_PGD_BOUNDARY] =
>> 		__pgd(__pa(swapper_kernel_pmd) | _PAGE_PRESENT);
>>@@ -2003,7 +2003,7 @@ void __init xen_setup_kernel_pagetable(pgd_t
>>*pgd, unsigned long max_pfn)
>> 	pmd_t *kernel_pmd;
>> 
>> 	initial_kernel_pmd =
>>-		extend_brk(sizeof(pmd_t) * PTRS_PER_PMD, PAGE_SIZE);
>>+		extend_brk(sizeof(pmd_t) * PTRS_PER_PMD, PAGE_SIZE, false);
>> 
>> 	max_pfn_mapped = PFN_DOWN(__pa(xen_start_info->pt_base) +
>> 				  xen_start_info->nr_pt_frames * PAGE_SIZE +
>>diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
>>index 95fb2aa..bbdcf20 100644
>>--- a/arch/x86/xen/p2m.c
>>+++ b/arch/x86/xen/p2m.c
>>@@ -281,13 +281,13 @@ void __ref xen_build_mfn_list_list(void)
>> 
>> 	/* Pre-initialize p2m_top_mfn to be completely missing */
>> 	if (p2m_top_mfn == NULL) {
>>-		p2m_mid_missing_mfn = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+		p2m_mid_missing_mfn = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 		p2m_mid_mfn_init(p2m_mid_missing_mfn);
>> 
>>-		p2m_top_mfn_p = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+		p2m_top_mfn_p = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 		p2m_top_mfn_p_init(p2m_top_mfn_p);
>> 
>>-		p2m_top_mfn = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+		p2m_top_mfn = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 		p2m_top_mfn_init(p2m_top_mfn);
>> 	} else {
>> 		/* Reinitialise, mfn's all change after migration */
>>@@ -322,7 +322,7 @@ void __ref xen_build_mfn_list_list(void)
>> 			 * runtime.  extend_brk() will BUG if we call
>> 			 * it too late.
>> 			 */
>>-			mid_mfn_p = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+			mid_mfn_p = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 			p2m_mid_mfn_init(mid_mfn_p);
>> 
>> 			p2m_top_mfn_p[topidx] = mid_mfn_p;
>>@@ -351,16 +351,16 @@ void __init
>>xen_build_dynamic_phys_to_machine(void)
>> 
>> 	xen_max_p2m_pfn = max_pfn;
>> 
>>-	p2m_missing = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+	p2m_missing = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 	p2m_init(p2m_missing);
>> 
>>-	p2m_mid_missing = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+	p2m_mid_missing = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 	p2m_mid_init(p2m_mid_missing);
>> 
>>-	p2m_top = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+	p2m_top = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 	p2m_top_init(p2m_top);
>> 
>>-	p2m_identity = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+	p2m_identity = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 	p2m_init(p2m_identity);
>> 
>> 	/*
>>@@ -373,7 +373,8 @@ void __init
>xen_build_dynamic_phys_to_machine(void)
>> 		unsigned mididx = p2m_mid_index(pfn);
>> 
>> 		if (p2m_top[topidx] == p2m_mid_missing) {
>>-			unsigned long **mid = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+			unsigned long **mid = extend_brk(PAGE_SIZE, PAGE_SIZE,
>>+							 false);
>> 			p2m_mid_init(mid);
>> 
>> 			p2m_top[topidx] = mid;
>>@@ -609,7 +610,7 @@ static bool __init early_alloc_p2m_middle(unsigned
>>long pfn, bool check_boundary
>> 		return false;
>> 
>> 	/* Boundary cross-over for the edges: */
>>-	p2m = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+	p2m = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 
>> 	p2m_init(p2m);
>> 
>>@@ -635,7 +636,7 @@ static bool __init early_alloc_p2m(unsigned long
>>pfn)
>> 	mid = p2m_top[topidx];
>> 	mid_mfn_p = p2m_top_mfn_p[topidx];
>> 	if (mid == p2m_mid_missing) {
>>-		mid = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+		mid = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 
>> 		p2m_mid_init(mid);
>> 
>>@@ -645,7 +646,7 @@ static bool __init early_alloc_p2m(unsigned long
>>pfn)
>> 	}
>> 	/* And the save/restore P2M tables.. */
>> 	if (mid_mfn_p == p2m_mid_missing_mfn) {
>>-		mid_mfn_p = extend_brk(PAGE_SIZE, PAGE_SIZE);
>>+		mid_mfn_p = extend_brk(PAGE_SIZE, PAGE_SIZE, false);
>> 		p2m_mid_mfn_init(mid_mfn_p);
>> 
>> 		p2m_top_mfn_p[topidx] = mid_mfn_p;
>>@@ -858,7 +859,7 @@ static void __init m2p_override_init(void)
>> 	unsigned i;
>> 
>>	m2p_overrides = extend_brk(sizeof(*m2p_overrides) *
>M2P_OVERRIDE_HASH,
>>-				   sizeof(unsigned long));
>>+				   sizeof(unsigned long), false);
>> 
>> 	for (i = 0; i < M2P_OVERRIDE_HASH; i++)
>> 		INIT_LIST_HEAD(&m2p_overrides[i]);
>>diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>>index 4c1baa7..dff7fcc 100644
>>--- a/drivers/acpi/osl.c
>>+++ b/drivers/acpi/osl.c
>>@@ -563,7 +563,7 @@ RESERVE_BRK(acpi_override_tables_alloc,
>>ACPI_OVERRIDE_TABLES_SIZE);
>> void __init early_alloc_acpi_override_tables_buf(void)
>> {
>> 	acpi_tables_addr = __pa(extend_brk(ACPI_OVERRIDE_TABLES_SIZE,
>>-					   PAGE_SIZE));
>>+					   PAGE_SIZE, false));
>> }
>> 
>> void __init acpi_initrd_override(void *data, size_t size)

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/