[PULL 0/4] OVMF patches for 2021-07-12

[PULL 0/4] OVMF patches for 2021-07-12

[Philippe Mathieu-Daudé]

The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/fw-edk2-20210712

for you to fetch changes up to d1e79210457323b225c369fa00e97577e0d0da95:

  MAINTAINERS: remove Laszlo Ersek's entries (2021-07-12 16:55:23 +0200)

----------------------------------------------------------------
Patches related to EDK2/OVMF

- MAINTAINERS: remove Laszlo Ersek's entries
- Introduce X86_FW_OVMF Kconfig symbol
- pc_system_ovmf_table_find: Assert that flash was parsed, document

----------------------------------------------------------------

Dov Murik (2):
  hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed
  hw/i386/pc: Document pc_system_ovmf_table_find

Laszlo Ersek (1):
  MAINTAINERS: remove Laszlo Ersek's entries

Philippe Mathieu-Daudé (1):
  hw/i386: Introduce X86_FW_OVMF Kconfig symbol

 include/hw/i386/pc.h          |   1 +
 hw/i386/pc_sysfw.c            | 107 ------------------------
 hw/i386/pc_sysfw_ovmf-stubs.c |  26 ++++++
 hw/i386/pc_sysfw_ovmf.c       | 151 ++++++++++++++++++++++++++++++++++
 MAINTAINERS                   |   4 +-
 hw/i386/Kconfig               |   4 +
 hw/i386/meson.build           |   2 +
 7 files changed, 185 insertions(+), 110 deletions(-)
 create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
 create mode 100644 hw/i386/pc_sysfw_ovmf.c

-- 
2.31.1

[PULL 1/4] hw/i386/pc: pc_system_ovmf_table_find: Assert that flash was parsed

[Philippe Mathieu-Daudé]

From: Dov Murik <dovmurik@linux.ibm.com>

Add assertion in pc_system_ovmf_table_find that verifies that the flash
was indeed previously parsed (looking for the OVMF table) by
pc_system_parse_ovmf_flash.

Now pc_system_ovmf_table_find distinguishes between "no one called
pc_system_parse_ovmf_flash" (which will abort due to assertion failure)
and "the flash was parsed but no OVMF table was found, or it is invalid"
(which will return false).

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210701052749.934744-2-dovmurik@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc_sysfw.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 6ce37a2b052..e353f2a4e9b 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -126,6 +126,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
 
 #define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
 
+static bool ovmf_flash_parsed;
 static uint8_t *ovmf_table;
 static int ovmf_table_len;
 
@@ -136,10 +137,12 @@ static void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
     int tot_len;
 
     /* should only be called once */
-    if (ovmf_table) {
+    if (ovmf_flash_parsed) {
         return;
     }
 
+    ovmf_flash_parsed = true;
+
     if (flash_size < TARGET_PAGE_SIZE) {
         return;
     }
@@ -183,6 +186,8 @@ bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
     int tot_len = ovmf_table_len;
     QemuUUID entry_guid;
 
+    assert(ovmf_flash_parsed);
+
     if (qemu_uuid_parse(entry, &entry_guid) < 0) {
         return false;
     }
-- 
2.31.1

[PULL 2/4] hw/i386/pc: Document pc_system_ovmf_table_find

[Philippe Mathieu-Daudé]

From: Dov Murik <dovmurik@linux.ibm.com>

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210701052749.934744-3-dovmurik@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc_sysfw.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index e353f2a4e9b..6ddce92a861 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -179,6 +179,17 @@ static void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
     ovmf_table += tot_len;
 }
 
+/**
+ * pc_system_ovmf_table_find - Find the data associated with an entry in OVMF's
+ * reset vector GUIDed table.
+ *
+ * @entry: GUID string of the entry to lookup
+ * @data: Filled with a pointer to the entry's value (if not NULL)
+ * @data_len: Filled with the length of the entry's value (if not NULL). Pass
+ *            NULL here if the length of data is known.
+ *
+ * Return: true if the entry was found in the OVMF table; false otherwise.
+ */
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len)
 {
-- 
2.31.1

[PULL 3/4] hw/i386: Introduce X86_FW_OVMF Kconfig symbol

[Philippe Mathieu-Daudé]

Introduce the X86_FW_OVMF Kconfig symbol for OVMF-specific code.
Move the OVMF-specific code from pc_sysfw.c to pc_sysfw_ovmf.c,
adding a pair of stubs.
Update MAINTAINERS to reach OVMF maintainers when these new
files are modified.

This fixes when building the microvm machine standalone:

  /usr/bin/ld: libqemu-i386-softmmu.fa.p/target_i386_monitor.c.o: in
  function `qmp_sev_inject_launch_secret':
  target/i386/monitor.c:749: undefined reference to `pc_system_ovmf_table_find'

Fixes: f522cef9b35 ("sev: update sev-inject-launch-secret to make gpa optional")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20210616204328.2611406-22-philmd@redhat.com>
---
 include/hw/i386/pc.h          |   1 +
 hw/i386/pc_sysfw.c            | 123 ---------------------------
 hw/i386/pc_sysfw_ovmf-stubs.c |  26 ++++++
 hw/i386/pc_sysfw_ovmf.c       | 151 ++++++++++++++++++++++++++++++++++
 MAINTAINERS                   |   1 +
 hw/i386/Kconfig               |   4 +
 hw/i386/meson.build           |   2 +
 7 files changed, 185 insertions(+), 123 deletions(-)
 create mode 100644 hw/i386/pc_sysfw_ovmf-stubs.c
 create mode 100644 hw/i386/pc_sysfw_ovmf.c

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 87294f2632e..0775f945d70 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -188,6 +188,7 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms);
 void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
 bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
                                int *data_len);
+void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
 
 
 /* acpi-build.c */
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 6ddce92a861..68d6b1f783e 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -124,129 +124,6 @@ void pc_system_flash_cleanup_unused(PCMachineState *pcms)
     }
 }
 
-#define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
-
-static bool ovmf_flash_parsed;
-static uint8_t *ovmf_table;
-static int ovmf_table_len;
-
-static void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
-{
-    uint8_t *ptr;
-    QemuUUID guid;
-    int tot_len;
-
-    /* should only be called once */
-    if (ovmf_flash_parsed) {
-        return;
-    }
-
-    ovmf_flash_parsed = true;
-
-    if (flash_size < TARGET_PAGE_SIZE) {
-        return;
-    }
-
-    /*
-     * if this is OVMF there will be a table footer
-     * guid 48 bytes before the end of the flash file.  If it's
-     * not found, silently abort the flash parsing.
-     */
-    qemu_uuid_parse(OVMF_TABLE_FOOTER_GUID, &guid);
-    guid = qemu_uuid_bswap(guid); /* guids are LE */
-    ptr = flash_ptr + flash_size - 48;
-    if (!qemu_uuid_is_equal((QemuUUID *)ptr, &guid)) {
-        return;
-    }
-
-    /* if found, just before is two byte table length */
-    ptr -= sizeof(uint16_t);
-    tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t);
-
-    if (tot_len <= 0) {
-        return;
-    }
-
-    ovmf_table = g_malloc(tot_len);
-    ovmf_table_len = tot_len;
-
-    /*
-     * ptr is the foot of the table, so copy it all to the newly
-     * allocated ovmf_table and then set the ovmf_table pointer
-     * to the table foot
-     */
-    memcpy(ovmf_table, ptr - tot_len, tot_len);
-    ovmf_table += tot_len;
-}
-
-/**
- * pc_system_ovmf_table_find - Find the data associated with an entry in OVMF's
- * reset vector GUIDed table.
- *
- * @entry: GUID string of the entry to lookup
- * @data: Filled with a pointer to the entry's value (if not NULL)
- * @data_len: Filled with the length of the entry's value (if not NULL). Pass
- *            NULL here if the length of data is known.
- *
- * Return: true if the entry was found in the OVMF table; false otherwise.
- */
-bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
-                               int *data_len)
-{
-    uint8_t *ptr = ovmf_table;
-    int tot_len = ovmf_table_len;
-    QemuUUID entry_guid;
-
-    assert(ovmf_flash_parsed);
-
-    if (qemu_uuid_parse(entry, &entry_guid) < 0) {
-        return false;
-    }
-
-    if (!ptr) {
-        return false;
-    }
-
-    entry_guid = qemu_uuid_bswap(entry_guid); /* guids are LE */
-    while (tot_len >= sizeof(QemuUUID) + sizeof(uint16_t)) {
-        int len;
-        QemuUUID *guid;
-
-        /*
-         * The data structure is
-         *   arbitrary length data
-         *   2 byte length of entire entry
-         *   16 byte guid
-         */
-        guid = (QemuUUID *)(ptr - sizeof(QemuUUID));
-        len = le16_to_cpu(*(uint16_t *)(ptr - sizeof(QemuUUID) -
-                                        sizeof(uint16_t)));
-
-        /*
-         * just in case the table is corrupt, wouldn't want to spin in
-         * the zero case
-         */
-        if (len < sizeof(QemuUUID) + sizeof(uint16_t)) {
-            return false;
-        } else if (len > tot_len) {
-            return false;
-        }
-
-        ptr -= len;
-        tot_len -= len;
-        if (qemu_uuid_is_equal(guid, &entry_guid)) {
-            if (data) {
-                *data = ptr;
-            }
-            if (data_len) {
-                *data_len = len - sizeof(QemuUUID) - sizeof(uint16_t);
-            }
-            return true;
-        }
-    }
-    return false;
-}
-
 /*
  * Map the pcms->flash[] from 4GiB downward, and realize.
  * Map them in descending order, i.e. pcms->flash[0] at the top,
diff --git a/hw/i386/pc_sysfw_ovmf-stubs.c b/hw/i386/pc_sysfw_ovmf-stubs.c
new file mode 100644
index 00000000000..aabe78b2710
--- /dev/null
+++ b/hw/i386/pc_sysfw_ovmf-stubs.c
@@ -0,0 +1,26 @@
+/*
+ * QEMU PC System Firmware (OVMF stubs)
+ *
+ * Copyright (c) 2021 Red Hat, Inc.
+ *
+ * Author:
+ *   Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+
+bool pc_system_ovmf_table_find(const char *entry, uint8_t **data, int *data_len)
+{
+    g_assert_not_reached();
+}
+
+void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
+{
+    g_assert_not_reached();
+}
diff --git a/hw/i386/pc_sysfw_ovmf.c b/hw/i386/pc_sysfw_ovmf.c
new file mode 100644
index 00000000000..f4dd92c5882
--- /dev/null
+++ b/hw/i386/pc_sysfw_ovmf.c
@@ -0,0 +1,151 @@
+/*
+ * QEMU PC System Firmware (OVMF specific)
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2011-2012 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
+#include "cpu.h"
+
+#define OVMF_TABLE_FOOTER_GUID "96b582de-1fb2-45f7-baea-a366c55a082d"
+
+static bool ovmf_flash_parsed;
+static uint8_t *ovmf_table;
+static int ovmf_table_len;
+
+void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size)
+{
+    uint8_t *ptr;
+    QemuUUID guid;
+    int tot_len;
+
+    /* should only be called once */
+    if (ovmf_flash_parsed) {
+        return;
+    }
+
+    ovmf_flash_parsed = true;
+
+    if (flash_size < TARGET_PAGE_SIZE) {
+        return;
+    }
+
+    /*
+     * if this is OVMF there will be a table footer
+     * guid 48 bytes before the end of the flash file.  If it's
+     * not found, silently abort the flash parsing.
+     */
+    qemu_uuid_parse(OVMF_TABLE_FOOTER_GUID, &guid);
+    guid = qemu_uuid_bswap(guid); /* guids are LE */
+    ptr = flash_ptr + flash_size - 48;
+    if (!qemu_uuid_is_equal((QemuUUID *)ptr, &guid)) {
+        return;
+    }
+
+    /* if found, just before is two byte table length */
+    ptr -= sizeof(uint16_t);
+    tot_len = le16_to_cpu(*(uint16_t *)ptr) - sizeof(guid) - sizeof(uint16_t);
+
+    if (tot_len <= 0) {
+        return;
+    }
+
+    ovmf_table = g_malloc(tot_len);
+    ovmf_table_len = tot_len;
+
+    /*
+     * ptr is the foot of the table, so copy it all to the newly
+     * allocated ovmf_table and then set the ovmf_table pointer
+     * to the table foot
+     */
+    memcpy(ovmf_table, ptr - tot_len, tot_len);
+    ovmf_table += tot_len;
+}
+
+/**
+ * pc_system_ovmf_table_find - Find the data associated with an entry in OVMF's
+ * reset vector GUIDed table.
+ *
+ * @entry: GUID string of the entry to lookup
+ * @data: Filled with a pointer to the entry's value (if not NULL)
+ * @data_len: Filled with the length of the entry's value (if not NULL). Pass
+ *            NULL here if the length of data is known.
+ *
+ * Return: true if the entry was found in the OVMF table; false otherwise.
+ */
+bool pc_system_ovmf_table_find(const char *entry, uint8_t **data,
+                               int *data_len)
+{
+    uint8_t *ptr = ovmf_table;
+    int tot_len = ovmf_table_len;
+    QemuUUID entry_guid;
+
+    assert(ovmf_flash_parsed);
+
+    if (qemu_uuid_parse(entry, &entry_guid) < 0) {
+        return false;
+    }
+
+    if (!ptr) {
+        return false;
+    }
+
+    entry_guid = qemu_uuid_bswap(entry_guid); /* guids are LE */
+    while (tot_len >= sizeof(QemuUUID) + sizeof(uint16_t)) {
+        int len;
+        QemuUUID *guid;
+
+        /*
+         * The data structure is
+         *   arbitrary length data
+         *   2 byte length of entire entry
+         *   16 byte guid
+         */
+        guid = (QemuUUID *)(ptr - sizeof(QemuUUID));
+        len = le16_to_cpu(*(uint16_t *)(ptr - sizeof(QemuUUID) -
+                                        sizeof(uint16_t)));
+
+        /*
+         * just in case the table is corrupt, wouldn't want to spin in
+         * the zero case
+         */
+        if (len < sizeof(QemuUUID) + sizeof(uint16_t)) {
+            return false;
+        } else if (len > tot_len) {
+            return false;
+        }
+
+        ptr -= len;
+        tot_len -= len;
+        if (qemu_uuid_is_equal(guid, &entry_guid)) {
+            if (data) {
+                *data = ptr;
+            }
+            if (data_len) {
+                *data_len = len - sizeof(QemuUUID) - sizeof(uint16_t);
+            }
+            return true;
+        }
+    }
+    return false;
+}
diff --git a/MAINTAINERS b/MAINTAINERS
index 3026b979b7f..7a2bd2e7184 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2943,6 +2943,7 @@ EDK2 Firmware
 M: Laszlo Ersek <lersek@redhat.com>
 M: Philippe Mathieu-Daudé <philmd@redhat.com>
 S: Supported
+F: hw/i386/*ovmf*
 F: pc-bios/descriptors/??-edk2-*.json
 F: pc-bios/edk2-*
 F: roms/Makefile.edk2
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index aacb6f6d964..bad6cf5b4e6 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -1,5 +1,9 @@
+config X86_FW_OVMF
+    bool
+
 config SEV
     bool
+    select X86_FW_OVMF
     depends on KVM
 
 config PC
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index e5d109f5c64..80dad29f2ba 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -24,6 +24,8 @@
   'pc_sysfw.c',
   'acpi-build.c',
   'port92.c'))
+i386_ss.add(when: 'CONFIG_X86_FW_OVMF', if_true: files('pc_sysfw_ovmf.c'),
+                                        if_false: files('pc_sysfw_ovmf-stubs.c'))
 
 subdir('kvm')
 subdir('xen')
-- 
2.31.1

[PULL 4/4] MAINTAINERS: remove Laszlo Ersek's entries

[Philippe Mathieu-Daudé]

From: Laszlo Ersek <lersek@redhat.com>

I've relinquished my edk2 roles with the following commit message [1] [2]
[3]:

> Maintainers.txt: remove Laszlo Ersek's entries
>
> I'm relinquishing all my roles listed in "Maintainers.txt", for personal
> reasons.
>
> My email address <lersek@redhat.com> remains functional.
>
> To my understanding, my employer is working to assign others engineers
> to the edk2 project (at their discretion).

[1] https://edk2.groups.io/g/devel/message/77585
[2] https://listman.redhat.com/archives/edk2-devel-archive/2021-July/msg00202.html
[3] http://mid.mail-archive.com/20210708070916.8937-1-lersek@redhat.com

Accordingly, remove my entries from QEMU's MAINTAINERS file as well, which
all relate to guest firmware.

Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Kashyap Chamarthy <kchamart@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210708071409.9671-1-lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 MAINTAINERS | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2bd2e7184..8543c7d1eb8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2200,7 +2200,6 @@ F: include/hw/southbridge/piix.h
 
 Firmware configuration (fw_cfg)
 M: Philippe Mathieu-Daudé <philmd@redhat.com>
-R: Laszlo Ersek <lersek@redhat.com>
 R: Gerd Hoffmann <kraxel@redhat.com>
 S: Supported
 F: docs/specs/fw_cfg.txt
@@ -2932,7 +2931,6 @@ F: include/hw/i2c/smbus_slave.h
 F: include/hw/i2c/smbus_eeprom.h
 
 Firmware schema specifications
-M: Laszlo Ersek <lersek@redhat.com>
 M: Philippe Mathieu-Daudé <philmd@redhat.com>
 R: Daniel P. Berrange <berrange@redhat.com>
 R: Kashyap Chamarthy <kchamart@redhat.com>
@@ -2940,7 +2938,6 @@ S: Maintained
 F: docs/interop/firmware.json
 
 EDK2 Firmware
-M: Laszlo Ersek <lersek@redhat.com>
 M: Philippe Mathieu-Daudé <philmd@redhat.com>
 S: Supported
 F: hw/i386/*ovmf*
-- 
2.31.1

Re: [PULL 0/4] OVMF patches for 2021-07-12

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/fw-edk2-20210712
>
> for you to fetch changes up to d1e79210457323b225c369fa00e97577e0d0da95:
>
>   MAINTAINERS: remove Laszlo Ersek's entries (2021-07-12 16:55:23 +0200)
>
> ----------------------------------------------------------------
> Patches related to EDK2/OVMF
>
> - MAINTAINERS: remove Laszlo Ersek's entries
> - Introduce X86_FW_OVMF Kconfig symbol
> - pc_system_ovmf_table_find: Assert that flash was parsed, document
>
> ----------------------------------------------------------------

So, even though this pullreq doesn't seem to be changing gitlab CI
config, I get a "yaml invalid" failure in the pipeline:
https://gitlab.com/qemu-project/qemu/-/pipelines/335995843

"'build-edk2' job needs 'docker-edk2' job but it was not added to the pipeline"

Any ideas what that's about?

thanks
-- PMM

Re: [PULL 0/4] OVMF patches for 2021-07-12

[Philippe Mathieu-Daudé]

On 7/13/21 12:25 PM, Peter Maydell wrote:
> On Mon, 12 Jul 2021 at 16:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
>>
>>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://github.com/philmd/qemu.git tags/fw-edk2-20210712
>>
>> for you to fetch changes up to d1e79210457323b225c369fa00e97577e0d0da95:
>>
>>   MAINTAINERS: remove Laszlo Ersek's entries (2021-07-12 16:55:23 +0200)
>>
>> ----------------------------------------------------------------
>> Patches related to EDK2/OVMF
>>
>> - MAINTAINERS: remove Laszlo Ersek's entries
>> - Introduce X86_FW_OVMF Kconfig symbol
>> - pc_system_ovmf_table_find: Assert that flash was parsed, document
>>
>> ----------------------------------------------------------------
> 
> So, even though this pullreq doesn't seem to be changing gitlab CI
> config, I get a "yaml invalid" failure in the pipeline:
> https://gitlab.com/qemu-project/qemu/-/pipelines/335995843
> 
> "'build-edk2' job needs 'docker-edk2' job but it was not added to the pipeline"
> 
> Any ideas what that's about?

For me this is related to what I tried to fix last year but
couldn't convince Daniel there was a problem. See this thread:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg773939.html

Re: [PULL 0/4] OVMF patches for 2021-07-12

[Daniel P. Berrangé]

On Tue, Jul 13, 2021 at 11:25:21AM +0100, Peter Maydell wrote:
> On Mon, 12 Jul 2021 at 16:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
> >
> >   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/philmd/qemu.git tags/fw-edk2-20210712
> >
> > for you to fetch changes up to d1e79210457323b225c369fa00e97577e0d0da95:
> >
> >   MAINTAINERS: remove Laszlo Ersek's entries (2021-07-12 16:55:23 +0200)
> >
> > ----------------------------------------------------------------
> > Patches related to EDK2/OVMF
> >
> > - MAINTAINERS: remove Laszlo Ersek's entries
> > - Introduce X86_FW_OVMF Kconfig symbol
> > - pc_system_ovmf_table_find: Assert that flash was parsed, document
> >
> > ----------------------------------------------------------------
> 
> So, even though this pullreq doesn't seem to be changing gitlab CI
> config, I get a "yaml invalid" failure in the pipeline:
> https://gitlab.com/qemu-project/qemu/-/pipelines/335995843
> 
> "'build-edk2' job needs 'docker-edk2' job but it was not added to the pipeline"
> 
> Any ideas what that's about?

The rules for these two jobs are not compatible with the depedency
declared between them.

The docker-edk2 job is set to exist only when edk2.yml or Dockerfile
are changed:

 docker-edk2:
   stage: containers
   rules: # Only run this job when the Dockerfile is modified
   - changes:
     - .gitlab-ci.d/edk2.yml
     - .gitlab-ci.d/edk2/Dockerfile
     when: always


The build-edk2 job is set to exist based on a wide variety of
changes

 build-edk2:
   stage: build
   needs: ['docker-edk2']
   rules: # Only run this job when ...
   - changes: # ... roms/edk2/ is modified (submodule updated)
     - roms/edk2/*
     when: always
   - if: '$CI_COMMIT_REF_NAME =~ /^edk2/' # or the branch/tag starts with 'edk2'
     when: always
   - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # or last commit description contains 'EDK2'
     when: always


If those jobs were independant those different sets of conditions
would be ok.  The build-edk2 job, however, has a 'needs' clause
adding a depdancy on 'docker-edk2'.

Given this dependancy, in *EVERY* scenario were 'build-edk2' job
exists, the 'docker-edk2' job *MUST* also exist. This isn't the
case, hence the failed pipeline.

This can be fixed by taking the 'rules' from 'build-edk2' and
also applying them to 'docker-edk2'.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 0/4] OVMF patches for 2021-07-12

[Daniel P. Berrangé]

On Tue, Jul 13, 2021 at 12:35:18PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/13/21 12:25 PM, Peter Maydell wrote:
> > On Mon, 12 Jul 2021 at 16:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >> The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
> >>
> >>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
> >>
> >> are available in the Git repository at:
> >>
> >>   https://github.com/philmd/qemu.git tags/fw-edk2-20210712
> >>
> >> for you to fetch changes up to d1e79210457323b225c369fa00e97577e0d0da95:
> >>
> >>   MAINTAINERS: remove Laszlo Ersek's entries (2021-07-12 16:55:23 +0200)
> >>
> >> ----------------------------------------------------------------
> >> Patches related to EDK2/OVMF
> >>
> >> - MAINTAINERS: remove Laszlo Ersek's entries
> >> - Introduce X86_FW_OVMF Kconfig symbol
> >> - pc_system_ovmf_table_find: Assert that flash was parsed, document
> >>
> >> ----------------------------------------------------------------
> > 
> > So, even though this pullreq doesn't seem to be changing gitlab CI
> > config, I get a "yaml invalid" failure in the pipeline:
> > https://gitlab.com/qemu-project/qemu/-/pipelines/335995843
> > 
> > "'build-edk2' job needs 'docker-edk2' job but it was not added to the pipeline"
> > 
> > Any ideas what that's about?
> 
> For me this is related to what I tried to fix last year but
> couldn't convince Daniel there was a problem. See this thread:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg773939.html

The situation was different back then, as there was no direct 'needs'
relation between the jobs.

This is also addressing a different issue, which I better understand
now. The filtering based on file names is unreliable when combined
with git pushes. Gitlab looks at the files modified in the delta beween
what is pushed as what already exists on the remote branch. So if you
push 3 changes to a branch, and then push another 2 changes later,
files modified by the first 3 changes won't be considered by the filter
rules, leading to unexpected execution behaviour.

Essentially filtering based on filename is only sane when used with
merge requests, not pushes, because a merge request has a well defined
base commit against which file changes are evaluated.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 0/4] OVMF patches for 2021-07-12

[Philippe Mathieu-Daudé]

On 7/13/21 1:16 PM, Daniel P. Berrangé wrote:
> On Tue, Jul 13, 2021 at 11:25:21AM +0100, Peter Maydell wrote:
>> On Mon, 12 Jul 2021 at 16:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> The following changes since commit bd38ae26cea0d1d6a97f930248df149204c210a2:
>>>
>>>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210710' into staging (2021-07-12 11:02:39 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>   https://github.com/philmd/qemu.git tags/fw-edk2-20210712
>>>
>>> for you to fetch changes up to d1e79210457323b225c369fa00e97577e0d0da95:
>>>
>>>   MAINTAINERS: remove Laszlo Ersek's entries (2021-07-12 16:55:23 +0200)
>>>
>>> ----------------------------------------------------------------
>>> Patches related to EDK2/OVMF
>>>
>>> - MAINTAINERS: remove Laszlo Ersek's entries
>>> - Introduce X86_FW_OVMF Kconfig symbol
>>> - pc_system_ovmf_table_find: Assert that flash was parsed, document
>>>
>>> ----------------------------------------------------------------
>>
>> So, even though this pullreq doesn't seem to be changing gitlab CI
>> config, I get a "yaml invalid" failure in the pipeline:
>> https://gitlab.com/qemu-project/qemu/-/pipelines/335995843
>>
>> "'build-edk2' job needs 'docker-edk2' job but it was not added to the pipeline"
>>
>> Any ideas what that's about?
> 
> The rules for these two jobs are not compatible with the depedency
> declared between them.
> 
> The docker-edk2 job is set to exist only when edk2.yml or Dockerfile
> are changed:
> 
>  docker-edk2:
>    stage: containers
>    rules: # Only run this job when the Dockerfile is modified
>    - changes:
>      - .gitlab-ci.d/edk2.yml
>      - .gitlab-ci.d/edk2/Dockerfile
>      when: always
> 
> 
> The build-edk2 job is set to exist based on a wide variety of
> changes
> 
>  build-edk2:
>    stage: build
>    needs: ['docker-edk2']
>    rules: # Only run this job when ...
>    - changes: # ... roms/edk2/ is modified (submodule updated)
>      - roms/edk2/*
>      when: always
>    - if: '$CI_COMMIT_REF_NAME =~ /^edk2/' # or the branch/tag starts with 'edk2'
>      when: always
>    - if: '$CI_COMMIT_MESSAGE =~ /edk2/i' # or last commit description contains 'EDK2'
>      when: always
> 
> 
> If those jobs were independant those different sets of conditions
> would be ok.  The build-edk2 job, however, has a 'needs' clause
> adding a depdancy on 'docker-edk2'.
> 
> Given this dependancy, in *EVERY* scenario were 'build-edk2' job
> exists, the 'docker-edk2' job *MUST* also exist. This isn't the
> case, hence the failed pipeline.
> 
> This can be fixed by taking the 'rules' from 'build-edk2' and
> also applying them to 'docker-edk2'.

Thank you for figuring the problem and clearly explaining it!

Phil.