Re: [Qemu-devel] [PATCH v4] hw/pc: Support system flash memory with -pflash parameter

2011-07-23 Thread Anthony Liguori

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

2011-07-23 Thread Jordan Justen
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

2011-07-23 Thread Anthony Liguori

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

2011-07-23 Thread Jordan Justen
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

2011-07-23 Thread Anthony Liguori

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

2011-07-23 Thread Jordan Justen
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

2011-07-23 Thread Anthony Liguori

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

2011-07-15 Thread Jordan Justen
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

2011-07-08 Thread Jordan Justen
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;