[PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Yinghai Lu]

Now we have arch_pfn_mapped array, and max_low_pfn_mapped should not
be used anymore.

Only user is ACPI_OVERRIDE, and it should not use that, as later
accessing is using early_remap. Change to try to 4G below and
then 4G above.

Other user is in drm/i915, but it is commented out.

Should use arch_pfn_mapped or just 1<<(32-PAGE_SHIFT) instead.

Suggested-by: H. Peter Anvin <hpa@linux.intel.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Airlie <airlied@linux.ie>
Cc: Jacob Shin <jacob.shin@amd.com>
Cc: linux-acpi@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
---
 arch/x86/include/asm/page_types.h      |    1 -
 arch/x86/kernel/setup.c                |    4 +---
 arch/x86/mm/init.c                     |    4 ----
 drivers/acpi/osl.c                     |    9 ++++++---
 drivers/gpu/drm/i915/i915_gem_stolen.c |    2 +-
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/page_types.h b/arch/x86/include/asm/page_types.h
index 54c9787..b012b82 100644
--- a/arch/x86/include/asm/page_types.h
+++ b/arch/x86/include/asm/page_types.h
@@ -43,7 +43,6 @@
 
 extern int devmem_is_allowed(unsigned long pagenr);
 
-extern unsigned long max_low_pfn_mapped;
 extern unsigned long max_pfn_mapped;
 
 static inline phys_addr_t get_max_mapped(void)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 90d8cc9..4dcaae7 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -113,13 +113,11 @@
 #include <asm/prom.h>
 
 /*
- * max_low_pfn_mapped: highest direct mapped pfn under 4GB
- * max_pfn_mapped:     highest direct mapped pfn over 4GB
+ * max_pfn_mapped:     highest direct mapped pfn
  *
  * The direct mapping only covers E820_RAM regions, so the ranges and gaps are
  * represented by pfn_mapped
  */
-unsigned long max_low_pfn_mapped;
 unsigned long max_pfn_mapped;
 
 #ifdef CONFIG_DMI
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 59b7fc4..abcc241 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -313,10 +313,6 @@ static void add_pfn_range_mapped(unsigned long start_pfn, unsigned long end_pfn)
 	nr_pfn_mapped = clean_sort_range(pfn_mapped, E820_X_MAX);
 
 	max_pfn_mapped = max(max_pfn_mapped, end_pfn);
-
-	if (start_pfn < (1UL<<(32-PAGE_SHIFT)))
-		max_low_pfn_mapped = max(max_low_pfn_mapped,
-					 min(end_pfn, 1UL<<(32-PAGE_SHIFT)));
 }
 
 bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 586e7e9..c9e36d7 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -624,9 +624,12 @@ void __init acpi_initrd_override(void *data, size_t size)
 	if (table_nr == 0)
 		return;
 
-	acpi_tables_addr =
-		memblock_find_in_range(0, max_low_pfn_mapped << PAGE_SHIFT,
-				       all_tables_size, PAGE_SIZE);
+	/* under 4G at first, then above 4G */
+	acpi_tables_addr = memblock_find_in_range(0, 1ULL<<32,
+					all_tables_size, PAGE_SIZE);
+	if (!acpi_tables_addr)
+		acpi_tables_addr = memblock_find_in_range(1ULL<<32, -1ULL,
+					all_tables_size, PAGE_SIZE);
 	if (!acpi_tables_addr) {
 		WARN_ON(1);
 		return;
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 69d97cb..7f9380b 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 		base -= dev_priv->mm.gtt->stolen_size;
 	} else {
 		/* Stolen is immediately above Top of Memory */
-		base = max_low_pfn_mapped << PAGE_SHIFT;
+		base = __REMOVED_CRAZY__ << PAGE_SHIFT;
 #endif
 	}
 
-- 
1.7.10.4

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Tejun Heo]

On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote:
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 69d97cb..7f9380b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  		base -= dev_priv->mm.gtt->stolen_size;
>  	} else {
>  		/* Stolen is immediately above Top of Memory */
> -		base = max_low_pfn_mapped << PAGE_SHIFT;
> +		base = __REMOVED_CRAZY__ << PAGE_SHIFT;

Huh?

-- 
tejun

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Yinghai Lu]

On Thu, Mar 7, 2013 at 9:10 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote:
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index 69d97cb..7f9380b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>>               base -= dev_priv->mm.gtt->stolen_size;
>>       } else {
>>               /* Stolen is immediately above Top of Memory */
>> -             base = max_low_pfn_mapped << PAGE_SHIFT;
>> +             base = __REMOVED_CRAZY__ << PAGE_SHIFT;
>
> Huh?

Whole function:

static unsigned long i915_stolen_to_physical(struct drm_device *dev)
{
	struct drm_i915_private *dev_priv = dev->dev_private;
	struct pci_dev *pdev = dev_priv->bridge_dev;
	u32 base;

	/* On the machines I have tested the Graphics Base of Stolen Memory
	 * is unreliable, so on those compute the base by subtracting the
	 * stolen memory from the Top of Low Usable DRAM which is where the
	 * BIOS places the graphics stolen memory.
	 *
	 * On gen2, the layout is slightly different with the Graphics Segment
	 * immediately following Top of Memory (or Top of Usable DRAM). Note
	 * it appears that TOUD is only reported by 865g, so we just use the
	 * top of memory as determined by the e820 probe.
	 *
	 * XXX gen2 requires an unavailable symbol and 945gm fails with
	 * its value of TOLUD.
	 */
	base = 0;
	if (INTEL_INFO(dev)->gen >= 6) {
		/* Read Base Data of Stolen Memory Register (BDSM) directly.
		 * Note that there is also a MCHBAR miror at 0x1080c0 or
		 * we could use device 2:0x5c instead.
		*/
		pci_read_config_dword(pdev, 0xB0, &base);
		base &= ~4095; /* lower bits used for locking register */
	} else if (INTEL_INFO(dev)->gen > 3 || IS_G33(dev)) {
		/* Read Graphics Base of Stolen Memory directly */
		pci_read_config_dword(pdev, 0xA4, &base);
#if 0
	} else if (IS_GEN3(dev)) {
		u8 val;
		/* Stolen is immediately below Top of Low Usable DRAM */
		pci_read_config_byte(pdev, 0x9c, &val);
		base = val >> 3 << 27;
		base -= dev_priv->mm.gtt->stolen_size;
	} else {
		/* Stolen is immediately above Top of Memory */
		base = __REMOVED_CRAZY__ << PAGE_SHIFT;
#endif
	}

	return base;
}

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Tejun Heo]

On Thu, Mar 7, 2013 at 9:22 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Mar 7, 2013 at 9:10 PM, Tejun Heo <tj@kernel.org> wrote:
>> On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote:
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>>> index 69d97cb..7f9380b 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>>> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>>>               base -= dev_priv->mm.gtt->stolen_size;
>>>       } else {
>>>               /* Stolen is immediately above Top of Memory */
>>> -             base = max_low_pfn_mapped << PAGE_SHIFT;
>>> +             base = __REMOVED_CRAZY__ << PAGE_SHIFT;
>>
>> Huh?
>
> Whole function:

Yeah, but can't we still just do 1LLU << 32 like other places? Or at
least explain what was there before? It's gonna confuse the hell out
of future readers of the code.

-- 
tejun

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Yinghai Lu]

On Thu, Mar 7, 2013 at 9:25 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 7, 2013 at 9:22 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Thu, Mar 7, 2013 at 9:10 PM, Tejun Heo <tj@kernel.org> wrote:
>>> On Thu, Mar 07, 2013 at 08:58:27PM -0800, Yinghai Lu wrote:
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>>>> index 69d97cb..7f9380b 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>>>> @@ -81,7 +81,7 @@ static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>>>>               base -= dev_priv->mm.gtt->stolen_size;
>>>>       } else {
>>>>               /* Stolen is immediately above Top of Memory */
>>>> -             base = max_low_pfn_mapped << PAGE_SHIFT;
>>>> +             base = __REMOVED_CRAZY__ << PAGE_SHIFT;
>>>
>>> Huh?
>>
>> Whole function:
>
> Yeah, but can't we still just do 1LLU << 32 like other places? Or at
> least explain what was there before? It's gonna confuse the hell out
> of future readers of the code.

They are not using memblock_find_in_range(), so 1ULL<< will not help.

Really hope i915 drm guys could clean that hacks.

Thanks

Yinghai

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Tejun Heo]

On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> They are not using memblock_find_in_range(), so 1ULL<< will not help.
>
> Really hope i915 drm guys could clean that hacks.

The code isn't being used.  Just leave it alone.  Maybe add a comment.
 The change is just making things more confusing.

-- 
tejun

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[H. Peter Anvin]

On 03/07/2013 09:28 PM, Tejun Heo wrote:
> On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> They are not using memblock_find_in_range(), so 1ULL<< will not help.
>>
>> Really hope i915 drm guys could clean that hacks.
> 
> The code isn't being used.  Just leave it alone.  Maybe add a comment.
>  The change is just making things more confusing.
> 

Indeed, but...

Daniel: can you guys clean this up or can we just remove the #if 0 clause?

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Daniel Vetter]

On Thu, Mar 07, 2013 at 10:09:26PM -0800, H. Peter Anvin wrote:
> On 03/07/2013 09:28 PM, Tejun Heo wrote:
> > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> >> They are not using memblock_find_in_range(), so 1ULL<< will not help.
> >>
> >> Really hope i915 drm guys could clean that hacks.
> > 
> > The code isn't being used.  Just leave it alone.  Maybe add a comment.
> >  The change is just making things more confusing.
> > 
> 
> Indeed, but...
> 
> Daniel: can you guys clean this up or can we just remove the #if 0 clause?

I guess we could just put this into a comment explaining where stolen
memory for the gfx devices is at on gen2. But tbh I don't mind if we just
keep the #if 0 code around. For all newer platforms we can get at that
offset through mch bar registers, so I don't really care.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[Chris Wilson]

On Mon, Mar 11, 2013 at 11:50:48PM +0100, Daniel Vetter wrote:
> On Thu, Mar 07, 2013 at 10:09:26PM -0800, H. Peter Anvin wrote:
> > On 03/07/2013 09:28 PM, Tejun Heo wrote:
> > > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > >> They are not using memblock_find_in_range(), so 1ULL<< will not help.
> > >>
> > >> Really hope i915 drm guys could clean that hacks.
> > > 
> > > The code isn't being used.  Just leave it alone.  Maybe add a comment.
> > >  The change is just making things more confusing.
> > > 
> > 
> > Indeed, but...
> > 
> > Daniel: can you guys clean this up or can we just remove the #if 0 clause?
> 
> I guess we could just put this into a comment explaining where stolen
> memory for the gfx devices is at on gen2. But tbh I don't mind if we just
> keep the #if 0 code around. For all newer platforms we can get at that
> offset through mch bar registers, so I don't really care.

If you want to keep the comment accurate
s/max_low_pfn_mapped/max_pfn_mapped/ as the machines in question don't
support more than 4GiB anyway. Or you can help address the underlying
issue of figuring out how we can derive the location of the stolen
memory which is reserved by the BIOS but not communicated to the OS.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 01/14] x86, ACPI, mm: Kill max_low_pfn_mapped

[H. Peter Anvin]

The problem is that the code will be broken, and so it makes no sense.  The #if 0 is really confusing.

Daniel Vetter <daniel@ffwll.ch> wrote:

>On Thu, Mar 07, 2013 at 10:09:26PM -0800, H. Peter Anvin wrote:
>> On 03/07/2013 09:28 PM, Tejun Heo wrote:
>> > On Thu, Mar 7, 2013 at 9:27 PM, Yinghai Lu <yinghai@kernel.org>
>wrote:
>> >> They are not using memblock_find_in_range(), so 1ULL<< will not
>help.
>> >>
>> >> Really hope i915 drm guys could clean that hacks.
>> > 
>> > The code isn't being used.  Just leave it alone.  Maybe add a
>comment.
>> >  The change is just making things more confusing.
>> > 
>> 
>> Indeed, but...
>> 
>> Daniel: can you guys clean this up or can we just remove the #if 0
>clause?
>
>I guess we could just put this into a comment explaining where stolen
>memory for the gfx devices is at on gen2. But tbh I don't mind if we
>just
>keep the #if 0 code around. For all newer platforms we can get at that
>offset through mch bar registers, so I don't really care.
>-Daniel

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

[PATCH 02/14] x86, ACPI: Split find/copy from acpi_initrd_override

[Yinghai Lu]

To parse srat early, we will need to move acpi table probing early.
and to keep acpi_initrd_table_override working, we need to move it
ahead.

But current that is called after init_mem_mapping and relocate_initrd().

Copying need to be after memblock is ready, because it need to allocate
some buffer for acpi tables.

Finding will be moved into head_32.S and head64.c, just like microcode
early scanning.

So split them at first.

Also move down functions declaration to avoid #ifdef in setup.c

Signed-off-by: Yinghai <yinghai@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Jacob Shin <jacob.shin@amd.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
---
 arch/x86/kernel/setup.c |    6 +++---
 drivers/acpi/osl.c      |   32 +++++++++++++++++++-------------
 include/linux/acpi.h    |   16 ++++++++--------
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 4dcaae7..e2913e9 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1092,9 +1092,9 @@ void __init setup_arch(char **cmdline_p)
 
 	reserve_initrd();
 
-#if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD)
-	acpi_initrd_override((void *)initrd_start, initrd_end - initrd_start);
-#endif
+	acpi_initrd_override_find((void *)initrd_start,
+					initrd_end - initrd_start);
+	acpi_initrd_override_copy();
 
 	reserve_crashkernel();
 
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index c9e36d7..b9d2ff0 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -539,6 +539,7 @@ acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
 
 static u64 acpi_tables_addr;
 static int all_tables_size;
+static int table_nr;
 
 /* Copied from acpica/tbutils.c:acpi_tb_checksum() */
 u8 __init acpi_table_checksum(u8 *buffer, u32 length)
@@ -569,18 +570,16 @@ static const char * const table_sigs[] = {
 
 #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
 
-/* Must not increase 10 or needs code modification below */
-#define ACPI_OVERRIDE_TABLES 10
+#define ACPI_OVERRIDE_TABLES 64
+static struct cpio_data __initdata early_initrd_files[ACPI_OVERRIDE_TABLES];
 
-void __init acpi_initrd_override(void *data, size_t size)
+void __init acpi_initrd_override_find(void *data, size_t size)
 {
-	int sig, no, table_nr = 0, total_offset = 0;
+	int sig, no;
 	long offset = 0;
 	struct acpi_table_header *table;
 	char cpio_path[32] = "kernel/firmware/acpi/";
 	struct cpio_data file;
-	struct cpio_data early_initrd_files[ACPI_OVERRIDE_TABLES];
-	char *p;
 
 	if (data == NULL || size == 0)
 		return;
@@ -621,7 +620,14 @@ void __init acpi_initrd_override(void *data, size_t size)
 		early_initrd_files[table_nr].size = file.size;
 		table_nr++;
 	}
-	if (table_nr == 0)
+}
+
+void __init acpi_initrd_override_copy(void)
+{
+	int no, total_offset = 0;
+	char *p;
+
+	if (!table_nr)
 		return;
 
 	/* under 4G at first, then above 4G */
@@ -647,14 +653,14 @@ void __init acpi_initrd_override(void *data, size_t size)
 	memblock_reserve(acpi_tables_addr, acpi_tables_addr + all_tables_size);
 	arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
 
-	p = early_ioremap(acpi_tables_addr, all_tables_size);
-
 	for (no = 0; no < table_nr; no++) {
-		memcpy(p + total_offset, early_initrd_files[no].data,
-		       early_initrd_files[no].size);
-		total_offset += early_initrd_files[no].size;
+		size_t size = early_initrd_files[no].size;
+
+		p = early_ioremap(acpi_tables_addr + total_offset, size);
+		memcpy(p, early_initrd_files[no].data, size);
+		early_iounmap(p, size);
+		total_offset += size;
 	}
-	early_iounmap(p, all_tables_size);
 }
 #endif /* CONFIG_ACPI_INITRD_TABLE_OVERRIDE */
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index bcbdd74..1654a241 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -79,14 +79,6 @@ typedef int (*acpi_tbl_table_handler)(struct acpi_table_header *table);
 typedef int (*acpi_tbl_entry_handler)(struct acpi_subtable_header *header,
 				      const unsigned long end);
 
-#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
-void acpi_initrd_override(void *data, size_t size);
-#else
-static inline void acpi_initrd_override(void *data, size_t size)
-{
-}
-#endif
-
 char * __acpi_map_table (unsigned long phys_addr, unsigned long size);
 void __acpi_unmap_table(char *map, unsigned long size);
 int early_acpi_boot_init(void);
@@ -485,6 +477,14 @@ static inline bool acpi_driver_match_device(struct device *dev,
 
 #endif	/* !CONFIG_ACPI */
 
+#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
+void acpi_initrd_override_find(void *data, size_t size);
+void acpi_initrd_override_copy(void);
+#else
+static inline void acpi_initrd_override_find(void *data, size_t size) { }
+static inline void acpi_initrd_override_copy(void) { }
+#endif
+
 #ifdef CONFIG_ACPI
 void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
 			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
-- 
1.7.10.4

Re: [PATCH 02/14] x86, ACPI: Split find/copy from acpi_initrd_override

[Tejun Heo]

On Thu, Mar 07, 2013 at 08:58:28PM -0800, Yinghai Lu wrote:
> To parse srat early, we will need to move acpi table probing early.
> and to keep acpi_initrd_table_override working, we need to move it
> ahead.
> 
> But current that is called after init_mem_mapping and relocate_initrd().
> 
> Copying need to be after memblock is ready, because it need to allocate
> some buffer for acpi tables.
> 
> Finding will be moved into head_32.S and head64.c, just like microcode
> early scanning.
> 
> So split them at first.
> 
> Also move down functions declaration to avoid #ifdef in setup.c
> 
> Signed-off-by: Yinghai <yinghai@kernel.org>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: Jacob Shin <jacob.shin@amd.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
...
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index c9e36d7..b9d2ff0 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -539,6 +539,7 @@ acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
>  
>  static u64 acpi_tables_addr;
>  static int all_tables_size;
> +static int table_nr;

Not particularly good choice of name for static variable visible to
multiple functions.  all_tables_size isn't a stellar choice either but
no need to continue the tradition.  Maybe acpi_nr_initrd_files?  Also,
why is this one defined here away from the actual table?

> -/* Must not increase 10 or needs code modification below */
> -#define ACPI_OVERRIDE_TABLES 10
> +#define ACPI_OVERRIDE_TABLES 64

What's up with the silent bumping of table size?

> +static struct cpio_data __initdata early_initrd_files[ACPI_OVERRIDE_TABLES];

acpi_initrd_files[]?  Do we really need the "early" designation
together with initrd?

> @@ -647,14 +653,14 @@ void __init acpi_initrd_override(void *data, size_t size)
>  	memblock_reserve(acpi_tables_addr, acpi_tables_addr + all_tables_size);
>  	arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
>  
> -	p = early_ioremap(acpi_tables_addr, all_tables_size);
> -
>  	for (no = 0; no < table_nr; no++) {
> -		memcpy(p + total_offset, early_initrd_files[no].data,
> -		       early_initrd_files[no].size);
> -		total_offset += early_initrd_files[no].size;
> +		size_t size = early_initrd_files[no].size;
> +
> +		p = early_ioremap(acpi_tables_addr + total_offset, size);
> +		memcpy(p, early_initrd_files[no].data, size);
> +		early_iounmap(p, size);
> +		total_offset += size;
>  	}
> -	early_iounmap(p, all_tables_size);

Why is this necessary?  Why no explanation in the description?

> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -79,14 +79,6 @@ typedef int (*acpi_tbl_table_handler)(struct acpi_table_header *table);
>  typedef int (*acpi_tbl_entry_handler)(struct acpi_subtable_header *header,
>  				      const unsigned long end);
>  
> -#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
> -void acpi_initrd_override(void *data, size_t size);
> -#else
> -static inline void acpi_initrd_override(void *data, size_t size)
> -{
> -}
> -#endif
> -
>  char * __acpi_map_table (unsigned long phys_addr, unsigned long size);
>  void __acpi_unmap_table(char *map, unsigned long size);
>  int early_acpi_boot_init(void);
> @@ -485,6 +477,14 @@ static inline bool acpi_driver_match_device(struct device *dev,
>  
>  #endif	/* !CONFIG_ACPI */
>  
> +#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
> +void acpi_initrd_override_find(void *data, size_t size);
> +void acpi_initrd_override_copy(void);
> +#else
> +static inline void acpi_initrd_override_find(void *data, size_t size) { }
> +static inline void acpi_initrd_override_copy(void) { }
> +#endif

I don't get this part either.  Why is it necessary to move the
prototypes to avoid #ifdefs in setup.c?  Ah, okay, you're brining it
outside CONFIG_ACPI so that they're defined regardless of that config
option.  Can you please add why you're moving the prototype in the
descriptoin?  Having "what" is nice but "why" is much nicer. :)

Thanks.

-- 
tejun

Re: [PATCH 02/14] x86, ACPI: Split find/copy from acpi_initrd_override

[Yinghai Lu]

On Thu, Mar 7, 2013 at 9:33 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 07, 2013 at 08:58:28PM -0800, Yinghai Lu wrote:
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index c9e36d7..b9d2ff0 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -539,6 +539,7 @@ acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
>>
>>  static u64 acpi_tables_addr;
>>  static int all_tables_size;
>> +static int table_nr;
>
> Not particularly good choice of name for static variable visible to
> multiple functions.  all_tables_size isn't a stellar choice either but
> no need to continue the tradition.  Maybe acpi_nr_initrd_files?  Also,
> why is this one defined here away from the actual table?

ok, acpi_nr_initrd_files.

will check if it could be killed.

>> -/* Must not increase 10 or needs code modification below */
>> -#define ACPI_OVERRIDE_TABLES 10
>> +#define ACPI_OVERRIDE_TABLES 64
>
> What's up with the silent bumping of table size?

will mention that in change log.

>
>> +static struct cpio_data __initdata early_initrd_files[ACPI_OVERRIDE_TABLES];
>
> acpi_initrd_files[]?  Do we really need the "early" designation
> together with initrd?

just move it out from acpi_initrd_override.

>
>> @@ -647,14 +653,14 @@ void __init acpi_initrd_override(void *data, size_t size)
>>       memblock_reserve(acpi_tables_addr, acpi_tables_addr + all_tables_size);
>>       arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
>>
>> -     p = early_ioremap(acpi_tables_addr, all_tables_size);
>> -
>>       for (no = 0; no < table_nr; no++) {
>> -             memcpy(p + total_offset, early_initrd_files[no].data,
>> -                    early_initrd_files[no].size);
>> -             total_offset += early_initrd_files[no].size;
>> +             size_t size = early_initrd_files[no].size;
>> +
>> +             p = early_ioremap(acpi_tables_addr + total_offset, size);
>> +             memcpy(p, early_initrd_files[no].data, size);
>> +             early_iounmap(p, size);
>> +             total_offset += size;
>>       }
>> -     early_iounmap(p, all_tables_size);
>
> Why is this necessary?  Why no explanation in the description?

actually it is the reason for bump table_nr to 64.

early_ioremap only can map 256k one time, so there will have limit for
overall size.

If map one by one, then we could increase the number of limit.

>
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -79,14 +79,6 @@ typedef int (*acpi_tbl_table_handler)(struct acpi_table_header *table);
>>  typedef int (*acpi_tbl_entry_handler)(struct acpi_subtable_header *header,
>>                                     const unsigned long end);
>>
>> -#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
>> -void acpi_initrd_override(void *data, size_t size);
>> -#else
>> -static inline void acpi_initrd_override(void *data, size_t size)
>> -{
>> -}
>> -#endif
>> -
>>  char * __acpi_map_table (unsigned long phys_addr, unsigned long size);
>>  void __acpi_unmap_table(char *map, unsigned long size);
>>  int early_acpi_boot_init(void);
>> @@ -485,6 +477,14 @@ static inline bool acpi_driver_match_device(struct device *dev,
>>
>>  #endif       /* !CONFIG_ACPI */
>>
>> +#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
>> +void acpi_initrd_override_find(void *data, size_t size);
>> +void acpi_initrd_override_copy(void);
>> +#else
>> +static inline void acpi_initrd_override_find(void *data, size_t size) { }
>> +static inline void acpi_initrd_override_copy(void) { }
>> +#endif
>
> I don't get this part either.  Why is it necessary to move the
> prototypes to avoid #ifdefs in setup.c?  Ah, okay, you're brining it
> outside CONFIG_ACPI so that they're defined regardless of that config
> option.  Can you please add why you're moving the prototype in the
> descriptoin?  Having "what" is nice but "why" is much nicer. :)

I think i have that in the log.

more detail is : ACPI_INITRD_TABLE_OVERRIDE depends
one ACPI and BLK_DEV_INITRD.

So could move it out safely.

Thanks

Yinghai

[PATCH 03/14] x86, ACPI: store override acpi tables phys addr

[Yinghai Lu]

As later 32bit only find table with phys address during 32bit flat mode
in head_32.S.

To keep 32bit and 64 bit consistent, use phys_addr for all.

Use early_ioremap to access during copying.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/osl.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index b9d2ff0..60317ea 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -616,7 +616,7 @@ void __init acpi_initrd_override_find(void *data, size_t size)
 			table->signature, cpio_path, file.name, table->length);
 
 		all_tables_size += table->length;
-		early_initrd_files[table_nr].data = file.data;
+		early_initrd_files[table_nr].data = (void *)__pa(file.data);
 		early_initrd_files[table_nr].size = file.size;
 		table_nr++;
 	}
@@ -625,7 +625,7 @@ void __init acpi_initrd_override_find(void *data, size_t size)
 void __init acpi_initrd_override_copy(void)
 {
 	int no, total_offset = 0;
-	char *p;
+	char *p, *q;
 
 	if (!table_nr)
 		return;
@@ -654,10 +654,13 @@ void __init acpi_initrd_override_copy(void)
 	arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
 
 	for (no = 0; no < table_nr; no++) {
-		size_t size = early_initrd_files[no].size;
+		unsigned long size = early_initrd_files[no].size;
 
 		p = early_ioremap(acpi_tables_addr + total_offset, size);
-		memcpy(p, early_initrd_files[no].data, size);
+		q = early_ioremap((unsigned long)early_initrd_files[no].data,
+					 size);
+		memcpy(p, q, size);
+		early_iounmap(q, size);
 		early_iounmap(p, size);
 		total_offset += size;
 	}
-- 
1.7.10.4

Re: [PATCH 03/14] x86, ACPI: store override acpi tables phys addr

[Tejun Heo]

On Thu, Mar 07, 2013 at 08:58:29PM -0800, Yinghai Lu wrote:
> As later 32bit only find table with phys address during 32bit flat mode
> in head_32.S.
> 
> To keep 32bit and 64 bit consistent, use phys_addr for all.
> 
> Use early_ioremap to access during copying.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: linux-acpi@vger.kernel.org
> ---
> @@ -654,10 +654,13 @@ void __init acpi_initrd_override_copy(void)
>  	arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
>  
>  	for (no = 0; no < table_nr; no++) {
> -		size_t size = early_initrd_files[no].size;
> +		unsigned long size = early_initrd_files[no].size;
>  
>  		p = early_ioremap(acpi_tables_addr + total_offset, size);
> -		memcpy(p, early_initrd_files[no].data, size);
> +		q = early_ioremap((unsigned long)early_initrd_files[no].data,
> +					 size);
> +		memcpy(p, q, size);
> +		early_iounmap(q, size);

Ah, okay, so the loop change in the previous patch was for this, I
suppose?  That chunk probably should either be a separate patch or
rolled into this one.

Thanks.

-- 
tejun

Re: [PATCH 03/14] x86, ACPI: store override acpi tables phys addr

[Yinghai Lu]

On Thu, Mar 7, 2013 at 9:36 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 07, 2013 at 08:58:29PM -0800, Yinghai Lu wrote:
>> As later 32bit only find table with phys address during 32bit flat mode
>> in head_32.S.
>>
>> To keep 32bit and 64 bit consistent, use phys_addr for all.
>>
>> Use early_ioremap to access during copying.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Thomas Renninger <trenn@suse.de>
>> Cc: Rafael J. Wysocki <rjw@sisk.pl>
>> Cc: linux-acpi@vger.kernel.org
>> ---
>> @@ -654,10 +654,13 @@ void __init acpi_initrd_override_copy(void)
>>       arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
>>
>>       for (no = 0; no < table_nr; no++) {
>> -             size_t size = early_initrd_files[no].size;
>> +             unsigned long size = early_initrd_files[no].size;
>>
>>               p = early_ioremap(acpi_tables_addr + total_offset, size);
>> -             memcpy(p, early_initrd_files[no].data, size);
>> +             q = early_ioremap((unsigned long)early_initrd_files[no].data,
>> +                                      size);
>> +             memcpy(p, q, size);
>> +             early_iounmap(q, size);
>
> Ah, okay, so the loop change in the previous patch was for this, I
> suppose?  That chunk probably should either be a separate patch or
> rolled into this one.

merge two patches?

Re: [PATCH 03/14] x86, ACPI: store override acpi tables phys addr

[Tejun Heo]

On Thu, Mar 07, 2013 at 10:49:04PM -0800, Yinghai Lu wrote:
> >> @@ -654,10 +654,13 @@ void __init acpi_initrd_override_copy(void)
> >>       arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
> >>
> >>       for (no = 0; no < table_nr; no++) {
> >> -             size_t size = early_initrd_files[no].size;
> >> +             unsigned long size = early_initrd_files[no].size;
> >>
> >>               p = early_ioremap(acpi_tables_addr + total_offset, size);
> >> -             memcpy(p, early_initrd_files[no].data, size);
> >> +             q = early_ioremap((unsigned long)early_initrd_files[no].data,
> >> +                                      size);
> >> +             memcpy(p, q, size);
> >> +             early_iounmap(q, size);
> >
> > Ah, okay, so the loop change in the previous patch was for this, I
> > suppose?  That chunk probably should either be a separate patch or
> > rolled into this one.
> 
> merge two patches?

Hmm... probably better to just move the related chunks from the
previous patch to this one with better explanation on what's going on.

Thanks.

-- 
tejun

[PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Yinghai Lu]

We will find acpi tables in initrd during head_32.S in 32bit flat mode.

So need acpi_initrd_override_find could take phys directly.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Jacob Shin <jacob.shin@amd.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
---
 arch/x86/kernel/setup.c |    2 +-
 drivers/acpi/osl.c      |   84 +++++++++++++++++++++++++++++++----------------
 include/linux/acpi.h    |    4 +--
 3 files changed, 58 insertions(+), 32 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index e2913e9..668e658 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1093,7 +1093,7 @@ void __init setup_arch(char **cmdline_p)
 	reserve_initrd();
 
 	acpi_initrd_override_find((void *)initrd_start,
-					initrd_end - initrd_start);
+					initrd_end - initrd_start, false);
 	acpi_initrd_override_copy();
 
 	reserve_crashkernel();
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 60317ea..b375159 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -552,38 +552,47 @@ u8 __init acpi_table_checksum(u8 *buffer, u32 length)
 	return sum;
 }
 
-/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
-static const char * const table_sigs[] = {
-	ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
-	ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
-	ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
-	ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
-	ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
-	ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
-	ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
-	ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
-	ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };
-
 /* Non-fatal errors: Affected tables/files are ignored */
 #define INVALID_TABLE(x, path, name)					\
-	{ pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); continue; }
+	do { pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); } while (0)
 
 #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
 
 #define ACPI_OVERRIDE_TABLES 64
 static struct cpio_data __initdata early_initrd_files[ACPI_OVERRIDE_TABLES];
 
-void __init acpi_initrd_override_find(void *data, size_t size)
+void __init acpi_initrd_override_find(void *data, size_t size, bool is_phys)
 {
 	int sig, no;
 	long offset = 0;
 	struct acpi_table_header *table;
 	char cpio_path[32] = "kernel/firmware/acpi/";
 	struct cpio_data file;
+	struct cpio_data *files = early_initrd_files;
+	int *all_tables_size_p = &all_tables_size;
+	int *table_nr_p = &table_nr;
+
+	/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
+	char *table_sigs[] = {
+		ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
+		ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
+		ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
+		ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
+		ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
+		ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
+		ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
+		ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
+		ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };
 
 	if (data == NULL || size == 0)
 		return;
 
+	if (is_phys) {
+		files = (struct cpio_data *)__pa_symbol(early_initrd_files);
+		all_tables_size_p = (int *)__pa_symbol(&all_tables_size);
+		table_nr_p = (int *)__pa_symbol(&table_nr);
+	}
+
 	for (no = 0; no < ACPI_OVERRIDE_TABLES; no++) {
 		file = find_cpio_data(cpio_path, data, size, &offset);
 		if (!file.data)
@@ -592,9 +601,12 @@ void __init acpi_initrd_override_find(void *data, size_t size)
 		data += offset;
 		size -= offset;
 
-		if (file.size < sizeof(struct acpi_table_header))
-			INVALID_TABLE("Table smaller than ACPI header",
+		if (file.size < sizeof(struct acpi_table_header)) {
+			if (!is_phys)
+				INVALID_TABLE("Table smaller than ACPI header",
 				      cpio_path, file.name);
+			continue;
+		}
 
 		table = file.data;
 
@@ -602,23 +614,34 @@ void __init acpi_initrd_override_find(void *data, size_t size)
 			if (!memcmp(table->signature, table_sigs[sig], 4))
 				break;
 
-		if (!table_sigs[sig])
-			INVALID_TABLE("Unknown signature",
+		if (!table_sigs[sig]) {
+			if (!is_phys)
+				 INVALID_TABLE("Unknown signature",
 				      cpio_path, file.name);
-		if (file.size != table->length)
-			INVALID_TABLE("File length does not match table length",
+			continue;
+		}
+		if (file.size != table->length) {
+			if (!is_phys)
+				INVALID_TABLE("File length does not match table length",
 				      cpio_path, file.name);
-		if (acpi_table_checksum(file.data, table->length))
-			INVALID_TABLE("Bad table checksum",
+			continue;
+		}
+		if (acpi_table_checksum(file.data, table->length)) {
+			if (!is_phys)
+				INVALID_TABLE("Bad table checksum",
 				      cpio_path, file.name);
+			continue;
+		}
 
-		pr_info("%4.4s ACPI table found in initrd [%s%s][0x%x]\n",
+		if (!is_phys)
+			pr_info("%4.4s ACPI table found in initrd [%s%s][0x%x]\n",
 			table->signature, cpio_path, file.name, table->length);
 
-		all_tables_size += table->length;
-		early_initrd_files[table_nr].data = (void *)__pa(file.data);
-		early_initrd_files[table_nr].size = file.size;
-		table_nr++;
+		(*all_tables_size_p) += table->length;
+		files[*table_nr_p].data = is_phys ?
+					    file.data : (void *)__pa(file.data);
+		files[*table_nr_p].size = file.size;
+		(*table_nr_p)++;
 	}
 }
 
@@ -654,11 +677,14 @@ void __init acpi_initrd_override_copy(void)
 	arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
 
 	for (no = 0; no < table_nr; no++) {
+		unsigned long phys_addr = (unsigned long)early_initrd_files[no].data;
 		unsigned long size = early_initrd_files[no].size;
 
+		q = early_ioremap(phys_addr, size);
+		pr_info("%4.4s ACPI table found in initrd [%#010lx-%#010lx]\n",
+				((struct acpi_table_header *)q)->signature,
+				phys_addr, phys_addr + size - 1);
 		p = early_ioremap(acpi_tables_addr + total_offset, size);
-		q = early_ioremap((unsigned long)early_initrd_files[no].data,
-					 size);
 		memcpy(p, q, size);
 		early_iounmap(q, size);
 		early_iounmap(p, size);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 1654a241..46a8a89 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -478,10 +478,10 @@ static inline bool acpi_driver_match_device(struct device *dev,
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
-void acpi_initrd_override_find(void *data, size_t size);
+void acpi_initrd_override_find(void *data, size_t size, bool is_phys);
 void acpi_initrd_override_copy(void);
 #else
-static inline void acpi_initrd_override_find(void *data, size_t size) { }
+static inline void acpi_initrd_override_find(void *data, size_t size, bool is_phys) { }
 static inline void acpi_initrd_override_copy(void) { }
 #endif
 
-- 
1.7.10.4

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Tejun Heo]

On Thu, Mar 07, 2013 at 08:58:30PM -0800, Yinghai Lu wrote:
> We will find acpi tables in initrd during head_32.S in 32bit flat mode.
> 
> So need acpi_initrd_override_find could take phys directly.

The patch description doesn't explain even half of what's going on.

> @@ -552,38 +552,47 @@ u8 __init acpi_table_checksum(u8 *buffer, u32 length)
>  	return sum;
>  }
>  
> -/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
> -static const char * const table_sigs[] = {
> -	ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
> -	ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
> -	ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
> -	ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
> -	ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
> -	ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
> -	ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
> -	ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
> -	ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };

Why is this table made a stack variable?  What's the benefit of doing
that?

>  /* Non-fatal errors: Affected tables/files are ignored */
>  #define INVALID_TABLE(x, path, name)					\
> -	{ pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); continue; }
> +	do { pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); } while (0)

Might as well rename the macro to something which indicates it's just
printing error message.  Urgh... who thought embedding control flow
directive like continue inside a macro was a good idea? :(

> -void __init acpi_initrd_override_find(void *data, size_t size)
> +void __init acpi_initrd_override_find(void *data, size_t size, bool is_phys)

Is it really necessary to make the function take both virtual and
physical addresses?  Can't we just make the function take phys_addr_t
and update everyone to call with physaddr?  Also @is_phys isn't simple
address switch.  It also changes error reporting.  If you're gonna
keep @is_phys, let's at least write up a function comment explaining
what's going on and why we need it.  But, really, if at all possible,
let's change the function to take single type of argument and
predicate error message printing on something else (e.g. early printk
initialized or whatever).

> @@ -654,11 +677,14 @@ void __init acpi_initrd_override_copy(void)
>  	arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
>  
>  	for (no = 0; no < table_nr; no++) {
> +		unsigned long phys_addr = (unsigned long)early_initrd_files[no].data;

Can we please use phys_addr_t for physical addresses?

>  		unsigned long size = early_initrd_files[no].size;
>  
> +		q = early_ioremap(phys_addr, size);
> +		pr_info("%4.4s ACPI table found in initrd [%#010lx-%#010lx]\n",
> +				((struct acpi_table_header *)q)->signature,
> +				phys_addr, phys_addr + size - 1);

Maybe putting pr_info after ioremapping both p and q would be easier
on the eyes?

>  		p = early_ioremap(acpi_tables_addr + total_offset, size);
> -		q = early_ioremap((unsigned long)early_initrd_files[no].data,
> -					 size);
>  		memcpy(p, q, size);
>  		early_iounmap(q, size);
>  		early_iounmap(p, size);

Thanks.

-- 
tejun

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Yinghai Lu]

On Thu, Mar 7, 2013 at 9:50 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 07, 2013 at 08:58:30PM -0800, Yinghai Lu wrote:
>> We will find acpi tables in initrd during head_32.S in 32bit flat mode.
>>
>> So need acpi_initrd_override_find could take phys directly.
>
> The patch description doesn't explain even half of what's going on.

hope HPA could understand.

Access initrd before relocate_initrd and init_memory mapping.

>
>> @@ -552,38 +552,47 @@ u8 __init acpi_table_checksum(u8 *buffer, u32 length)
>>       return sum;
>>  }
>>
>> -/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
>> -static const char * const table_sigs[] = {
>> -     ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
>> -     ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
>> -     ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
>> -     ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
>> -     ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
>> -     ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
>> -     ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
>> -     ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
>> -     ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };
>
> Why is this table made a stack variable?  What's the benefit of doing
> that?

so I do need to switch global variables to phys and access it.

>
>>  /* Non-fatal errors: Affected tables/files are ignored */
>>  #define INVALID_TABLE(x, path, name)                                 \
>> -     { pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); continue; }
>> +     do { pr_err("ACPI OVERRIDE: " x " [%s%s]\n", path, name); } while (0)
>
> Might as well rename the macro to something which indicates it's just
> printing error message.  Urgh... who thought embedding control flow
> directive like continue inside a macro was a good idea? :(

so I removed it.

>
>> -void __init acpi_initrd_override_find(void *data, size_t size)
>> +void __init acpi_initrd_override_find(void *data, size_t size, bool is_phys)
>
> Is it really necessary to make the function take both virtual and
> physical addresses?  Can't we just make the function take phys_addr_t
> and update everyone to call with physaddr?  Also @is_phys isn't simple
> address switch.  It also changes error reporting.  If you're gonna
> keep @is_phys, let's at least write up a function comment explaining
> what's going on and why we need it.  But, really, if at all possible,
> let's change the function to take single type of argument and
> predicate error message printing on something else (e.g. early printk
> initialized or whatever).

yes, one for 32bit from head_32.S, phys.
one for 64bit from head64.c. with _va.

Not sure if I could use early_printk from head_32.S, as Fenghua does
not print out
from microcode updating early in the same parts.

Will check that.

>
>> @@ -654,11 +677,14 @@ void __init acpi_initrd_override_copy(void)
>>       arch_reserve_mem_area(acpi_tables_addr, all_tables_size);
>>
>>       for (no = 0; no < table_nr; no++) {
>> +             unsigned long phys_addr = (unsigned long)early_initrd_files[no].data;
>
> Can we please use phys_addr_t for physical addresses?

ok.

>
>>               unsigned long size = early_initrd_files[no].size;
>>
>> +             q = early_ioremap(phys_addr, size);
>> +             pr_info("%4.4s ACPI table found in initrd [%#010lx-%#010lx]\n",
>> +                             ((struct acpi_table_header *)q)->signature,
>> +                             phys_addr, phys_addr + size - 1);
>
> Maybe putting pr_info after ioremapping both p and q would be easier
> on the eyes?

ok.

>
>>               p = early_ioremap(acpi_tables_addr + total_offset, size);
>> -             q = early_ioremap((unsigned long)early_initrd_files[no].data,
>> -                                      size);
>>               memcpy(p, q, size);
>>               early_iounmap(q, size);
>>               early_iounmap(p, size);

Thanks

Yinghai

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Tejun Heo]

Hello, Yinghai.

On Thu, Mar 07, 2013 at 10:57:21PM -0800, Yinghai Lu wrote:
> On Thu, Mar 7, 2013 at 9:50 PM, Tejun Heo <tj@kernel.org> wrote:
> > On Thu, Mar 07, 2013 at 08:58:30PM -0800, Yinghai Lu wrote:
> >> We will find acpi tables in initrd during head_32.S in 32bit flat mode.
> >>
> >> So need acpi_initrd_override_find could take phys directly.
> >
> > The patch description doesn't explain even half of what's going on.
> 
> hope HPA could understand.
> 
> Access initrd before relocate_initrd and init_memory mapping.

I really hope the changelogs were better.  Eh well...

> >> -/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
> >> -static const char * const table_sigs[] = {
> >> -     ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
> >> -     ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
> >> -     ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
> >> -     ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
> >> -     ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
> >> -     ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
> >> -     ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
> >> -     ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
> >> -     ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };
> >
> > Why is this table made a stack variable?  What's the benefit of doing
> > that?
> 
> so I do need to switch global variables to phys and access it.

I can't really understand what your response means.  Can you please
elaborate?

> > Is it really necessary to make the function take both virtual and
> > physical addresses?  Can't we just make the function take phys_addr_t
> > and update everyone to call with physaddr?  Also @is_phys isn't simple
> > address switch.  It also changes error reporting.  If you're gonna
> > keep @is_phys, let's at least write up a function comment explaining
> > what's going on and why we need it.  But, really, if at all possible,
> > let's change the function to take single type of argument and
> > predicate error message printing on something else (e.g. early printk
> > initialized or whatever).
> 
> yes, one for 32bit from head_32.S, phys.
> one for 64bit from head64.c. with _va.

head64.c can't call with phys?  Why not?

> Not sure if I could use early_printk from head_32.S, as Fenghua does
> not print out
> from microcode updating early in the same parts.

ISTR it works but it doens't have to (although it would be much nicer
if it did).  You can test whether printk is online and skip if it
isn't online yet.

Thanks.

-- 
tejun

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Yinghai Lu]

On Thu, Mar 7, 2013 at 11:06 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello, Yinghai.
>
> On Thu, Mar 07, 2013 at 10:57:21PM -0800, Yinghai Lu wrote:
>> On Thu, Mar 7, 2013 at 9:50 PM, Tejun Heo <tj@kernel.org> wrote:
>> > On Thu, Mar 07, 2013 at 08:58:30PM -0800, Yinghai Lu wrote:
>> >> We will find acpi tables in initrd during head_32.S in 32bit flat mode.
>> >>
>> >> So need acpi_initrd_override_find could take phys directly.
>> >
>> > The patch description doesn't explain even half of what's going on.
>>
>> hope HPA could understand.
>>
>> Access initrd before relocate_initrd and init_memory mapping.
>
> I really hope the changelogs were better.  Eh well...
>
>> >> -/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
>> >> -static const char * const table_sigs[] = {
>> >> -     ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
>> >> -     ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
>> >> -     ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
>> >> -     ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
>> >> -     ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
>> >> -     ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
>> >> -     ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
>> >> -     ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
>> >> -     ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };
>> >
>> > Why is this table made a stack variable?  What's the benefit of doing
>> > that?
>>
>> so I do need to switch global variables to phys and access it.
>
> I can't really understand what your response means.  Can you please
> elaborate?

sorry, I missed NOT.

so I do NOT need to switch global variables from kernel virtual addr
to phys address and access it
in 32bit flat mode.

>
>> > Is it really necessary to make the function take both virtual and
>> > physical addresses?  Can't we just make the function take phys_addr_t
>> > and update everyone to call with physaddr?  Also @is_phys isn't simple
>> > address switch.  It also changes error reporting.  If you're gonna
>> > keep @is_phys, let's at least write up a function comment explaining
>> > what's going on and why we need it.  But, really, if at all possible,
>> > let's change the function to take single type of argument and
>> > predicate error message printing on something else (e.g. early printk
>> > initialized or whatever).
>>
>> yes, one for 32bit from head_32.S, phys.
>> one for 64bit from head64.c. with _va.
>
> head64.c can't call with phys?  Why not?

HPA's #PF set up page table only handle kernel low mapping address.

and after reset_early_page_tables, only kernel high mapping address is
there. and other low mapping will be supported via #PF handler.

>
>> Not sure if I could use early_printk from head_32.S, as Fenghua does
>> not print out
>> from microcode updating early in the same parts.
>
> ISTR it works but it doens't have to (although it would be much nicer
> if it did).  You can test whether printk is online and skip if it
> isn't online yet.

ok, will give it try.

Thanks

Yinghai

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Tejun Heo]

On Thu, Mar 07, 2013 at 11:25:14PM -0800, Yinghai Lu wrote:
> >> > Why is this table made a stack variable?  What's the benefit of doing
> >> > that?
> >>
> >> so I do need to switch global variables to phys and access it.
> >
> > I can't really understand what your response means.  Can you please
> > elaborate?
> 
> sorry, I missed NOT.
> 
> so I do NOT need to switch global variables from kernel virtual addr
> to phys address and access it
> in 32bit flat mode.

Ah, okay, so the function is called with a completely different
address mode and so you actually want to build the table on stack so
that you don't have to flip the address mode for the global address.

> >> yes, one for 32bit from head_32.S, phys.
> >> one for 64bit from head64.c. with _va.
> >
> > head64.c can't call with phys?  Why not?
> 
> HPA's #PF set up page table only handle kernel low mapping address.
> 
> and after reset_early_page_tables, only kernel high mapping address is
> there. and other low mapping will be supported via #PF handler.

Okay, it now makes sense.  Ah.... You'll definitely need a lot of
documentation explanining what's going on.

Thanks.

-- 
tejun

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Andrew Morton]

On Thu, 7 Mar 2013 22:57:21 -0800 Yinghai Lu <yinghai@kernel.org> wrote:

> >
> >> @@ -552,38 +552,47 @@ u8 __init acpi_table_checksum(u8 *buffer, u32 length)
> >>       return sum;
> >>  }
> >>
> >> -/* All but ACPI_SIG_RSDP and ACPI_SIG_FACS: */
> >> -static const char * const table_sigs[] = {
> >> -     ACPI_SIG_BERT, ACPI_SIG_CPEP, ACPI_SIG_ECDT, ACPI_SIG_EINJ,
> >> -     ACPI_SIG_ERST, ACPI_SIG_HEST, ACPI_SIG_MADT, ACPI_SIG_MSCT,
> >> -     ACPI_SIG_SBST, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_ASF,
> >> -     ACPI_SIG_BOOT, ACPI_SIG_DBGP, ACPI_SIG_DMAR, ACPI_SIG_HPET,
> >> -     ACPI_SIG_IBFT, ACPI_SIG_IVRS, ACPI_SIG_MCFG, ACPI_SIG_MCHI,
> >> -     ACPI_SIG_SLIC, ACPI_SIG_SPCR, ACPI_SIG_SPMI, ACPI_SIG_TCPA,
> >> -     ACPI_SIG_UEFI, ACPI_SIG_WAET, ACPI_SIG_WDAT, ACPI_SIG_WDDT,
> >> -     ACPI_SIG_WDRT, ACPI_SIG_DSDT, ACPI_SIG_FADT, ACPI_SIG_PSDT,
> >> -     ACPI_SIG_RSDT, ACPI_SIG_XSDT, ACPI_SIG_SSDT, NULL };
> >
> > Why is this table made a stack variable?  What's the benefit of doing
> > that?
> 
> so I do need to switch global variables to phys and access it.

What Tejun means is that it should be marked "static" within
acpi_initrd_override(), so we don't have to build a copy on the stack
at runtime each time acpi_initrd_override() is called.

While we're there, it should be __initdata also.

Re: [PATCH 04/14] x86, ACPI: make acpi override finding work with 32bit flat mode

[Thomas Gleixner]

Yinghai,

On Thu, 7 Mar 2013, Yinghai Lu wrote:

> On Thu, Mar 7, 2013 at 9:50 PM, Tejun Heo <tj@kernel.org> wrote:
> > On Thu, Mar 07, 2013 at 08:58:30PM -0800, Yinghai Lu wrote:
> >> We will find acpi tables in initrd during head_32.S in 32bit flat mode.
> >>
> >> So need acpi_initrd_override_find could take phys directly.
> >
> > The patch description doesn't explain even half of what's going on.
> 
> hope HPA could understand.

What the heck? Is HPA your personal decryptor?

I'm really tired of this nonsense. There is a track record of people
complaining about your completely useless and sloppy changelogs and
your unwillingness to properly explain and discuss your patches.

Just to make it clear. We all are able and willing to cope with
developers who are not native english speakers and have limited
language skills. I'm rewriting changelogs on a regular base without
complaining about that.

But that's not the problem at hand. I met you personally at KS and
found out that your english skills are not those of the random Chinese
person. You just prefer to hide your excellent language skills behind
your Chinese name.

That's an utter waste of time and resources!

Please grow up and use your language and technical skills in a way
which does not offend the people you need to interact with.

Seriously, if you can't convince yourself that a proper communication
with maintainers and other developers is a primary task, please don't
be surprised if you end up on the /dev/null filters and auto-NAK bots
of quite a bunch of affected people.

That would be a sad outcome, really. But that's solely your decision.

Thanks,

	tglx

[PATCH 05/14] x86, ACPI: Find acpi tables in initrd early at head_32.S/head64.c

[Yinghai Lu]

head64.c could use #PF handler set page table to access initrd before
mapping and relocating.

head_32.S could use 32bit flat mode to access initrd before mapping
and relocating.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Jacob Shin <jacob.shin@amd.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
---
 arch/x86/include/asm/setup.h |    2 ++
 arch/x86/kernel/head64.c     |    2 ++
 arch/x86/kernel/head_32.S    |    4 ++++
 arch/x86/kernel/setup.c      |   28 ++++++++++++++++++++++++++--
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index b7bf350..b09db26 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -42,6 +42,8 @@ extern void visws_early_detect(void);
 static inline void visws_early_detect(void) { }
 #endif
 
+void x86_acpi_override_find(void);
+
 extern unsigned long saved_video_mode;
 
 extern void reserve_standard_io_resources(void);
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index c5e403f..a31bc63 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -174,6 +174,8 @@ void __init x86_64_start_kernel(char * real_mode_data)
 	if (console_loglevel == 10)
 		early_printk("Kernel alive\n");
 
+	x86_acpi_override_find();
+
 	clear_page(init_level4_pgt);
 	/* set init_level4_pgt kernel high mapping*/
 	init_level4_pgt[511] = early_level4_pgt[511];
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 73afd11..ca08f0e 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -149,6 +149,10 @@ ENTRY(startup_32)
 	call load_ucode_bsp
 #endif
 
+#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
+	call x86_acpi_override_find
+#endif
+
 /*
  * Initialize page tables.  This creates a PDE and a set of page
  * tables, which are located immediately beyond __brk_base.  The variable
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 668e658..d43545a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -424,6 +424,32 @@ static void __init reserve_initrd(void)
 }
 #endif /* CONFIG_BLK_DEV_INITRD */
 
+#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
+void __init x86_acpi_override_find(void)
+{
+	unsigned long ramdisk_image, ramdisk_size;
+	unsigned char *p = NULL;
+
+#ifdef CONFIG_X86_32
+	struct boot_params *boot_params_p;
+
+	boot_params_p = (struct boot_params *)__pa_symbol(&boot_params);
+	ramdisk_image = boot_params_p->hdr.ramdisk_image;
+	ramdisk_size  = boot_params_p->hdr.ramdisk_size;
+	p = (unsigned char *)ramdisk_image;
+	acpi_initrd_override_find(p, ramdisk_size, true);
+#else
+	ramdisk_image = get_ramdisk_image();
+	ramdisk_size  = get_ramdisk_size();
+	if (ramdisk_image)
+		p = __va(ramdisk_image);
+	acpi_initrd_override_find(p, ramdisk_size, false);
+#endif
+}
+#else
+void __init x86_acpi_override_find(void) { }
+#endif
+
 static void __init parse_setup_data(void)
 {
 	struct setup_data *data;
@@ -1092,8 +1118,6 @@ void __init setup_arch(char **cmdline_p)
 
 	reserve_initrd();
 
-	acpi_initrd_override_find((void *)initrd_start,
-					initrd_end - initrd_start, false);
 	acpi_initrd_override_copy();
 
 	reserve_crashkernel();
-- 
1.7.10.4

Re: [PATCH 05/14] x86, ACPI: Find acpi tables in initrd early at head_32.S/head64.c

[Tejun Heo]

On Thu, Mar 07, 2013 at 08:58:31PM -0800, Yinghai Lu wrote:
> diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
> index 73afd11..ca08f0e 100644
> --- a/arch/x86/kernel/head_32.S
> +++ b/arch/x86/kernel/head_32.S
> @@ -149,6 +149,10 @@ ENTRY(startup_32)
>  	call load_ucode_bsp
>  #endif
>  
> +#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
> +	call x86_acpi_override_find
> +#endif

The function is always defined.  We can probalby lose ifdef here?

Also, does it really have to be called from head_32.S?  No way this
can be done after entering C code?  It would be great if you can
explain overall design choices in the head message (and important
patches).

> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 668e658..d43545a 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -424,6 +424,32 @@ static void __init reserve_initrd(void)
>  }
>  #endif /* CONFIG_BLK_DEV_INITRD */
>  
> +#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
> +void __init x86_acpi_override_find(void)
> +{
> +	unsigned long ramdisk_image, ramdisk_size;
> +	unsigned char *p = NULL;
> +
> +#ifdef CONFIG_X86_32
> +	struct boot_params *boot_params_p;
> +
> +	boot_params_p = (struct boot_params *)__pa_symbol(&boot_params);
> +	ramdisk_image = boot_params_p->hdr.ramdisk_image;
> +	ramdisk_size  = boot_params_p->hdr.ramdisk_size;
> +	p = (unsigned char *)ramdisk_image;
> +	acpi_initrd_override_find(p, ramdisk_size, true);
> +#else
> +	ramdisk_image = get_ramdisk_image();
> +	ramdisk_size  = get_ramdisk_size();
> +	if (ramdisk_image)
> +		p = __va(ramdisk_image);
> +	acpi_initrd_override_find(p, ramdisk_size, false);
> +#endif
> +}
> +#else
> +void __init x86_acpi_override_find(void) { }

And add a comment here why we're not doing static inline for the dummy
function?

Thanks.

-- 
tejun

Re: [PATCH 05/14] x86, ACPI: Find acpi tables in initrd early at head_32.S/head64.c

[Yinghai Lu]

On Thu, Mar 7, 2013 at 9:57 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 07, 2013 at 08:58:31PM -0800, Yinghai Lu wrote:
>> diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
>> index 73afd11..ca08f0e 100644
>> --- a/arch/x86/kernel/head_32.S
>> +++ b/arch/x86/kernel/head_32.S
>> @@ -149,6 +149,10 @@ ENTRY(startup_32)
>>       call load_ucode_bsp
>>  #endif
>>
>> +#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
>> +     call x86_acpi_override_find
>> +#endif
>
> The function is always defined.  We can probalby lose ifdef here?

just mimic microcode updating again.

>
> Also, does it really have to be called from head_32.S?  No way this
> can be done after entering C code?  It would be great if you can
> explain overall design choices in the head message (and important
> patches).

have to be with head_32.S and it is with 32bit flat mode, so could access
4G blow without setting page table.

Will try add to more in the change log.

>
>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>> index 668e658..d43545a 100644
>> --- a/arch/x86/kernel/setup.c
>> +++ b/arch/x86/kernel/setup.c
>> @@ -424,6 +424,32 @@ static void __init reserve_initrd(void)
>>  }
>>  #endif /* CONFIG_BLK_DEV_INITRD */
>>
>> +#ifdef CONFIG_ACPI_INITRD_TABLE_OVERRIDE
>> +void __init x86_acpi_override_find(void)
>> +{
>> +     unsigned long ramdisk_image, ramdisk_size;
>> +     unsigned char *p = NULL;
>> +
>> +#ifdef CONFIG_X86_32
>> +     struct boot_params *boot_params_p;
>> +
>> +     boot_params_p = (struct boot_params *)__pa_symbol(&boot_params);
>> +     ramdisk_image = boot_params_p->hdr.ramdisk_image;
>> +     ramdisk_size  = boot_params_p->hdr.ramdisk_size;
>> +     p = (unsigned char *)ramdisk_image;
>> +     acpi_initrd_override_find(p, ramdisk_size, true);
>> +#else
>> +     ramdisk_image = get_ramdisk_image();
>> +     ramdisk_size  = get_ramdisk_size();
>> +     if (ramdisk_image)
>> +             p = __va(ramdisk_image);
>> +     acpi_initrd_override_find(p, ramdisk_size, false);
>> +#endif
>> +}
>> +#else
>> +void __init x86_acpi_override_find(void) { }
>
> And add a comment here why we're not doing static inline for the dummy
> function?

...

Re: [PATCH 05/14] x86, ACPI: Find acpi tables in initrd early at head_32.S/head64.c

[Tejun Heo]

On Thu, Mar 07, 2013 at 11:02:15PM -0800, Yinghai Lu wrote:
> > Also, does it really have to be called from head_32.S?  No way this
> > can be done after entering C code?  It would be great if you can
> > explain overall design choices in the head message (and important
> > patches).
> 
> have to be with head_32.S and it is with 32bit flat mode, so could access
> 4G blow without setting page table.
> 
> Will try add to more in the change log.

Yes, please.  In the comment too.

-- 
tejun

[PATCH 11/14] x86, acpi, numa: split SLIT handling out

[Yinghai Lu]

We need to handle slit later, as it need to allocate buffer.

Also we only need srat info before init_mem_mapping.

x86_acpi_numa_init become x86_acpi_numa_init_only_slit
x86_acpi_numa_init_no_slit.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: linux-acpi@vger.kernel.org
---
 arch/x86/include/asm/acpi.h |    3 ++-
 arch/x86/mm/numa.c          |   13 ++++++++++++-
 arch/x86/mm/srat.c          |    8 ++++++--
 drivers/acpi/numa.c         |   22 +++++++++++++++++++---
 include/linux/acpi.h        |    2 ++
 5 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index b31bf97..9f171a7 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -178,7 +178,8 @@ static inline void disable_acpi(void) { }
 
 #ifdef CONFIG_ACPI_NUMA
 extern int acpi_numa;
-extern int x86_acpi_numa_init(void);
+int x86_acpi_numa_init_no_slit(void);
+void x86_acpi_numa_init_only_slit(void);
 #endif /* CONFIG_ACPI_NUMA */
 
 #define acpi_unlazy_tlb(x)	leave_mm(x)
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index ace0370..23ec6ba 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -640,6 +640,10 @@ static int __init dummy_numa_init(void)
 	return 0;
 }
 
+#ifdef CONFIG_ACPI_NUMA
+static bool srat_used __initdata;
+#endif
+
 /**
  * x86_numa_init - Initialize NUMA
  *
@@ -658,8 +662,10 @@ void __init x86_numa_init(void)
 			goto out;
 #endif
 #ifdef CONFIG_ACPI_NUMA
-		if (!numa_init(x86_acpi_numa_init))
+		if (!numa_init(x86_acpi_numa_init_no_slit)) {
+			srat_used = true;
 			goto out;
+		}
 #endif
 #ifdef CONFIG_AMD_NUMA
 		if (!numa_init(amd_numa_init))
@@ -670,6 +676,11 @@ void __init x86_numa_init(void)
 	numa_init(dummy_numa_init);
 
 out:
+#ifdef CONFIG_ACPI_NUMA
+	if (srat_used)
+		x86_acpi_numa_init_only_slit();
+#endif
+
 	numa_emulation(&numa_meminfo, numa_distance_cnt);
 
 	for (i = 0; i < mi->nr_blks; i++) {
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index cdd0da9..47a62b2 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -187,11 +187,15 @@ out_err:
 
 void __init acpi_numa_arch_fixup(void) {}
 
-int __init x86_acpi_numa_init(void)
+void __init x86_acpi_numa_init_only_slit(void)
+{
+	acpi_numa_init_only_slit();
+}
+int __init x86_acpi_numa_init_no_slit(void)
 {
 	int ret;
 
-	ret = acpi_numa_init();
+	ret = acpi_numa_init_no_slit();
 	if (ret < 0)
 		return ret;
 	return srat_disabled() ? -EINVAL : 0;
diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
index 33e609f..2215718 100644
--- a/drivers/acpi/numa.c
+++ b/drivers/acpi/numa.c
@@ -282,7 +282,13 @@ acpi_table_parse_srat(enum acpi_srat_type id,
 					    handler, max_entries);
 }
 
-int __init acpi_numa_init(void)
+void __init acpi_numa_init_only_slit(void)
+{
+	/* SLIT: System Locality Information Table */
+	acpi_table_parse(ACPI_SIG_SLIT, acpi_parse_slit);
+}
+
+static int __init __acpi_numa_init(bool with_slit)
 {
 	int cnt = 0;
 
@@ -303,8 +309,8 @@ int __init acpi_numa_init(void)
 					    NR_NODE_MEMBLKS);
 	}
 
-	/* SLIT: System Locality Information Table */
-	acpi_table_parse(ACPI_SIG_SLIT, acpi_parse_slit);
+	if (with_slit)
+		acpi_numa_init_only_slit();
 
 	acpi_numa_arch_fixup();
 
@@ -315,6 +321,16 @@ int __init acpi_numa_init(void)
 	return 0;
 }
 
+int __init acpi_numa_init(void)
+{
+	return __acpi_numa_init(true);
+}
+
+int __init acpi_numa_init_no_slit(void)
+{
+	return __acpi_numa_init(false);
+}
+
 int acpi_get_pxm(acpi_handle h)
 {
 	unsigned long long pxm;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 46a8a89..bfd2852 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -86,6 +86,8 @@ int acpi_boot_init (void);
 void acpi_boot_table_init (void);
 int acpi_mps_check (void);
 int acpi_numa_init (void);
+int acpi_numa_init_no_slit(void);
+void acpi_numa_init_only_slit(void);
 
 int acpi_table_init (void);
 int acpi_table_parse(char *id, acpi_tbl_table_handler handler);
-- 
1.7.10.4

Re: [PATCH 11/14] x86, acpi, numa: split SLIT handling out

[Tejun Heo]

On Thu, Mar 07, 2013 at 08:58:37PM -0800, Yinghai Lu wrote:
> +void __init acpi_numa_init_only_slit(void)
> +{
> +	/* SLIT: System Locality Information Table */
> +	acpi_table_parse(ACPI_SIG_SLIT, acpi_parse_slit);
> +}
> +
> +static int __init __acpi_numa_init(bool with_slit)
>  {
>  	int cnt = 0;

Hmmm.... how about just having the following two functions

	acpi_numa_init_srat();
	acpi_numa_init_slit();

and update both x86 and ia64 to use the two functions?

-- 
tejun

Re: [PATCH 11/14] x86, acpi, numa: split SLIT handling out

[Yinghai Lu]

On Thu, Mar 7, 2013 at 10:46 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 07, 2013 at 08:58:37PM -0800, Yinghai Lu wrote:
>> +void __init acpi_numa_init_only_slit(void)
>> +{
>> +     /* SLIT: System Locality Information Table */
>> +     acpi_table_parse(ACPI_SIG_SLIT, acpi_parse_slit);
>> +}
>> +
>> +static int __init __acpi_numa_init(bool with_slit)
>>  {
>>       int cnt = 0;
>
> Hmmm.... how about just having the following two functions
>
>         acpi_numa_init_srat();
>         acpi_numa_init_slit();

ok.


> and update both x86 and ia64 to use the two functions?

ia64 like to call in this seqence
acpi_numa_init()
parse srat
parse slit
then
acpi_numa_arch_fixup()

in this arch_fixup, it will try to fill dummy distance_matrix.

so would to keep acpi_numa_init ...

Re: [PATCH 11/14] x86, acpi, numa: split SLIT handling out

[Tejun Heo]

On Thu, Mar 7, 2013 at 11:18 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> ia64 like to call in this seqence
> acpi_numa_init()
> parse srat
> parse slit
> then
> acpi_numa_arch_fixup()
>
> in this arch_fixup, it will try to fill dummy distance_matrix.
>
> so would to keep acpi_numa_init ...

Can't it just call acpi_numa_init_srat() and then init_slit()?  What
am I missing?

-- 
tejun

Re: [PATCH 11/14] x86, acpi, numa: split SLIT handling out

[Yinghai Lu]

On Thu, Mar 7, 2013 at 11:19 PM, Tejun Heo <tj@kernel.org> wrote:
> On Thu, Mar 7, 2013 at 11:18 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> ia64 like to call in this seqence
>> acpi_numa_init()
>> parse srat
>> parse slit
>> then
>> acpi_numa_arch_fixup()
>>
>> in this arch_fixup, it will try to fill dummy distance_matrix.
>>
>> so would to keep acpi_numa_init ...
>
> Can't it just call acpi_numa_init_srat() and then init_slit()?  What
> am I missing?

Yes, but need to break acpi_numa_init calling
in arch/ia64/kernel/setup.c

with
acpi_numa_init_srat()
acpi_numa_init_slit()
acpi_numa_arch_fixup()

current code is
acpi_numa_init calling arch_numa_arch_fixup()

Thanks

Yinghai