Patchwork [v8,0/2] PC system flash support

login
register
mail settings
Submitter Markus Armbruster
Date Nov. 30, 2011, 8:04 a.m.
Message ID <m3fwh6ox2i.fsf@blackfin.pond.sub.org>
Download mbox | patch
Permalink /patch/128429/
State New
Headers show

Comments

Markus Armbruster - Nov. 30, 2011, 8:04 a.m.
Avi Kivity <avi@redhat.com> writes:

> On 11/29/2011 09:03 AM, Jordan Justen wrote:
>> On Mon, Nov 28, 2011 at 02:28, Avi Kivity <avi@redhat.com> wrote:
>> > On 11/28/2011 04:26 AM, Jordan Justen wrote:
>> >> Enable flash emulation in a PC system using pflash_cfi01.
>> >
>> > The new memory layout should be made conditional on the machine type (-M
>> > pc-1.1 or later) to allow migration from/to pc-1.0 and earlier.
>> >
>> > Memory layout in this context are the names of memory ranges given to
>> > memory_region_init_ram(), sizes, and locations.  Also, the flash
>> > interface should not be there for older machines.
>>
>> Is this the right way to go about it?
>> 1. Create a new pc-sysfw qdev
>> 2. Use a flash property in pc_piix.c to selectively enable the flash
>>
>> Or, is there an easier or better way to say pc > pc-1.0?
>
> No idea, copying some qdev people.
>
>> How do I enable "migration from/to pc-1.0 and earlier"?  It seems like
>> some code will need to shuttle the data (rom <=> pflash).  Can you
>> point me an an example?
>
> It should be completely transparent, so long as you instantiate a ROM
> for <= pc-1.0 (say based on a property) and flash for >= pc-1.1.
>
> One way to verify is to to 'info qdev' and 'info mtree' with qemu-1.0
> and qemu-1.1 -M pc-1.0, and see that you get the same results.

Gerd created the compatibility machinery (commit b6b61144), and I'm not
really familiar with it.  But I can give it a try.

Our tool to configure devices is qdev properties.

The device defines property default values.  The board can override
them, with QEMUMachine member compat_props.

For examples, peruse git-log -Scompat_props.  Random pick with [my
explanations in brackets]:

commit b1aeb92666d2fde413c34578b3b42bbfe5f2a506
Author: Isaku Yamahata <yamahata@valinux.co.jp>
Date:   Fri Nov 26 21:01:41 2010 +0900

    pci: make command SERR bit writable
    
    pcie aer needs SERR bit to be writable, and the PCI spec requires
    this as well.  For compatibility, introduce compat global property
    command_serr_enable and make this bit readonly for a pre 0.14 pc
    machine.
    
    Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

[Patch hunks reordered to make the patch easier to follow]
 
 struct PCIDevice {
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7d29d43..a2fb554 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
[Change default to off for pc-0.13 and all the older machine types.]
@@ -217,6 +217,14 @@ static QEMUMachine pc_machine = {
     .desc = "Standard PC",
     .init = pc_init_pci,
     .max_cpus = 255,
+    .compat_props = (GlobalProperty[]) {
+        {
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
+        },
+        { /* end of list */ }
+    },
     .is_default = 1,
 };

@@ -265,6 +273,10 @@ static QEMUMachine pc_machine_v0_12 = {
             .driver   = "vmware-svga",
             .property = "rombar",
             .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
         },
         { /* end of list */ }
     }
@@ -300,6 +312,10 @@ static QEMUMachine pc_machine_v0_11 = {
             .driver   = "PCI",
             .property = "rombar",
             .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
         },
         { /* end of list */ }
     }
@@ -347,6 +363,10 @@ static QEMUMachine pc_machine_v0_10 = {
             .driver   = "PCI",
             .property = "rombar",
             .value    = stringify(0),
+        },{
+            .driver   = "PCI",
+            .property = "command_serr_enable",
+            .value    = "off",
         },
         { /* end of list */ }
     },
@@ -568,6 +570,9 @@ static void pci_init_wmask(PCIDevice *dev)
[Compatible behavior when QEMU_PCI_CAP_SERR_BITNR is off:]
     pci_set_word(dev->wmask + PCI_COMMAND,
                  PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
                  PCI_COMMAND_INTX_DISABLE);
+    if (dev->cap_present & QEMU_PCI_CAP_SERR) {
+        pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR);
+    }
 
     memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
            config_size - PCI_CONFIG_HEADER_SIZE);
Markus Armbruster - Nov. 30, 2011, 8:38 a.m.
Markus Armbruster <armbru@redhat.com> writes:

> Avi Kivity <avi@redhat.com> writes:
>
>> On 11/29/2011 09:03 AM, Jordan Justen wrote:
>>> On Mon, Nov 28, 2011 at 02:28, Avi Kivity <avi@redhat.com> wrote:
>>> > On 11/28/2011 04:26 AM, Jordan Justen wrote:
>>> >> Enable flash emulation in a PC system using pflash_cfi01.
>>> >
>>> > The new memory layout should be made conditional on the machine type (-M
>>> > pc-1.1 or later) to allow migration from/to pc-1.0 and earlier.
>>> >
>>> > Memory layout in this context are the names of memory ranges given to
>>> > memory_region_init_ram(), sizes, and locations.  Also, the flash
>>> > interface should not be there for older machines.
>>>
>>> Is this the right way to go about it?
>>> 1. Create a new pc-sysfw qdev
>>> 2. Use a flash property in pc_piix.c to selectively enable the flash
>>>
>>> Or, is there an easier or better way to say pc > pc-1.0?
>>
>> No idea, copying some qdev people.
>>
>>> How do I enable "migration from/to pc-1.0 and earlier"?  It seems like
>>> some code will need to shuttle the data (rom <=> pflash).  Can you
>>> point me an an example?
>>
>> It should be completely transparent, so long as you instantiate a ROM
>> for <= pc-1.0 (say based on a property) and flash for >= pc-1.1.
>>
>> One way to verify is to to 'info qdev' and 'info mtree' with qemu-1.0
>> and qemu-1.1 -M pc-1.0, and see that you get the same results.
>
> Gerd created the compatibility machinery (commit b6b61144), and I'm not
> really familiar with it.  But I can give it a try.
>
> Our tool to configure devices is qdev properties.
>
> The device defines property default values.  The board can override
> them, with QEMUMachine member compat_props.

If you need more than just configuring a device differently, you
probably need to hack the board init method.  Check out commit 0ec329da
for an example.  Works roughly like this:

* Add suitable parameter to pc_init1()

* Change pc_init_pci() to pass the "new behavior on" argument.

* Define pc_init_pci_no_kvmclock() like pc_init_pci(), except pass the
  "new behavior off" argument.  Change old boards to use it.

[...]

Patch

diff --git a/hw/pci.c b/hw/pci.c
index 0c15b13..ca878e8 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -57,6 +57,8 @@  struct BusInfo pci_bus_info = {
[Define a property to make the incompatible change configurable.  Store
in PCIDevice member cap_present bit QEMU_PCI_CAP_SERR_BITNR, default is
true.]
         DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
         DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
                         QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
+        DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
+                        QEMU_PCI_CAP_SERR_BITNR, true),
         DEFINE_PROP_END_OF_LIST()
     }
 };
diff --git a/hw/pci.h b/hw/pci.h
index 89f7b76..099c251 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -118,6 +118,10 @@  enum {
[Since we use a new bit of the existing cap_present, we don't have
to define a new member.  Instead we need to define the new bit:]
     /* multifunction capable device */
 #define QEMU_PCI_CAP_MULTIFUNCTION_BITNR        3
     QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
+
+    /* command register SERR bit enabled */
+#define QEMU_PCI_CAP_SERR_BITNR 4
+    QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
 };