| Message ID | 20190524063553.5339-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 |
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 > > >
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 > >
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);
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(-)