* [Qemu-devel] [PATCH 01/20] hw/i386/pc: Use unsigned type to index arrays
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 11:57 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array Philippe Mathieu-Daudé
` (19 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 5 +++--
include/hw/i386/pc.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2632b73f80..fc38b59d2d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -870,7 +870,7 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
{
- int index = le32_to_cpu(e820_reserve.count);
+ unsigned int index = le32_to_cpu(e820_reserve.count);
struct e820_entry *entry;
if (type != E820_RAM) {
@@ -902,7 +902,8 @@ int e820_get_num_entries(void)
return e820_entries;
}
-bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length)
+bool e820_get_entry(unsigned int idx, uint32_t type,
+ uint64_t *address, uint64_t *length)
{
if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
*address = le64_to_cpu(e820_table[idx].address);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 43df7230a2..ad3a75d8fa 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -291,7 +291,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
int e820_add_entry(uint64_t, uint64_t, uint32_t);
int e820_get_num_entries(void);
-bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
+bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
extern GlobalProperty pc_compat_4_0[];
extern const size_t pc_compat_4_0_len;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 01/20] hw/i386/pc: Use unsigned type to index arrays
2019-05-24 6:35 ` [Qemu-devel] [PATCH 01/20] hw/i386/pc: Use unsigned type to index arrays Philippe Mathieu-Daudé
@ 2019-05-24 11:57 ` Li Qiang
0 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 11:57 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:40写道:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/pc.c | 5 +++--
> include/hw/i386/pc.h | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2632b73f80..fc38b59d2d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -870,7 +870,7 @@ static void handle_a20_line_change(void *opaque, int
> irq, int level)
>
> int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> {
> - int index = le32_to_cpu(e820_reserve.count);
> + unsigned int index = le32_to_cpu(e820_reserve.count);
> struct e820_entry *entry;
>
> if (type != E820_RAM) {
> @@ -902,7 +902,8 @@ int e820_get_num_entries(void)
> return e820_entries;
> }
>
> -bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t
> *length)
> +bool e820_get_entry(unsigned int idx, uint32_t type,
> + uint64_t *address, uint64_t *length)
> {
> if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
> *address = le64_to_cpu(e820_table[idx].address);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 43df7230a2..ad3a75d8fa 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -291,7 +291,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
>
> int e820_add_entry(uint64_t, uint64_t, uint32_t);
> int e820_get_num_entries(void);
> -bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> +bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
>
> extern GlobalProperty pc_compat_4_0[];
> extern const size_t pc_compat_4_0_len;
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 01/20] hw/i386/pc: Use unsigned type to index arrays Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 11:58 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 03/20] hw/i386/pc: Let e820_add_entry() return a ssize_t type Philippe Mathieu-Daudé
` (18 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 4 ++--
include/hw/i386/pc.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fc38b59d2d..df8600ac24 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -104,7 +104,7 @@ struct e820_table {
static struct e820_table e820_reserve;
static struct e820_entry *e820_table;
-static unsigned e820_entries;
+static size_t e820_entries;
struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
/* Physical Address of PVH entry point read from kernel ELF NOTE */
@@ -897,7 +897,7 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
return e820_entries;
}
-int e820_get_num_entries(void)
+size_t e820_get_num_entries(void)
{
return e820_entries;
}
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ad3a75d8fa..28b19173b0 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -290,7 +290,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
#define E820_UNUSABLE 5
int e820_add_entry(uint64_t, uint64_t, uint32_t);
-int e820_get_num_entries(void);
+size_t e820_get_num_entries(void);
bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
extern GlobalProperty pc_compat_4_0[];
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array
2019-05-24 6:35 ` [Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array Philippe Mathieu-Daudé
@ 2019-05-24 11:58 ` Li Qiang
0 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 11:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:37写道:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/pc.c | 4 ++--
> include/hw/i386/pc.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index fc38b59d2d..df8600ac24 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -104,7 +104,7 @@ struct e820_table {
>
> static struct e820_table e820_reserve;
> static struct e820_entry *e820_table;
> -static unsigned e820_entries;
> +static size_t e820_entries;
> struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>
> /* Physical Address of PVH entry point read from kernel ELF NOTE */
> @@ -897,7 +897,7 @@ int e820_add_entry(uint64_t address, uint64_t length,
> uint32_t type)
> return e820_entries;
> }
>
> -int e820_get_num_entries(void)
> +size_t e820_get_num_entries(void)
> {
> return e820_entries;
> }
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ad3a75d8fa..28b19173b0 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -290,7 +290,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> #define E820_UNUSABLE 5
>
> int e820_add_entry(uint64_t, uint64_t, uint32_t);
> -int e820_get_num_entries(void);
> +size_t e820_get_num_entries(void);
> bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
>
> extern GlobalProperty pc_compat_4_0[];
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 03/20] hw/i386/pc: Let e820_add_entry() return a ssize_t type
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 01/20] hw/i386/pc: Use unsigned type to index arrays Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 12:03 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type Philippe Mathieu-Daudé
` (17 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
e820_add_entry() returns an array size on success, or a negative
value on error.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 2 +-
include/hw/i386/pc.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index df8600ac24..1245028dd6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -868,7 +868,7 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
x86_cpu_set_a20(cpu, level);
}
-int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
+ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
{
unsigned int index = le32_to_cpu(e820_reserve.count);
struct e820_entry *entry;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 28b19173b0..2bc48c03c6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -289,7 +289,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
#define E820_NVS 4
#define E820_UNUSABLE 5
-int e820_add_entry(uint64_t, uint64_t, uint32_t);
+ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
size_t e820_get_num_entries(void);
bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 03/20] hw/i386/pc: Let e820_add_entry() return a ssize_t type
2019-05-24 6:35 ` [Qemu-devel] [PATCH 03/20] hw/i386/pc: Let e820_add_entry() return a ssize_t type Philippe Mathieu-Daudé
@ 2019-05-24 12:03 ` Li Qiang
0 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 12:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:43写道:
> e820_add_entry() returns an array size on success, or a negative
> value on error.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/pc.c | 2 +-
> include/hw/i386/pc.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index df8600ac24..1245028dd6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -868,7 +868,7 @@ static void handle_a20_line_change(void *opaque, int
> irq, int level)
> x86_cpu_set_a20(cpu, level);
> }
>
> -int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> +ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> {
> unsigned int index = le32_to_cpu(e820_reserve.count);
> struct e820_entry *entry;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 28b19173b0..2bc48c03c6 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -289,7 +289,7 @@ void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> #define E820_NVS 4
> #define E820_UNUSABLE 5
>
> -int e820_add_entry(uint64_t, uint64_t, uint32_t);
> +ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
> size_t e820_get_num_entries(void);
> bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
>
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 03/20] hw/i386/pc: Let e820_add_entry() return a ssize_t type Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 12:33 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 05/20] hw/i386/pc: Add documentation to the e820_*() functions Philippe Mathieu-Daudé
` (16 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
This ensure we won't use an incorrect value.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 12 +++++++-----
include/hw/i386/pc.h | 16 ++++++++++------
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 1245028dd6..ac8343c728 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -868,9 +868,10 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
x86_cpu_set_a20(cpu, level);
}
-ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
{
unsigned int index = le32_to_cpu(e820_reserve.count);
+ uint32_t utype = (uint32_t)type;
struct e820_entry *entry;
if (type != E820_RAM) {
@@ -882,7 +883,7 @@ ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
entry->address = cpu_to_le64(address);
entry->length = cpu_to_le64(length);
- entry->type = cpu_to_le32(type);
+ entry->type = cpu_to_le32(utype);
e820_reserve.count = cpu_to_le32(index);
}
@@ -891,7 +892,7 @@ ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
e820_table[e820_entries].address = cpu_to_le64(address);
e820_table[e820_entries].length = cpu_to_le64(length);
- e820_table[e820_entries].type = cpu_to_le32(type);
+ e820_table[e820_entries].type = cpu_to_le32(utype);
e820_entries++;
return e820_entries;
@@ -902,10 +903,11 @@ size_t e820_get_num_entries(void)
return e820_entries;
}
-bool e820_get_entry(unsigned int idx, uint32_t type,
+bool e820_get_entry(unsigned int idx, E820Type type,
uint64_t *address, uint64_t *length)
{
- if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
+ uint32_t utype = (uint32_t)type;
+ if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
*address = le64_to_cpu(e820_table[idx].address);
*length = le64_to_cpu(e820_table[idx].length);
return true;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 2bc48c03c6..10e77a40ce 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -282,12 +282,16 @@ void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
-/* e820 types */
-#define E820_RAM 1
-#define E820_RESERVED 2
-#define E820_ACPI 3
-#define E820_NVS 4
-#define E820_UNUSABLE 5
+/**
+ * E820Type: Type of the e820 address range.
+ */
+typedef enum {
+ E820_RAM = 1,
+ E820_RESERVED = 2,
+ E820_ACPI = 3,
+ E820_NVS = 4,
+ E820_UNUSABLE = 5
+} E820Type;
ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
size_t e820_get_num_entries(void);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type
2019-05-24 6:35 ` [Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type Philippe Mathieu-Daudé
@ 2019-05-24 12:33 ` Li Qiang
2019-05-30 16:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 34+ messages in thread
From: Li Qiang @ 2019-05-24 12:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:45写道:
> This ensure we won't use an incorrect value.
>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/i386/pc.c | 12 +++++++-----
> include/hw/i386/pc.h | 16 ++++++++++------
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 1245028dd6..ac8343c728 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -868,9 +868,10 @@ static void handle_a20_line_change(void *opaque, int
> irq, int level)
> x86_cpu_set_a20(cpu, level);
> }
>
> -ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
> +ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
> {
> unsigned int index = le32_to_cpu(e820_reserve.count);
> + uint32_t utype = (uint32_t)type;
>
I don't have strong opinion for this, as I don't like add an explicit
conversion.
> struct e820_entry *entry;
>
> if (type != E820_RAM) {
> @@ -882,7 +883,7 @@ ssize_t e820_add_entry(uint64_t address, uint64_t
> length, uint32_t type)
>
> entry->address = cpu_to_le64(address);
> entry->length = cpu_to_le64(length);
> - entry->type = cpu_to_le32(type);
> + entry->type = cpu_to_le32(utype);
>
> e820_reserve.count = cpu_to_le32(index);
> }
> @@ -891,7 +892,7 @@ ssize_t e820_add_entry(uint64_t address, uint64_t
> length, uint32_t type)
> e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
> e820_table[e820_entries].address = cpu_to_le64(address);
> e820_table[e820_entries].length = cpu_to_le64(length);
> - e820_table[e820_entries].type = cpu_to_le32(type);
> + e820_table[e820_entries].type = cpu_to_le32(utype);
> e820_entries++;
>
> return e820_entries;
> @@ -902,10 +903,11 @@ size_t e820_get_num_entries(void)
> return e820_entries;
> }
>
> -bool e820_get_entry(unsigned int idx, uint32_t type,
> +bool e820_get_entry(unsigned int idx, E820Type type,
> uint64_t *address, uint64_t *length)
> {
> - if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(type)) {
> + uint32_t utype = (uint32_t)type;
> + if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype))
> {
> *address = le64_to_cpu(e820_table[idx].address);
> *length = le64_to_cpu(e820_table[idx].length);
> return true;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 2bc48c03c6..10e77a40ce 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -282,12 +282,16 @@ void pc_system_firmware_init(PCMachineState *pcms,
> MemoryRegion *rom_memory);
> void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> const CPUArchIdList *apic_ids, GArray *entry);
>
> -/* e820 types */
> -#define E820_RAM 1
> -#define E820_RESERVED 2
> -#define E820_ACPI 3
> -#define E820_NVS 4
> -#define E820_UNUSABLE 5
> +/**
> + * E820Type: Type of the e820 address range.
> + */
> +typedef enum {
> + E820_RAM = 1,
> + E820_RESERVED = 2,
> + E820_ACPI = 3,
> + E820_NVS = 4,
> + E820_UNUSABLE = 5
> +} E820Type;
>
> ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
> size_t e820_get_num_entries(void);
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type
2019-05-24 12:33 ` Li Qiang
@ 2019-05-30 16:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-30 16:34 UTC (permalink / raw)
To: Li Qiang
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Hi Li,
On 5/24/19 2:33 PM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于
> 2019年5月24日周五 下午2:45写道:
>
> This ensure we won't use an incorrect value.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>>
> ---
> hw/i386/pc.c | 12 +++++++-----
> include/hw/i386/pc.h | 16 ++++++++++------
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 1245028dd6..ac8343c728 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -868,9 +868,10 @@ static void handle_a20_line_change(void
> *opaque, int irq, int level)
> x86_cpu_set_a20(cpu, level);
> }
>
> -ssize_t e820_add_entry(uint64_t address, uint64_t length, uint32_t
> type)
> +ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type
> type)
> {
> unsigned int index = le32_to_cpu(e820_reserve.count);
> + uint32_t utype = (uint32_t)type;
>
>
> I don't have strong opinion for this, as I don't like add an explicit
> conversion.
Usually I try to not over-cast, but I guess remember I added that
because some Clang build was failing, but I started a build on Travis-CI
and all passed, so I might have been trying in a local build directory
with stricter CPPFLAGS.
I'll clean that out.
Thanks for your review of this series!
Phil.
> struct e820_entry *entry;
>
> if (type != E820_RAM) {
> @@ -882,7 +883,7 @@ ssize_t e820_add_entry(uint64_t address,
> uint64_t length, uint32_t type)
>
> entry->address = cpu_to_le64(address);
> entry->length = cpu_to_le64(length);
> - entry->type = cpu_to_le32(type);
> + entry->type = cpu_to_le32(utype);
>
> e820_reserve.count = cpu_to_le32(index);
> }
> @@ -891,7 +892,7 @@ ssize_t e820_add_entry(uint64_t address,
> uint64_t length, uint32_t type)
> e820_table = g_renew(struct e820_entry, e820_table,
> e820_entries + 1);
> e820_table[e820_entries].address = cpu_to_le64(address);
> e820_table[e820_entries].length = cpu_to_le64(length);
> - e820_table[e820_entries].type = cpu_to_le32(type);
> + e820_table[e820_entries].type = cpu_to_le32(utype);
> e820_entries++;
>
> return e820_entries;
> @@ -902,10 +903,11 @@ size_t e820_get_num_entries(void)
> return e820_entries;
> }
>
> -bool e820_get_entry(unsigned int idx, uint32_t type,
> +bool e820_get_entry(unsigned int idx, E820Type type,
> uint64_t *address, uint64_t *length)
> {
> - if (idx < e820_entries && e820_table[idx].type ==
> cpu_to_le32(type)) {
> + uint32_t utype = (uint32_t)type;
> + if (idx < e820_entries && e820_table[idx].type ==
> cpu_to_le32(utype)) {
> *address = le64_to_cpu(e820_table[idx].address);
> *length = le64_to_cpu(e820_table[idx].length);
> return true;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 2bc48c03c6..10e77a40ce 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -282,12 +282,16 @@ void pc_system_firmware_init(PCMachineState
> *pcms, MemoryRegion *rom_memory);
> void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> const CPUArchIdList *apic_ids, GArray *entry);
>
> -/* e820 types */
> -#define E820_RAM 1
> -#define E820_RESERVED 2
> -#define E820_ACPI 3
> -#define E820_NVS 4
> -#define E820_UNUSABLE 5
> +/**
> + * E820Type: Type of the e820 address range.
> + */
> +typedef enum {
> + E820_RAM = 1,
> + E820_RESERVED = 2,
> + E820_ACPI = 3,
> + E820_NVS = 4,
> + E820_UNUSABLE = 5
> +} E820Type;
>
> ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
> size_t e820_get_num_entries(void);
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 05/20] hw/i386/pc: Add documentation to the e820_*() functions
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 04/20] hw/i386/pc: Add the E820Type enum type Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 06/20] hw/i386/pc: Use e820_get_num_entries() to access e820_entries Philippe Mathieu-Daudé
` (15 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Philippe Mathieu-Daudé,
Samuel Ortiz
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
include/hw/i386/pc.h | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 10e77a40ce..95bf3278f2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -293,9 +293,42 @@ typedef enum {
E820_UNUSABLE = 5
} E820Type;
-ssize_t e820_add_entry(uint64_t, uint64_t, uint32_t);
+/**
+ * e820_add_entry: Add an #e820_entry to the @e820_table.
+ *
+ * Returns the number of entries of the e820_table on success,
+ * or a negative errno otherwise.
+ *
+ * @address: The base address of the structure which the BIOS is to fill in.
+ * @length: The length in bytes of the structure passed to the BIOS.
+ * @type: The #E820Type of the address range.
+ */
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
+
+/**
+ * e820_get_num_entries: The number of entries of the @e820_table.
+ *
+ * Returns the number of entries of the e820_table.
+ */
size_t e820_get_num_entries(void);
-bool e820_get_entry(unsigned int, uint32_t, uint64_t *, uint64_t *);
+
+/**
+ * e820_get_entry: Get the address/length of an #e820_entry.
+ *
+ * If the #e820_entry stored at @index is of #E820Type @type, fills @address
+ * and @length with the #e820_entry values and return @true.
+ * Return @false otherwise.
+ *
+ * @index: The index of the #e820_entry to get values.
+ * @type: The @E820Type of the address range expected.
+ * @address: Pointer to the base address of the #e820_entry structure to
+ * be filled.
+ * @length: Pointer to the length (in bytes) of the #e820_entry structure
+ * to be filled.
+ * @return: true if the entry was found, false otherwise.
+ */
+bool e820_get_entry(unsigned int index, E820Type type,
+ uint64_t *address, uint64_t *length);
extern GlobalProperty pc_compat_4_0[];
extern const size_t pc_compat_4_0_len;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 06/20] hw/i386/pc: Use e820_get_num_entries() to access e820_entries
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 05/20] hw/i386/pc: Add documentation to the e820_*() functions Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 13:11 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] " Philippe Mathieu-Daudé
` (14 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
To be able to extract the e820* code out of this file (in the next
patch), access e820_entries with its correct helper.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ac8343c728..2e195049a5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1022,7 +1022,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
&e820_reserve, sizeof(e820_reserve));
fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
- sizeof(struct e820_entry) * e820_entries);
+ sizeof(struct e820_entry) * e820_get_num_entries());
fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
/* allocate memory for the NUMA channel: one (64bit) word for the number
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 06/20] hw/i386/pc: Use e820_get_num_entries() to access e820_entries
2019-05-24 6:35 ` [Qemu-devel] [PATCH 06/20] hw/i386/pc: Use e820_get_num_entries() to access e820_entries Philippe Mathieu-Daudé
@ 2019-05-24 13:11 ` Li Qiang
0 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 13:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:48写道:
> To be able to extract the e820* code out of this file (in the next
> patch), access e820_entries with its correct helper.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/pc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index ac8343c728..2e195049a5 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1022,7 +1022,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as,
> PCMachineState *pcms)
> fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
> &e820_reserve, sizeof(e820_reserve));
> fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
> - sizeof(struct e820_entry) * e820_entries);
> + sizeof(struct e820_entry) * e820_get_num_entries());
>
> fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
> /* allocate memory for the NUMA channel: one (64bit) word for the
> number
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 07/20] hw/i386/pc: Extract e820 memory layout code
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 02/20] hw/i386/pc: Use size_t type to hold/return a size of array Philippe Mathieu-Daudé
` (19 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Samuel Ortiz, Yang Zhong, Paolo Bonzini,
Marcelo Tosatti, Rob Bradford, Michael S. Tsirkin,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
Richard Henderson, open list:X86
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/Makefile.objs | 2 +-
hw/i386/e820_memory_layout.c | 62 +++++++++++++++++++++++++++++
hw/i386/e820_memory_layout.h | 76 ++++++++++++++++++++++++++++++++++++
hw/i386/pc.c | 64 +-----------------------------
include/hw/i386/pc.h | 48 -----------------------
target/i386/kvm.c | 1 +
6 files changed, 141 insertions(+), 112 deletions(-)
create mode 100644 hw/i386/e820_memory_layout.c
create mode 100644 hw/i386/e820_memory_layout.h
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 5d9c9efd5f..d3374e0831 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -1,5 +1,5 @@
obj-$(CONFIG_KVM) += kvm/
-obj-y += multiboot.o
+obj-y += e820_memory_layout.o multiboot.o
obj-y += pc.o
obj-$(CONFIG_I440FX) += pc_piix.o
obj-$(CONFIG_Q35) += pc_q35.o
diff --git a/hw/i386/e820_memory_layout.c b/hw/i386/e820_memory_layout.c
new file mode 100644
index 0000000000..b9be08536c
--- /dev/null
+++ b/hw/i386/e820_memory_layout.c
@@ -0,0 +1,62 @@
+/*
+ * QEMU BIOS e820 routines
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "e820_memory_layout.h"
+
+static size_t e820_entries;
+struct e820_table e820_reserve;
+struct e820_entry *e820_table;
+
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
+{
+ unsigned int index = le32_to_cpu(e820_reserve.count);
+ uint32_t utype = (uint32_t)type;
+ struct e820_entry *entry;
+
+ if (type != E820_RAM) {
+ /* old FW_CFG_E820_TABLE entry -- reservations only */
+ if (index >= E820_NR_ENTRIES) {
+ return -EBUSY;
+ }
+ entry = &e820_reserve.entry[index++];
+
+ entry->address = cpu_to_le64(address);
+ entry->length = cpu_to_le64(length);
+ entry->type = cpu_to_le32(utype);
+
+ e820_reserve.count = cpu_to_le32(index);
+ }
+
+ /* new "etc/e820" file -- include ram too */
+ e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
+ e820_table[e820_entries].address = cpu_to_le64(address);
+ e820_table[e820_entries].length = cpu_to_le64(length);
+ e820_table[e820_entries].type = cpu_to_le32(utype);
+ e820_entries++;
+
+ return e820_entries;
+}
+
+size_t e820_get_num_entries(void)
+{
+ return e820_entries;
+}
+
+bool e820_get_entry(unsigned int idx, E820Type type,
+ uint64_t *address, uint64_t *length)
+{
+ uint32_t utype = (uint32_t)type;
+ if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
+ *address = le64_to_cpu(e820_table[idx].address);
+ *length = le64_to_cpu(e820_table[idx].length);
+ return true;
+ }
+ return false;
+}
diff --git a/hw/i386/e820_memory_layout.h b/hw/i386/e820_memory_layout.h
new file mode 100644
index 0000000000..64e88e4772
--- /dev/null
+++ b/hw/i386/e820_memory_layout.h
@@ -0,0 +1,76 @@
+/*
+ * QEMU BIOS e820 routines
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef HW_I386_E820_H
+#define HW_I386_E820_H
+
+/**
+ * E820Type: Type of the e820 address range.
+ */
+typedef enum {
+ E820_RAM = 1,
+ E820_RESERVED = 2,
+ E820_ACPI = 3,
+ E820_NVS = 4,
+ E820_UNUSABLE = 5
+} E820Type;
+
+#define E820_NR_ENTRIES 16
+
+struct e820_entry {
+ uint64_t address;
+ uint64_t length;
+ uint32_t type;
+} QEMU_PACKED __attribute((__aligned__(4)));
+
+struct e820_table {
+ uint32_t count;
+ struct e820_entry entry[E820_NR_ENTRIES];
+} QEMU_PACKED __attribute((__aligned__(4)));
+
+extern struct e820_table e820_reserve;
+extern struct e820_entry *e820_table;
+
+/**
+ * e820_add_entry: Add an #e820_entry to the @e820_table.
+ *
+ * Returns the number of entries of the e820_table on success,
+ * or a negative errno otherwise.
+ *
+ * @address: The base address of the structure which the BIOS is to fill in.
+ * @length: The length in bytes of the structure passed to the BIOS.
+ * @type: The #E820Type of the address range.
+ */
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
+
+/**
+ * e820_get_num_entries: The number of entries of the @e820_table.
+ *
+ * Returns the number of entries of the e820_table.
+ */
+size_t e820_get_num_entries(void);
+
+/**
+ * e820_get_entry: Get the address/length of an #e820_entry.
+ *
+ * If the #e820_entry stored at @index is of #E820Type @type, fills @address
+ * and @length with the #e820_entry values and return @true.
+ * Return @false otherwise.
+ *
+ * @index: The index of the #e820_entry to get values.
+ * @type: The @E820Type of the address range expected.
+ * @address: Pointer to the base address of the #e820_entry structure to
+ * be filled.
+ * @length: Pointer to the length (in bytes) of the #e820_entry structure
+ * to be filled.
+ * @return: true if the entry was found, false otherwise.
+ */
+bool e820_get_entry(unsigned int index, E820Type type,
+ uint64_t *address, uint64_t *length);
+
+#endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2e195049a5..fc22779ac1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,6 +78,7 @@
#include "hw/i386/intel_iommu.h"
#include "hw/net/ne2000-isa.h"
#include "standard-headers/asm-x86/bootparam.h"
+#include "e820_memory_layout.h"
/* debug PC/ISA interrupts */
//#define DEBUG_IRQ
@@ -89,22 +90,6 @@
#define DPRINTF(fmt, ...)
#endif
-#define E820_NR_ENTRIES 16
-
-struct e820_entry {
- uint64_t address;
- uint64_t length;
- uint32_t type;
-} QEMU_PACKED __attribute((__aligned__(4)));
-
-struct e820_table {
- uint32_t count;
- struct e820_entry entry[E820_NR_ENTRIES];
-} QEMU_PACKED __attribute((__aligned__(4)));
-
-static struct e820_table e820_reserve;
-static struct e820_entry *e820_table;
-static size_t e820_entries;
struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
/* Physical Address of PVH entry point read from kernel ELF NOTE */
@@ -868,53 +853,6 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
x86_cpu_set_a20(cpu, level);
}
-ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
-{
- unsigned int index = le32_to_cpu(e820_reserve.count);
- uint32_t utype = (uint32_t)type;
- struct e820_entry *entry;
-
- if (type != E820_RAM) {
- /* old FW_CFG_E820_TABLE entry -- reservations only */
- if (index >= E820_NR_ENTRIES) {
- return -EBUSY;
- }
- entry = &e820_reserve.entry[index++];
-
- entry->address = cpu_to_le64(address);
- entry->length = cpu_to_le64(length);
- entry->type = cpu_to_le32(utype);
-
- e820_reserve.count = cpu_to_le32(index);
- }
-
- /* new "etc/e820" file -- include ram too */
- e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
- e820_table[e820_entries].address = cpu_to_le64(address);
- e820_table[e820_entries].length = cpu_to_le64(length);
- e820_table[e820_entries].type = cpu_to_le32(utype);
- e820_entries++;
-
- return e820_entries;
-}
-
-size_t e820_get_num_entries(void)
-{
- return e820_entries;
-}
-
-bool e820_get_entry(unsigned int idx, E820Type type,
- uint64_t *address, uint64_t *length)
-{
- uint32_t utype = (uint32_t)type;
- if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
- *address = le64_to_cpu(e820_table[idx].address);
- *length = le64_to_cpu(e820_table[idx].length);
- return true;
- }
- return false;
-}
-
/* Enables contiguous-apic-ID mode, for compatibility */
static bool compat_apic_id_mode;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 95bf3278f2..0f1bf667ae 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -282,54 +282,6 @@ void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
-/**
- * E820Type: Type of the e820 address range.
- */
-typedef enum {
- E820_RAM = 1,
- E820_RESERVED = 2,
- E820_ACPI = 3,
- E820_NVS = 4,
- E820_UNUSABLE = 5
-} E820Type;
-
-/**
- * e820_add_entry: Add an #e820_entry to the @e820_table.
- *
- * Returns the number of entries of the e820_table on success,
- * or a negative errno otherwise.
- *
- * @address: The base address of the structure which the BIOS is to fill in.
- * @length: The length in bytes of the structure passed to the BIOS.
- * @type: The #E820Type of the address range.
- */
-ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
-
-/**
- * e820_get_num_entries: The number of entries of the @e820_table.
- *
- * Returns the number of entries of the e820_table.
- */
-size_t e820_get_num_entries(void);
-
-/**
- * e820_get_entry: Get the address/length of an #e820_entry.
- *
- * If the #e820_entry stored at @index is of #E820Type @type, fills @address
- * and @length with the #e820_entry values and return @true.
- * Return @false otherwise.
- *
- * @index: The index of the #e820_entry to get values.
- * @type: The @E820Type of the address range expected.
- * @address: Pointer to the base address of the #e820_entry structure to
- * be filled.
- * @length: Pointer to the length (in bytes) of the #e820_entry structure
- * to be filled.
- * @return: true if the entry was found, false otherwise.
- */
-bool e820_get_entry(unsigned int index, E820Type type,
- uint64_t *address, uint64_t *length);
-
extern GlobalProperty pc_compat_4_0[];
extern const size_t pc_compat_4_0_len;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b29ce5c0d..dbf890005e 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -39,6 +39,7 @@
#include "hw/i386/apic-msidef.h"
#include "hw/i386/intel_iommu.h"
#include "hw/i386/x86-iommu.h"
+#include "hw/i386/e820_memory_layout.h"
#include "hw/pci/pci.h"
#include "hw/pci/msi.h"
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 07/20] hw/i386/pc: Extract e820 memory layout code
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, open list:X86, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/Makefile.objs | 2 +-
hw/i386/e820_memory_layout.c | 62 +++++++++++++++++++++++++++++
hw/i386/e820_memory_layout.h | 76 ++++++++++++++++++++++++++++++++++++
hw/i386/pc.c | 64 +-----------------------------
include/hw/i386/pc.h | 48 -----------------------
target/i386/kvm.c | 1 +
6 files changed, 141 insertions(+), 112 deletions(-)
create mode 100644 hw/i386/e820_memory_layout.c
create mode 100644 hw/i386/e820_memory_layout.h
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 5d9c9efd5f..d3374e0831 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -1,5 +1,5 @@
obj-$(CONFIG_KVM) += kvm/
-obj-y += multiboot.o
+obj-y += e820_memory_layout.o multiboot.o
obj-y += pc.o
obj-$(CONFIG_I440FX) += pc_piix.o
obj-$(CONFIG_Q35) += pc_q35.o
diff --git a/hw/i386/e820_memory_layout.c b/hw/i386/e820_memory_layout.c
new file mode 100644
index 0000000000..b9be08536c
--- /dev/null
+++ b/hw/i386/e820_memory_layout.c
@@ -0,0 +1,62 @@
+/*
+ * QEMU BIOS e820 routines
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "e820_memory_layout.h"
+
+static size_t e820_entries;
+struct e820_table e820_reserve;
+struct e820_entry *e820_table;
+
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
+{
+ unsigned int index = le32_to_cpu(e820_reserve.count);
+ uint32_t utype = (uint32_t)type;
+ struct e820_entry *entry;
+
+ if (type != E820_RAM) {
+ /* old FW_CFG_E820_TABLE entry -- reservations only */
+ if (index >= E820_NR_ENTRIES) {
+ return -EBUSY;
+ }
+ entry = &e820_reserve.entry[index++];
+
+ entry->address = cpu_to_le64(address);
+ entry->length = cpu_to_le64(length);
+ entry->type = cpu_to_le32(utype);
+
+ e820_reserve.count = cpu_to_le32(index);
+ }
+
+ /* new "etc/e820" file -- include ram too */
+ e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
+ e820_table[e820_entries].address = cpu_to_le64(address);
+ e820_table[e820_entries].length = cpu_to_le64(length);
+ e820_table[e820_entries].type = cpu_to_le32(utype);
+ e820_entries++;
+
+ return e820_entries;
+}
+
+size_t e820_get_num_entries(void)
+{
+ return e820_entries;
+}
+
+bool e820_get_entry(unsigned int idx, E820Type type,
+ uint64_t *address, uint64_t *length)
+{
+ uint32_t utype = (uint32_t)type;
+ if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
+ *address = le64_to_cpu(e820_table[idx].address);
+ *length = le64_to_cpu(e820_table[idx].length);
+ return true;
+ }
+ return false;
+}
diff --git a/hw/i386/e820_memory_layout.h b/hw/i386/e820_memory_layout.h
new file mode 100644
index 0000000000..64e88e4772
--- /dev/null
+++ b/hw/i386/e820_memory_layout.h
@@ -0,0 +1,76 @@
+/*
+ * QEMU BIOS e820 routines
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef HW_I386_E820_H
+#define HW_I386_E820_H
+
+/**
+ * E820Type: Type of the e820 address range.
+ */
+typedef enum {
+ E820_RAM = 1,
+ E820_RESERVED = 2,
+ E820_ACPI = 3,
+ E820_NVS = 4,
+ E820_UNUSABLE = 5
+} E820Type;
+
+#define E820_NR_ENTRIES 16
+
+struct e820_entry {
+ uint64_t address;
+ uint64_t length;
+ uint32_t type;
+} QEMU_PACKED __attribute((__aligned__(4)));
+
+struct e820_table {
+ uint32_t count;
+ struct e820_entry entry[E820_NR_ENTRIES];
+} QEMU_PACKED __attribute((__aligned__(4)));
+
+extern struct e820_table e820_reserve;
+extern struct e820_entry *e820_table;
+
+/**
+ * e820_add_entry: Add an #e820_entry to the @e820_table.
+ *
+ * Returns the number of entries of the e820_table on success,
+ * or a negative errno otherwise.
+ *
+ * @address: The base address of the structure which the BIOS is to fill in.
+ * @length: The length in bytes of the structure passed to the BIOS.
+ * @type: The #E820Type of the address range.
+ */
+ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
+
+/**
+ * e820_get_num_entries: The number of entries of the @e820_table.
+ *
+ * Returns the number of entries of the e820_table.
+ */
+size_t e820_get_num_entries(void);
+
+/**
+ * e820_get_entry: Get the address/length of an #e820_entry.
+ *
+ * If the #e820_entry stored at @index is of #E820Type @type, fills @address
+ * and @length with the #e820_entry values and return @true.
+ * Return @false otherwise.
+ *
+ * @index: The index of the #e820_entry to get values.
+ * @type: The @E820Type of the address range expected.
+ * @address: Pointer to the base address of the #e820_entry structure to
+ * be filled.
+ * @length: Pointer to the length (in bytes) of the #e820_entry structure
+ * to be filled.
+ * @return: true if the entry was found, false otherwise.
+ */
+bool e820_get_entry(unsigned int index, E820Type type,
+ uint64_t *address, uint64_t *length);
+
+#endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2e195049a5..fc22779ac1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,6 +78,7 @@
#include "hw/i386/intel_iommu.h"
#include "hw/net/ne2000-isa.h"
#include "standard-headers/asm-x86/bootparam.h"
+#include "e820_memory_layout.h"
/* debug PC/ISA interrupts */
//#define DEBUG_IRQ
@@ -89,22 +90,6 @@
#define DPRINTF(fmt, ...)
#endif
-#define E820_NR_ENTRIES 16
-
-struct e820_entry {
- uint64_t address;
- uint64_t length;
- uint32_t type;
-} QEMU_PACKED __attribute((__aligned__(4)));
-
-struct e820_table {
- uint32_t count;
- struct e820_entry entry[E820_NR_ENTRIES];
-} QEMU_PACKED __attribute((__aligned__(4)));
-
-static struct e820_table e820_reserve;
-static struct e820_entry *e820_table;
-static size_t e820_entries;
struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
/* Physical Address of PVH entry point read from kernel ELF NOTE */
@@ -868,53 +853,6 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
x86_cpu_set_a20(cpu, level);
}
-ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
-{
- unsigned int index = le32_to_cpu(e820_reserve.count);
- uint32_t utype = (uint32_t)type;
- struct e820_entry *entry;
-
- if (type != E820_RAM) {
- /* old FW_CFG_E820_TABLE entry -- reservations only */
- if (index >= E820_NR_ENTRIES) {
- return -EBUSY;
- }
- entry = &e820_reserve.entry[index++];
-
- entry->address = cpu_to_le64(address);
- entry->length = cpu_to_le64(length);
- entry->type = cpu_to_le32(utype);
-
- e820_reserve.count = cpu_to_le32(index);
- }
-
- /* new "etc/e820" file -- include ram too */
- e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
- e820_table[e820_entries].address = cpu_to_le64(address);
- e820_table[e820_entries].length = cpu_to_le64(length);
- e820_table[e820_entries].type = cpu_to_le32(utype);
- e820_entries++;
-
- return e820_entries;
-}
-
-size_t e820_get_num_entries(void)
-{
- return e820_entries;
-}
-
-bool e820_get_entry(unsigned int idx, E820Type type,
- uint64_t *address, uint64_t *length)
-{
- uint32_t utype = (uint32_t)type;
- if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype)) {
- *address = le64_to_cpu(e820_table[idx].address);
- *length = le64_to_cpu(e820_table[idx].length);
- return true;
- }
- return false;
-}
-
/* Enables contiguous-apic-ID mode, for compatibility */
static bool compat_apic_id_mode;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 95bf3278f2..0f1bf667ae 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -282,54 +282,6 @@ void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
-/**
- * E820Type: Type of the e820 address range.
- */
-typedef enum {
- E820_RAM = 1,
- E820_RESERVED = 2,
- E820_ACPI = 3,
- E820_NVS = 4,
- E820_UNUSABLE = 5
-} E820Type;
-
-/**
- * e820_add_entry: Add an #e820_entry to the @e820_table.
- *
- * Returns the number of entries of the e820_table on success,
- * or a negative errno otherwise.
- *
- * @address: The base address of the structure which the BIOS is to fill in.
- * @length: The length in bytes of the structure passed to the BIOS.
- * @type: The #E820Type of the address range.
- */
-ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
-
-/**
- * e820_get_num_entries: The number of entries of the @e820_table.
- *
- * Returns the number of entries of the e820_table.
- */
-size_t e820_get_num_entries(void);
-
-/**
- * e820_get_entry: Get the address/length of an #e820_entry.
- *
- * If the #e820_entry stored at @index is of #E820Type @type, fills @address
- * and @length with the #e820_entry values and return @true.
- * Return @false otherwise.
- *
- * @index: The index of the #e820_entry to get values.
- * @type: The @E820Type of the address range expected.
- * @address: Pointer to the base address of the #e820_entry structure to
- * be filled.
- * @length: Pointer to the length (in bytes) of the #e820_entry structure
- * to be filled.
- * @return: true if the entry was found, false otherwise.
- */
-bool e820_get_entry(unsigned int index, E820Type type,
- uint64_t *address, uint64_t *length);
-
extern GlobalProperty pc_compat_4_0[];
extern const size_t pc_compat_4_0_len;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b29ce5c0d..dbf890005e 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -39,6 +39,7 @@
#include "hw/i386/apic-msidef.h"
#include "hw/i386/intel_iommu.h"
#include "hw/i386/x86-iommu.h"
+#include "hw/i386/e820_memory_layout.h"
#include "hw/pci/pci.h"
#include "hw/pci/msi.h"
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 07/20] hw/i386/pc: Extract e820 memory layout code
2019-05-24 6:35 ` [Qemu-devel] " Philippe Mathieu-Daudé
(?)
@ 2019-05-24 13:13 ` Li Qiang
-1 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 13:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Samuel Ortiz, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, open list:X86, Paolo Bonzini,
Richard Henderson, Eduardo Habkost
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:41写道:
> Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/Makefile.objs | 2 +-
> hw/i386/e820_memory_layout.c | 62 +++++++++++++++++++++++++++++
> hw/i386/e820_memory_layout.h | 76 ++++++++++++++++++++++++++++++++++++
> hw/i386/pc.c | 64 +-----------------------------
> include/hw/i386/pc.h | 48 -----------------------
> target/i386/kvm.c | 1 +
> 6 files changed, 141 insertions(+), 112 deletions(-)
> create mode 100644 hw/i386/e820_memory_layout.c
> create mode 100644 hw/i386/e820_memory_layout.h
>
> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
> index 5d9c9efd5f..d3374e0831 100644
> --- a/hw/i386/Makefile.objs
> +++ b/hw/i386/Makefile.objs
> @@ -1,5 +1,5 @@
> obj-$(CONFIG_KVM) += kvm/
> -obj-y += multiboot.o
> +obj-y += e820_memory_layout.o multiboot.o
> obj-y += pc.o
> obj-$(CONFIG_I440FX) += pc_piix.o
> obj-$(CONFIG_Q35) += pc_q35.o
> diff --git a/hw/i386/e820_memory_layout.c b/hw/i386/e820_memory_layout.c
> new file mode 100644
> index 0000000000..b9be08536c
> --- /dev/null
> +++ b/hw/i386/e820_memory_layout.c
> @@ -0,0 +1,62 @@
> +/*
> + * QEMU BIOS e820 routines
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/bswap.h"
> +#include "e820_memory_layout.h"
> +
> +static size_t e820_entries;
> +struct e820_table e820_reserve;
> +struct e820_entry *e820_table;
> +
> +ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
> +{
> + unsigned int index = le32_to_cpu(e820_reserve.count);
> + uint32_t utype = (uint32_t)type;
> + struct e820_entry *entry;
> +
> + if (type != E820_RAM) {
> + /* old FW_CFG_E820_TABLE entry -- reservations only */
> + if (index >= E820_NR_ENTRIES) {
> + return -EBUSY;
> + }
> + entry = &e820_reserve.entry[index++];
> +
> + entry->address = cpu_to_le64(address);
> + entry->length = cpu_to_le64(length);
> + entry->type = cpu_to_le32(utype);
> +
> + e820_reserve.count = cpu_to_le32(index);
> + }
> +
> + /* new "etc/e820" file -- include ram too */
> + e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
> + e820_table[e820_entries].address = cpu_to_le64(address);
> + e820_table[e820_entries].length = cpu_to_le64(length);
> + e820_table[e820_entries].type = cpu_to_le32(utype);
> + e820_entries++;
> +
> + return e820_entries;
> +}
> +
> +size_t e820_get_num_entries(void)
> +{
> + return e820_entries;
> +}
> +
> +bool e820_get_entry(unsigned int idx, E820Type type,
> + uint64_t *address, uint64_t *length)
> +{
> + uint32_t utype = (uint32_t)type;
> + if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype))
> {
> + *address = le64_to_cpu(e820_table[idx].address);
> + *length = le64_to_cpu(e820_table[idx].length);
> + return true;
> + }
> + return false;
> +}
> diff --git a/hw/i386/e820_memory_layout.h b/hw/i386/e820_memory_layout.h
> new file mode 100644
> index 0000000000..64e88e4772
> --- /dev/null
> +++ b/hw/i386/e820_memory_layout.h
> @@ -0,0 +1,76 @@
> +/*
> + * QEMU BIOS e820 routines
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#ifndef HW_I386_E820_H
> +#define HW_I386_E820_H
> +
> +/**
> + * E820Type: Type of the e820 address range.
> + */
> +typedef enum {
> + E820_RAM = 1,
> + E820_RESERVED = 2,
> + E820_ACPI = 3,
> + E820_NVS = 4,
> + E820_UNUSABLE = 5
> +} E820Type;
> +
> +#define E820_NR_ENTRIES 16
> +
> +struct e820_entry {
> + uint64_t address;
> + uint64_t length;
> + uint32_t type;
> +} QEMU_PACKED __attribute((__aligned__(4)));
> +
> +struct e820_table {
> + uint32_t count;
> + struct e820_entry entry[E820_NR_ENTRIES];
> +} QEMU_PACKED __attribute((__aligned__(4)));
> +
> +extern struct e820_table e820_reserve;
> +extern struct e820_entry *e820_table;
> +
> +/**
> + * e820_add_entry: Add an #e820_entry to the @e820_table.
> + *
> + * Returns the number of entries of the e820_table on success,
> + * or a negative errno otherwise.
> + *
> + * @address: The base address of the structure which the BIOS is to fill
> in.
> + * @length: The length in bytes of the structure passed to the BIOS.
> + * @type: The #E820Type of the address range.
> + */
> +ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
> +
> +/**
> + * e820_get_num_entries: The number of entries of the @e820_table.
> + *
> + * Returns the number of entries of the e820_table.
> + */
> +size_t e820_get_num_entries(void);
> +
> +/**
> + * e820_get_entry: Get the address/length of an #e820_entry.
> + *
> + * If the #e820_entry stored at @index is of #E820Type @type, fills
> @address
> + * and @length with the #e820_entry values and return @true.
> + * Return @false otherwise.
> + *
> + * @index: The index of the #e820_entry to get values.
> + * @type: The @E820Type of the address range expected.
> + * @address: Pointer to the base address of the #e820_entry structure to
> + * be filled.
> + * @length: Pointer to the length (in bytes) of the #e820_entry structure
> + * to be filled.
> + * @return: true if the entry was found, false otherwise.
> + */
> +bool e820_get_entry(unsigned int index, E820Type type,
> + uint64_t *address, uint64_t *length);
> +
> +#endif
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2e195049a5..fc22779ac1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -78,6 +78,7 @@
> #include "hw/i386/intel_iommu.h"
> #include "hw/net/ne2000-isa.h"
> #include "standard-headers/asm-x86/bootparam.h"
> +#include "e820_memory_layout.h"
>
> /* debug PC/ISA interrupts */
> //#define DEBUG_IRQ
> @@ -89,22 +90,6 @@
> #define DPRINTF(fmt, ...)
> #endif
>
> -#define E820_NR_ENTRIES 16
> -
> -struct e820_entry {
> - uint64_t address;
> - uint64_t length;
> - uint32_t type;
> -} QEMU_PACKED __attribute((__aligned__(4)));
> -
> -struct e820_table {
> - uint32_t count;
> - struct e820_entry entry[E820_NR_ENTRIES];
> -} QEMU_PACKED __attribute((__aligned__(4)));
> -
> -static struct e820_table e820_reserve;
> -static struct e820_entry *e820_table;
> -static size_t e820_entries;
> struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
>
> /* Physical Address of PVH entry point read from kernel ELF NOTE */
> @@ -868,53 +853,6 @@ static void handle_a20_line_change(void *opaque, int
> irq, int level)
> x86_cpu_set_a20(cpu, level);
> }
>
> -ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type)
> -{
> - unsigned int index = le32_to_cpu(e820_reserve.count);
> - uint32_t utype = (uint32_t)type;
> - struct e820_entry *entry;
> -
> - if (type != E820_RAM) {
> - /* old FW_CFG_E820_TABLE entry -- reservations only */
> - if (index >= E820_NR_ENTRIES) {
> - return -EBUSY;
> - }
> - entry = &e820_reserve.entry[index++];
> -
> - entry->address = cpu_to_le64(address);
> - entry->length = cpu_to_le64(length);
> - entry->type = cpu_to_le32(utype);
> -
> - e820_reserve.count = cpu_to_le32(index);
> - }
> -
> - /* new "etc/e820" file -- include ram too */
> - e820_table = g_renew(struct e820_entry, e820_table, e820_entries + 1);
> - e820_table[e820_entries].address = cpu_to_le64(address);
> - e820_table[e820_entries].length = cpu_to_le64(length);
> - e820_table[e820_entries].type = cpu_to_le32(utype);
> - e820_entries++;
> -
> - return e820_entries;
> -}
> -
> -size_t e820_get_num_entries(void)
> -{
> - return e820_entries;
> -}
> -
> -bool e820_get_entry(unsigned int idx, E820Type type,
> - uint64_t *address, uint64_t *length)
> -{
> - uint32_t utype = (uint32_t)type;
> - if (idx < e820_entries && e820_table[idx].type == cpu_to_le32(utype))
> {
> - *address = le64_to_cpu(e820_table[idx].address);
> - *length = le64_to_cpu(e820_table[idx].length);
> - return true;
> - }
> - return false;
> -}
> -
> /* Enables contiguous-apic-ID mode, for compatibility */
> static bool compat_apic_id_mode;
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 95bf3278f2..0f1bf667ae 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -282,54 +282,6 @@ void pc_system_firmware_init(PCMachineState *pcms,
> MemoryRegion *rom_memory);
> void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
> const CPUArchIdList *apic_ids, GArray *entry);
>
> -/**
> - * E820Type: Type of the e820 address range.
> - */
> -typedef enum {
> - E820_RAM = 1,
> - E820_RESERVED = 2,
> - E820_ACPI = 3,
> - E820_NVS = 4,
> - E820_UNUSABLE = 5
> -} E820Type;
> -
> -/**
> - * e820_add_entry: Add an #e820_entry to the @e820_table.
> - *
> - * Returns the number of entries of the e820_table on success,
> - * or a negative errno otherwise.
> - *
> - * @address: The base address of the structure which the BIOS is to fill
> in.
> - * @length: The length in bytes of the structure passed to the BIOS.
> - * @type: The #E820Type of the address range.
> - */
> -ssize_t e820_add_entry(uint64_t address, uint64_t length, E820Type type);
> -
> -/**
> - * e820_get_num_entries: The number of entries of the @e820_table.
> - *
> - * Returns the number of entries of the e820_table.
> - */
> -size_t e820_get_num_entries(void);
> -
> -/**
> - * e820_get_entry: Get the address/length of an #e820_entry.
> - *
> - * If the #e820_entry stored at @index is of #E820Type @type, fills
> @address
> - * and @length with the #e820_entry values and return @true.
> - * Return @false otherwise.
> - *
> - * @index: The index of the #e820_entry to get values.
> - * @type: The @E820Type of the address range expected.
> - * @address: Pointer to the base address of the #e820_entry structure to
> - * be filled.
> - * @length: Pointer to the length (in bytes) of the #e820_entry structure
> - * to be filled.
> - * @return: true if the entry was found, false otherwise.
> - */
> -bool e820_get_entry(unsigned int index, E820Type type,
> - uint64_t *address, uint64_t *length);
> -
> extern GlobalProperty pc_compat_4_0[];
> extern const size_t pc_compat_4_0_len;
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 3b29ce5c0d..dbf890005e 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -39,6 +39,7 @@
> #include "hw/i386/apic-msidef.h"
> #include "hw/i386/intel_iommu.h"
> #include "hw/i386/x86-iommu.h"
> +#include "hw/i386/e820_memory_layout.h"
>
> #include "hw/pci/pci.h"
> #include "hw/pci/msi.h"
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 08/20] hw/i386/pc: Use address_space_memory in place
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] " Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 13:22 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 09/20] hw/i386/pc: Rename bochs_bios_init() more generic as x86_create_fw_cfg() Philippe Mathieu-Daudé
` (12 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
The address_space_memory variable is used once.
Use it in place and remove the argument.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fc22779ac1..a3936bb29d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
}
}
-static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
+static FWCfgState *bochs_bios_init(PCMachineState *pcms)
{
FWCfgState *fw_cfg;
uint64_t *numa_fw_cfg;
@@ -936,7 +936,8 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
const CPUArchIdList *cpus;
MachineClass *mc = MACHINE_GET_CLASS(pcms);
- fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4, as);
+ fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
+ &address_space_memory);
fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
/* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
@@ -1761,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = bochs_bios_init(&address_space_memory, pcms);
+ fw_cfg = bochs_bios_init(pcms);
rom_set_fw(fw_cfg);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 08/20] hw/i386/pc: Use address_space_memory in place
2019-05-24 6:35 ` [Qemu-devel] [PATCH 08/20] hw/i386/pc: Use address_space_memory in place Philippe Mathieu-Daudé
@ 2019-05-24 13:22 ` Li Qiang
0 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 13:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:38写道:
> The address_space_memory variable is used once.
> Use it in place and remove the argument.
>
> Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/pc.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index fc22779ac1..a3936bb29d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
> }
> }
>
> -static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
> +static FWCfgState *bochs_bios_init(PCMachineState *pcms)
> {
> FWCfgState *fw_cfg;
> uint64_t *numa_fw_cfg;
> @@ -936,7 +936,8 @@ static FWCfgState *bochs_bios_init(AddressSpace *as,
> PCMachineState *pcms)
> const CPUArchIdList *cpus;
> MachineClass *mc = MACHINE_GET_CLASS(pcms);
>
> - fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4, as);
> + fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
> + &address_space_memory);
> fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
>
> /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
> @@ -1761,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
> option_rom_mr,
> 1);
>
> - fw_cfg = bochs_bios_init(&address_space_memory, pcms);
> + fw_cfg = bochs_bios_init(pcms);
>
> rom_set_fw(fw_cfg);
>
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 09/20] hw/i386/pc: Rename bochs_bios_init() more generic as x86_create_fw_cfg()
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 08/20] hw/i386/pc: Use address_space_memory in place Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 13:26 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument Philippe Mathieu-Daudé
` (11 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
The bochs_bios_init() is not restricted to the Bochs BIOS and is
useful to other BIOS. Rename it to be more generic.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a3936bb29d..264074489b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
}
}
-static FWCfgState *bochs_bios_init(PCMachineState *pcms)
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
{
FWCfgState *fw_cfg;
uint64_t *numa_fw_cfg;
@@ -1508,7 +1508,7 @@ void pc_cpus_init(PCMachineState *pcms)
* Limit for the APIC ID value, so that all
* CPU APIC IDs are < pcms->apic_id_limit.
*
- * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
+ * This is used for FW_CFG_MAX_CPUS. See comments on x86_create_fw_cfg().
*/
pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
possible_cpus = mc->possible_cpu_arch_ids(ms);
@@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = bochs_bios_init(pcms);
+ fw_cfg = x86_create_fw_cfg(pcms);
rom_set_fw(fw_cfg);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 09/20] hw/i386/pc: Rename bochs_bios_init() more generic as x86_create_fw_cfg()
2019-05-24 6:35 ` [Qemu-devel] [PATCH 09/20] hw/i386/pc: Rename bochs_bios_init() more generic as x86_create_fw_cfg() Philippe Mathieu-Daudé
@ 2019-05-24 13:26 ` Li Qiang
0 siblings, 0 replies; 34+ messages in thread
From: Li Qiang @ 2019-05-24 13:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:46写道:
> The bochs_bios_init() is not restricted to the Bochs BIOS and is
> useful to other BIOS. Rename it to be more generic.
>
> Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/i386/pc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a3936bb29d..264074489b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
> }
> }
>
> -static FWCfgState *bochs_bios_init(PCMachineState *pcms)
> +static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
> {
> FWCfgState *fw_cfg;
> uint64_t *numa_fw_cfg;
> @@ -1508,7 +1508,7 @@ void pc_cpus_init(PCMachineState *pcms)
> * Limit for the APIC ID value, so that all
> * CPU APIC IDs are < pcms->apic_id_limit.
> *
> - * This is used for FW_CFG_MAX_CPUS. See comments on
> bochs_bios_init().
> + * This is used for FW_CFG_MAX_CPUS. See comments on
> x86_create_fw_cfg().
> */
> pcms->apic_id_limit = x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
> possible_cpus = mc->possible_cpu_arch_ids(ms);
> @@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
> option_rom_mr,
> 1);
>
> - fw_cfg = bochs_bios_init(pcms);
> + fw_cfg = x86_create_fw_cfg(pcms);
>
> rom_set_fw(fw_cfg);
>
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 09/20] hw/i386/pc: Rename bochs_bios_init() more generic as x86_create_fw_cfg() Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 15:30 ` Li Qiang
2019-05-24 6:35 ` [Qemu-devel] [PATCH 11/20] hw/i386/pc: Pass the apic_id_limit " Philippe Mathieu-Daudé
` (10 subsequent siblings)
20 siblings, 1 reply; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
The boot_cpus is used once. Pass it by argument, this will
allow us to remove the PCMachineState argument later.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 264074489b..01894b9875 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
}
}
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t boot_cpus)
{
FWCfgState *fw_cfg;
uint64_t *numa_fw_cfg;
@@ -938,7 +938,7 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
&address_space_memory);
- fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
+ fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
/* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
*
@@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = x86_create_fw_cfg(pcms);
+ fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
rom_set_fw(fw_cfg);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument
2019-05-24 6:35 ` [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument Philippe Mathieu-Daudé
@ 2019-05-24 15:30 ` Li Qiang
2019-05-30 15:19 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 34+ messages in thread
From: Li Qiang @ 2019-05-24 15:30 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> 于2019年5月24日周五 下午2:50写道:
> The boot_cpus is used once. Pass it by argument, this will
> allow us to remove the PCMachineState argument later.
>
> Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/i386/pc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 264074489b..01894b9875 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
> }
> }
>
> -static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
> +static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t
> boot_cpus)
>
For the patches 10/11/12, I don't think this is an elegant solution. When
we add more data like 'boot_cpus'
we need add more arguments?
Thanks,
Li Qiang
> {
> FWCfgState *fw_cfg;
> uint64_t *numa_fw_cfg;
> @@ -938,7 +938,7 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState
> *pcms)
>
> fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
> &address_space_memory);
> - fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
> + fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
>
> /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
> *
> @@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
> option_rom_mr,
> 1);
>
> - fw_cfg = x86_create_fw_cfg(pcms);
> + fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
>
> rom_set_fw(fw_cfg);
>
> --
> 2.20.1
>
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument
2019-05-24 15:30 ` Li Qiang
@ 2019-05-30 15:19 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-30 15:19 UTC (permalink / raw)
To: Li Qiang
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Qemu Developers, Samuel Ortiz, Paolo Bonzini,
Richard Henderson
On 5/24/19 5:30 PM, Li Qiang wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> 于
> 2019年5月24日周五 下午2:50写道:
>
> The boot_cpus is used once. Pass it by argument, this will
> allow us to remove the PCMachineState argument later.
>
> Suggested-by: Samuel Ortiz <sameo@linux.intel.com
> <mailto:sameo@linux.intel.com>>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
> <mailto:philmd@redhat.com>>
> ---
> hw/i386/pc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 264074489b..01894b9875 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
> }
> }
>
> -static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms)
> +static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t
> boot_cpus)
>
>
> For the patches 10/11/12, I don't think this is an elegant solution.
> When we add more data like 'boot_cpus'
> we need add more arguments?
This fonction is called once at machine creation, so there is no
performance penalty. To keep the code modularizable (reusable) it is an
acceptable tradeoff :)
>
> {
> FWCfgState *fw_cfg;
> uint64_t *numa_fw_cfg;
> @@ -938,7 +938,7 @@ static FWCfgState
> *x86_create_fw_cfg(PCMachineState *pcms)
>
> fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
> &address_space_memory);
> - fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
> + fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
>
> /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
> *
> @@ -1762,7 +1762,7 @@ void pc_memory_init(PCMachineState *pcms,
> option_rom_mr,
> 1);
>
> - fw_cfg = x86_create_fw_cfg(pcms);
> + fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
>
> rom_set_fw(fw_cfg);
>
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 11/20] hw/i386/pc: Pass the apic_id_limit value by argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 10/20] hw/i386/pc: Pass the boot_cpus value by argument Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 12/20] hw/i386/pc: Pass the CPUArchIdList array " Philippe Mathieu-Daudé
` (9 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Pass the apic_id_limit value by argument, this will
allow us to remove the PCMachineState argument later.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 01894b9875..fe07baeb1d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,8 @@ static void pc_build_smbios(PCMachineState *pcms)
}
}
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, uint16_t boot_cpus)
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
+ uint16_t boot_cpus, uint16_t apic_id_limit)
{
FWCfgState *fw_cfg;
uint64_t *numa_fw_cfg;
@@ -1762,7 +1763,7 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus);
+ fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus, pcms->apic_id_limit);
rom_set_fw(fw_cfg);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 12/20] hw/i386/pc: Pass the CPUArchIdList array by argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 11/20] hw/i386/pc: Pass the apic_id_limit " Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 13/20] hw/i386/pc: Let fw_cfg_init() use the generic MachineState Philippe Mathieu-Daudé
` (8 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Pass the CPUArchIdList array by argument, this will
allow us to remove the PCMachineState argument later.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fe07baeb1d..6cfdb09f34 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,14 +928,12 @@ static void pc_build_smbios(PCMachineState *pcms)
}
}
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
+static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, const CPUArchIdList *cpus,
uint16_t boot_cpus, uint16_t apic_id_limit)
{
FWCfgState *fw_cfg;
uint64_t *numa_fw_cfg;
int i;
- const CPUArchIdList *cpus;
- MachineClass *mc = MACHINE_GET_CLASS(pcms);
fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
&address_space_memory);
@@ -953,7 +951,7 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
* So for compatibility reasons with old BIOSes we are stuck with
* "etc/max-cpus" actually being apic_id_limit
*/
- fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
+ fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
acpi_tables, acpi_tables_len);
@@ -969,20 +967,19 @@ static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms,
* of nodes, one word for each VCPU->node and one word for each node to
* hold the amount of memory.
*/
- numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
+ numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
- cpus = mc->possible_cpu_arch_ids(MACHINE(pcms));
for (i = 0; i < cpus->len; i++) {
unsigned int apic_id = cpus->cpus[i].arch_id;
- assert(apic_id < pcms->apic_id_limit);
+ assert(apic_id < apic_id_limit);
numa_fw_cfg[apic_id + 1] = cpu_to_le64(cpus->cpus[i].props.node_id);
}
for (i = 0; i < nb_numa_nodes; i++) {
- numa_fw_cfg[pcms->apic_id_limit + 1 + i] =
+ numa_fw_cfg[apic_id_limit + 1 + i] =
cpu_to_le64(numa_info[i].node_mem);
}
fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
- (1 + pcms->apic_id_limit + nb_numa_nodes) *
+ (1 + apic_id_limit + nb_numa_nodes) *
sizeof(*numa_fw_cfg));
return fw_cfg;
@@ -1763,7 +1760,8 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = x86_create_fw_cfg(pcms, pcms->boot_cpus, pcms->apic_id_limit);
+ fw_cfg = x86_create_fw_cfg(pcms, mc->possible_cpu_arch_ids(machine),
+ pcms->boot_cpus, pcms->apic_id_limit);
rom_set_fw(fw_cfg);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 13/20] hw/i386/pc: Let fw_cfg_init() use the generic MachineState
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 12/20] hw/i386/pc: Pass the CPUArchIdList array " Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 14/20] hw/i386/pc: Let pc_build_smbios() take a FWCfgState argument Philippe Mathieu-Daudé
` (7 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
We removed the PCMachineState access, we can now let the fw_cfg_init()
function to take a generic MachineState object.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6cfdb09f34..e8813ef2b6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -928,7 +928,7 @@ static void pc_build_smbios(PCMachineState *pcms)
}
}
-static FWCfgState *x86_create_fw_cfg(PCMachineState *pcms, const CPUArchIdList *cpus,
+static FWCfgState *x86_create_fw_cfg(MachineState *ms, const CPUArchIdList *cpus,
uint16_t boot_cpus, uint16_t apic_id_limit)
{
FWCfgState *fw_cfg;
@@ -1664,6 +1664,7 @@ void pc_memory_init(PCMachineState *pcms,
MemoryRegion *ram_below_4g, *ram_above_4g;
FWCfgState *fw_cfg;
MachineState *machine = MACHINE(pcms);
+ MachineClass *mc = MACHINE_GET_CLASS(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
assert(machine->ram_size == pcms->below_4g_mem_size +
@@ -1760,7 +1761,7 @@ void pc_memory_init(PCMachineState *pcms,
option_rom_mr,
1);
- fw_cfg = x86_create_fw_cfg(pcms, mc->possible_cpu_arch_ids(machine),
+ fw_cfg = x86_create_fw_cfg(machine, mc->possible_cpu_arch_ids(machine),
pcms->boot_cpus, pcms->apic_id_limit);
rom_set_fw(fw_cfg);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 14/20] hw/i386/pc: Let pc_build_smbios() take a FWCfgState argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 13/20] hw/i386/pc: Let fw_cfg_init() use the generic MachineState Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 15/20] hw/i386/pc: Let pc_build_smbios() take a generic MachineState argument Philippe Mathieu-Daudé
` (6 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Pass the FWCfgState object by argument, this will
allow us to remove the PCMachineState argument later.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e8813ef2b6..b029aee6c7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -886,7 +886,7 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
}
}
-static void pc_build_smbios(PCMachineState *pcms)
+static void pc_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
{
uint8_t *smbios_tables, *smbios_anchor;
size_t smbios_tables_len, smbios_anchor_len;
@@ -900,7 +900,7 @@ static void pc_build_smbios(PCMachineState *pcms)
smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
if (smbios_tables) {
- fw_cfg_add_bytes(pcms->fw_cfg, FW_CFG_SMBIOS_ENTRIES,
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
smbios_tables, smbios_tables_len);
}
@@ -921,9 +921,9 @@ static void pc_build_smbios(PCMachineState *pcms)
g_free(mem_array);
if (smbios_anchor) {
- fw_cfg_add_file(pcms->fw_cfg, "etc/smbios/smbios-tables",
+ fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
smbios_tables, smbios_tables_len);
- fw_cfg_add_file(pcms->fw_cfg, "etc/smbios/smbios-anchor",
+ fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
smbios_anchor, smbios_anchor_len);
}
}
@@ -1587,7 +1587,7 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
- pc_build_smbios(pcms);
+ pc_build_smbios(pcms, pcms->fw_cfg);
pc_build_feature_control_file(pcms);
/* update FW_CFG_NB_CPUS to account for -device added CPUs */
fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 15/20] hw/i386/pc: Let pc_build_smbios() take a generic MachineState argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (13 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 14/20] hw/i386/pc: Let pc_build_smbios() take a FWCfgState argument Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 16/20] hw/i386/pc: Rename pc_build_smbios() as generic fw_cfg_build_smbios() Philippe Mathieu-Daudé
` (5 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Let the pc_build_smbios() function take a generic MachineState
argument.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b029aee6c7..a79b344eb5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -886,13 +886,12 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
}
}
-static void pc_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
+static void pc_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
{
uint8_t *smbios_tables, *smbios_anchor;
size_t smbios_tables_len, smbios_anchor_len;
struct smbios_phys_mem_area *mem_array;
unsigned i, array_count;
- MachineState *ms = MACHINE(pcms);
X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
/* tell smbios about cpuid version and features */
@@ -1587,7 +1586,7 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
- pc_build_smbios(pcms, pcms->fw_cfg);
+ pc_build_smbios(MACHINE(pcms), pcms->fw_cfg);
pc_build_feature_control_file(pcms);
/* update FW_CFG_NB_CPUS to account for -device added CPUs */
fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 16/20] hw/i386/pc: Rename pc_build_smbios() as generic fw_cfg_build_smbios()
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (14 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 15/20] hw/i386/pc: Let pc_build_smbios() take a generic MachineState argument Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 17/20] hw/i386/pc: Let pc_build_feature_control() take a FWCfgState argument Philippe Mathieu-Daudé
` (4 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Now that the pc_build_smbios() function has been refactored to not
depend of PC specific types, rename it to a more generic name.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a79b344eb5..2804c4dc1b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -886,7 +886,7 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
}
}
-static void pc_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
+static void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
{
uint8_t *smbios_tables, *smbios_anchor;
size_t smbios_tables_len, smbios_anchor_len;
@@ -1586,7 +1586,7 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
- pc_build_smbios(MACHINE(pcms), pcms->fw_cfg);
+ fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
pc_build_feature_control_file(pcms);
/* update FW_CFG_NB_CPUS to account for -device added CPUs */
fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 17/20] hw/i386/pc: Let pc_build_feature_control() take a FWCfgState argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (15 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 16/20] hw/i386/pc: Rename pc_build_smbios() as generic fw_cfg_build_smbios() Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 18/20] hw/i386/pc: Let pc_build_feature_control() take a MachineState argument Philippe Mathieu-Daudé
` (3 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Pass the FWCfgState object by argument, this will
allow us to remove the PCMachineState argument later.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2804c4dc1b..0db0ba3893 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1515,7 +1515,8 @@ void pc_cpus_init(PCMachineState *pcms)
}
}
-static void pc_build_feature_control_file(PCMachineState *pcms)
+static void pc_build_feature_control_file(PCMachineState *pcms,
+ FWCfgState *fw_cfg)
{
MachineState *ms = MACHINE(pcms);
X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
@@ -1541,7 +1542,7 @@ static void pc_build_feature_control_file(PCMachineState *pcms)
val = g_malloc(sizeof(*val));
*val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
- fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
+ fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
}
static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
@@ -1587,7 +1588,7 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
- pc_build_feature_control_file(pcms);
+ pc_build_feature_control_file(pcms, pcms->fw_cfg);
/* update FW_CFG_NB_CPUS to account for -device added CPUs */
fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 18/20] hw/i386/pc: Let pc_build_feature_control() take a MachineState argument
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (16 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 17/20] hw/i386/pc: Let pc_build_feature_control() take a FWCfgState argument Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 19/20] hw/i386/pc: Rename pc_build_feature_control() as generic fw_cfg_build_* Philippe Mathieu-Daudé
` (2 subsequent siblings)
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Let the pc_build_feature_control_file() function take a generic MachineState
argument.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0db0ba3893..0ff2cea1ee 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1515,10 +1515,9 @@ void pc_cpus_init(PCMachineState *pcms)
}
}
-static void pc_build_feature_control_file(PCMachineState *pcms,
+static void pc_build_feature_control_file(MachineState *ms,
FWCfgState *fw_cfg)
{
- MachineState *ms = MACHINE(pcms);
X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
CPUX86State *env = &cpu->env;
uint32_t unused, ecx, edx;
@@ -1588,7 +1587,7 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
- pc_build_feature_control_file(pcms, pcms->fw_cfg);
+ pc_build_feature_control_file(MACHINE(pcms), pcms->fw_cfg);
/* update FW_CFG_NB_CPUS to account for -device added CPUs */
fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 19/20] hw/i386/pc: Rename pc_build_feature_control() as generic fw_cfg_build_*
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (17 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 18/20] hw/i386/pc: Let pc_build_feature_control() take a MachineState argument Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-24 6:35 ` [Qemu-devel] [PATCH 20/20] hw/i386/pc: Extract the x86 generic fw_cfg code Philippe Mathieu-Daudé
2019-05-29 3:21 ` [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Michael S. Tsirkin
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Now that the pc_build_feature_control_file() function has been
refactored to not depend of PC specific types, rename it to a
more generic name.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/pc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0ff2cea1ee..4d333aba82 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1515,8 +1515,8 @@ void pc_cpus_init(PCMachineState *pcms)
}
}
-static void pc_build_feature_control_file(MachineState *ms,
- FWCfgState *fw_cfg)
+static void fw_cfg_build_feature_control(MachineState *ms,
+ FWCfgState *fw_cfg)
{
X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
CPUX86State *env = &cpu->env;
@@ -1587,7 +1587,7 @@ void pc_machine_done(Notifier *notifier, void *data)
acpi_setup();
if (pcms->fw_cfg) {
fw_cfg_build_smbios(MACHINE(pcms), pcms->fw_cfg);
- pc_build_feature_control_file(MACHINE(pcms), pcms->fw_cfg);
+ fw_cfg_build_feature_control(MACHINE(pcms), pcms->fw_cfg);
/* update FW_CFG_NB_CPUS to account for -device added CPUs */
fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH 20/20] hw/i386/pc: Extract the x86 generic fw_cfg code
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (18 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 19/20] hw/i386/pc: Rename pc_build_feature_control() as generic fw_cfg_build_* Philippe Mathieu-Daudé
@ 2019-05-24 6:35 ` Philippe Mathieu-Daudé
2019-05-29 3:21 ` [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Michael S. Tsirkin
20 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-24 6:35 UTC (permalink / raw)
To: qemu-devel
Cc: Yang Zhong, Eduardo Habkost, Rob Bradford, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, Richard Henderson,
Philippe Mathieu-Daudé,
Samuel Ortiz
Extract all the functions that are not PC-machine specific into
the (arch-specific) fw_cfg.c file. This will allow other X86-machine
to re-use these functions.
Suggested-by: Samuel Ortiz <sameo@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/i386/fw_cfg.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++
hw/i386/fw_cfg.h | 6 +++
hw/i386/pc.c | 128 +-------------------------------------------
3 files changed, 142 insertions(+), 127 deletions(-)
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 380a819230..bdd744c040 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -13,8 +13,15 @@
*/
#include "qemu/osdep.h"
+#include "sysemu/numa.h"
+#include "hw/acpi/acpi.h"
+#include "hw/firmware/smbios.h"
+#include "hw/i386/pc.h"
#include "hw/i386/fw_cfg.h"
+#include "hw/timer/hpet.h"
#include "hw/nvram/fw_cfg.h"
+#include "e820_memory_layout.h"
+#include "kvm_i386.h"
const char *fw_cfg_arch_key_name(uint16_t key)
{
@@ -36,3 +43,131 @@ const char *fw_cfg_arch_key_name(uint16_t key)
}
return NULL;
}
+
+void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
+{
+ uint8_t *smbios_tables, *smbios_anchor;
+ size_t smbios_tables_len, smbios_anchor_len;
+ struct smbios_phys_mem_area *mem_array;
+ unsigned i, array_count;
+ X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
+
+ /* tell smbios about cpuid version and features */
+ smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
+
+ smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
+ if (smbios_tables) {
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
+ smbios_tables, smbios_tables_len);
+ }
+
+ /* build the array of physical mem area from e820 table */
+ mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());
+ for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
+ uint64_t addr, len;
+
+ if (e820_get_entry(i, E820_RAM, &addr, &len)) {
+ mem_array[array_count].address = addr;
+ mem_array[array_count].length = len;
+ array_count++;
+ }
+ }
+ smbios_get_tables(mem_array, array_count,
+ &smbios_tables, &smbios_tables_len,
+ &smbios_anchor, &smbios_anchor_len);
+ g_free(mem_array);
+
+ if (smbios_anchor) {
+ fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
+ smbios_tables, smbios_tables_len);
+ fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
+ smbios_anchor, smbios_anchor_len);
+ }
+}
+
+void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg)
+{
+ X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
+ CPUX86State *env = &cpu->env;
+ uint32_t unused, ecx, edx;
+ uint64_t feature_control_bits = 0;
+ uint64_t *val;
+
+ cpu_x86_cpuid(env, 1, 0, &unused, &unused, &ecx, &edx);
+ if (ecx & CPUID_EXT_VMX) {
+ feature_control_bits |= FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+ }
+
+ if ((edx & (CPUID_EXT2_MCE | CPUID_EXT2_MCA)) ==
+ (CPUID_EXT2_MCE | CPUID_EXT2_MCA) &&
+ (env->mcg_cap & MCG_LMCE_P)) {
+ feature_control_bits |= FEATURE_CONTROL_LMCE;
+ }
+
+ if (!feature_control_bits) {
+ return;
+ }
+
+ val = g_malloc(sizeof(*val));
+ *val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
+ fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
+}
+
+FWCfgState *x86_create_fw_cfg(MachineState *ms, const CPUArchIdList *cpus,
+ uint16_t boot_cpus, uint16_t apic_id_limit)
+{
+ FWCfgState *fw_cfg;
+ uint64_t *numa_fw_cfg;
+ int i;
+
+ fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
+ &address_space_memory);
+ fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
+
+ /*
+ * FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
+ *
+ * For machine types prior to 1.8, SeaBIOS needs FW_CFG_MAX_CPUS for
+ * building MPTable, ACPI MADT, ACPI CPU hotplug and ACPI SRAT table,
+ * that tables are based on xAPIC ID and QEMU<->SeaBIOS interface
+ * for CPU hotplug also uses APIC ID and not "CPU index".
+ * This means that FW_CFG_MAX_CPUS is not the "maximum number of CPUs",
+ * but the "limit to the APIC ID values SeaBIOS may see".
+ *
+ * So for compatibility reasons with old BIOSes we are stuck with
+ * "etc/max-cpus" actually being apic_id_limit
+ */
+ fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
+ fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
+ acpi_tables, acpi_tables_len);
+ fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
+
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
+ &e820_reserve, sizeof(e820_reserve));
+ fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
+ sizeof(struct e820_entry) * e820_get_num_entries());
+
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
+ /*
+ * allocate memory for the NUMA channel: one (64bit) word for the number
+ * of nodes, one word for each VCPU->node and one word for each node to
+ * hold the amount of memory.
+ */
+ numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+ numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
+ for (i = 0; i < cpus->len; i++) {
+ unsigned int apic_id = cpus->cpus[i].arch_id;
+ assert(apic_id < apic_id_limit);
+ numa_fw_cfg[apic_id + 1] = cpu_to_le64(cpus->cpus[i].props.node_id);
+ }
+ for (i = 0; i < nb_numa_nodes; i++) {
+ numa_fw_cfg[apic_id_limit + 1 + i] =
+ cpu_to_le64(numa_info[i].node_mem);
+ }
+ fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
+ (1 + apic_id_limit + nb_numa_nodes) *
+ sizeof(*numa_fw_cfg));
+
+ return fw_cfg;
+}
diff --git a/hw/i386/fw_cfg.h b/hw/i386/fw_cfg.h
index 17a4bc32f2..14a9223374 100644
--- a/hw/i386/fw_cfg.h
+++ b/hw/i386/fw_cfg.h
@@ -9,6 +9,7 @@
#ifndef HW_I386_FW_CFG_H
#define HW_I386_FW_CFG_H
+#include "hw/boards.h"
#include "hw/nvram/fw_cfg.h"
#define FW_CFG_ACPI_TABLES (FW_CFG_ARCH_LOCAL + 0)
@@ -17,4 +18,9 @@
#define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
#define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
+FWCfgState *x86_create_fw_cfg(MachineState *ms, const CPUArchIdList *cpus,
+ uint16_t boot_cpus, uint16_t apic_id_limit);
+void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg);
+void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg);
+
#endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4d333aba82..0e59756ed5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -79,6 +79,7 @@
#include "hw/net/ne2000-isa.h"
#include "standard-headers/asm-x86/bootparam.h"
#include "e820_memory_layout.h"
+#include "fw_cfg.h"
/* debug PC/ISA interrupts */
//#define DEBUG_IRQ
@@ -886,104 +887,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
}
}
-static void fw_cfg_build_smbios(MachineState *ms, FWCfgState *fw_cfg)
-{
- uint8_t *smbios_tables, *smbios_anchor;
- size_t smbios_tables_len, smbios_anchor_len;
- struct smbios_phys_mem_area *mem_array;
- unsigned i, array_count;
- X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
-
- /* tell smbios about cpuid version and features */
- smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
-
- smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
- if (smbios_tables) {
- fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
- smbios_tables, smbios_tables_len);
- }
-
- /* build the array of physical mem area from e820 table */
- mem_array = g_malloc0(sizeof(*mem_array) * e820_get_num_entries());
- for (i = 0, array_count = 0; i < e820_get_num_entries(); i++) {
- uint64_t addr, len;
-
- if (e820_get_entry(i, E820_RAM, &addr, &len)) {
- mem_array[array_count].address = addr;
- mem_array[array_count].length = len;
- array_count++;
- }
- }
- smbios_get_tables(mem_array, array_count,
- &smbios_tables, &smbios_tables_len,
- &smbios_anchor, &smbios_anchor_len);
- g_free(mem_array);
-
- if (smbios_anchor) {
- fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-tables",
- smbios_tables, smbios_tables_len);
- fw_cfg_add_file(fw_cfg, "etc/smbios/smbios-anchor",
- smbios_anchor, smbios_anchor_len);
- }
-}
-
-static FWCfgState *x86_create_fw_cfg(MachineState *ms, const CPUArchIdList *cpus,
- uint16_t boot_cpus, uint16_t apic_id_limit)
-{
- FWCfgState *fw_cfg;
- uint64_t *numa_fw_cfg;
- int i;
-
- fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
- &address_space_memory);
- fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
-
- /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
- *
- * For machine types prior to 1.8, SeaBIOS needs FW_CFG_MAX_CPUS for
- * building MPTable, ACPI MADT, ACPI CPU hotplug and ACPI SRAT table,
- * that tables are based on xAPIC ID and QEMU<->SeaBIOS interface
- * for CPU hotplug also uses APIC ID and not "CPU index".
- * This means that FW_CFG_MAX_CPUS is not the "maximum number of CPUs",
- * but the "limit to the APIC ID values SeaBIOS may see".
- *
- * So for compatibility reasons with old BIOSes we are stuck with
- * "etc/max-cpus" actually being apic_id_limit
- */
- fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
- fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
- fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
- acpi_tables, acpi_tables_len);
- fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
-
- fw_cfg_add_bytes(fw_cfg, FW_CFG_E820_TABLE,
- &e820_reserve, sizeof(e820_reserve));
- fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
- sizeof(struct e820_entry) * e820_get_num_entries());
-
- fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
- /* allocate memory for the NUMA channel: one (64bit) word for the number
- * of nodes, one word for each VCPU->node and one word for each node to
- * hold the amount of memory.
- */
- numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
- numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
- for (i = 0; i < cpus->len; i++) {
- unsigned int apic_id = cpus->cpus[i].arch_id;
- assert(apic_id < apic_id_limit);
- numa_fw_cfg[apic_id + 1] = cpu_to_le64(cpus->cpus[i].props.node_id);
- }
- for (i = 0; i < nb_numa_nodes; i++) {
- numa_fw_cfg[apic_id_limit + 1 + i] =
- cpu_to_le64(numa_info[i].node_mem);
- }
- fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
- (1 + apic_id_limit + nb_numa_nodes) *
- sizeof(*numa_fw_cfg));
-
- return fw_cfg;
-}
-
static long get_file_size(FILE *f)
{
long where, size;
@@ -1515,35 +1418,6 @@ void pc_cpus_init(PCMachineState *pcms)
}
}
-static void fw_cfg_build_feature_control(MachineState *ms,
- FWCfgState *fw_cfg)
-{
- X86CPU *cpu = X86_CPU(ms->possible_cpus->cpus[0].cpu);
- CPUX86State *env = &cpu->env;
- uint32_t unused, ecx, edx;
- uint64_t feature_control_bits = 0;
- uint64_t *val;
-
- cpu_x86_cpuid(env, 1, 0, &unused, &unused, &ecx, &edx);
- if (ecx & CPUID_EXT_VMX) {
- feature_control_bits |= FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
- }
-
- if ((edx & (CPUID_EXT2_MCE | CPUID_EXT2_MCA)) ==
- (CPUID_EXT2_MCE | CPUID_EXT2_MCA) &&
- (env->mcg_cap & MCG_LMCE_P)) {
- feature_control_bits |= FEATURE_CONTROL_LMCE;
- }
-
- if (!feature_control_bits) {
- return;
- }
-
- val = g_malloc(sizeof(*val));
- *val = cpu_to_le64(feature_control_bits | FEATURE_CONTROL_LOCKED);
- fw_cfg_add_file(fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
-}
-
static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
{
if (cpus_count > 0xff) {
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine
2019-05-24 6:35 [Qemu-devel] [PATCH 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine Philippe Mathieu-Daudé
` (19 preceding siblings ...)
2019-05-24 6:35 ` [Qemu-devel] [PATCH 20/20] hw/i386/pc: Extract the x86 generic fw_cfg code Philippe Mathieu-Daudé
@ 2019-05-29 3:21 ` Michael S. Tsirkin
20 siblings, 0 replies; 34+ messages in thread
From: Michael S. Tsirkin @ 2019-05-29 3:21 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Yang Zhong, Samuel Ortiz, Rob Bradford, Marcelo Tosatti,
qemu-devel, Paolo Bonzini, Eduardo Habkost
On Fri, May 24, 2019 at 08:35:33AM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
>
> This is my take at salvaging some NEMU good work.
> Samuel worked in adding the fw_cfg device to the x86-virt NEMU machine.
> This series is inspired by NEMU's commit 3cb92d080835 [*] and adapted
> to upstream style. The result makes the upstream codebase more
> modularizable.
> There are very little logical changes, this is mostly a cleanup
> refactor.
>
> Regards,
>
> Phil.
So I take issue with some of the renames. When I see fw_cfg_XXXX I
expect the definition be in fw_cfg.c not in pc.c
Please use prefixes that match the file name -
avoids namespace conflicts and makes it easy to
find files.
This is not to say that the current system is perfect:
we have bochs_bios_init which isn't in bochs.c, should
be pc_bochs_bios_init or whatever.
Thanks!
> [*] https://github.com/intel/nemu/commit/3cb92d080835ac8d47c8b713156338afa33cff5c
>
> Philippe Mathieu-Daudé (20):
> hw/i386/pc: Use unsigned type to index arrays
> hw/i386/pc: Use size_t type to hold/return a size of array
> hw/i386/pc: Let e820_add_entry() return a ssize_t type
> hw/i386/pc: Add the E820Type enum type
> hw/i386/pc: Add documentation to the e820_*() functions
> hw/i386/pc: Use e820_get_num_entries() to access e820_entries
> hw/i386/pc: Extract e820 memory layout code
> hw/i386/pc: Use address_space_memory in place
> hw/i386/pc: Rename bochs_bios_init() more generic as
> x86_create_fw_cfg()
> hw/i386/pc: Pass the boot_cpus value by argument
> hw/i386/pc: Pass the apic_id_limit value by argument
> hw/i386/pc: Pass the CPUArchIdList array by argument
> hw/i386/pc: Let fw_cfg_init() use the generic MachineState
> hw/i386/pc: Let pc_build_smbios() take a FWCfgState argument
> hw/i386/pc: Let pc_build_smbios() take a generic MachineState argument
> hw/i386/pc: Rename pc_build_smbios() as generic fw_cfg_build_smbios()
> hw/i386/pc: Let pc_build_feature_control() take a FWCfgState argument
> hw/i386/pc: Let pc_build_feature_control() take a MachineState
> argument
> hw/i386/pc: Rename pc_build_feature_control() as generic
> fw_cfg_build_*
> hw/i386/pc: Extract the x86 generic fw_cfg code
>
> hw/i386/Makefile.objs | 2 +-
> hw/i386/e820_memory_layout.c | 62 +++++++++++
> hw/i386/e820_memory_layout.h | 76 +++++++++++++
> hw/i386/fw_cfg.c | 135 +++++++++++++++++++++++
> hw/i386/fw_cfg.h | 6 ++
> hw/i386/pc.c | 201 ++---------------------------------
> include/hw/i386/pc.h | 11 --
> target/i386/kvm.c | 1 +
> 8 files changed, 289 insertions(+), 205 deletions(-)
> create mode 100644 hw/i386/e820_memory_layout.c
> create mode 100644 hw/i386/e820_memory_layout.h
>
> --
> 2.20.1
^ permalink raw reply [flat|nested] 34+ messages in thread