All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] igvm: fix byte_offset handling in parameter directives
@ 2026-09-07 14:58 Luigi Leonardi
  2026-09-07 14:58 ` [PATCH v3 1/2] igvm: validate byte_offset before using it " Luigi Leonardi
  2026-09-07 14:58 ` [PATCH v3 2/2] igvm: honor byte_offset when writing memory map, MADT and device tree Luigi Leonardi
  0 siblings, 2 replies; 5+ messages in thread
From: Luigi Leonardi @ 2026-09-07 14:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Stefano Garzarella, Ani Sinha, Paolo Bonzini,
	Zhao Liu, qemu-stable, Luigi Leonardi

Several IGVM directives (memory map, vp-count, environment info, MADT,
device tree) _can_ place their data at a byte_offset within a shared
parameter area, letting multiple parameters be packed into one area.

Patch 1 introduces qigvm_get_param_data(), which looks up a
parameter area and validates that byte_offset falls within it in one
step, and uses it in the vp-count and environment-info handlers, which
already relied on byte_offset.

Patch 2 fixes the memory map, MADT and device tree handlers, which
ignored byte_offset entirely and always wrote at the start of the
buffer, potentially corrupting earlier data when several parameters
share an area.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
Changes in v3:
- Renamed the function to `qigvm_get_param_data`. [Stefano]
- `qigvm_get_param_data` now returns the data pointer directly [Stefano]
- Added fixes tags. [Stefano]
- Renamed variables to param_data and param_size [Stefano]
- Link to v2: https://lore.kernel.org/qemu-devel/20260904-fix_offset-v2-0-f5bb4cf6d4b0@redhat.com

Changes in v2:
- Inverted commit order: first I introduce the helper and use it where
  necessary, then in commit 2 I handle the `offset`. [Stefano]
- The helper now returns data and size, that already consider the byte
  offset [Stefano]
- Link to v1: https://lore.kernel.org/qemu-devel/20260902-fix_offset-v1-0-04b18f7595b2@redhat.com

---
Luigi Leonardi (2):
      igvm: validate byte_offset before using it in parameter directives
      igvm: honor byte_offset when writing memory map, MADT and device tree

 backends/igvm.c                | 96 ++++++++++++++++++++++++++++++++----------
 include/system/igvm-internal.h |  6 +++
 target/i386/igvm.c             | 13 +++---
 3 files changed, 86 insertions(+), 29 deletions(-)
---
base-commit: d2e570cc0f97b936902a5b1b86b73c0f5998b475
change-id: 20260902-fix_offset-a268cb12aafc

Best regards,
-- 
Luigi Leonardi <leonardi@redhat.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/2] igvm: validate byte_offset before using it in parameter directives
  2026-09-07 14:58 [PATCH v3 0/2] igvm: fix byte_offset handling in parameter directives Luigi Leonardi
@ 2026-09-07 14:58 ` Luigi Leonardi
  2026-09-10  9:35   ` Stefano Garzarella
  2026-09-07 14:58 ` [PATCH v3 2/2] igvm: honor byte_offset when writing memory map, MADT and device tree Luigi Leonardi
  1 sibling, 1 reply; 5+ messages in thread
From: Luigi Leonardi @ 2026-09-07 14:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Stefano Garzarella, Ani Sinha, Paolo Bonzini,
	Zhao Liu, qemu-stable, Luigi Leonardi

None of the directive handlers that place data at a byte_offset within
a parameter area validated that byte_offset actually falls within the
parameter area's size. A malformed IGVM file with byte_offset > size would
underflow the "size - byte_offset" computation used to determine
remaining space, wrapping to a huge value and defeating the size
check, then write out of bounds through param_entry->data +
byte_offset.

Add qigvm_get_param_data(), which looks up the parameter area,
checks byte_offset against its size, and returns the offset-adjusted
data pointer directly (NULL on failure), with the remaining space
returned via an output parameter, instead of the raw
QIgvmParameterData entry. This keeps the byte_offset arithmetic in
one place instead of repeating param_entry->data + byte_offset and
param_entry->size - byte_offset at every call site.

Use it in the vp-count and environment-info handlers, adding an
explicit check that the fixed-size write fits in the remaining space.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/igvm.c                | 70 +++++++++++++++++++++++++++++++++++-------
 include/system/igvm-internal.h |  6 ++++
 2 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index 7b7bdc72b7..a8340d7d55 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -101,6 +101,39 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
     return NULL;
 }
 
+/*
+ * Look up a parameter area and check that param->byte_offset falls within
+ * it, in one step. On success, returns the offset-adjusted write location
+ * within the parameter area and sets *param_size to the remaining space
+ * there, so callers never need to touch param->byte_offset themselves.
+ * Returns NULL on failure.
+ */
+uint8_t *
+qigvm_get_param_data(QIgvm *igvm, uint32_t parameter_area_index,
+                     const IGVM_VHS_PARAMETER *param,
+                     uint32_t *param_size,
+                     Error **errp)
+{
+    QIgvmParameterData *param_entry;
+
+    assert(param_size);
+
+    param_entry = qigvm_find_param_entry(igvm, parameter_area_index, errp);
+    if (!param_entry) {
+        return NULL;
+    }
+
+    if (param->byte_offset > param_entry->size) {
+        error_setg(errp,
+                   "IGVM: byte_offset 0x%x exceeds parameter area size 0x%x",
+                   param->byte_offset, param_entry->size);
+        return NULL;
+    }
+
+    *param_size = param_entry->size - param->byte_offset;
+    return param_entry->data + param->byte_offset;
+}
+
 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,
@@ -682,17 +715,25 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
                                     Error **errp)
 {
     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
-    QIgvmParameterData *param_entry;
+    uint8_t *param_data;
+    uint32_t param_size;
     uint32_t *vp_count;
     CPUState *cpu;
 
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+                                       param, &param_size, errp);
+    if (!param_data) {
         return -1;
     }
 
-    vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
+    if (sizeof(*vp_count) > param_size) {
+        error_setg(errp,
+                   "IGVM: vp-count parameter exceeds parameter area "
+                   "defined in IGVM file");
+        return -1;
+    }
+
+    vp_count = (uint32_t *)param_data;
     *vp_count = 0;
     CPU_FOREACH(cpu)
     {
@@ -707,17 +748,24 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
                                             Error **errp)
 {
     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
-    QIgvmParameterData *param_entry;
+    uint8_t *param_data;
+    uint32_t param_size;
     IgvmEnvironmentInfo *environmental_state;
 
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+                                       param, &param_size, errp);
+    if (!param_data) {
+        return -1;
+    }
+
+    if (sizeof(*environmental_state) > param_size) {
+        error_setg(errp,
+                   "IGVM: environment-info parameter exceeds parameter area "
+                   "defined in IGVM file");
         return -1;
     }
 
-    environmental_state =
-        (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
+    environmental_state = (IgvmEnvironmentInfo *)param_data;
     environmental_state->memory_is_shared = 1;
 
     return 0;
diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
index 9e9fa1d9af..f5ee2d5b1b 100644
--- a/include/system/igvm-internal.h
+++ b/include/system/igvm-internal.h
@@ -81,4 +81,10 @@ QIgvmParameterData*
 qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
                        Error **errp);
 
+uint8_t *
+qigvm_get_param_data(QIgvm *igvm, uint32_t parameter_area_index,
+                     const IGVM_VHS_PARAMETER *param,
+                     uint32_t *param_size,
+                     Error **errp);
+
 #endif

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] igvm: honor byte_offset when writing memory map, MADT and device tree
  2026-09-07 14:58 [PATCH v3 0/2] igvm: fix byte_offset handling in parameter directives Luigi Leonardi
  2026-09-07 14:58 ` [PATCH v3 1/2] igvm: validate byte_offset before using it " Luigi Leonardi
@ 2026-09-07 14:58 ` Luigi Leonardi
  2026-09-10  9:47   ` Stefano Garzarella
  1 sibling, 1 reply; 5+ messages in thread
From: Luigi Leonardi @ 2026-09-07 14:58 UTC (permalink / raw)
  To: qemu-devel
  Cc: Gerd Hoffmann, Stefano Garzarella, Ani Sinha, Paolo Bonzini,
	Zhao Liu, qemu-stable, Luigi Leonardi

qigvm_directive_memory_map(), qigvm_directive_madt() and
qigvm_directive_device_tree() all wrote their data unconditionally at
the start of the parameter area's buffer, ignoring param->byte_offset
from the IGVM_VHS_PARAMETER header. This is harmless when a
directive's offset happens to be 0, but breaks for IGVM files that pack
multiple parameters into a single shared parameter area at different
offsets: a later directive would overwrite the data written by an earlier
one at the start of the buffer, corrupting it.

Switch these handlers to qigvm_get_param_data(), introduced in
the previous patch, and use the offset-adjusted data pointer it
returns and the remaining size it sets instead of writing at
param_entry->data and sizing checks against param_entry->size
directly. This both honors byte_offset and validates it against the
parameter area size before it is used.

Fixes: c1d466d267 ("backends/igvm: Add IGVM loader and configuration")
Fixes: dea1f68a5c ("igvm: Fill MADT IGVM parameter field on x86_64")
Fixes: 1c4bd8f13c ("igvm: add device tree parameter support")
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/igvm.c    | 26 ++++++++++++++------------
 target/i386/igvm.c | 13 +++++++------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index a8340d7d55..b3ff6cba4a 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -638,7 +638,8 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
     int (*get_mem_map_entry)(int index, ConfidentialGuestMemoryMapEntry *entry,
                              Error **errp) = NULL;
-    QIgvmParameterData *param_entry;
+    uint8_t *param_data;
+    uint32_t param_size;
     int max_entry_count;
     int entry = 0;
     IGVM_VHS_MEMORY_MAP_ENTRY *mm_entry;
@@ -659,14 +660,14 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
     }
 
     /* Find the parameter area that should hold the memory map */
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+                                       param, &param_size, errp);
+    if (!param_data) {
         return -1;
     }
 
-    max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
-    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
+    max_entry_count = param_size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
+    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_data;
 
     retval = get_mem_map_entry(entry, &cgmm_entry, errp);
     while (retval == 0) {
@@ -862,12 +863,13 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
 {
     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
     g_autofree void *fdt_packed = NULL;
-    QIgvmParameterData *param_entry;
+    uint8_t *param_data;
+    uint32_t param_size;
     uint32_t fdt_size;
 
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+                                       param, &param_size, errp);
+    if (!param_data) {
         return -1;
     }
 
@@ -885,14 +887,14 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
     }
 
     fdt_size = fdt_totalsize(fdt_packed);
-    if (fdt_size > param_entry->size) {
+    if (fdt_size > param_size) {
         error_setg(errp,
                    "IGVM: device tree size exceeds parameter area"
                    " defined in IGVM file");
         return -1;
     }
 
-    memcpy(param_entry->data, fdt_packed, fdt_size);
+    memcpy(param_data, fdt_packed, fdt_size);
 
     return 0;
 }
diff --git a/target/i386/igvm.c b/target/i386/igvm.c
index ad9bf87761..6ae662b033 100644
--- a/target/i386/igvm.c
+++ b/target/i386/igvm.c
@@ -187,20 +187,21 @@ void qigvm_x86_bsp_reset(CPUX86State *env)
 int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
 {
     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
-    QIgvmParameterData *param_entry;
+    uint8_t *param_data;
+    uint32_t param_size;
     int result = 0;
 
     /* Find the parameter area that should hold the MADT data */
-    param_entry = qigvm_find_param_entry(ctx,
-                                         param->parameter_area_index, errp);
-    if (param_entry == NULL) {
+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
+                                       param, &param_size, errp);
+    if (!param_data) {
         return -1;
     }
 
     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
 
-    if (madt->len <= param_entry->size) {
-        memcpy(param_entry->data, madt->data, madt->len);
+    if (madt->len <= param_size) {
+        memcpy(param_data, madt->data, madt->len);
     } else {
         error_setg(
             errp,

-- 
2.55.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] igvm: validate byte_offset before using it in parameter directives
  2026-09-07 14:58 ` [PATCH v3 1/2] igvm: validate byte_offset before using it " Luigi Leonardi
@ 2026-09-10  9:35   ` Stefano Garzarella
  0 siblings, 0 replies; 5+ messages in thread
From: Stefano Garzarella @ 2026-09-10  9:35 UTC (permalink / raw)
  To: Luigi Leonardi
  Cc: qemu-devel, Gerd Hoffmann, Ani Sinha, Paolo Bonzini, Zhao Liu,
	qemu-stable

On Mon, Sep 07, 2026 at 04:58:55PM +0200, Luigi Leonardi wrote:
>None of the directive handlers that place data at a byte_offset within
>a parameter area validated that byte_offset actually falls within the
>parameter area's size. A malformed IGVM file with byte_offset > size would
>underflow the "size - byte_offset" computation used to determine
>remaining space, wrapping to a huge value and defeating the size
>check, then write out of bounds through param_entry->data +
>byte_offset.
>
>Add qigvm_get_param_data(), which looks up the parameter area,
>checks byte_offset against its size, and returns the offset-adjusted
>data pointer directly (NULL on failure), with the remaining space
>returned via an output parameter, instead of the raw
>QIgvmParameterData entry. This keeps the byte_offset arithmetic in
>one place instead of repeating param_entry->data + byte_offset and
>param_entry->size - byte_offset at every call site.
>
>Use it in the vp-count and environment-info handlers, adding an
>explicit check that the fixed-size write fits in the remaining space.
>

Should we add a Fixes tag?

>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/igvm.c                | 70 +++++++++++++++++++++++++++++++++++-------
> include/system/igvm-internal.h |  6 ++++
> 2 files changed, 65 insertions(+), 11 deletions(-)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 7b7bdc72b7..a8340d7d55 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -101,6 +101,39 @@ qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
>     return NULL;
> }
>
>+/*
>+ * Look up a parameter area and check that param->byte_offset falls within
>+ * it, in one step. On success, returns the offset-adjusted write location
>+ * within the parameter area and sets *param_size to the remaining space
>+ * there, so callers never need to touch param->byte_offset themselves.
>+ * Returns NULL on failure.
>+ */

How about something a little more concise and straight to the point?

/*
  * Get parameter area data at byte_offset with bounds validation.
  * On success, returns offset-adjusted data pointer and sets param_size
  * to remaining space.
  * Returns NULL on failure.
  */

>+uint8_t *
>+qigvm_get_param_data(QIgvm *igvm, uint32_t parameter_area_index,

IIUC all callers are just passing param->parameter_area_index, since we 
have `param`, can we avoid passsing `parameter_area_index` as parameter?

>+                     const IGVM_VHS_PARAMETER *param,
>+                     uint32_t *param_size,
>+                     Error **errp)
>+{
>+    QIgvmParameterData *param_entry;
>+
>+    assert(param_size);
>+
>+    param_entry = qigvm_find_param_entry(igvm, parameter_area_index, errp);
>+    if (!param_entry) {
>+        return NULL;
>+    }
>+
>+    if (param->byte_offset > param_entry->size) {
>+        error_setg(errp,
>+                   "IGVM: byte_offset 0x%x exceeds parameter area size 0x%x",
>+                   param->byte_offset, param_entry->size);
>+        return NULL;
>+    }
>+
>+    *param_size = param_entry->size - param->byte_offset;
>+    return param_entry->data + param->byte_offset;
>+}
>+
> 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,
>@@ -682,17 +715,25 @@ static int qigvm_directive_vp_count(QIgvm *ctx, const uint8_t *header_data,
>                                     Error **errp)
> {
>     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>-    QIgvmParameterData *param_entry;
>+    uint8_t *param_data;
>+    uint32_t param_size;
>     uint32_t *vp_count;
>     CPUState *cpu;
>
>-    param_entry = qigvm_find_param_entry(ctx,
>-                                         param->parameter_area_index, errp);
>-    if (param_entry == NULL) {
>+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,

Can we avoid `param_data` at all and assing it directly to `vp_count` ?

>+                                       param, &param_size, errp);
>+    if (!param_data) {
>         return -1;
>     }
>
>-    vp_count = (uint32_t *)(param_entry->data + param->byte_offset);
>+    if (sizeof(*vp_count) > param_size) {
>+        error_setg(errp,
>+                   "IGVM: vp-count parameter exceeds parameter area "
>+                   "defined in IGVM file");
>+        return -1;
>+    }
>+
>+    vp_count = (uint32_t *)param_data;
>     *vp_count = 0;
>     CPU_FOREACH(cpu)
>     {
>@@ -707,17 +748,24 @@ static int qigvm_directive_environment_info(QIgvm *ctx,
>                                             Error **errp)
> {
>     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>-    QIgvmParameterData *param_entry;
>+    uint8_t *param_data;
>+    uint32_t param_size;
>     IgvmEnvironmentInfo *environmental_state;
>
>-    param_entry = qigvm_find_param_entry(ctx,
>-                                         param->parameter_area_index, errp);
>-    if (param_entry == NULL) {
>+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
>+                                       param, &param_size, errp);

Ditto.

>+    if (!param_data) {
>+        return -1;
>+    }
>+
>+    if (sizeof(*environmental_state) > param_size) {
>+        error_setg(errp,
>+                   "IGVM: environment-info parameter exceeds parameter area "
>+                   "defined in IGVM file");
>         return -1;
>     }
>
>-    environmental_state =
>-        (IgvmEnvironmentInfo *)(param_entry->data + param->byte_offset);
>+    environmental_state = (IgvmEnvironmentInfo *)param_data;
>     environmental_state->memory_is_shared = 1;
>
>     return 0;
>diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
>index 9e9fa1d9af..f5ee2d5b1b 100644
>--- a/include/system/igvm-internal.h
>+++ b/include/system/igvm-internal.h
>@@ -81,4 +81,10 @@ QIgvmParameterData*
> qigvm_find_param_entry(QIgvm *igvm, uint32_t parameter_area_index,
>                        Error **errp);
>
>+uint8_t *
>+qigvm_get_param_data(QIgvm *igvm, uint32_t parameter_area_index,
>+                     const IGVM_VHS_PARAMETER *param,
>+                     uint32_t *param_size,
>+                     Error **errp);
>+
> #endif
>
>-- 
>2.55.0
>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/2] igvm: honor byte_offset when writing memory map, MADT and device tree
  2026-09-07 14:58 ` [PATCH v3 2/2] igvm: honor byte_offset when writing memory map, MADT and device tree Luigi Leonardi
@ 2026-09-10  9:47   ` Stefano Garzarella
  0 siblings, 0 replies; 5+ messages in thread
From: Stefano Garzarella @ 2026-09-10  9:47 UTC (permalink / raw)
  To: Luigi Leonardi
  Cc: qemu-devel, Gerd Hoffmann, Ani Sinha, Paolo Bonzini, Zhao Liu,
	qemu-stable

On Mon, Sep 07, 2026 at 04:58:56PM +0200, Luigi Leonardi wrote:
>qigvm_directive_memory_map(), qigvm_directive_madt() and
>qigvm_directive_device_tree() all wrote their data unconditionally at
>the start of the parameter area's buffer, ignoring param->byte_offset
>from the IGVM_VHS_PARAMETER header. This is harmless when a
>directive's offset happens to be 0, but breaks for IGVM files that pack
>multiple parameters into a single shared parameter area at different
>offsets: a later directive would overwrite the data written by an earlier
>one at the start of the buffer, corrupting it.
>
>Switch these handlers to qigvm_get_param_data(), introduced in
>the previous patch, and use the offset-adjusted data pointer it
>returns and the remaining size it sets instead of writing at
>param_entry->data and sizing checks against param_entry->size
>directly. This both honors byte_offset and validates it against the
>parameter area size before it is used.
>
>Fixes: c1d466d267 ("backends/igvm: Add IGVM loader and configuration")

IIUC this one should be also in the first patch, right?

That said, since that commit introduced qigvm_directive_memory_map() 
should we move changes to it in the first patch of this series?

Or just squash everything in a single patch.

>Fixes: dea1f68a5c ("igvm: Fill MADT IGVM parameter field on x86_64")
>Fixes: 1c4bd8f13c ("igvm: add device tree parameter support")
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/igvm.c    | 26 ++++++++++++++------------
> target/i386/igvm.c | 13 +++++++------
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index a8340d7d55..b3ff6cba4a 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -638,7 +638,8 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>     int (*get_mem_map_entry)(int index, ConfidentialGuestMemoryMapEntry *entry,
>                              Error **errp) = NULL;
>-    QIgvmParameterData *param_entry;
>+    uint8_t *param_data;
>+    uint32_t param_size;
>     int max_entry_count;
>     int entry = 0;
>     IGVM_VHS_MEMORY_MAP_ENTRY *mm_entry;
>@@ -659,14 +660,14 @@ static int qigvm_directive_memory_map(QIgvm *ctx, const uint8_t *header_data,
>     }
>
>     /* Find the parameter area that should hold the memory map */
>-    param_entry = qigvm_find_param_entry(ctx,
>-                                         param->parameter_area_index, errp);
>-    if (param_entry == NULL) {
>+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
>+                                       param, &param_size, errp);

Ditto, why not assigning it to mm_entry ?

The rest LGTM.

Thanks,
Stefano

>+    if (!param_data) {
>         return -1;
>     }
>
>-    max_entry_count = param_entry->size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>-    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_entry->data;
>+    max_entry_count = param_size / sizeof(IGVM_VHS_MEMORY_MAP_ENTRY);
>+    mm_entry = (IGVM_VHS_MEMORY_MAP_ENTRY *)param_data;
>
>     retval = get_mem_map_entry(entry, &cgmm_entry, errp);
>     while (retval == 0) {
>@@ -862,12 +863,13 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
> {
>     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>     g_autofree void *fdt_packed = NULL;
>-    QIgvmParameterData *param_entry;
>+    uint8_t *param_data;
>+    uint32_t param_size;
>     uint32_t fdt_size;
>
>-    param_entry = qigvm_find_param_entry(ctx,
>-                                         param->parameter_area_index, errp);
>-    if (param_entry == NULL) {
>+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
>+                                       param, &param_size, errp);
>+    if (!param_data) {
>         return -1;
>     }
>
>@@ -885,14 +887,14 @@ static int qigvm_directive_device_tree(QIgvm *ctx, const uint8_t *header_data,
>     }
>
>     fdt_size = fdt_totalsize(fdt_packed);
>-    if (fdt_size > param_entry->size) {
>+    if (fdt_size > param_size) {
>         error_setg(errp,
>                    "IGVM: device tree size exceeds parameter area"
>                    " defined in IGVM file");
>         return -1;
>     }
>
>-    memcpy(param_entry->data, fdt_packed, fdt_size);
>+    memcpy(param_data, fdt_packed, fdt_size);
>
>     return 0;
> }
>diff --git a/target/i386/igvm.c b/target/i386/igvm.c
>index ad9bf87761..6ae662b033 100644
>--- a/target/i386/igvm.c
>+++ b/target/i386/igvm.c
>@@ -187,20 +187,21 @@ void qigvm_x86_bsp_reset(CPUX86State *env)
> int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
> {
>     const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>-    QIgvmParameterData *param_entry;
>+    uint8_t *param_data;
>+    uint32_t param_size;
>     int result = 0;
>
>     /* Find the parameter area that should hold the MADT data */
>-    param_entry = qigvm_find_param_entry(ctx,
>-                                         param->parameter_area_index, errp);
>-    if (param_entry == NULL) {
>+    param_data = qigvm_get_param_data(ctx, param->parameter_area_index,
>+                                       param, &param_size, errp);
>+    if (!param_data) {
>         return -1;
>     }
>
>     GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
>
>-    if (madt->len <= param_entry->size) {
>-        memcpy(param_entry->data, madt->data, madt->len);
>+    if (madt->len <= param_size) {
>+        memcpy(param_data, madt->data, madt->len);
>     } else {
>         error_setg(
>             errp,
>
>-- 
>2.55.0
>



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-10  9:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 14:58 [PATCH v3 0/2] igvm: fix byte_offset handling in parameter directives Luigi Leonardi
2026-09-07 14:58 ` [PATCH v3 1/2] igvm: validate byte_offset before using it " Luigi Leonardi
2026-09-10  9:35   ` Stefano Garzarella
2026-09-07 14:58 ` [PATCH v3 2/2] igvm: honor byte_offset when writing memory map, MADT and device tree Luigi Leonardi
2026-09-10  9:47   ` Stefano Garzarella

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.