Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On 07/23/2011 07:18 PM, Jordan Justen wrote: In terms of other flash devices, I don't think it's that simple. Flash is tied to the mobo layout so I don't think index> 0 really makes sense unless you allow a specific mapping address. I doubt that's terribly useful. I think VM's have a different situation than real hardware. I'm not sure an all ROM or all flash decision will work well for qemu. In most cases it may work better to make a ROM image available just below 4GB, and add a flash image below this ROM. This allows the qemu's firmware to be updated as usual in ${prefix}/share/bios.bin, but still allows a flash memory to be available below this. (The flash below the ROM could be used only for storing UEFI variables.) Otherwise when a new qemu is released along with a new firmware image, the VM instance using writable flash will continue to use the old firmware image. Yes, that's a feature. We could have a second nvram for other purposes of course. That could be defined as index=1. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On Sat, Jul 23, 2011 at 15:26, Anthony Liguori wrote: > On 07/23/2011 05:06 PM, Jordan Justen wrote: >> >> On Sat, Jul 23, 2011 at 14:25, Anthony Liguori >> wrote: >>> >>> On 07/23/2011 03:19 PM, Jordan Justen wrote: On Sat, Jul 23, 2011 at 08:51, Anthony Liguori wrote: > > On 07/08/2011 02:37 PM, Jordan Justen wrote: >> >> If -pflash is specified and -bios is specified then pflash will >> be mapped just below the system rom using hw/pflash_cfi01.c. >> >> If -pflash is specified on the command line, but -bios is >> not specified, then 'bios.bin' will NOT be loaded, and >> instead the -pflash flash image will be mapped just below >> 4GB in place of the normal rom image. > > This is way too tied to the pc platform to be this generic. > > I think a better approach would be to default to having unit=0 of > IF_PFLASH > default to a read-only BDS that points to bios.bin. -bios would just > be > a > short cut to use a different file name but you should be able to > override > with -drive too. > > And to really simplify things, you could add a readonly flag to -bios > such > that you could do: > > -bios foo.img,readonly=off > > Which is what I think you're looking for semantically. There seemed to be some feedback on the list interested in preserving a read-only firmware, and just adding a flash region. So, for example, the firmware could be read from a common system location like is generally done with bios.bin today, and VM instance specific flash region could still be added. >>> >>> You can have multiple flash regions. >> >> So, is your recommendation that we support N pflash images in >> x86/x86-64? Instance/index 0 is mapped just under 4GB, and the rest >> follow below this? > > No. There should be a flash device, pflash index 0 is fine, but it should > be mapped under 4GB and also in the legacy BIOS space. > > This is the PC firmware flash. By default it should be read-only and it > should be created by using ${prefix}/share/bios.bin. But it should be > possible to override both the filename and the read-only flag. > > In terms of other flash devices, I don't think it's that simple. Flash is > tied to the mobo layout so I don't think index > 0 really makes sense unless > you allow a specific mapping address. I doubt that's terribly useful. I think VM's have a different situation than real hardware. I'm not sure an all ROM or all flash decision will work well for qemu. In most cases it may work better to make a ROM image available just below 4GB, and add a flash image below this ROM. This allows the qemu's firmware to be updated as usual in ${prefix}/share/bios.bin, but still allows a flash memory to be available below this. (The flash below the ROM could be used only for storing UEFI variables.) Otherwise when a new qemu is released along with a new firmware image, the VM instance using writable flash will continue to use the old firmware image. -Jordan > > Regards, > > Anthony Liguori > >> >> This seems like a good plan, although I can't see a usage for more >> than 2 instances. >> >> -Jordan >> >>> You're introducing two modes. In one mode, we emulate a flash device and >>> expose it for the BIOS ROM. In the second mode, we don't emulate a >>> device >>> but we expose the BIOS ROM based on a file in a shared read-only >>> location. >>> >>> I'm suggesting always emulating a flash device, but by default make the >>> device read-only and have it be loaded from a file in a shared read-only >>> location. >>> >>> That means we have a single code path and a consistent view from a >>> management tooling perspective. IOW, management tools will always see >>> that >>> there is a BIOS block device, and they need to worry about making sure >>> that >>> BIOS block device is there. >>> If the entire firmware is moved to a separate VM instance specific flash, then firmware update also gets complicated. It is no longer just a matter of updating the qemu firmware package in your distro's package management system. >>> >>> I think the bit your misunderstanding is that you should default the >>> firmware to be created from a common file as a read-only device. >>> >>> Regards, >>> >>> Anthony Liguori >>> What about taking your idea, but adding a second drive that would be mapped just below the 1st if it is specified with -drive? Thanks, -Jordan > > Regards, > > Anthony Liguori > >> >> Signed-off-by: Jordan Justen >> Reviewed-by: Aurelien Jarno > > > >> --- >> default-configs/i386-softmmu.mak | 1 + >> default-configs/x86_64-softmmu.mak | 1 + >> hw/pc.c | 161 >> +++- >> 3 files changed, 125 insertions(+), 38 deletions(-) >> >> diff --
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On 07/23/2011 05:06 PM, Jordan Justen wrote: On Sat, Jul 23, 2011 at 14:25, Anthony Liguori wrote: On 07/23/2011 03:19 PM, Jordan Justen wrote: On Sat, Jul 23, 2011 at 08:51, Anthony Liguori wrote: On 07/08/2011 02:37 PM, Jordan Justen wrote: If -pflash is specified and -bios is specified then pflash will be mapped just below the system rom using hw/pflash_cfi01.c. If -pflash is specified on the command line, but -bios is not specified, then 'bios.bin' will NOT be loaded, and instead the -pflash flash image will be mapped just below 4GB in place of the normal rom image. This is way too tied to the pc platform to be this generic. I think a better approach would be to default to having unit=0 of IF_PFLASH default to a read-only BDS that points to bios.bin. -bios would just be a short cut to use a different file name but you should be able to override with -drive too. And to really simplify things, you could add a readonly flag to -bios such that you could do: -bios foo.img,readonly=off Which is what I think you're looking for semantically. There seemed to be some feedback on the list interested in preserving a read-only firmware, and just adding a flash region. So, for example, the firmware could be read from a common system location like is generally done with bios.bin today, and VM instance specific flash region could still be added. You can have multiple flash regions. So, is your recommendation that we support N pflash images in x86/x86-64? Instance/index 0 is mapped just under 4GB, and the rest follow below this? No. There should be a flash device, pflash index 0 is fine, but it should be mapped under 4GB and also in the legacy BIOS space. This is the PC firmware flash. By default it should be read-only and it should be created by using ${prefix}/share/bios.bin. But it should be possible to override both the filename and the read-only flag. In terms of other flash devices, I don't think it's that simple. Flash is tied to the mobo layout so I don't think index > 0 really makes sense unless you allow a specific mapping address. I doubt that's terribly useful. Regards, Anthony Liguori This seems like a good plan, although I can't see a usage for more than 2 instances. -Jordan You're introducing two modes. In one mode, we emulate a flash device and expose it for the BIOS ROM. In the second mode, we don't emulate a device but we expose the BIOS ROM based on a file in a shared read-only location. I'm suggesting always emulating a flash device, but by default make the device read-only and have it be loaded from a file in a shared read-only location. That means we have a single code path and a consistent view from a management tooling perspective. IOW, management tools will always see that there is a BIOS block device, and they need to worry about making sure that BIOS block device is there. If the entire firmware is moved to a separate VM instance specific flash, then firmware update also gets complicated. It is no longer just a matter of updating the qemu firmware package in your distro's package management system. I think the bit your misunderstanding is that you should default the firmware to be created from a common file as a read-only device. Regards, Anthony Liguori What about taking your idea, but adding a second drive that would be mapped just below the 1st if it is specified with -drive? Thanks, -Jordan Regards, Anthony Liguori Signed-off-by: Jordan Justen Reviewed-by: Aurelien Jarno --- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/pc.c| 161 +++- 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 55589fa..8697cd4 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 8895028..eca9284 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc.c b/hw/pc.c index a3e8539..e25354f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -41,6 +41,7 @@ #include "sysemu.h" #include "blockdev.h" #include "ui/qemu-spice.h" +#include "flash.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) } } -void pc_memory_init(const char *kernel_filename, -const char *kernel_cmdline, -const char *initrd_filename, -ram_addr_t below_4g_mem_size, -ram_addr_t above_4g_mem_size) +static void pc_isa_bios_in
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On Sat, Jul 23, 2011 at 14:25, Anthony Liguori wrote: > On 07/23/2011 03:19 PM, Jordan Justen wrote: >> >> On Sat, Jul 23, 2011 at 08:51, Anthony Liguori >> wrote: >>> >>> On 07/08/2011 02:37 PM, Jordan Justen wrote: If -pflash is specified and -bios is specified then pflash will be mapped just below the system rom using hw/pflash_cfi01.c. If -pflash is specified on the command line, but -bios is not specified, then 'bios.bin' will NOT be loaded, and instead the -pflash flash image will be mapped just below 4GB in place of the normal rom image. >>> >>> This is way too tied to the pc platform to be this generic. >>> >>> I think a better approach would be to default to having unit=0 of >>> IF_PFLASH >>> default to a read-only BDS that points to bios.bin. -bios would just be >>> a >>> short cut to use a different file name but you should be able to override >>> with -drive too. >>> >>> And to really simplify things, you could add a readonly flag to -bios >>> such >>> that you could do: >>> >>> -bios foo.img,readonly=off >>> >>> Which is what I think you're looking for semantically. >> >> There seemed to be some feedback on the list interested in preserving >> a read-only firmware, and just adding a flash region. >> >> So, for example, the firmware could be read from a common system >> location like is generally done with bios.bin today, and VM instance >> specific flash region could still be added. > > You can have multiple flash regions. So, is your recommendation that we support N pflash images in x86/x86-64? Instance/index 0 is mapped just under 4GB, and the rest follow below this? This seems like a good plan, although I can't see a usage for more than 2 instances. -Jordan > You're introducing two modes. In one mode, we emulate a flash device and > expose it for the BIOS ROM. In the second mode, we don't emulate a device > but we expose the BIOS ROM based on a file in a shared read-only location. > > I'm suggesting always emulating a flash device, but by default make the > device read-only and have it be loaded from a file in a shared read-only > location. > > That means we have a single code path and a consistent view from a > management tooling perspective. IOW, management tools will always see that > there is a BIOS block device, and they need to worry about making sure that > BIOS block device is there. > >> >> If the entire firmware is moved to a separate VM instance specific >> flash, then firmware update also gets complicated. It is no longer >> just a matter of updating the qemu firmware package in your distro's >> package management system. > > I think the bit your misunderstanding is that you should default the > firmware to be created from a common file as a read-only device. > > Regards, > > Anthony Liguori > >> >> What about taking your idea, but adding a second drive that would be >> mapped just below the 1st if it is specified with -drive? >> >> Thanks, >> >> -Jordan >> >>> >>> Regards, >>> >>> Anthony Liguori >>> Signed-off-by: Jordan Justen Reviewed-by: Aurelien Jarno >>> >>> >>> --- default-configs/i386-softmmu.mak | 1 + default-configs/x86_64-softmmu.mak | 1 + hw/pc.c | 161 +++- 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 55589fa..8697cd4 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 8895028..eca9284 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc.c b/hw/pc.c index a3e8539..e25354f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -41,6 +41,7 @@ #include "sysemu.h" #include "blockdev.h" #include "ui/qemu-spice.h" +#include "flash.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) } } -void pc_memory_init(const char *kernel_filename, - const char *kernel_cmdline, - const char *initrd_filename, - ram_addr_t below_4g_mem_size, - ram_addr_t above_4g_mem_size) +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) { - char *filename; - int ret, linux_boot, i; - ram_addr_t ram_addr, bios_offset, option_rom_offset; - int bios_si
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On 07/23/2011 03:19 PM, Jordan Justen wrote: On Sat, Jul 23, 2011 at 08:51, Anthony Liguori wrote: On 07/08/2011 02:37 PM, Jordan Justen wrote: If -pflash is specified and -bios is specified then pflash will be mapped just below the system rom using hw/pflash_cfi01.c. If -pflash is specified on the command line, but -bios is not specified, then 'bios.bin' will NOT be loaded, and instead the -pflash flash image will be mapped just below 4GB in place of the normal rom image. This is way too tied to the pc platform to be this generic. I think a better approach would be to default to having unit=0 of IF_PFLASH default to a read-only BDS that points to bios.bin. -bios would just be a short cut to use a different file name but you should be able to override with -drive too. And to really simplify things, you could add a readonly flag to -bios such that you could do: -bios foo.img,readonly=off Which is what I think you're looking for semantically. There seemed to be some feedback on the list interested in preserving a read-only firmware, and just adding a flash region. So, for example, the firmware could be read from a common system location like is generally done with bios.bin today, and VM instance specific flash region could still be added. You can have multiple flash regions. You're introducing two modes. In one mode, we emulate a flash device and expose it for the BIOS ROM. In the second mode, we don't emulate a device but we expose the BIOS ROM based on a file in a shared read-only location. I'm suggesting always emulating a flash device, but by default make the device read-only and have it be loaded from a file in a shared read-only location. That means we have a single code path and a consistent view from a management tooling perspective. IOW, management tools will always see that there is a BIOS block device, and they need to worry about making sure that BIOS block device is there. If the entire firmware is moved to a separate VM instance specific flash, then firmware update also gets complicated. It is no longer just a matter of updating the qemu firmware package in your distro's package management system. I think the bit your misunderstanding is that you should default the firmware to be created from a common file as a read-only device. Regards, Anthony Liguori What about taking your idea, but adding a second drive that would be mapped just below the 1st if it is specified with -drive? Thanks, -Jordan Regards, Anthony Liguori Signed-off-by: Jordan Justen Reviewed-by: Aurelien Jarno --- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/pc.c| 161 +++- 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 55589fa..8697cd4 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 8895028..eca9284 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc.c b/hw/pc.c index a3e8539..e25354f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -41,6 +41,7 @@ #include "sysemu.h" #include "blockdev.h" #include "ui/qemu-spice.h" +#include "flash.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) } } -void pc_memory_init(const char *kernel_filename, -const char *kernel_cmdline, -const char *initrd_filename, -ram_addr_t below_4g_mem_size, -ram_addr_t above_4g_mem_size) +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) { -char *filename; -int ret, linux_boot, i; -ram_addr_t ram_addr, bios_offset, option_rom_offset; -int bios_size, isa_bios_size; -void *fw_cfg; - -linux_boot = (kernel_filename != NULL); +int isa_bios_size; -/* allocate RAM */ -ram_addr = qemu_ram_alloc(NULL, "pc.ram", - below_4g_mem_size + above_4g_mem_size); -cpu_register_physical_memory(0, 0xa, ram_addr); -cpu_register_physical_memory(0x10, - below_4g_mem_size - 0x10, - ram_addr + 0x10); -if (above_4g_mem_size>0) { -cpu_register_physical_memory(0x1ULL, above_4g_mem_size, - ram_addr + below_4g_mem_size); +/* map the last 128KB of the BIOS in ISA space */ +isa_bios_size = ram_size; +if (isa_bios_size>(128 * 1024)) { +isa_bios_size = 1
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On Sat, Jul 23, 2011 at 08:51, Anthony Liguori wrote: > On 07/08/2011 02:37 PM, Jordan Justen wrote: >> >> If -pflash is specified and -bios is specified then pflash will >> be mapped just below the system rom using hw/pflash_cfi01.c. >> >> If -pflash is specified on the command line, but -bios is >> not specified, then 'bios.bin' will NOT be loaded, and >> instead the -pflash flash image will be mapped just below >> 4GB in place of the normal rom image. > > This is way too tied to the pc platform to be this generic. > > I think a better approach would be to default to having unit=0 of IF_PFLASH > default to a read-only BDS that points to bios.bin. -bios would just be a > short cut to use a different file name but you should be able to override > with -drive too. > > And to really simplify things, you could add a readonly flag to -bios such > that you could do: > > -bios foo.img,readonly=off > > Which is what I think you're looking for semantically. There seemed to be some feedback on the list interested in preserving a read-only firmware, and just adding a flash region. So, for example, the firmware could be read from a common system location like is generally done with bios.bin today, and VM instance specific flash region could still be added. If the entire firmware is moved to a separate VM instance specific flash, then firmware update also gets complicated. It is no longer just a matter of updating the qemu firmware package in your distro's package management system. What about taking your idea, but adding a second drive that would be mapped just below the 1st if it is specified with -drive? Thanks, -Jordan > > Regards, > > Anthony Liguori > >> >> Signed-off-by: Jordan Justen >> Reviewed-by: Aurelien Jarno > > > >> --- >> default-configs/i386-softmmu.mak | 1 + >> default-configs/x86_64-softmmu.mak | 1 + >> hw/pc.c | 161 >> +++- >> 3 files changed, 125 insertions(+), 38 deletions(-) >> >> diff --git a/default-configs/i386-softmmu.mak >> b/default-configs/i386-softmmu.mak >> index 55589fa..8697cd4 100644 >> --- a/default-configs/i386-softmmu.mak >> +++ b/default-configs/i386-softmmu.mak >> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >> CONFIG_SOUND=y >> CONFIG_HPET=y >> CONFIG_APPLESMC=y >> +CONFIG_PFLASH_CFI01=y >> diff --git a/default-configs/x86_64-softmmu.mak >> b/default-configs/x86_64-softmmu.mak >> index 8895028..eca9284 100644 >> --- a/default-configs/x86_64-softmmu.mak >> +++ b/default-configs/x86_64-softmmu.mak >> @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y >> CONFIG_SOUND=y >> CONFIG_HPET=y >> CONFIG_APPLESMC=y >> +CONFIG_PFLASH_CFI01=y >> diff --git a/hw/pc.c b/hw/pc.c >> index a3e8539..e25354f 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -41,6 +41,7 @@ >> #include "sysemu.h" >> #include "blockdev.h" >> #include "ui/qemu-spice.h" >> +#include "flash.h" >> >> /* output Bochs bios info messages */ >> //#define DEBUG_BIOS >> @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) >> } >> } >> >> -void pc_memory_init(const char *kernel_filename, >> - const char *kernel_cmdline, >> - const char *initrd_filename, >> - ram_addr_t below_4g_mem_size, >> - ram_addr_t above_4g_mem_size) >> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) >> { >> - char *filename; >> - int ret, linux_boot, i; >> - ram_addr_t ram_addr, bios_offset, option_rom_offset; >> - int bios_size, isa_bios_size; >> - void *fw_cfg; >> - >> - linux_boot = (kernel_filename != NULL); >> + int isa_bios_size; >> >> - /* allocate RAM */ >> - ram_addr = qemu_ram_alloc(NULL, "pc.ram", >> - below_4g_mem_size + above_4g_mem_size); >> - cpu_register_physical_memory(0, 0xa, ram_addr); >> - cpu_register_physical_memory(0x10, >> - below_4g_mem_size - 0x10, >> - ram_addr + 0x10); >> - if (above_4g_mem_size> 0) { >> - cpu_register_physical_memory(0x1ULL, above_4g_mem_size, >> - ram_addr + below_4g_mem_size); >> + /* map the last 128KB of the BIOS in ISA space */ >> + isa_bios_size = ram_size; >> + if (isa_bios_size> (128 * 1024)) { >> + isa_bios_size = 128 * 1024; >> } >> + ram_offset = ram_offset + ram_size - isa_bios_size; >> + cpu_register_physical_memory(0x10 - isa_bios_size, >> + isa_bios_size, >> + ram_offset | IO_MEM_ROM); >> +} >> + >> +static int pc_system_rom_init(void) >> +{ >> + int ret; >> + int bios_size; >> + ram_addr_t bios_offset; >> + char *filename; >> >> /* BIOS load */ >> - if (bios_name == NULL) >> + if (bios_name == NULL) { >> bios_name = BIOS_FILENAME; >> + } >> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); >> if (filename) { >>
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
On 07/08/2011 02:37 PM, Jordan Justen wrote: If -pflash is specified and -bios is specified then pflash will be mapped just below the system rom using hw/pflash_cfi01.c. If -pflash is specified on the command line, but -bios is not specified, then 'bios.bin' will NOT be loaded, and instead the -pflash flash image will be mapped just below 4GB in place of the normal rom image. This is way too tied to the pc platform to be this generic. I think a better approach would be to default to having unit=0 of IF_PFLASH default to a read-only BDS that points to bios.bin. -bios would just be a short cut to use a different file name but you should be able to override with -drive too. And to really simplify things, you could add a readonly flag to -bios such that you could do: -bios foo.img,readonly=off Which is what I think you're looking for semantically. Regards, Anthony Liguori Signed-off-by: Jordan Justen Reviewed-by: Aurelien Jarno --- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/pc.c| 161 +++- 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 55589fa..8697cd4 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 8895028..eca9284 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc.c b/hw/pc.c index a3e8539..e25354f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -41,6 +41,7 @@ #include "sysemu.h" #include "blockdev.h" #include "ui/qemu-spice.h" +#include "flash.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) } } -void pc_memory_init(const char *kernel_filename, -const char *kernel_cmdline, -const char *initrd_filename, -ram_addr_t below_4g_mem_size, -ram_addr_t above_4g_mem_size) +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) { -char *filename; -int ret, linux_boot, i; -ram_addr_t ram_addr, bios_offset, option_rom_offset; -int bios_size, isa_bios_size; -void *fw_cfg; - -linux_boot = (kernel_filename != NULL); +int isa_bios_size; -/* allocate RAM */ -ram_addr = qemu_ram_alloc(NULL, "pc.ram", - below_4g_mem_size + above_4g_mem_size); -cpu_register_physical_memory(0, 0xa, ram_addr); -cpu_register_physical_memory(0x10, - below_4g_mem_size - 0x10, - ram_addr + 0x10); -if (above_4g_mem_size> 0) { -cpu_register_physical_memory(0x1ULL, above_4g_mem_size, - ram_addr + below_4g_mem_size); +/* map the last 128KB of the BIOS in ISA space */ +isa_bios_size = ram_size; +if (isa_bios_size> (128 * 1024)) { +isa_bios_size = 128 * 1024; } +ram_offset = ram_offset + ram_size - isa_bios_size; +cpu_register_physical_memory(0x10 - isa_bios_size, + isa_bios_size, + ram_offset | IO_MEM_ROM); +} + +static int pc_system_rom_init(void) +{ +int ret; +int bios_size; +ram_addr_t bios_offset; +char *filename; /* BIOS load */ -if (bios_name == NULL) +if (bios_name == NULL) { bios_name = BIOS_FILENAME; +} filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); if (filename) { bios_size = get_image_size(filename); } else { bios_size = -1; } -if (bios_size<= 0 || -(bios_size % 65536) != 0) { -goto bios_error; + +if (bios_size<= 0 || (bios_size % 65536) != 0) { +ret = -1; +} else { +bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); +ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); } -bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); -ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); + if (ret != 0) { -bios_error: fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name); exit(1); } + if (filename) { qemu_free(filename); } -/* map the last 128KB of the BIOS in ISA space */ -isa_bios_size = bios_size; -if (isa_bios_size> (128 * 1024)) -isa_bios_size = 128 * 1024; -cpu_register_physical_memory(0x10 - isa_bios_size, - isa_bios_size, -
Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
Hi all, Are there any concerns with this patch? I haven't heard much feedback, except: * Jes Sorensen - March 28 - code style * Aurelien Jarno - April 18 - Reviewed-by Thanks, -Jordan On Fri, Jul 8, 2011 at 12:37, Jordan Justen wrote: > If -pflash is specified and -bios is specified then pflash will > be mapped just below the system rom using hw/pflash_cfi01.c. > > If -pflash is specified on the command line, but -bios is > not specified, then 'bios.bin' will NOT be loaded, and > instead the -pflash flash image will be mapped just below > 4GB in place of the normal rom image. > > Signed-off-by: Jordan Justen > Reviewed-by: Aurelien Jarno > --- > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/pc.c | 161 > +++- > 3 files changed, 125 insertions(+), 38 deletions(-) > > diff --git a/default-configs/i386-softmmu.mak > b/default-configs/i386-softmmu.mak > index 55589fa..8697cd4 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y > CONFIG_SOUND=y > CONFIG_HPET=y > CONFIG_APPLESMC=y > +CONFIG_PFLASH_CFI01=y > diff --git a/default-configs/x86_64-softmmu.mak > b/default-configs/x86_64-softmmu.mak > index 8895028..eca9284 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y > CONFIG_SOUND=y > CONFIG_HPET=y > CONFIG_APPLESMC=y > +CONFIG_PFLASH_CFI01=y > diff --git a/hw/pc.c b/hw/pc.c > index a3e8539..e25354f 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -41,6 +41,7 @@ > #include "sysemu.h" > #include "blockdev.h" > #include "ui/qemu-spice.h" > +#include "flash.h" > > /* output Bochs bios info messages */ > //#define DEBUG_BIOS > @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) > } > } > > -void pc_memory_init(const char *kernel_filename, > - const char *kernel_cmdline, > - const char *initrd_filename, > - ram_addr_t below_4g_mem_size, > - ram_addr_t above_4g_mem_size) > +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) > { > - char *filename; > - int ret, linux_boot, i; > - ram_addr_t ram_addr, bios_offset, option_rom_offset; > - int bios_size, isa_bios_size; > - void *fw_cfg; > - > - linux_boot = (kernel_filename != NULL); > + int isa_bios_size; > > - /* allocate RAM */ > - ram_addr = qemu_ram_alloc(NULL, "pc.ram", > - below_4g_mem_size + above_4g_mem_size); > - cpu_register_physical_memory(0, 0xa, ram_addr); > - cpu_register_physical_memory(0x10, > - below_4g_mem_size - 0x10, > - ram_addr + 0x10); > - if (above_4g_mem_size > 0) { > - cpu_register_physical_memory(0x1ULL, above_4g_mem_size, > - ram_addr + below_4g_mem_size); > + /* map the last 128KB of the BIOS in ISA space */ > + isa_bios_size = ram_size; > + if (isa_bios_size > (128 * 1024)) { > + isa_bios_size = 128 * 1024; > } > + ram_offset = ram_offset + ram_size - isa_bios_size; > + cpu_register_physical_memory(0x10 - isa_bios_size, > + isa_bios_size, > + ram_offset | IO_MEM_ROM); > +} > + > +static int pc_system_rom_init(void) > +{ > + int ret; > + int bios_size; > + ram_addr_t bios_offset; > + char *filename; > > /* BIOS load */ > - if (bios_name == NULL) > + if (bios_name == NULL) { > bios_name = BIOS_FILENAME; > + } > filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); > if (filename) { > bios_size = get_image_size(filename); > } else { > bios_size = -1; > } > - if (bios_size <= 0 || > - (bios_size % 65536) != 0) { > - goto bios_error; > + > + if (bios_size <= 0 || (bios_size % 65536) != 0) { > + ret = -1; > + } else { > + bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); > + ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); > } > - bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); > - ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); > + > if (ret != 0) { > - bios_error: > fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name); > exit(1); > } > + > if (filename) { > qemu_free(filename); > } > - /* map the last 128KB of the BIOS in ISA space */ > - isa_bios_size = bios_size; > - if (isa_bios_size > (128 * 1024)) > - isa_bios_size = 128 * 1024; > - cpu_register_physical_memory(0x10 - isa_bios_size, > - isa_bios_size, > - (bios_offset + bios_size - isa_bios_size) | > IO_MEM_ROM); > > - option_rom_offset
[Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter
If -pflash is specified and -bios is specified then pflash will be mapped just below the system rom using hw/pflash_cfi01.c. If -pflash is specified on the command line, but -bios is not specified, then 'bios.bin' will NOT be loaded, and instead the -pflash flash image will be mapped just below 4GB in place of the normal rom image. Signed-off-by: Jordan Justen Reviewed-by: Aurelien Jarno --- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/pc.c| 161 +++- 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 55589fa..8697cd4 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 8895028..eca9284 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -21,3 +21,4 @@ CONFIG_PIIX_PCI=y CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc.c b/hw/pc.c index a3e8539..e25354f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -41,6 +41,7 @@ #include "sysemu.h" #include "blockdev.h" #include "ui/qemu-spice.h" +#include "flash.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -957,70 +958,154 @@ void pc_cpus_init(const char *cpu_model) } } -void pc_memory_init(const char *kernel_filename, -const char *kernel_cmdline, -const char *initrd_filename, -ram_addr_t below_4g_mem_size, -ram_addr_t above_4g_mem_size) +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) { -char *filename; -int ret, linux_boot, i; -ram_addr_t ram_addr, bios_offset, option_rom_offset; -int bios_size, isa_bios_size; -void *fw_cfg; - -linux_boot = (kernel_filename != NULL); +int isa_bios_size; -/* allocate RAM */ -ram_addr = qemu_ram_alloc(NULL, "pc.ram", - below_4g_mem_size + above_4g_mem_size); -cpu_register_physical_memory(0, 0xa, ram_addr); -cpu_register_physical_memory(0x10, - below_4g_mem_size - 0x10, - ram_addr + 0x10); -if (above_4g_mem_size > 0) { -cpu_register_physical_memory(0x1ULL, above_4g_mem_size, - ram_addr + below_4g_mem_size); +/* map the last 128KB of the BIOS in ISA space */ +isa_bios_size = ram_size; +if (isa_bios_size > (128 * 1024)) { +isa_bios_size = 128 * 1024; } +ram_offset = ram_offset + ram_size - isa_bios_size; +cpu_register_physical_memory(0x10 - isa_bios_size, + isa_bios_size, + ram_offset | IO_MEM_ROM); +} + +static int pc_system_rom_init(void) +{ +int ret; +int bios_size; +ram_addr_t bios_offset; +char *filename; /* BIOS load */ -if (bios_name == NULL) +if (bios_name == NULL) { bios_name = BIOS_FILENAME; +} filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); if (filename) { bios_size = get_image_size(filename); } else { bios_size = -1; } -if (bios_size <= 0 || -(bios_size % 65536) != 0) { -goto bios_error; + +if (bios_size <= 0 || (bios_size % 65536) != 0) { +ret = -1; +} else { +bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); +ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); } -bios_offset = qemu_ram_alloc(NULL, "pc.bios", bios_size); -ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); + if (ret != 0) { -bios_error: fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", bios_name); exit(1); } + if (filename) { qemu_free(filename); } -/* map the last 128KB of the BIOS in ISA space */ -isa_bios_size = bios_size; -if (isa_bios_size > (128 * 1024)) -isa_bios_size = 128 * 1024; -cpu_register_physical_memory(0x10 - isa_bios_size, - isa_bios_size, - (bios_offset + bios_size - isa_bios_size) | IO_MEM_ROM); -option_rom_offset = qemu_ram_alloc(NULL, "pc.rom", PC_ROM_SIZE); -cpu_register_physical_memory(PC_ROM_MIN_VGA, PC_ROM_SIZE, option_rom_offset); +pc_isa_bios_init(bios_offset, bios_size); /* map all the bios at the top of memory */ cpu_register_physical_memory((uint32_t)(-bios_size), bios_size, bios_offset | IO_MEM_ROM); +return bios_size; +} + +static void pc_system_flash_init(DriveInfo *pflash_drv, int rom_size) +{ +BlockDriverState *bdrv;