Re: [PATCH v2] hw/misc/vmfwupdate: Introduce hypervisor fw-cfg interface support

[Ani Sinha]

> On 17 Dec 2024, at 8:51 PM, Gerd Hoffman <kraxel@redhat.com> wrote:
> 
>  Hi,
> 
>> +Fw-cfg Files
>> +************
>> +
>> +Guests drive vmfwupdate through special ``fw-cfg`` files that control its 
>> flow
>> +followed by a standard system reset operation. When vmfwupdate is available,
>> +it provides the following ``fw-cfg`` files:
>> +
>> +* ``vmfwupdate/cap`` (``u64``) - Read-only Little Endian encoded bitmap of 
>> additional
>> +  capabilities the interface supports. List of available capabilities:
>> +
>> +     ``VMFWUPDATE_CAP_BIOS_RESIZE        0x0000000000000001``
>> +
>> +* ``vmfwupdate/bios-size`` (``u32``) - Little Endian encoded size of the 
>> BIOS region.
>> +  Read-only by default. Optionally Read-write if ``vmfwupdate/cap`` contains
>> +  ``VMFWUPDATE_CAP_BIOS_RESIZE``. On write, the BIOS region may resize. 
>> Guests are
>> +  required to read the value after writing and compare it with the 
>> requested size
>> +  to determine whether the resize was successful. Note, x86 BIOS regions 
>> always
>> +  start at 4GiB - bios-size.
>> +
>> +* ``vmfwupdate/opaque`` (``1024 bytes``) - A 1KiB buffer that survives the 
>> BIOS replacement
>> +  flow. Can be used by the guest to propagate guest physical addresses of 
>> payloads
>> +  to its BIOS stage. It’s recommended to make the new BIOS clear this file 
>> on boot
>> +  if it exists. Contents of this file are under control by the hypervisor. 
>> In an
>> +  environment that considers the hypervisor outside of its trust boundary, 
>> guests
>> +  are advised to validate its contents before consumption.
>> +
>> +* ``vmfwupdate/disable`` (``u8``) - Indicates whether the interface is 
>> disabled.
>> +  Returns 0 for enabled, 1 for disabled. Writing any value disables it. 
>> Writing is
>> +  only allowed if the value is 0. When the interface is disabled, the 
>> replace file
>> +  is ignored on reset. This value resets to 0 on system reset.
>> +
>> +* ``vmfwupdate/bios-addr`` (``u64``) - A 64bit Little Endian encoded guest 
>> physical address
>> +  at the beginning of the replacement BIOS region. The provided payload 
>> must reside
>> +  in shared memory. 0 on system reset.
> 
> I'd suggest to make all integers u64 (little endian).
> 
> Any specific reason why vmfwupdate/opaque is 1024 bytes?
> How about using 4096 (aka PAGE_SIZE) bytes?
> 
>> +On reset, the hypervisor evaluates whether ``vmfwupdate/disable`` is ``1``. 
>> If it is, it ignores
>> +any other vmfwupdate values and performs a standard system reset.
>> +
>> +If ``vmfwupdate/disable`` is ``0``, the hypervisor checks if bios-addr is 
>> ``0``. If it is, it
>> +performs a standard system reset.
> 
> What is 'standard system reset' in SEV-SNP mode?  I think qemu does not
> support it right now and will simply poweroff the guest instead.  Will
> that continue to be the case?  

No. Work is in progress to support VM resets even in SEV-SNP.

> Or should qemu re-create the VM context
> in that case, simliar to the firmware update case,
> except that qemu
> would have to use the default firmware image then?

Fw update works both in coco and non-coco cases. Will update the doc in v3.

> 
> Will the vmfwupdate be supported without SEV-SNP too?  Might be useful
> for development + testing.  If so the spec should clarify what happens
> in that case, because the concept of "shared memory" does not exist
> then.

Will update the doc in v3.