Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
Hi Michael, Apologies for the delay, I just sent out a rebased patch. Based on latest tags looks like release may already have been started? -Erich From: McMillan, Erich Sent: Wednesday, December 2, 2020 8:31 AM To: Michael S. Tsirkin Cc: qemu-devel@nongnu.org ; ler...@redhat.com ; dgilb...@redhat.com ; marcel.apfelb...@gmail.com ; imamm...@redhat.com ; kra...@redhat.com Subject: Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option Sure, no problem. From: Michael S. Tsirkin Sent: Wednesday, December 2, 2020 3:56 AM To: McMillan, Erich Cc: qemu-devel@nongnu.org ; ler...@redhat.com ; dgilb...@redhat.com ; marcel.apfelb...@gmail.com ; imamm...@redhat.com ; kra...@redhat.com Subject: Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option Could you help by rebasing this on master? Shouldn't be hard ... Thanks!
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
Sure, no problem. From: Michael S. Tsirkin Sent: Wednesday, December 2, 2020 3:56 AM To: McMillan, Erich Cc: qemu-devel@nongnu.org ; ler...@redhat.com ; dgilb...@redhat.com ; marcel.apfelb...@gmail.com ; imamm...@redhat.com ; kra...@redhat.com Subject: Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option Could you help by rebasing this on master? Shouldn't be hard ... Thanks!
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
Could you help by rebasing this on master? Shouldn't be hard ... Thanks!
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
Thanks Michael. -Erich From: Michael S. Tsirkin Sent: Tuesday, November 17, 2020 10:53 AM To: McMillan, Erich Cc: qemu-devel@nongnu.org ; ler...@redhat.com ; dgilb...@redhat.com ; marcel.apfelb...@gmail.com ; imamm...@redhat.com ; kra...@redhat.com Subject: Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option On Fri, Sep 25, 2020 at 05:57:51PM +, Erich Mcmillan wrote: > From: Erich McMillan > > At Hewlett Packard Inc. we have a need for increased fw size to enable > testing of our custom fw. > Move return statement for early return > > Signed-off-by: Erich McMillan My bad that I dropped it by mistake before code freeze. I will queue it for the next release. > --- > > Changes since v5: > > Move return statement for pc_machine_set_max_fw_size() to follow > error_setg() as early return. > > hw/i386/pc.c | 51 > hw/i386/pc_sysfw.c | 13 ++- > include/hw/i386/pc.h | 2 ++ > 3 files changed, 55 insertions(+), 11 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d11daacc23..70c8c9adcf 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1869,6 +1869,50 @@ static void pc_machine_set_max_ram_below_4g(Object > *obj, Visitor *v, > pcms->max_ram_below_4g = value; > } > > +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +PCMachineState *pcms = PC_MACHINE(obj); > +uint64_t value = pcms->max_fw_size; > + > +visit_type_size(v, name, , errp); > +} > + > +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +PCMachineState *pcms = PC_MACHINE(obj); > +Error *error = NULL; > +uint64_t value; > + > +visit_type_size(v, name, , ); > +if (error) { > +error_propagate(errp, error); > +return; > +} > + > +/* > +* We don't have a theoretically justifiable exact lower bound on the base > +* address of any flash mapping. In practice, the IO-APIC MMIO range is > +* [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving > free > +* only 18MB-4KB below 4G. For now, restrict the cumulative mapping to > 8MB in > +* size. > +*/ > +if (value > 16 * MiB) { > +error_setg(errp, > + "User specified max allowed firmware size %" PRIu64 " is " > + "greater than 16MiB. If combined firwmare size exceeds " > + "16MiB the system may not boot, or experience > intermittent" > + "stability issues.", > + value); > +return; > +} > + > +pcms->max_fw_size = value; > +} > + > static void pc_machine_initfn(Object *obj) > { > PCMachineState *pcms = PC_MACHINE(obj); > @@ -1884,6 +1928,7 @@ static void pc_machine_initfn(Object *obj) > pcms->smbus_enabled = true; > pcms->sata_enabled = true; > pcms->pit_enabled = true; > +pcms->max_fw_size = 8 * MiB; > > pc_system_flash_create(pcms); > pcms->pcspk = isa_new(TYPE_PC_SPEAKER); > @@ -2004,6 +2049,12 @@ static void pc_machine_class_init(ObjectClass *oc, > void *data) > > object_class_property_add_bool(oc, PC_MACHINE_PIT, > pc_machine_get_pit, pc_machine_set_pit); > + > +object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size", > +pc_machine_get_max_fw_size, pc_machine_set_max_fw_size, > +NULL, NULL); > +object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE, > +"Maximum combined firmware size"); > } > > static const TypeInfo pc_machine_info = { > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index b6c0822fe3..22450ba0ef 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -39,15 +39,6 @@ > #include "hw/block/flash.h" > #include "sysemu/kvm.h" > > -/* > - * We don't have a theoretically justifiable exact lower bound on the base > - * address of any flash mapping. In practice, the IO-APIC MMIO range is > - * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free > - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in > - * size. > - */ > -#define FLASH_SIZE_LIMIT (8 * MiB) > - > #define FLASH_SECTOR_SIZE 4096
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
On Fri, Sep 25, 2020 at 05:57:51PM +, Erich Mcmillan wrote: > From: Erich McMillan > > At Hewlett Packard Inc. we have a need for increased fw size to enable > testing of our custom fw. > Move return statement for early return > > Signed-off-by: Erich McMillan My bad that I dropped it by mistake before code freeze. I will queue it for the next release. > --- > > Changes since v5: > > Move return statement for pc_machine_set_max_fw_size() to follow > error_setg() as early return. > > hw/i386/pc.c | 51 > hw/i386/pc_sysfw.c | 13 ++- > include/hw/i386/pc.h | 2 ++ > 3 files changed, 55 insertions(+), 11 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d11daacc23..70c8c9adcf 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1869,6 +1869,50 @@ static void pc_machine_set_max_ram_below_4g(Object > *obj, Visitor *v, > pcms->max_ram_below_4g = value; > } > > +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +PCMachineState *pcms = PC_MACHINE(obj); > +uint64_t value = pcms->max_fw_size; > + > +visit_type_size(v, name, , errp); > +} > + > +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +PCMachineState *pcms = PC_MACHINE(obj); > +Error *error = NULL; > +uint64_t value; > + > +visit_type_size(v, name, , ); > +if (error) { > +error_propagate(errp, error); > +return; > +} > + > +/* > +* We don't have a theoretically justifiable exact lower bound on the base > +* address of any flash mapping. In practice, the IO-APIC MMIO range is > +* [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving > free > +* only 18MB-4KB below 4G. For now, restrict the cumulative mapping to > 8MB in > +* size. > +*/ > +if (value > 16 * MiB) { > +error_setg(errp, > + "User specified max allowed firmware size %" PRIu64 " is " > + "greater than 16MiB. If combined firwmare size exceeds " > + "16MiB the system may not boot, or experience > intermittent" > + "stability issues.", > + value); > +return; > +} > + > +pcms->max_fw_size = value; > +} > + > static void pc_machine_initfn(Object *obj) > { > PCMachineState *pcms = PC_MACHINE(obj); > @@ -1884,6 +1928,7 @@ static void pc_machine_initfn(Object *obj) > pcms->smbus_enabled = true; > pcms->sata_enabled = true; > pcms->pit_enabled = true; > +pcms->max_fw_size = 8 * MiB; > > pc_system_flash_create(pcms); > pcms->pcspk = isa_new(TYPE_PC_SPEAKER); > @@ -2004,6 +2049,12 @@ static void pc_machine_class_init(ObjectClass *oc, > void *data) > > object_class_property_add_bool(oc, PC_MACHINE_PIT, > pc_machine_get_pit, pc_machine_set_pit); > + > +object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size", > +pc_machine_get_max_fw_size, pc_machine_set_max_fw_size, > +NULL, NULL); > +object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE, > +"Maximum combined firmware size"); > } > > static const TypeInfo pc_machine_info = { > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index b6c0822fe3..22450ba0ef 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -39,15 +39,6 @@ > #include "hw/block/flash.h" > #include "sysemu/kvm.h" > > -/* > - * We don't have a theoretically justifiable exact lower bound on the base > - * address of any flash mapping. In practice, the IO-APIC MMIO range is > - * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free > - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in > - * size. > - */ > -#define FLASH_SIZE_LIMIT (8 * MiB) > - > #define FLASH_SECTOR_SIZE 4096 > > static void pc_isa_bios_init(MemoryRegion *rom_memory, > @@ -182,10 +173,10 @@ static void pc_system_flash_map(PCMachineState *pcms, > } > if ((hwaddr)size != size > || total_size > HWADDR_MAX - size > -|| total_size + size > FLASH_SIZE_LIMIT) { > +|| total_size + size > pcms->max_fw_size) { > error_report("combined size of system firmware exceeds " > "%" PRIu64 " bytes", > - FLASH_SIZE_LIMIT); > + pcms->max_fw_size); > exit(1); > } > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index fe52e165b2..f7c8e7cbfe 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -43,6 +43,7 @@ struct PCMachineState { > bool smbus_enabled; >
Re: [PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
On 09/25/20 19:57, Erich Mcmillan wrote: > From: Erich McMillan > > At Hewlett Packard Inc. we have a need for increased fw size to enable > testing of our custom fw. > Move return statement for early return > > Signed-off-by: Erich McMillan > --- > > Changes since v5: > > Move return statement for pc_machine_set_max_fw_size() to follow > error_setg() as early return. > > hw/i386/pc.c | 51 > hw/i386/pc_sysfw.c | 13 ++- > include/hw/i386/pc.h | 2 ++ > 3 files changed, 55 insertions(+), 11 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d11daacc23..70c8c9adcf 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1869,6 +1869,50 @@ static void pc_machine_set_max_ram_below_4g(Object > *obj, Visitor *v, > pcms->max_ram_below_4g = value; > } > > +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +PCMachineState *pcms = PC_MACHINE(obj); > +uint64_t value = pcms->max_fw_size; > + > +visit_type_size(v, name, , errp); > +} > + > +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +PCMachineState *pcms = PC_MACHINE(obj); > +Error *error = NULL; > +uint64_t value; > + > +visit_type_size(v, name, , ); > +if (error) { > +error_propagate(errp, error); > +return; > +} > + > +/* > +* We don't have a theoretically justifiable exact lower bound on the base > +* address of any flash mapping. In practice, the IO-APIC MMIO range is > +* [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving > free > +* only 18MB-4KB below 4G. For now, restrict the cumulative mapping to > 8MB in > +* size. > +*/ > +if (value > 16 * MiB) { > +error_setg(errp, > + "User specified max allowed firmware size %" PRIu64 " is " > + "greater than 16MiB. If combined firwmare size exceeds " > + "16MiB the system may not boot, or experience > intermittent" > + "stability issues.", > + value); > +return; > +} > + > +pcms->max_fw_size = value; > +} > + > static void pc_machine_initfn(Object *obj) > { > PCMachineState *pcms = PC_MACHINE(obj); > @@ -1884,6 +1928,7 @@ static void pc_machine_initfn(Object *obj) > pcms->smbus_enabled = true; > pcms->sata_enabled = true; > pcms->pit_enabled = true; > +pcms->max_fw_size = 8 * MiB; > > pc_system_flash_create(pcms); > pcms->pcspk = isa_new(TYPE_PC_SPEAKER); > @@ -2004,6 +2049,12 @@ static void pc_machine_class_init(ObjectClass *oc, > void *data) > > object_class_property_add_bool(oc, PC_MACHINE_PIT, > pc_machine_get_pit, pc_machine_set_pit); > + > +object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size", > +pc_machine_get_max_fw_size, pc_machine_set_max_fw_size, > +NULL, NULL); > +object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE, > +"Maximum combined firmware size"); > } > > static const TypeInfo pc_machine_info = { > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index b6c0822fe3..22450ba0ef 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -39,15 +39,6 @@ > #include "hw/block/flash.h" > #include "sysemu/kvm.h" > > -/* > - * We don't have a theoretically justifiable exact lower bound on the base > - * address of any flash mapping. In practice, the IO-APIC MMIO range is > - * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free > - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in > - * size. > - */ > -#define FLASH_SIZE_LIMIT (8 * MiB) > - > #define FLASH_SECTOR_SIZE 4096 > > static void pc_isa_bios_init(MemoryRegion *rom_memory, > @@ -182,10 +173,10 @@ static void pc_system_flash_map(PCMachineState *pcms, > } > if ((hwaddr)size != size > || total_size > HWADDR_MAX - size > -|| total_size + size > FLASH_SIZE_LIMIT) { > +|| total_size + size > pcms->max_fw_size) { > error_report("combined size of system firmware exceeds " > "%" PRIu64 " bytes", > - FLASH_SIZE_LIMIT); > + pcms->max_fw_size); > exit(1); > } > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index fe52e165b2..f7c8e7cbfe 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -43,6 +43,7 @@ struct PCMachineState { > bool smbus_enabled; > bool sata_enabled; > bool pit_enabled; > +uint64_t max_fw_size; > > /* NUMA information: */ >
[PATCH v6] hw/i386/pc: add max combined fw size as machine configuration option
From: Erich McMillan At Hewlett Packard Inc. we have a need for increased fw size to enable testing of our custom fw. Move return statement for early return Signed-off-by: Erich McMillan --- Changes since v5: Move return statement for pc_machine_set_max_fw_size() to follow error_setg() as early return. hw/i386/pc.c | 51 hw/i386/pc_sysfw.c | 13 ++- include/hw/i386/pc.h | 2 ++ 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index d11daacc23..70c8c9adcf 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1869,6 +1869,50 @@ static void pc_machine_set_max_ram_below_4g(Object *obj, Visitor *v, pcms->max_ram_below_4g = value; } +static void pc_machine_get_max_fw_size(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ +PCMachineState *pcms = PC_MACHINE(obj); +uint64_t value = pcms->max_fw_size; + +visit_type_size(v, name, , errp); +} + +static void pc_machine_set_max_fw_size(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ +PCMachineState *pcms = PC_MACHINE(obj); +Error *error = NULL; +uint64_t value; + +visit_type_size(v, name, , ); +if (error) { +error_propagate(errp, error); +return; +} + +/* +* We don't have a theoretically justifiable exact lower bound on the base +* address of any flash mapping. In practice, the IO-APIC MMIO range is +* [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free +* only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in +* size. +*/ +if (value > 16 * MiB) { +error_setg(errp, + "User specified max allowed firmware size %" PRIu64 " is " + "greater than 16MiB. If combined firwmare size exceeds " + "16MiB the system may not boot, or experience intermittent" + "stability issues.", + value); +return; +} + +pcms->max_fw_size = value; +} + static void pc_machine_initfn(Object *obj) { PCMachineState *pcms = PC_MACHINE(obj); @@ -1884,6 +1928,7 @@ static void pc_machine_initfn(Object *obj) pcms->smbus_enabled = true; pcms->sata_enabled = true; pcms->pit_enabled = true; +pcms->max_fw_size = 8 * MiB; pc_system_flash_create(pcms); pcms->pcspk = isa_new(TYPE_PC_SPEAKER); @@ -2004,6 +2049,12 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) object_class_property_add_bool(oc, PC_MACHINE_PIT, pc_machine_get_pit, pc_machine_set_pit); + +object_class_property_add(oc, PC_MACHINE_MAX_FW_SIZE, "size", +pc_machine_get_max_fw_size, pc_machine_set_max_fw_size, +NULL, NULL); +object_class_property_set_description(oc, PC_MACHINE_MAX_FW_SIZE, +"Maximum combined firmware size"); } static const TypeInfo pc_machine_info = { diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index b6c0822fe3..22450ba0ef 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -39,15 +39,6 @@ #include "hw/block/flash.h" #include "sysemu/kvm.h" -/* - * We don't have a theoretically justifiable exact lower bound on the base - * address of any flash mapping. In practice, the IO-APIC MMIO range is - * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free - * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in - * size. - */ -#define FLASH_SIZE_LIMIT (8 * MiB) - #define FLASH_SECTOR_SIZE 4096 static void pc_isa_bios_init(MemoryRegion *rom_memory, @@ -182,10 +173,10 @@ static void pc_system_flash_map(PCMachineState *pcms, } if ((hwaddr)size != size || total_size > HWADDR_MAX - size -|| total_size + size > FLASH_SIZE_LIMIT) { +|| total_size + size > pcms->max_fw_size) { error_report("combined size of system firmware exceeds " "%" PRIu64 " bytes", - FLASH_SIZE_LIMIT); + pcms->max_fw_size); exit(1); } diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index fe52e165b2..f7c8e7cbfe 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -43,6 +43,7 @@ struct PCMachineState { bool smbus_enabled; bool sata_enabled; bool pit_enabled; +uint64_t max_fw_size; /* NUMA information: */ uint64_t numa_nodes; @@ -59,6 +60,7 @@ struct PCMachineState { #define PC_MACHINE_SMBUS"smbus" #define PC_MACHINE_SATA "sata" #define PC_MACHINE_PIT "pit" +#define PC_MACHINE_MAX_FW_SIZE "max-fw-size" /** * PCMachineClass: -- 2.25.1