Hi,

> +        /*
> +         * If TDVF temp memory describe in TDVF metadata lays in RAM, reserve
> +         * the region property.
> +         */
> +        if (entry->address >= 4 * GiB + x86ms->above_4g_mem_size ||
> +            entry->address + entry->size >= 4 * GiB + 
> x86ms->above_4g_mem_size) {
> +            error_report("TDVF type %u address 0x%" PRIx64 " size 0x%" PRIx64
> +                         " above high memory",
> +                         entry->type, entry->address, entry->size);
> +            exit(1);
> +        }

I think you can simply use dma_memory_map() API, then just work with
guest physical addresses and drop the messy and error-prone memory
region offset calculations.

> +    entry->mem_ptr = memory_region_get_ram_ptr(entry->mr);
> +    if (entry->data_len) {
> +        /*
> +         * The memory_region api doesn't allow partial file mapping, create
> +         * ram and copy the contents
> +         */
> +        if (lseek(fd, entry->data_offset, SEEK_SET) != entry->data_offset) {
> +            error_report("can't seek to 0x%x %s", entry->data_offset, 
> filename);
> +            exit(1);
> +        }
> +        if (read(fd, entry->mem_ptr, entry->data_len) != entry->data_len) {
> +            error_report("can't read 0x%x %s", entry->data_len, filename);
> +            exit(1);
> +        }
> +    }

Wouldn't a simple rom_add_blob work here?

> +int load_tdvf(const char *filename)
> +{

> +    for_each_fw_entry(fw, entry) {
> +        if (entry->address < x86ms->below_4g_mem_size ||
> +            entry->address > 4 * GiB) {
> +            tdvf_init_ram_memory(ms, entry);
> +        } else {
> +            tdvf_init_bios_memory(fd, filename, entry);
> +        }
> +    }

Why there are two different ways to load the firmware?

Also: why is all this firmware volume parsing needed?  The normal ovmf
firmware can simply be mapped just below 4G, why can't tdvf work the
same way?

thanks,
  Gerd


Reply via email to