| Message ID | 20190613143446.23937-5-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/i386/pc: Do not restrict the fw_cfg functions to the PC machine | expand |
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
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
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);
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(-)