Re: [RFC PATCH v2 20/44] i386/tdx: Parse tdx metadata and store the result into TdxGuestState

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 8/26/2021 7:18 PM, Gerd Hoffmann wrote:
>> +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?

because there are two different parts in TDVF:
  a) one is firmware volume (BFV and CFV, i.e., OVMF_CODE.fd and 
OVMF_VAR.fd). Those are ROMs;

  b) the other is some RAM regions, e.g., temp memory for BFV early 
running and TD HOB to pass info to TDVF; Those are RAMs which is already 
added to TDX VM;

> 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?

Ideally, the firmware (part a above) can be mapped just below 4G like 
what we do for OVMF.

But it needs additional when map part a) to parse the metadata and get 
location of part b) and initialize the RAM of part b). Yes, the 
additional work can be added in existing OVMF laoding flow as pflash.


+ Laszlo,

Regarding laoding TDVF as pflash, I have some questions:

- pflash requires KVM to support readonly mmeory. However, for TDX, it 
doesn't support readonly memory. Is it a must? or we can make an 
exception for TDX?

- I saw from 
https://lists.gnu.org/archive/html/qemu-discuss/2018-04/msg00045.html, 
you said when load OVMF as pflash, it's MMIO. But for TDVF, it's treated 
as private memory. I'm not sure whether it will cause some potential 
problem if loading TDVF with pflash.

Anyway I tried changing the existing pflash approach to load TDVF. It 
can boot a TDX VM and no issue.

> thanks,
>    Gerd
>