Now that the -bios and -pflash code paths work the same it is possible to have a
common implementation.
While at it convert the magic number 0x100000 (== 1MiB) to increase readability.
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
include/hw/i386/x86.h | 2 ++
hw/i386/pc_sysfw.c | 28 ++++------------------------
hw/i386/x86.c | 29 ++++++++++++++++++-----------
3 files changed, 24 insertions(+), 35 deletions(-)
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 4dc30dcb4d..8e6ba4a726 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -116,6 +116,8 @@ void x86_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
void x86_cpu_unplug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp);
+void x86_isa_bios_init(MemoryRegion *rom_memory, MemoryRegion *bios,
+ bool isapc_ram_fw);
void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
MemoryRegion *rom_memory, bool isapc_ram_fw);
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 6e89671c26..e529182b48 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -28,7 +28,6 @@
#include "sysemu/block-backend.h"
#include "qemu/error-report.h"
#include "qemu/option.h"
-#include "qemu/units.h"
#include "hw/sysbus.h"
#include "hw/i386/x86.h"
#include "hw/i386/pc.h"
@@ -40,27 +39,6 @@
#define FLASH_SECTOR_SIZE 4096
-static void pc_isa_bios_init(MemoryRegion *rom_memory,
- MemoryRegion *flash_mem)
-{
- int isa_bios_size;
- MemoryRegion *isa_bios;
- uint64_t flash_size;
-
- flash_size = memory_region_size(flash_mem);
-
- /* map the last 128KB of the BIOS in ISA space */
- isa_bios_size = MIN(flash_size, 128 * KiB);
- isa_bios = g_malloc(sizeof(*isa_bios));
- memory_region_init_alias(isa_bios, NULL, "isa-bios", flash_mem,
- flash_size - isa_bios_size, isa_bios_size);
- memory_region_add_subregion_overlap(rom_memory,
- 0x100000 - isa_bios_size,
- isa_bios,
- 1);
- memory_region_set_readonly(isa_bios, true);
-}
-
static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
const char *name,
const char *alias_prop_name)
@@ -121,7 +99,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
* pcms->max_fw_size.
*
* If pcms->flash[0] has a block backend, its memory is passed to
- * pc_isa_bios_init(). Merging several flash devices for isa-bios is
+ * x86_isa_bios_init(). Merging several flash devices for isa-bios is
* not supported.
*/
static void pc_system_flash_map(PCMachineState *pcms,
@@ -176,7 +154,9 @@ static void pc_system_flash_map(PCMachineState *pcms,
if (i == 0) {
flash_mem = pflash_cfi01_get_memory(system_flash);
- pc_isa_bios_init(rom_memory, flash_mem);
+
+ /* Map the last 128KB of the BIOS in ISA space */
+ x86_isa_bios_init(rom_memory, flash_mem, false);
/* Encrypt the pflash boot ROM */
if (sev_enabled()) {
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 32cd22a4a8..7366b0cee4 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1136,13 +1136,28 @@ void x86_load_linux(X86MachineState *x86ms,
nb_option_roms++;
}
+void x86_isa_bios_init(MemoryRegion *rom_memory, MemoryRegion *bios,
+ bool isapc_ram_fw)
+{
+ int bios_size = memory_region_size(bios);
+ int isa_bios_size = MIN(bios_size, 128 * KiB);
+ MemoryRegion *isa_bios;
+
+ isa_bios = g_malloc(sizeof(*isa_bios));
+ memory_region_init_alias(isa_bios, NULL, "isa-bios", bios,
+ bios_size - isa_bios_size, isa_bios_size);
+ memory_region_add_subregion_overlap(rom_memory, 1 * MiB - isa_bios_size,
+ isa_bios, 1);
+ memory_region_set_readonly(isa_bios, !isapc_ram_fw);
+}
+
void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
MemoryRegion *rom_memory, bool isapc_ram_fw)
{
const char *bios_name;
char *filename;
- MemoryRegion *bios, *isa_bios;
- int bios_size, isa_bios_size;
+ MemoryRegion *bios;
+ int bios_size;
ssize_t ret;
/* BIOS load */
@@ -1180,15 +1195,7 @@ void x86_bios_rom_init(MachineState *ms, const char *default_firmware,
g_free(filename);
/* map the last 128KB of the BIOS in ISA space */
- isa_bios_size = MIN(bios_size, 128 * KiB);
- isa_bios = g_malloc(sizeof(*isa_bios));
- memory_region_init_alias(isa_bios, NULL, "isa-bios", bios,
- bios_size - isa_bios_size, isa_bios_size);
- memory_region_add_subregion_overlap(rom_memory,
- 0x100000 - isa_bios_size,
- isa_bios,
- 1);
- memory_region_set_readonly(isa_bios, !isapc_ram_fw);
+ x86_isa_bios_init(rom_memory, bios, isapc_ram_fw);
/* map all the bios at the top of memory */
memory_region_add_subregion(rom_memory,
--
2.44.0Hi Bernhard,
On 22/4/24 22:06, Bernhard Beschow wrote:
> Now that the -bios and -pflash code paths work the same it is possible to have a
> common implementation.
>
> While at it convert the magic number 0x100000 (== 1MiB) to increase readability.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> include/hw/i386/x86.h | 2 ++
> hw/i386/pc_sysfw.c | 28 ++++------------------------
> hw/i386/x86.c | 29 ++++++++++++++++++-----------
> 3 files changed, 24 insertions(+), 35 deletions(-)
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index 6e89671c26..e529182b48 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -28,7 +28,6 @@
> #include "sysemu/block-backend.h"
> #include "qemu/error-report.h"
> #include "qemu/option.h"
> -#include "qemu/units.h"
> #include "hw/sysbus.h"
> #include "hw/i386/x86.h"
> #include "hw/i386/pc.h"
> @@ -40,27 +39,6 @@
>
> #define FLASH_SECTOR_SIZE 4096
>
> -static void pc_isa_bios_init(MemoryRegion *rom_memory,
> - MemoryRegion *flash_mem)
> -{
> - int isa_bios_size;
> - MemoryRegion *isa_bios;
> - uint64_t flash_size;
> -
> - flash_size = memory_region_size(flash_mem);
> -
> - /* map the last 128KB of the BIOS in ISA space */
> - isa_bios_size = MIN(flash_size, 128 * KiB);
> - isa_bios = g_malloc(sizeof(*isa_bios));
> - memory_region_init_alias(isa_bios, NULL, "isa-bios", flash_mem,
> - flash_size - isa_bios_size, isa_bios_size);
> - memory_region_add_subregion_overlap(rom_memory,
> - 0x100000 - isa_bios_size,
> - isa_bios,
> - 1);
> - memory_region_set_readonly(isa_bios, true);
> -}
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 32cd22a4a8..7366b0cee4 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -1136,13 +1136,28 @@ void x86_load_linux(X86MachineState *x86ms,
> nb_option_roms++;
> }
>
> +void x86_isa_bios_init(MemoryRegion *rom_memory, MemoryRegion *bios,
> + bool isapc_ram_fw)
> +{
> + int bios_size = memory_region_size(bios);
> + int isa_bios_size = MIN(bios_size, 128 * KiB);
> + MemoryRegion *isa_bios;
> +
> + isa_bios = g_malloc(sizeof(*isa_bios));
Pre-existing, but we shouldn't leak MR like that.
Probably better to pass pre-allocated as argument,
smth like:
/**
* x86_isa_bios_init: ... at fixed addr ...
* @isa_bios: MR to initialize
* @isa_mr: ISA bus
* @bios: BIOS MR to map on ISA bus
* @read_only: Map the BIOS as read-only
*/
void x86_isa_bios_init(MemoryRegion *isa_bios,
const MemoryRegion *isa_mr,
const MemoryRegion *bios,
bool read_only);
> + memory_region_init_alias(isa_bios, NULL, "isa-bios", bios,
> + bios_size - isa_bios_size, isa_bios_size);
> + memory_region_add_subregion_overlap(rom_memory, 1 * MiB - isa_bios_size,
> + isa_bios, 1);
> + memory_region_set_readonly(isa_bios, !isapc_ram_fw);
> +}
Am 25. April 2024 07:19:27 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Hi Bernhard,
>
>On 22/4/24 22:06, Bernhard Beschow wrote:
>> Now that the -bios and -pflash code paths work the same it is possible to have a
>> common implementation.
>>
>> While at it convert the magic number 0x100000 (== 1MiB) to increase readability.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> include/hw/i386/x86.h | 2 ++
>> hw/i386/pc_sysfw.c | 28 ++++------------------------
>> hw/i386/x86.c | 29 ++++++++++++++++++-----------
>> 3 files changed, 24 insertions(+), 35 deletions(-)
>
>
>> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>> index 6e89671c26..e529182b48 100644
>> --- a/hw/i386/pc_sysfw.c
>> +++ b/hw/i386/pc_sysfw.c
>> @@ -28,7 +28,6 @@
>> #include "sysemu/block-backend.h"
>> #include "qemu/error-report.h"
>> #include "qemu/option.h"
>> -#include "qemu/units.h"
>> #include "hw/sysbus.h"
>> #include "hw/i386/x86.h"
>> #include "hw/i386/pc.h"
>> @@ -40,27 +39,6 @@
>> #define FLASH_SECTOR_SIZE 4096
>> -static void pc_isa_bios_init(MemoryRegion *rom_memory,
>> - MemoryRegion *flash_mem)
>> -{
>> - int isa_bios_size;
>> - MemoryRegion *isa_bios;
>> - uint64_t flash_size;
>> -
>> - flash_size = memory_region_size(flash_mem);
>> -
>> - /* map the last 128KB of the BIOS in ISA space */
>> - isa_bios_size = MIN(flash_size, 128 * KiB);
>> - isa_bios = g_malloc(sizeof(*isa_bios));
>> - memory_region_init_alias(isa_bios, NULL, "isa-bios", flash_mem,
>> - flash_size - isa_bios_size, isa_bios_size);
>> - memory_region_add_subregion_overlap(rom_memory,
>> - 0x100000 - isa_bios_size,
>> - isa_bios,
>> - 1);
>> - memory_region_set_readonly(isa_bios, true);
>> -}
>
>
>> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> index 32cd22a4a8..7366b0cee4 100644
>> --- a/hw/i386/x86.c
>> +++ b/hw/i386/x86.c
>> @@ -1136,13 +1136,28 @@ void x86_load_linux(X86MachineState *x86ms,
>> nb_option_roms++;
>> }
>> +void x86_isa_bios_init(MemoryRegion *rom_memory, MemoryRegion *bios,
>> + bool isapc_ram_fw)
>> +{
>> + int bios_size = memory_region_size(bios);
>> + int isa_bios_size = MIN(bios_size, 128 * KiB);
>> + MemoryRegion *isa_bios;
>> +
>> + isa_bios = g_malloc(sizeof(*isa_bios));
>
>Pre-existing, but we shouldn't leak MR like that.
>
>Probably better to pass pre-allocated as argument,
>smth like:
>
> /**
> * x86_isa_bios_init: ... at fixed addr ...
> * @isa_bios: MR to initialize
> * @isa_mr: ISA bus
> * @bios: BIOS MR to map on ISA bus
> * @read_only: Map the BIOS as read-only
> */
> void x86_isa_bios_init(MemoryRegion *isa_bios,
> const MemoryRegion *isa_mr,
> const MemoryRegion *bios,
> bool read_only);
>
Same would apply for the "pc.bios" region. I'll fix both then.
Best regards,
Bernhard
>> + memory_region_init_alias(isa_bios, NULL, "isa-bios", bios,
>> + bios_size - isa_bios_size, isa_bios_size);
>> + memory_region_add_subregion_overlap(rom_memory, 1 * MiB - isa_bios_size,
>> + isa_bios, 1);
>> + memory_region_set_readonly(isa_bios, !isapc_ram_fw);
>> +}
>
© 2016 - 2026 Red Hat, Inc.