[Qemu-devel] [PULL 0/1] nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

[Qemu-devel] [PULL 0/1] nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

[Gerd Hoffmann]

  Hi,

Here comes fwcfg patch queue, a bit shorter than planned as I've ran
into issues while verifying linuxboot_dma fw_cfg patches.

please pull,
  Gerd

The following changes since commit 10ae9d76388e3f4a31f6a1475b5e2d1f28404a10:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.6-20160201' into staging (2016-02-02 09:13:10 +0000)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-fwcfg-20160202-1

for you to fetch changes up to a907ec52cc1aefc820768b6e341b56f8f3caaca7:

  nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file (2016-02-02 12:45:01 +0100)

----------------------------------------------------------------
nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

----------------------------------------------------------------
Laszlo Ersek (1):
      nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

 hw/block/nvme.c | 42 +++++-------------------------------------
 1 file changed, 5 insertions(+), 37 deletions(-)

[Qemu-devel] [PULL 1/1] nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

[Gerd Hoffmann]

From: Laszlo Ersek <lersek@redhat.com>

Background on QEMU boot indices
-------------------------------

Normally, the "bootindex" property is configured for bootable devices
with:

  DEVICE_instance_init()
    device_add_bootindex_property(..., "bootindex", ...)
      object_property_add(..., device_get_bootindex,
                          device_set_bootindex, ...)

and when the bootindex is set on the QEMU command line, with

  -device DEVICE,...,bootindex=N

the setter that was configured above is invoked:

  device_set_bootindex()
    /* parse boot index */
    visit_type_int32()

    /* verify unicity */
    check_boot_index()

    /* store parsed boot index */
    ...

    /* insert device path to boot order */
    add_boot_device_path()

In the last step, add_boot_device_path() ensures that an OpenFirmware
device path will show up in the "bootorder" fw_cfg file, at a position
corresponding to the device's boot index. Thus guest firmware (SeaBIOS and
OVMF) can try to boot off the device with the right priority.

NVMe boot index
---------------

In QEMU commit 33739c712982,

  nvma: ide: add bootindex to qom property

the following generic setters / getters:
- device_set_bootindex()
- device_get_bootindex()

were open-coded for NVMe, under the names
- nvme_set_bootindex()
- nvme_get_bootindex()

Plus nvme_instance_init() was added to configure the "bootindex" property
manually, designating the open-coded getter & setter, rather than calling
device_add_bootindex_property().

Crucially, nvme_set_bootindex() avoided the final add_boot_device_path()
call. This fact is spelled out in the message of commit 33739c712982, and
it was presumably the entire reason for all of the code duplication.

Now, Vladislav filed an RFE for OVMF
<https://github.com/tianocore/edk2/issues/48>; OVMF should boot off NVMe
devices. It is simple to build edk2's existent NvmExpressDxe driver into
OVMF, but the boot order matching logic in OVMF can only handle NVMe if
the "bootorder" fw_cfg file includes such devices.

Therefore this patch converts the NVMe device model to
device_set_bootindex() all the way.

Device paths
------------

device_set_bootindex() accepts an optional parameter called "suffix". When
present, it is expected to take the form of an OpenFirmware device path
node, and it gets appended as last node to the otherwise auto-generated
OFW path.

For NVMe, the auto-generated part is

  /pci@i0cf8/pci8086,5845@6[,1]
       ^     ^            ^  ^
       |     |            PCI slot and (present when nonzero)
       |     |            function of the NVMe controller, both hex
       |     "driver name" component, built from PCI vendor & device IDs
       PCI root at system bus port, PIO

to which here we append the suffix

  /namespace@1,0
             ^ ^
             | big endian (MSB at lowest address) numeric interpretation
             | of the 64-bit IEEE Extended Unique Identifier, aka EUI-64,
             | hex
             32-bit NVMe namespace identifier, aka NSID, hex

resulting in the OFW device path

  /pci@i0cf8/pci8086,5845@6[,1]/namespace@1,0

The reason for including the NSID and the EUI-64 is that an NVMe device
can in theory produce several different namespaces (distinguished by
NSID). Additionally, each of those may (optionally) have an EUI-64 value.

For now, QEMU only provides namespace 1.

Furthermore, QEMU doesn't even represent the EUI-64 as a standalone field;
it is embedded (and left unused) inside the "NvmeIdNs.res30" array, at the
last eight bytes. (Which is fine, since EUI-64 can be left zero-filled if
unsupported by the device.)

Based on the above, we set the "unit address" part of the last
("namespace") node to fixed "1,0".

OVMF will then map the above OFW device path to the following UEFI device
path fragment, for boot order processing:

  PciRoot(0x0)/Pci(0x6,0x1)/NVMe(0x1,00-00-00-00-00-00-00-00)
          ^        ^   ^    ^    ^   ^
          |        |   |    |    |   octets of the EUI-64 in address order
          |        |   |    |    NSID
          |        |   |    NVMe namespace messaging device path node
          |        PCI slot and function
          PCI root bridge

Cc: Keith Busch <keith.busch@intel.com> (supporter:nvme)
Cc: Kevin Wolf <kwolf@redhat.com> (supporter:Block layer core)
Cc: qemu-block@nongnu.org (open list:nvme)
Cc: Gonglei <arei.gonglei@huawei.com>
Cc: Vladislav Vovchenko <vladislav.vovchenko@sk.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Gonglei <arei.gonglei@huawei.com>
Acked-by: Keith Busch <keith.busch@intel.com>
Tested-by: Vladislav Vovchenko <vladislav.vovchenko@sk.com>
Message-id: 1453850483-27511-1-git-send-email-lersek@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/block/nvme.c | 42 +++++-------------------------------------
 1 file changed, 5 insertions(+), 37 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a5fedb2..c68b625 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -916,45 +916,13 @@ static void nvme_class_init(ObjectClass *oc, void *data)
     dc->vmsd = &nvme_vmstate;
 }
 
-static void nvme_get_bootindex(Object *obj, Visitor *v, void *opaque,
-                                  const char *name, Error **errp)
-{
-    NvmeCtrl *s = NVME(obj);
-
-    visit_type_int32(v, &s->conf.bootindex, name, errp);
-}
-
-static void nvme_set_bootindex(Object *obj, Visitor *v, void *opaque,
-                                  const char *name, Error **errp)
-{
-    NvmeCtrl *s = NVME(obj);
-    int32_t boot_index;
-    Error *local_err = NULL;
-
-    visit_type_int32(v, &boot_index, name, &local_err);
-    if (local_err) {
-        goto out;
-    }
-    /* check whether bootindex is present in fw_boot_order list  */
-    check_boot_index(boot_index, &local_err);
-    if (local_err) {
-        goto out;
-    }
-    /* change bootindex to a new one */
-    s->conf.bootindex = boot_index;
-
-out:
-    if (local_err) {
-        error_propagate(errp, local_err);
-    }
-}
-
 static void nvme_instance_init(Object *obj)
 {
-    object_property_add(obj, "bootindex", "int32",
-                        nvme_get_bootindex,
-                        nvme_set_bootindex, NULL, NULL, NULL);
-    object_property_set_int(obj, -1, "bootindex", NULL);
+    NvmeCtrl *s = NVME(obj);
+
+    device_add_bootindex_property(obj, &s->conf.bootindex,
+                                  "bootindex", "/namespace@1,0",
+                                  DEVICE(obj), &error_abort);
 }
 
 static const TypeInfo nvme_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/1] nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

[Peter Maydell]

On 2 February 2016 at 12:56, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes fwcfg patch queue, a bit shorter than planned as I've ran
> into issues while verifying linuxboot_dma fw_cfg patches.
>
> please pull,
>   Gerd
>
> The following changes since commit 10ae9d76388e3f4a31f6a1475b5e2d1f28404a10:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.6-20160201' into staging (2016-02-02 09:13:10 +0000)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-fwcfg-20160202-1
>
> for you to fetch changes up to a907ec52cc1aefc820768b6e341b56f8f3caaca7:
>
>   nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file (2016-02-02 12:45:01 +0100)
>
> ----------------------------------------------------------------
> nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 0/1] nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file

[Laszlo Ersek]

On 02/02/16 15:54, Peter Maydell wrote:
> On 2 February 2016 at 12:56, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>   Hi,
>>
>> Here comes fwcfg patch queue, a bit shorter than planned as I've ran
>> into issues while verifying linuxboot_dma fw_cfg patches.
>>
>> please pull,
>>   Gerd
>>
>> The following changes since commit 10ae9d76388e3f4a31f6a1475b5e2d1f28404a10:
>>
>>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.6-20160201' into staging (2016-02-02 09:13:10 +0000)
>>
>> are available in the git repository at:
>>
>>
>>   git://git.kraxel.org/qemu tags/pull-fwcfg-20160202-1
>>
>> for you to fetch changes up to a907ec52cc1aefc820768b6e341b56f8f3caaca7:
>>
>>   nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file (2016-02-02 12:45:01 +0100)
>>
>> ----------------------------------------------------------------
>> nvme: generate OpenFirmware device path in the "bootorder" fw_cfg file
>>
>> ----------------------------------------------------------------
> 
> Applied, thanks.
> 
> -- PMM
> 

\o/

Thank you both!
Laszlo