| Message ID | 20220629193701.734154-1-dionnaglaze@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] target/i386: Add unaccepted memory configuration | expand |
> 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
On Wed, Jun 29, 2022 at 07:37:01PM +0000, Dionna Glaze wrote: > 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. This talks about something supported for SEV-SNP, but.... > 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."); > } ..this is adding a property to the 'sev-guest' object, which only targets SEV/SEV-ES currently AFAIK. The most recent patches I recall for SEV-SNP introduced a new 'sev-snp-guest' object instead of overloading the existing 'sev-guest' object: https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04757.html With regards, Daniel
On 6/30/22 03:14, Daniel P. Berrangé wrote: > On Wed, Jun 29, 2022 at 07:37:01PM +0000, Dionna Glaze wrote: >> 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. > > This talks about something supported for SEV-SNP, but.... > >> 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."); >> } > > ..this is adding a property to the 'sev-guest' object, which only > targets SEV/SEV-ES currently AFAIK. > > The most recent patches I recall for SEV-SNP introduced a new > 'sev-snp-guest' object instead of overloading the existing > 'sev-guest' object: > > https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04757.html > Correct, the SNP support for Qemu is only RFC at this point until the KVM support for SNP is (near) finalized. Thanks, Tom > > > With regards, > Daniel
> > The most recent patches I recall for SEV-SNP introduced a new > > 'sev-snp-guest' object instead of overloading the existing > > 'sev-guest' object: > > > > https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg04757.html > > > > Correct, the SNP support for Qemu is only RFC at this point until the KVM > support for SNP is (near) finalized. > Ah okay, should I wait until that RFC patch set is merged to propose an extension to it, or should I coordinate with y'all at AMD to include this in your patch set? Apologies Pankaj, I forgot the change log (still new to git send-email). The change is that the configuration option is no longer in MachineState, but part of SevGuestState, with accessor functions for fw_cfg.c to know if it needs to add the fw_cfg file and what its value should be. That was the main feedback on v1.
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
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> --- hw/i386/fw_cfg.c | 6 ++++++ target/i386/sev.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ target/i386/sev.h | 2 ++ 3 files changed, 57 insertions(+)