[Qemu-devel] [PATCH v7 0/4] KVM flash memory support

[Qemu-devel] [PATCH v7 0/4] KVM flash memory support

[Jordan Justen]

git://github.com/jljusten/qemu.git kvm-flash-v7

Utilize KVM_CAP_READONLY_MEM to support PC system flash emulation
with KVM.

v7:
 * Update for readable => romd_mode rename (5f9a5ea1)

v6:
 * Rebase to master following 9e1c2ec8
 * Make sure patch 1 "isapc: Fix non-KVM qemu boot" can be
   applied individually. (It is a candidate for 1.5.)

v5:
 * Remove patch to pflash_cfi01 which enabled readonly mode
 * Adjust kvm code to use KVM READONLY support for ranges that
   either have the readonly flag set, or for devices with
   readable set.

v4:
 * With a machine type of isapc, don't mark the BIOS as read-only.
   isapc + seabios will not boot if the BIOS is read-only. This
   matches the current behavior of isapc with KVM, which is the
   only mode under which isapc currently works.

v3:
 * Squash patch 2 & 3 based on Xiao's feedback that what I
   was calling a 'workaround' in patch 3 was actually what
   is required by the KVM READONLY memory support.

v2:
 * Remove rom_only from PC_COMPAT_1_4
 * Only enable flash when a pflash drive is created.

Jordan Justen (4):
  isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)
  kvm: add kvm_readonly_mem_enabled
  kvm: support using KVM_MEM_READONLY flag for regions
  pc_sysfw: allow flash (-pflash) memory to be used with KVM

 hw/block/pc_sysfw.c  |   62 ++++++++++++++++++++++++++++++++------------------
 hw/i386/pc_piix.c    |    5 ++++
 include/sysemu/kvm.h |   10 ++++++++
 kvm-all.c            |   43 ++++++++++++++++++++++++++--------
 kvm-stub.c           |    1 +
 5 files changed, 90 insertions(+), 31 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Jordan Justen]

The isapc machine with seabios currently requires the BIOS region
to be read/write memory rather than read-only memory.

KVM currently cannot support the BIOS as a ROM region, but qemu
in non-KVM mode can. Based on this, isapc machine currently only
works with KVM.

To work-around this isapc issue, this change avoids marking the
BIOS as readonly for isapc.

This change also will allow KVM to start supporting ROM mode
via KVM_CAP_READONLY_MEM.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/pc_sysfw.c |   16 +++++++++++-----
 hw/i386/pc_piix.c   |    5 +++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/block/pc_sysfw.c b/hw/block/pc_sysfw.c
index 4f17668..ea1a355 100644
--- a/hw/block/pc_sysfw.c
+++ b/hw/block/pc_sysfw.c
@@ -39,6 +39,7 @@
 typedef struct PcSysFwDevice {
     SysBusDevice busdev;
     uint8_t rom_only;
+    uint8_t isapc_ram_fw;
 } PcSysFwDevice;
 
 static void pc_isa_bios_init(MemoryRegion *rom_memory,
@@ -139,7 +140,7 @@ static void pc_system_flash_init(MemoryRegion *rom_memory,
     pc_isa_bios_init(rom_memory, flash_mem, size);
 }
 
-static void old_pc_system_rom_init(MemoryRegion *rom_memory)
+static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
 {
     char *filename;
     MemoryRegion *bios, *isa_bios;
@@ -163,7 +164,9 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
     bios = g_malloc(sizeof(*bios));
     memory_region_init_ram(bios, "pc.bios", bios_size);
     vmstate_register_ram_global(bios);
-    memory_region_set_readonly(bios, true);
+    if (!isapc_ram_fw) {
+        memory_region_set_readonly(bios, true);
+    }
     ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
     if (ret != 0) {
     bios_error:
@@ -186,7 +189,9 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
                                         0x100000 - isa_bios_size,
                                         isa_bios,
                                         1);
-    memory_region_set_readonly(isa_bios, true);
+    if (!isapc_ram_fw) {
+        memory_region_set_readonly(isa_bios, true);
+    }
 
     /* map all the bios at the top of memory */
     memory_region_add_subregion(rom_memory,
@@ -216,7 +221,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
     qdev_init_nofail(DEVICE(sysfw_dev));
 
     if (sysfw_dev->rom_only) {
-        old_pc_system_rom_init(rom_memory);
+        old_pc_system_rom_init(rom_memory, sysfw_dev->isapc_ram_fw);
         return;
     }
 
@@ -234,7 +239,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
             exit(1);
         } else {
             sysfw_dev->rom_only = 1;
-            old_pc_system_rom_init(rom_memory);
+            old_pc_system_rom_init(rom_memory, sysfw_dev->isapc_ram_fw);
             return;
         }
     }
@@ -255,6 +260,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
 }
 
 static Property pcsysfw_properties[] = {
+    DEFINE_PROP_UINT8("isapc_ram_fw", PcSysFwDevice, isapc_ram_fw, 0),
     DEFINE_PROP_UINT8("rom_only", PcSysFwDevice, rom_only, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 43ab480..530b6ab 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -713,6 +713,11 @@ static QEMUMachine isapc_machine = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "pc-sysfw",
+            .property = "isapc_ram_fw",
+            .value    = stringify(1),
+        },
         { /* end of list */ }
     },
     DEFAULT_MACHINE_OPTIONS,
-- 
1.7.10.4

[Qemu-devel] [PATCH v7 2/4] kvm: add kvm_readonly_mem_enabled

[Jordan Justen]

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/kvm.h |   10 ++++++++++
 kvm-all.c            |    6 ++++++
 kvm-stub.c           |    1 +
 3 files changed, 17 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 08284ef..8b19322 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -45,6 +45,7 @@ extern bool kvm_async_interrupts_allowed;
 extern bool kvm_irqfds_allowed;
 extern bool kvm_msi_via_irqfd_allowed;
 extern bool kvm_gsi_routing_allowed;
+extern bool kvm_readonly_mem_allowed;
 
 #if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled()           (kvm_allowed)
@@ -97,6 +98,14 @@ extern bool kvm_gsi_routing_allowed;
  */
 #define kvm_gsi_routing_enabled() (kvm_gsi_routing_allowed)
 
+/**
+ * kvm_readonly_mem_enabled:
+ *
+ * Returns: true if KVM readonly memory is enabled (ie the kernel
+ * supports it and we're running in a configuration that permits it).
+ */
+#define kvm_readonly_mem_enabled() (kvm_readonly_mem_allowed)
+
 #else
 #define kvm_enabled()           (0)
 #define kvm_irqchip_in_kernel() (false)
@@ -104,6 +113,7 @@ extern bool kvm_gsi_routing_allowed;
 #define kvm_irqfds_enabled() (false)
 #define kvm_msi_via_irqfd_enabled() (false)
 #define kvm_gsi_routing_allowed() (false)
+#define kvm_readonly_mem_enabled() (false)
 #endif
 
 struct kvm_run;
diff --git a/kvm-all.c b/kvm-all.c
index 8222729..327ae12 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -111,6 +111,7 @@ bool kvm_irqfds_allowed;
 bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
 bool kvm_allowed;
+bool kvm_readonly_mem_allowed;
 
 static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_INFO(USER_MEMORY),
@@ -1425,6 +1426,11 @@ int kvm_init(void)
         s->irq_set_ioctl = KVM_IRQ_LINE_STATUS;
     }
 
+#ifdef KVM_CAP_READONLY_MEM
+    kvm_readonly_mem_allowed =
+        (kvm_check_extension(s, KVM_CAP_READONLY_MEM) > 0);
+#endif
+
     ret = kvm_arch_init(s);
     if (ret < 0) {
         goto err;
diff --git a/kvm-stub.c b/kvm-stub.c
index b2c8f9b..22eaff0 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -26,6 +26,7 @@ bool kvm_irqfds_allowed;
 bool kvm_msi_via_irqfd_allowed;
 bool kvm_gsi_routing_allowed;
 bool kvm_allowed;
+bool kvm_readonly_mem_allowed;
 
 int kvm_init_vcpu(CPUState *cpu)
 {
-- 
1.7.10.4

[Qemu-devel] [PATCH v7 3/4] kvm: support using KVM_MEM_READONLY flag for regions

[Jordan Justen]

For readonly memory regions and rom devices in romd_mode,
we make use of the KVM_MEM_READONLY. A slot that uses
KVM_MEM_READONLY can be read from and code can execute from the
region, but writes will exit to qemu.

For rom devices with !romd_mode, we force the slot to be
removed so reads or writes to the region will exit to qemu.
(Note that a memory region in this state is not executable
within kvm.)

v7:
 * Update for readable => romd_mode rename (5f9a5ea1)

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> (v4)
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> (v5)
---
 kvm-all.c |   37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 327ae12..8e7bbf8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -201,12 +201,18 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
 
     mem.slot = slot->slot;
     mem.guest_phys_addr = slot->start_addr;
-    mem.memory_size = slot->memory_size;
     mem.userspace_addr = (unsigned long)slot->ram;
     mem.flags = slot->flags;
     if (s->migration_log) {
         mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
     }
+    if (mem.flags & KVM_MEM_READONLY) {
+        /* Set the slot size to 0 before setting the slot to the desired
+         * value. This is needed based on KVM commit 75d61fbc. */
+        mem.memory_size = 0;
+        kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
+    }
+    mem.memory_size = slot->memory_size;
     return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
 }
 
@@ -268,9 +274,14 @@ err:
  * dirty pages logging control
  */
 
-static int kvm_mem_flags(KVMState *s, bool log_dirty)
+static int kvm_mem_flags(KVMState *s, bool log_dirty, bool readonly)
 {
-    return log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
+    int flags = 0;
+    flags = log_dirty ? KVM_MEM_LOG_DIRTY_PAGES : 0;
+    if (readonly && kvm_readonly_mem_allowed) {
+        flags |= KVM_MEM_READONLY;
+    }
+    return flags;
 }
 
 static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
@@ -281,7 +292,7 @@ static int kvm_slot_dirty_pages_log_change(KVMSlot *mem, bool log_dirty)
 
     old_flags = mem->flags;
 
-    flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty);
+    flags = (mem->flags & ~mask) | kvm_mem_flags(s, log_dirty, false);
     mem->flags = flags;
 
     /* If nothing changed effectively, no need to issue ioctl */
@@ -619,6 +630,8 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     int err;
     MemoryRegion *mr = section->mr;
     bool log_dirty = memory_region_is_logging(mr);
+    bool writeable = !mr->readonly && !mr->rom_device;
+    bool readonly_flag = mr->readonly || memory_region_is_romd(mr);
     hwaddr start_addr = section->offset_within_address_space;
     ram_addr_t size = section->size;
     void *ram = NULL;
@@ -638,7 +651,13 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     }
 
     if (!memory_region_is_ram(mr)) {
-        return;
+        if (writeable || !kvm_readonly_mem_allowed) {
+            return;
+        } else if (!mr->romd_mode) {
+            /* If the memory device is not in romd_mode, then we actually want
+             * to remove the kvm memory slot so all accesses will trap. */
+            add = false;
+        }
     }
 
     ram = memory_region_get_ram_ptr(mr) + section->offset_within_region + delta;
@@ -687,7 +706,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             mem->memory_size = old.memory_size;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags = kvm_mem_flags(s, log_dirty);
+            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -708,7 +727,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             mem->memory_size = start_addr - old.start_addr;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags =  kvm_mem_flags(s, log_dirty);
+            mem->flags =  kvm_mem_flags(s, log_dirty, readonly_flag);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -732,7 +751,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
             size_delta = mem->start_addr - old.start_addr;
             mem->memory_size = old.memory_size - size_delta;
             mem->ram = old.ram + size_delta;
-            mem->flags = kvm_mem_flags(s, log_dirty);
+            mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -754,7 +773,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add)
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->ram = ram;
-    mem->flags = kvm_mem_flags(s, log_dirty);
+    mem->flags = kvm_mem_flags(s, log_dirty, readonly_flag);
 
     err = kvm_set_user_memory_region(s, mem);
     if (err) {
-- 
1.7.10.4

[Qemu-devel] [PATCH v7 4/4] pc_sysfw: allow flash (-pflash) memory to be used with KVM

[Jordan Justen]

When pc-sysfw.rom_only == 0, flash memory will be
usable with kvm. In order to enable flash memory mode,
a pflash device must be created. (For example, by
using the -pflash command line parameter.)

Usage of a flash memory device with kvm requires
KVM_CAP_READONLY_MEM, and kvm will abort if
a flash device is used with an older kvm which does
not support this capability.

If a flash device is not used, then qemu/kvm will
operate in the original rom-mode.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/block/pc_sysfw.c |   50 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/hw/block/pc_sysfw.c b/hw/block/pc_sysfw.c
index ea1a355..412d1b0 100644
--- a/hw/block/pc_sysfw.c
+++ b/hw/block/pc_sysfw.c
@@ -220,28 +220,40 @@ void pc_system_firmware_init(MemoryRegion *rom_memory)
 
     qdev_init_nofail(DEVICE(sysfw_dev));
 
-    if (sysfw_dev->rom_only) {
-        old_pc_system_rom_init(rom_memory, sysfw_dev->isapc_ram_fw);
-        return;
-    }
-
     pflash_drv = drive_get(IF_PFLASH, 0, 0);
 
-    /* Currently KVM cannot execute from device memory.
-       Use old rom based firmware initialization for KVM. */
-    /*
-     * This is a Bad Idea, because it makes enabling/disabling KVM
-     * guest-visible.  Let's fix it for real in QEMU 1.6.
-     */
-    if (kvm_enabled()) {
-        if (pflash_drv != NULL) {
-            fprintf(stderr, "qemu: pflash cannot be used with kvm enabled\n");
-            exit(1);
-        } else {
-            sysfw_dev->rom_only = 1;
-            old_pc_system_rom_init(rom_memory, sysfw_dev->isapc_ram_fw);
-            return;
+    if (pc_sysfw_flash_vs_rom_bug_compatible) {
+        /*
+         * This is a Bad Idea, because it makes enabling/disabling KVM
+         * guest-visible.  Do it only in bug-compatibility mode.
+         */
+        if (kvm_enabled()) {
+            if (pflash_drv != NULL) {
+                fprintf(stderr, "qemu: pflash cannot be used with kvm enabled\n");
+                exit(1);
+            } else {
+                /* In old pc_sysfw_flash_vs_rom_bug_compatible mode, we assume
+                 * that KVM cannot execute from device memory. In this case, we
+                 * use old rom based firmware initialization for KVM. But, since
+                 * this is different from non-kvm mode, this behavior is
+                 * undesirable */
+                sysfw_dev->rom_only = 1;
+            }
         }
+    } else if (pflash_drv == NULL) {
+        /* When a pflash drive is not found, use rom-mode */
+        sysfw_dev->rom_only = 1;
+    } else if (kvm_enabled() && !kvm_readonly_mem_enabled()) {
+        /* Older KVM cannot execute from device memory. So, flash memory
+         * cannot be used unless the readonly memory kvm capability is present. */
+        fprintf(stderr, "qemu: pflash with kvm requires KVM readonly memory support\n");
+        exit(1);
+    }
+
+    /* If rom-mode is active, use the old pc system rom initialization. */
+    if (sysfw_dev->rom_only) {
+        old_pc_system_rom_init(rom_memory, sysfw_dev->isapc_ram_fw);
+        return;
     }
 
     /* If a pflash drive is not found, then create one using
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Kevin O'Connor]

On Wed, May 29, 2013 at 01:27:24AM -0700, Jordan Justen wrote:
> The isapc machine with seabios currently requires the BIOS region
> to be read/write memory rather than read-only memory.
> 
> KVM currently cannot support the BIOS as a ROM region, but qemu
> in non-KVM mode can. Based on this, isapc machine currently only
> works with KVM.
> 
> To work-around this isapc issue, this change avoids marking the
> BIOS as readonly for isapc.

How about changing the code to always make the ROM read/write (instead
of doing it only on isapc).  Currently, if the rom is marked as
read-only, then SeaBIOS makes it read/write as the first thing it
does.  So, making it read-only doesn't really serve any purpose.

-Kevin

Re: [Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Jordan Justen]

On Thu, May 30, 2013 at 7:06 PM, Kevin O'Connor <kevin@koconnor.net> wrote:
> On Wed, May 29, 2013 at 01:27:24AM -0700, Jordan Justen wrote:
>> The isapc machine with seabios currently requires the BIOS region
>> to be read/write memory rather than read-only memory.
>>
>> KVM currently cannot support the BIOS as a ROM region, but qemu
>> in non-KVM mode can. Based on this, isapc machine currently only
>> works with KVM.
>>
>> To work-around this isapc issue, this change avoids marking the
>> BIOS as readonly for isapc.
>
> How about changing the code to always make the ROM read/write (instead
> of doing it only on isapc).  Currently, if the rom is marked as
> read-only, then SeaBIOS makes it read/write as the first thing it
> does.  So, making it read-only doesn't really serve any purpose.

Are you talking about PAM registers? I think these should only affect
0xe0000-0xfffff and 0xfffe000-0xffffffff. So, the PAM registers should
only impact part of the potential ROM range.

Also, I think the PAM registers should default to ROM mode after reset
and on power-on.

-Jordan

Re: [Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Laszlo Ersek]

On 05/31/13 06:41, Jordan Justen wrote:
> On Thu, May 30, 2013 at 7:06 PM, Kevin O'Connor <kevin@koconnor.net> wrote:
>> On Wed, May 29, 2013 at 01:27:24AM -0700, Jordan Justen wrote:
>>> The isapc machine with seabios currently requires the BIOS region
>>> to be read/write memory rather than read-only memory.
>>>
>>> KVM currently cannot support the BIOS as a ROM region, but qemu
>>> in non-KVM mode can. Based on this, isapc machine currently only
>>> works with KVM.
>>>
>>> To work-around this isapc issue, this change avoids marking the
>>> BIOS as readonly for isapc.
>>
>> How about changing the code to always make the ROM read/write (instead
>> of doing it only on isapc).  Currently, if the rom is marked as
>> read-only, then SeaBIOS makes it read/write as the first thing it
>> does.  So, making it read-only doesn't really serve any purpose.
> 
> Are you talking about PAM registers? I think these should only affect
> 0xe0000-0xfffff and 0xfffe000-0xffffffff. So, the PAM registers should
> only impact part of the potential ROM range.
> 
> Also, I think the PAM registers should default to ROM mode after reset
> and on power-on.

I think we've been here before...

- always resetting the PAM registers broke S3 resume:
http://thread.gmane.org/gmane.comp.emulators.qemu/195931/focus=195932
http://thread.gmane.org/gmane.comp.emulators.qemu/195931/focus=196081

- there was a patch to distinguish soft reset from hard reset:
http://thread.gmane.org/gmane.comp.emulators.qemu/195351

- another approach for soft vs. hard, and how it relates to resume:
http://thread.gmane.org/gmane.comp.emulators.qemu/198545/focus=198546

I believe currently qemu doesn't distinguish soft from hard reset. Hard
reset would have to reset PAM registers, soft reset must not. Since we
only have one (mixed?) kind, and S3 resume depends on PAM registers
being intact, we can't clear them during the one kind of reset that we
currently have.

No idea what the final resolution was, but I vaguely believe none of the
"distinguish soft from hard reset" submissions got all aspects right, or
some such.

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Paolo Bonzini]

Il 31/05/2013 04:06, Kevin O'Connor ha scritto:
> On Wed, May 29, 2013 at 01:27:24AM -0700, Jordan Justen wrote:
>> > The isapc machine with seabios currently requires the BIOS region
>> > to be read/write memory rather than read-only memory.
>> > 
>> > KVM currently cannot support the BIOS as a ROM region, but qemu
>> > in non-KVM mode can. Based on this, isapc machine currently only
>> > works with KVM.
>> > 
>> > To work-around this isapc issue, this change avoids marking the
>> > BIOS as readonly for isapc.
> How about changing the code to always make the ROM read/write (instead
> of doing it only on isapc).  Currently, if the rom is marked as
> read-only, then SeaBIOS makes it read/write as the first thing it
> does.  So, making it read-only doesn't really serve any purpose.

I don't think SeaBIOS can rely on the value of the PAM registers when it
is started via a CPU reset (INIT), can it?

Paolo

Re: [Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Paolo Bonzini]

Il 31/05/2013 14:40, Laszlo Ersek ha scritto:
> I think we've been here before...
> 
> - always resetting the PAM registers broke S3 resume:
> http://thread.gmane.org/gmane.comp.emulators.qemu/195931/focus=195932
> http://thread.gmane.org/gmane.comp.emulators.qemu/195931/focus=196081
> 
> - there was a patch to distinguish soft reset from hard reset:
> http://thread.gmane.org/gmane.comp.emulators.qemu/195351

This is still planned, but it won't touch the PAM registers so as not to
break S3.  Peter Stuge said that the memory controller registers keep
their content across S3 in real machines too.

> - another approach for soft vs. hard, and how it relates to resume:
> http://thread.gmane.org/gmane.comp.emulators.qemu/198545/focus=198546
> 
> I believe currently qemu doesn't distinguish soft from hard reset. Hard
> reset would have to reset PAM registers, soft reset must not.

Hard reset = powerdown, soft reset = reset or S3.

QEMU only cares about soft reset.  There is one case where that matters,
which is "-no-shutdown -no-reboot".  Here you can go to S4 or S5 and
then type "cont", and the machine will behave as if you actually did a
soft reset.  I don't think it matters much in practice, though.

Paolo

Re: [Qemu-devel] [PATCH v7 0/4] KVM flash memory support

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v7 1/4] isapc: Fix non-KVM qemu boot (read/write memory for isapc BIOS)

[Kevin O'Connor]

On Fri, May 31, 2013 at 02:48:17PM +0200, Paolo Bonzini wrote:
> Il 31/05/2013 04:06, Kevin O'Connor ha scritto:
> > On Wed, May 29, 2013 at 01:27:24AM -0700, Jordan Justen wrote:
> >> > The isapc machine with seabios currently requires the BIOS region
> >> > to be read/write memory rather than read-only memory.
> >> > 
> >> > KVM currently cannot support the BIOS as a ROM region, but qemu
> >> > in non-KVM mode can. Based on this, isapc machine currently only
> >> > works with KVM.
> >> > 
> >> > To work-around this isapc issue, this change avoids marking the
> >> > BIOS as readonly for isapc.
> > How about changing the code to always make the ROM read/write (instead
> > of doing it only on isapc).  Currently, if the rom is marked as
> > read-only, then SeaBIOS makes it read/write as the first thing it
> > does.  So, making it read-only doesn't really serve any purpose.
> 
> I don't think SeaBIOS can rely on the value of the PAM registers when it
> is started via a CPU reset (INIT), can it?

SeaBIOS currently detects the PAM registers at startup - if it is in
read-only mode it makes it read/write.  If it's in read/write mode it
leaves it there.  That's why I think it is silly to start in read-only
mode - the first thing the guest does is make it read/write.

-Kevin