Change in ...coreboot[master]: mb/emulation/qemu-i440fx: add e820 interface

[Thomas Heijligen (Code Review)]

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30848

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 30 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/30848/1

diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
index c5d6253..1890e66 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
@@ -83,6 +83,30 @@
    return fw_cfg_find_file(file, name);
 }

+static uint32_t fw_cfg_e820_size;
+static uint32_t fw_cfg_e820_pos;
+
+int fw_cfg_e820_select(void)
+{
+	FWCfgFile file;
+
+	if (!fw_cfg_present() || fw_cfg_find_file(&file, "etc/e820"))
+		return -1;
+	fw_cfg_select(file.select);
+	fw_cfg_e820_size = file.size;
+	return 0;
+}
+
+int fw_cfg_e820_read(FwCfgE820Entry *entry)
+{
+	if (fw_cfg_e820_pos + sizeof(*entry) > fw_cfg_e820_size)
+		return -1;
+
+	fw_cfg_read(entry, sizeof(*entry));
+	fw_cfg_e820_pos += sizeof(*entry);
+	return 0;
+}
+
 int fw_cfg_max_cpus(void)
 {
    unsigned short max_cpus;
diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.h b/src/mainboard/emulation/qemu-i440fx/fw_cfg.h
index b5cdb92..eb07847 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.h
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.h
@@ -18,5 +18,11 @@
 int fw_cfg_check_file(FWCfgFile *file, const char *name);
 int fw_cfg_max_cpus(void);
 unsigned long fw_cfg_smbios_tables(int *handle, unsigned long *current);
+/*
+ * Do not call any other fw_cfg_ function between fw_cfg_e820_select and
+ * fw_cfg_e820_read. This causes undefind behavior.
+ */
+int fw_cfg_e820_select(void);
+int fw_cfg_e820_read(FwCfgE820Entry *entry);

#endif /* FW_CFG_H */

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/30848/1/src/mainboard/emulation/qemu-i440fx/... 
File src/mainboard/emulation/qemu-i440fx/fw_cfg.h:

https://review.coreboot.org/#/c/30848/1/src/mainboard/emulation/qemu-i440fx/... 
PS1, Line 22:  * Do not call any other fw_cfg_ function between fw_cfg_e820_select and
That's a bad API design.
Please add a function that returns TOLUD based on e820 tables.

[Thomas Heijligen (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/30848

to look at the new patch set (#3).

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 45 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/30848/3

[Thomas Heijligen (Code Review)]

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/30848/1/src/mainboard/emulation/qemu-i440fx/... 
File src/mainboard/emulation/qemu-i440fx/fw_cfg.h:

https://review.coreboot.org/#/c/30848/1/src/mainboard/emulation/qemu-i440fx/... 
PS1, Line 22:  * Do not call any other fw_cfg_ function between fw_cfg_e820_select and
> That's a bad API design. […]

There is a new function to get top of memory. All read and writes are now done in fw_cfg.c internal.

[Thomas Heijligen (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/30848

to look at the new patch set (#4).

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 38 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/30848/4

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/30848/4/src/mainboard/emulation/qemu-i440fx/... 
File src/mainboard/emulation/qemu-i440fx/fw_cfg.c:

https://review.coreboot.org/#/c/30848/4/src/mainboard/emulation/qemu-i440fx/... 
PS4, Line 96: static int fw_cfg_e820_read(FwCfgE820Entry *entry, uint32_t *size, uint32_t *pos)
line over 80 characters

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 4: Code-Review+1

(3 comments)

https://review.coreboot.org/#/c/30848/4/src/mainboard/emulation/qemu-i440fx/... 
File src/mainboard/emulation/qemu-i440fx/fw_cfg.c:

https://review.coreboot.org/#/c/30848/4/src/mainboard/emulation/qemu-i440fx/... 
PS4, Line 105: 
add description what this functions returns.
What's returned on error.

https://review.coreboot.org/#/c/30848/4/src/mainboard/emulation/qemu-i440fx/... 
PS4, Line 106: unsigned long fw_cfg_cbmem_top(void)
please name it tolud, to make clear that is the top of dram below 4 GiB.
It's not related to cbmem at all.
return uintptr_t

https://review.coreboot.org/#/c/30848/4/src/mainboard/emulation/qemu-i440fx/... 
PS4, Line 114: 			uint64_t limit = (uint64_t)e.address + e.length;
no need for cast. e.address is of type uint64_t

[Thomas Heijligen (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/30848

to look at the new patch set (#5).

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 38 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/30848/5

[Thomas Heijligen (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/30848

to look at the new patch set (#6).

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 39 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/30848/6

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/#/c/30848/6/src/mainboard/emulation/qemu-i440fx/... 
File src/mainboard/emulation/qemu-i440fx/fw_cfg.c:

https://review.coreboot.org/#/c/30848/6/src/mainboard/emulation/qemu-i440fx/... 
PS6, Line 96: static int fw_cfg_e820_read(FwCfgE820Entry *entry, uint32_t *size, uint32_t *pos)
line over 80 characters

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 6: Code-Review+1

What Jenkins says.

[Thomas Heijligen (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/30848

to look at the new patch set (#7).

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 40 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/30848/7

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

Patch Set 8: Code-Review+2

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30848 )

Change subject: mb/emulation/qemu-i440fx: add e820 interface
......................................................................

mb/emulation/qemu-i440fx: add e820 interface

Qemu provides e820 table at fw_cfg interface. Add functions to access it.

Change-Id: I547bc7fef09999baa28149a6325cbca91e31e99b
Signed-off-by: Thomas Heijligen 
Reviewed-on: https://review.coreboot.org/c/30848
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
M src/mainboard/emulation/qemu-i440fx/fw_cfg.h
2 files changed, 40 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
index b859bdb..c5d5a47 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
@@ -83,6 +83,45 @@
    return fw_cfg_find_file(file, name);
 }

+static int fw_cfg_e820_select(uint32_t *size)
+{
+	FWCfgFile file;
+
+	if (!fw_cfg_present() || fw_cfg_find_file(&file, "etc/e820"))
+		return -1;
+	fw_cfg_select(file.select);
+	*size = file.size;
+	return 0;
+}
+
+static int fw_cfg_e820_read(FwCfgE820Entry *entry, uint32_t *size,
+		uint32_t *pos)
+{
+	if (*pos + sizeof(*entry) > *size)
+		return -1;
+
+	fw_cfg_read(entry, sizeof(*entry));
+	*pos += sizeof(*entry);
+	return 0;
+}
+
+/* returns tolud on success or 0 on failure */
+uintptr_t fw_cfg_tolud(void)
+{
+	FwCfgE820Entry e;
+	uint64_t top = 0;
+	uint32_t size = 0, pos = 0;
+
+	if (fw_cfg_e820_select(&size)) {
+		while (!fw_cfg_e820_read(&e, &size, &pos)) {
+			uint64_t limit = e.address + e.length;
+			if (e.type == 1 && limit < 4ULL * GiB && limit > top)
+				top = limit;
+		}
+	}
+	return (uintptr_t)top;
+}
+
 int fw_cfg_max_cpus(void)
 {
    unsigned short max_cpus;
diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.h b/src/mainboard/emulation/qemu-i440fx/fw_cfg.h
index 91b758c..975801b 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.h
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.h
@@ -18,5 +18,6 @@
 int fw_cfg_check_file(FWCfgFile *file, const char *name);
 int fw_cfg_max_cpus(void);
 unsigned long fw_cfg_smbios_tables(int *handle, unsigned long *current);
+uintptr_t fw_cfg_tolud(void);

#endif /* FW_CFG_H */