Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Luigi Leonardi <leonardi@redhat.com>
To: Oliver Steffen <osteffen@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	 Eduardo Habkost <eduardo@habkost.net>,
	Gerd Hoffmann <kraxel@redhat.com>,
	 Stefano Garzarella <sgarzare@redhat.com>,
	Zhao Liu <zhao1.liu@intel.com>,
	 Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Igor Mammedov <imammedo@redhat.com>,
	 Marcelo Tosatti <mtosatti@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	 Ani Sinha <anisinha@redhat.com>,
	kvm@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.com>,
	 Joerg Roedel <joerg.roedel@amd.com>
Subject: Re: [PATCH v5 3/6] igvm: Add common function for finding parameter entries
Date: Thu, 29 Jan 2026 12:09:41 +0100	[thread overview]
Message-ID: <aXs_30LoEwVjCSg1@leonardi-redhat> (raw)
In-Reply-To: <20260127100257.1074104-4-osteffen@redhat.com>

On Tue, Jan 27, 2026 at 11:02:54AM +0100, Oliver Steffen wrote:
>Move repeating code for finding the parameter entries in the IGVM
>backend out of the parameter handlers into a common function.
>
>A warning message is emitted in case a no parameter entry can be found
>for a given index.
>
>Signed-off-by: Oliver Steffen <osteffen@redhat.com>
>---
> backends/igvm.c | 143 ++++++++++++++++++++++++++----------------------
> 1 file changed, 77 insertions(+), 66 deletions(-)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 4cf7b57234..213c9d337e 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -12,6 +12,7 @@
> #include "qemu/osdep.h"
>
> #include "qapi/error.h"
>+#include "qemu/error-report.h"
> #include "qemu/target-info-qapi.h"
> #include "system/igvm.h"
> #include "system/igvm-cfg.h"
>@@ -97,6 +98,20 @@ typedef struct QIgvm {
>     unsigned region_page_count;
> } QIgvm;
>
>+static QIgvmParameterData*
>+qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index)
>+{
>+    QIgvmParameterData *param_entry;
>+    QTAILQ_FOREACH(param_entry, &igvm->parameter_data, next)
>+    {
>+        if (param_entry->index == parameter_area_index) {
>+            return param_entry;
>+        }
>+    }
>+    warn_report("IGVM: No parameter area for index %u", parameter_area_index);
>+    return NULL;
>+}
>+
> static int qigvm_directive_page_data(QIgvm *ctx, const uint8_t *header_data,
>                                      Error **errp);
> static int qigvm_directive_vp_context(QIgvm *ctx, const uint8_t *header_data,
>@@ -571,58 +586,54 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>     }
>
>     /* Find the parameter area that should hold the memory map */
>-    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>-    {
>-        if (param_entry->index == param->parameter_area_index) {
>-            max_entry_count =
>-                param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>-            mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
>-
>-            retval = get_mem_map_entry(entry, &cgmm_entry, errp);
>-            while (retval == 0) {
>-                if (entry >= max_entry_count) {
>-                    error_setg(
>-                        errp,
>-                        "IGVM: guest memory map size exceeds parameter area defined in IGVM file");
>-                    return -1;
>-                }
>-                mm_entry[entry].starting_gpa_page_number = cgmm_entry.gpa >> 12;
>-                mm_entry[entry].number_of_pages = cgmm_entry.size >> 12;
>-
>-                switch (cgmm_entry.type) {
>-                case CGS_MEM_RAM:
>-                    mm_entry[entry].entry_type =
>-                        IGVM_MEMORY_MAP_ENTRY_TYPE_MEMORY;
>-                    break;
>-                case CGS_MEM_RESERVED:
>-                    mm_entry[entry].entry_type =
>-                        IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>-                    break;
>-                case CGS_MEM_ACPI:
>-                    mm_entry[entry].entry_type =
>-                        IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>-                    break;
>-                case CGS_MEM_NVS:
>-                    mm_entry[entry].entry_type =
>-                        IGVM_MEMORY_MAP_ENTRY_TYPE_PERSISTENT;
>-                    break;
>-                case CGS_MEM_UNUSABLE:
>-                    mm_entry[entry].entry_type =
>-                        IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>-                    break;
>-                }
>-                retval = get_mem_map_entry(++entry, &cgmm_entry, errp);
>-            }
>-            if (retval < 0) {
>-                return retval;
>-            }
>-            /* The entries need to be sorted */
>-            qsort(mm_entry, entry, sizeof(IGVM_VHS_MEMORY_MAP_ENTRY),
>-                  qigvm_cmp_mm_entry);
>+    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
>+    if (param_entry == NULL) {
>+        return 0;
>+    }
>+
>+    max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>+    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
>+
>+    retval = get_mem_map_entry(entry, &cgmm_entry, errp);
>+    while (retval == 0) {
>+        if (entry >= max_entry_count) {
>+            error_setg(
>+                errp,
>+                "IGVM: guest memory map size exceeds parameter area defined "
>+                "in IGVM file");
>+            return -1;
>+        }
>+        mm_entry[entry].starting_gpa_page_number = cgmm_entry.gpa >> 12;
>+        mm_entry[entry].number_of_pages = cgmm_entry.size >> 12;
>
>+        switch (cgmm_entry.type) {
>+        case CGS_MEM_RAM:
>+            mm_entry[entry].entry_type = IGVM_MEMORY_MAP_ENTRY_TYPE_MEMORY;
>+            break;
>+        case CGS_MEM_RESERVED:
>+            mm_entry[entry].entry_type =
>+                IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>+            break;
>+        case CGS_MEM_ACPI:
>+            mm_entry[entry].entry_type =
>+                IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>+            break;
>+        case CGS_MEM_NVS:
>+            mm_entry[entry].entry_type = IGVM_MEMORY_MAP_ENTRY_TYPE_PERSISTENT;
>+            break;
>+        case CGS_MEM_UNUSABLE:
>+            mm_entry[entry].entry_type =
>+                IGVM_MEMORY_MAP_ENTRY_TYPE_PLATFORM_RESERVED;
>             break;
>         }
>+        retval = get_mem_map_entry(++entry, &cgmm_entry, errp);
>+    }
>+    if (retval < 0) {
>+        return retval;
>     }
>+    /* The entries need to be sorted */
>+    qsort(mm_entry, entry, sizeof(IGVM_VHS_MEMORY_MAP_ENTRY),
>+          qigvm_cmp_mm_entry);
>     return 0;
> }
>
>@@ -634,18 +645,18 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
>     uint32_t *vp_count;
>     CPUState *cpu;
>
>-    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>+    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
>+    if (param_entry == NULL) {
>+        return 0;
>+    }
>+
>+    vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
>+    *vp_count = 0;
>+    CPU_FOREACH(cpu)
>     {
>-        if (param_entry->index == param->parameter_area_index) {
>-            vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
>-            *vp_count = 0;
>-            CPU_FOREACH(cpu)
>-            {
>-                (*vp_count)++;
>-            }
>-            break;
>-        }
>+        (*vp_count)++;
>     }
>+
>     return 0;
> }
>
>@@ -657,15 +668,15 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
>     QIgvmParameterData *param_entry;
>     IgvmEnvironmentInfo *environmental_state;
>
>-    QTAILQ_FOREACH(param_entry, &ctx->parameter_data, next)
>-    {
>-        if (param_entry->index == param->parameter_area_index) {
>-            environmental_state =
>-                (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
>-            environmental_state->memory_is_shared = 1;
>-            break;
>-        }
>+    param_entry = qigvm_find_param_entry(ctx, param->parameter_area_index);
>+    if (param_entry == NULL) {
>+        return 0;
>     }
>+
>+    environmental_state =
>+        (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
>+    environmental_state->memory_is_shared = 1;
>+
>     return 0;
> }
>
>-- 
>2.52.0
>

LGTM!

Reviewed-by: Luigi Leonardi <leonardi@redhat.com>


  reply	other threads:[~2026-01-29 11:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-27 10:02 [PATCH v5 0/6] igvm: Supply MADT via IGVM parameter Oliver Steffen
2026-01-27 10:02 ` [PATCH v5 1/6] hw/acpi: Make BIOS linker optional Oliver Steffen
2026-01-29 10:51   ` Luigi Leonardi
2026-01-27 10:02 ` [PATCH v5 2/6] hw/acpi: Add standalone function to build MADT Oliver Steffen
2026-01-27 10:02 ` [PATCH v5 3/6] igvm: Add common function for finding parameter entries Oliver Steffen
2026-01-29 11:09   ` Luigi Leonardi [this message]
2026-01-27 10:02 ` [PATCH v5 4/6] igvm: Refactor qigvm_parameter_insert Oliver Steffen
2026-01-29 11:10   ` Luigi Leonardi
2026-01-27 10:02 ` [PATCH v5 5/6] igvm: Pass machine state to IGVM file processing Oliver Steffen
2026-01-27 10:02 ` [PATCH v5 6/6] igvm: Fill MADT IGVM parameter field Oliver Steffen
2026-01-27 15:54   ` Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aXs_30LoEwVjCSg1@leonardi-redhat \
    --to=leonardi@redhat.com \
    --cc=anisinha@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=imammedo@redhat.com \
    --cc=joerg.roedel@amd.com \
    --cc=kraxel@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=osteffen@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=sgarzare@redhat.com \
    --cc=zhao1.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox