1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 00/20] hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine
  4. View patch

[Qemu-devel] [PATCH v2 04/20] hw/i386/pc: Add the E820Type enum type

Philippe Mathieu-Daudé posted 20 patches 6 years, 8 months ago
Maintainers: Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <rth@twiddle.net>
There is a newer version of this series
[Qemu-devel] [PATCH v2 04/20] hw/i386/pc: Add the E820Type enum type
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
This ensure we won't use an incorrect value.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Do not cast the enum (Li)
---
 hw/i386/pc.c         |  4 ++--
 include/hw/i386/pc.h | 16 ++++++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5a7cffbb1a..86ba554439 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -872,7 +872,7 @@ 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);
     struct e820_entry *entry;
@@ -906,7 +906,7 @@ 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)) {
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c56116e6f6..7c07185dd5 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
Re: [Qemu-devel] [PATCH v2 04/20] hw/i386/pc: Add the E820Type enum type
Posted by Michael S. Tsirkin 6 years, 7 months ago 
On Thu, Jun 13, 2019 at 04:34:30PM +0200, Philippe Mathieu-Daudé wrote:
> This ensure we won't use an incorrect value.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

It doesn't actually ensure anything: compiler does not check IIUC.

And OTOH it's stored in type field in struct e820_entry.

> ---
> v2: Do not cast the enum (Li)
> ---
>  hw/i386/pc.c         |  4 ++--
>  include/hw/i386/pc.h | 16 ++++++++++------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5a7cffbb1a..86ba554439 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -872,7 +872,7 @@ 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);
>      struct e820_entry *entry;
> @@ -906,7 +906,7 @@ 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)) {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index c56116e6f6..7c07185dd5 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
Re: [Qemu-devel] [PATCH v2 04/20] hw/i386/pc: Add the E820Type enum type
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago 
On 6/20/19 5:31 PM, Michael S. Tsirkin wrote:
> On Thu, Jun 13, 2019 at 04:34:30PM +0200, Philippe Mathieu-Daudé wrote:
>> This ensure we won't use an incorrect value.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> It doesn't actually ensure anything: compiler does not check IIUC.
> 
> And OTOH it's stored in type field in struct e820_entry.

I totally missed that... Thanks!

>> ---
>> v2: Do not cast the enum (Li)
>> ---
>>  hw/i386/pc.c         |  4 ++--
>>  include/hw/i386/pc.h | 16 ++++++++++------
>>  2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 5a7cffbb1a..86ba554439 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -872,7 +872,7 @@ 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);
>>      struct e820_entry *entry;
>> @@ -906,7 +906,7 @@ 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)) {
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index c56116e6f6..7c07185dd5 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.