Re: [PATCH v2] target/i386: Add unaccepted memory configuration

[Gupta, Pankaj]

> For SEV-SNP, an OS is "SEV-SNP capable" without supporting this UEFI
> v2.9 memory type. In order for OVMF to be able to avoid pre-validating
> potentially hundreds of gibibytes of data before booting, it needs to
> know if the guest OS can support its use of the new type of memory in
> the memory map.
>
> Cc: Xu, Min M <min.m.xu@intel.com>
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Thomas Lendacky <Thomas.Lendacky@amd.com>
> Cc: Gerd Hoffman <kraxel@redhat.com>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---

Wondering what changed in v2. Did I miss change log?

>   hw/i386/fw_cfg.c  |  6 ++++++
>   target/i386/sev.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
>   target/i386/sev.h |  2 ++
>   3 files changed, 57 insertions(+)
>
> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index a283785a8d..9c069ddebe 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -23,6 +23,7 @@
>   #include "e820_memory_layout.h"
>   #include "kvm/kvm_i386.h"
>   #include "qapi/error.h"
> +#include "target/i386/sev.h"
>   #include CONFIG_DEVICES
>   
>   struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
> @@ -131,6 +132,11 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
>                        &e820_reserve, sizeof(e820_reserve));
>       fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
>                       sizeof(struct e820_entry) * e820_get_num_entries());
> +    if (sev_has_accept_all_memory(ms->cgs)) {
> +        bool accept_all = sev_accept_all_memory(ms->cgs);
> +        fw_cfg_add_file(fw_cfg, "opt/ovmf/AcceptAllMemory",
> +                        &accept_all, sizeof(accept_all));
> +    }
>   
>       fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
>       /* allocate memory for the NUMA channel: one (64bit) word for the number
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 32f7dbac4e..01399a304c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -64,6 +64,7 @@ struct SevGuestState {
>       uint32_t cbitpos;
>       uint32_t reduced_phys_bits;
>       bool kernel_hashes;
> +    int accept_all_memory;
>   
>       /* runtime state */
>       uint32_t handle;
> @@ -155,6 +156,15 @@ static const char *const sev_fw_errlist[] = {
>       [SEV_RET_SECURE_DATA_INVALID]    = "Part-specific integrity check 
> failure",
>   };
>   
> +static QEnumLookup memory_acceptance_lookup = {
> +    .array = (const char *const[]) {
> +        "default",
> +        "true",
> +        "false",
> +    },
> +    .size = 3,
> +};
> +
>   #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
>   
>   static int
> @@ -353,6 +363,21 @@ static void sev_guest_set_kernel_hashes(Object *obj, bool 
> value, Error **errp)
>       sev->kernel_hashes = value;
>   }
>   
> +static int sev_guest_get_accept_all_memory(Object *obj, Error **errp)
> +{
> +    SevGuestState *sev = SEV_GUEST(obj);
> +
> +    return sev->accept_all_memory;
> +}
> +
> +static void
> +sev_guest_set_accept_all_memory(Object *obj, int value, Error **errp)
> +{
> +    SevGuestState *sev = SEV_GUEST(obj);
> +
> +    sev->accept_all_memory = value;
> +}
> +
>   static void
>   sev_guest_class_init(ObjectClass *oc, void *data)
>   {
> @@ -376,6 +401,14 @@ sev_guest_class_init(ObjectClass *oc, void *data)
>                                      sev_guest_set_kernel_hashes);
>       object_class_property_set_description(oc, "kernel-hashes",
>               "add kernel hashes to guest firmware for measured Linux boot");
> +    object_class_property_add_enum(oc, "accept-all-memory",
> +                                   "MemoryAcceptance",
> +                                   &memory_acceptance_lookup,
> +        sev_guest_get_accept_all_memory, sev_guest_set_accept_all_memory);
> +    object_class_property_set_description(
> +        oc, "accept-all-memory",
> +        "false: Accept all memory, true: Accept up to 4G and leave the rest 
> unaccepted (UEFI"
> +        " v2.9 memory type), default: default firmware behavior.");
>   }
>   
>   static void
> @@ -906,6 +939,22 @@ sev_vm_state_change(void *opaque, bool running, RunState 
> state)
>       }
>   }
>   
> +int sev_has_accept_all_memory(ConfidentialGuestSupport *cgs)
> +{
> +    SevGuestState *sev
> +        = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
> +
> +    return sev && sev->accept_all_memory != 0;
> +}
> +
> +int sev_accept_all_memory(ConfidentialGuestSupport *cgs)
> +{
> +    SevGuestState *sev
> +        = (SevGuestState *)object_dynamic_cast(OBJECT(cgs), TYPE_SEV_GUEST);
> +
> +    return sev && sev->accept_all_memory == 1;
> +}
> +
>   int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>   {
>       SevGuestState *sev
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index 7b1528248a..d61b6e9443 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -58,5 +58,7 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t 
> flash_size);
>   void sev_es_set_reset_vector(CPUState *cpu);
>   
>   int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp);
> +int sev_has_accept_all_memory(ConfidentialGuestSupport *cgs);
> +int sev_accept_all_memory(ConfidentialGuestSupport *cgs);
>   
>   #endif