> On 3 Jul 2025, at 6:56 PM, Roy Hopkins <[email protected]> wrote:
>
> On Fri, 2025-06-27 at 15:41 +0530, Ani Sinha wrote:
>> On Fri, Jun 13, 2025 at 7:24 PM Roy Hopkins <[email protected]>
>> wrote:
>>>
>>> Adds an IGVM loader to QEMU which processes a given IGVM file and
>>> applies the directives within the file to the current guest
>>> configuration.
>>>
>>> The IGVM loader can be used to configure both confidential and
>>> non-confidential guests. For confidential guests, the
>>> ConfidentialGuestSupport object for the system is used to encrypt
>>> memory, apply the initial CPU state and perform other confidential guest
>>> operations.
>>>
>>> The loader is configured via a new IgvmCfg QOM object which allows the
>>> user to provide a path to the IGVM file to process.
>>>
>>> Signed-off-by: Roy Hopkins <[email protected]>
>>> Acked-by: Michael S. Tsirkin <[email protected]>
>>> Acked-by: Gerd Hoffman <[email protected]>
>>> Reviewed-by: Stefano Garzarella <[email protected]>
>>> ---
>>> backends/igvm-cfg.c | 51 +++
>>> backends/igvm.c | 807 ++++++++++++++++++++++++++++++++++++++
>>> backends/igvm.h | 22 ++
>>> backends/meson.build | 2 +
>>> include/system/igvm-cfg.h | 46 +++
>>> qapi/qom.json | 17 +
>>> 6 files changed, 945 insertions(+)
>>> create mode 100644 backends/igvm-cfg.c
>>> create mode 100644 backends/igvm.c
>>> create mode 100644 backends/igvm.h
>>> create mode 100644 include/system/igvm-cfg.h
>>>
>>
>> <snip>
>>
>>> +
>>> +static int qigvm_process_mem_region(QIgvm *ctx, unsigned start_index,
>>> + uint64_t gpa_start, unsigned
>>> page_count,
>>> + const IgvmPageDataFlags *flags,
>>> + const IgvmPageDataType page_type,
>>> + Error **errp)
>>> +{
>>> + uint8_t *region;
>>> + IgvmHandle data_handle;
>>> + const void *data;
>>> + uint32_t data_size;
>>> + unsigned page_index;
>>> + bool zero = true;
>>> + const uint64_t page_size = flags->is_2mb_page ? 0x200000 : 0x1000;
>>> + int result;
>>> + int cgs_page_type;
>>> +
>>> + region = qigvm_prepare_memory(ctx, gpa_start, page_count * page_size,
>>> + start_index, errp);
>>> + if (!region) {
>>> + return -1;
>>> + }
>>> +
>>> + for (page_index = 0; page_index < page_count; page_index++) {
>>> + data_handle = igvm_get_header_data(
>>> + ctx->file, IGVM_HEADER_SECTION_DIRECTIVE, page_index +
>>> start_index);
>>> + if (data_handle == IGVMAPI_NO_DATA) {
>>> + /* No data indicates a zero page */
>>> + memset(®ion[page_index * page_size], 0, page_size);
>>> + } else if (data_handle < 0) {
>>> + error_setg(
>>> + errp,
>>> + "IGVM file contains invalid page data for directive with "
>>> + "index %d",
>>> + page_index + start_index);
>>> + return -1;
>>> + } else {
>>> + zero = false;
>>> + data_size = igvm_get_buffer_size(ctx->file, data_handle);
>>> + if (data_size < page_size) {
>>> + memset(®ion[page_index * page_size], 0, page_size);
>>> + } else if (data_size > page_size) {
>>> + error_setg(errp,
>>> + "IGVM file contains page data with invalid size
>>> for "
>>> + "directive with index %d",
>>> + page_index + start_index);
>>> + return -1;
>>> + }
>>> + data = igvm_get_buffer(ctx->file, data_handle);
>>> + memcpy(®ion[page_index * page_size], data, data_size);
>>> + igvm_free_buffer(ctx->file, data_handle);
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * If a confidential guest support object is provided then use it to
>>> set the
>>> + * guest state.
>>> + */
>>> + if (ctx->cgs) {
>>> + cgs_page_type =
>>> + qigvm_type_to_cgs_type(page_type, flags->unmeasured, zero);
>>> + if (cgs_page_type < 0) {
>>> + error_setg(errp,
>>> + "Invalid page type in IGVM file. Directives: %d to
>>> %d, "
>>> + "page type: %d",
>>> + start_index, start_index + page_count, page_type);
>>> + return -1;
>>> + }
>>> +
>>> + result = ctx->cgsc->set_guest_state(
>>> + gpa_start, region, page_size * page_count, cgs_page_type, 0,
>>> errp);
>>> + if (result < 0) {
>>> + return result;
>>> + }
>>> + }
>>> + return 0;
>>> +}
>>> +
>>> +static int qigvm_process_mem_page(QIgvm *ctx,
>>> + const IGVM_VHS_PAGE_DATA *page_data,
>>> + Error **errp)
>>> +{
>>> + if (page_data) {
>>> + if (ctx->region_page_count == 0) {
>>> + ctx->region_start = page_data->gpa;
>>> + ctx->region_start_index = ctx->current_header_index;
>>> + } else {
>>> + if (!qigvm_page_attrs_equal(ctx->file,
>>> ctx->current_header_index,
>>> + page_data,
>>> + &ctx->region_prev_page_data) ||
>>> + ((ctx->region_prev_page_data.gpa +
>>> + (ctx->region_prev_page_data.flags.is_2mb_page ? 0x200000
>>> :
>>> + 0x1000))
>>> !=
>>> + page_data->gpa) ||
>>> + (ctx->region_last_index != (ctx->current_header_index -
>>> 1))) {
>>> + /* End of current region */
>>> + if (qigvm_process_mem_region(
>>> + ctx, ctx->region_start_index, ctx->region_start,
>>> + ctx->region_page_count,
>>> + &ctx->region_prev_page_data.flags,
>>> + ctx->region_prev_page_data.data_type, errp) < 0) {
>>> + return -1;
>>> + }
>>> + ctx->region_page_count = 0;
>>> + ctx->region_start = page_data->gpa;
>>> + ctx->region_start_index = ctx->current_header_index;
>>
>> Should we return here? Is there any need for the memcpy() below?
>>
>
> No. In this case the new region is not contiguous with the previous region so
> the
> previous region is completed and the current region variables initialized
> (using
> the same values as the case where no previous region existed above). The code
> then
> falls through to populate the new region with the first page, hence the
> memcpy.
Yes with your explanation it makes sense. Can you please add some comments to
this code if you need to respin, otherwise I can add it once it merges.
>
>>> + }
>>> + }
>>> + memcpy(&ctx->region_prev_page_data, page_data,
>>> + sizeof(ctx->region_prev_page_data));
>>> + ctx->region_last_index = ctx->current_header_index;
>>> + ctx->region_page_count++;
>>> + } else {
>>> + if (ctx->region_page_count > 0) {
>>> + if (qigvm_process_mem_region(
>>> + ctx, ctx->region_start_index, ctx->region_start,
>>> + ctx->region_page_count,
>>> &ctx->region_prev_page_data.flags,
>>> + ctx->region_prev_page_data.data_type, errp) < 0) {
>>> + return -1;
>>> + }
>>> + ctx->region_page_count = 0;
>>> + }
>>> + }
>>> + return 0;
>>> +}
>>> +
