Message ID | 20211108134840.2757206-2-dovmurik@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | SEV: add kernel-hashes=on for measured -kernel launch | expand |
Dov Murik <dovmurik@linux.ibm.com> writes: > Introduce new boolean 'kernel-hashes' option on the sev-guest object. > It will be used to to decide whether to add the hashes of > kernel/initrd/cmdline to SEV guest memory when booting with -kernel. > The default value is 'off'. > > Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > --- > qapi/qom.json | 7 ++++++- > target/i386/sev.c | 20 ++++++++++++++++++++ > qemu-options.hx | 6 +++++- > 3 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/qapi/qom.json b/qapi/qom.json > index ccd1167808..4fd5d1716b 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -769,6 +769,10 @@ > # @reduced-phys-bits: number of bits in physical addresses that become > # unavailable when SEV is enabled > # > +# @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a > +# designated guest firmware page for measured boot > +# with -kernel (default: false) Missing: (since 7.0) > +# > # Since: 2.12 > ## > { 'struct': 'SevGuestProperties', > @@ -778,7 +782,8 @@ > '*policy': 'uint32', > '*handle': 'uint32', > '*cbitpos': 'uint32', > - 'reduced-phys-bits': 'uint32' } } > + 'reduced-phys-bits': 'uint32', > + '*kernel-hashes': 'bool' } } > > ## > # @ObjectType: [...]
On 08/11/2021 17:51, Markus Armbruster wrote: > Dov Murik <dovmurik@linux.ibm.com> writes: > >> Introduce new boolean 'kernel-hashes' option on the sev-guest object. >> It will be used to to decide whether to add the hashes of >> kernel/initrd/cmdline to SEV guest memory when booting with -kernel. >> The default value is 'off'. >> >> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> >> --- >> qapi/qom.json | 7 ++++++- >> target/i386/sev.c | 20 ++++++++++++++++++++ >> qemu-options.hx | 6 +++++- >> 3 files changed, 31 insertions(+), 2 deletions(-) >> >> diff --git a/qapi/qom.json b/qapi/qom.json >> index ccd1167808..4fd5d1716b 100644 >> --- a/qapi/qom.json >> +++ b/qapi/qom.json >> @@ -769,6 +769,10 @@ >> # @reduced-phys-bits: number of bits in physical addresses that become >> # unavailable when SEV is enabled >> # >> +# @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a >> +# designated guest firmware page for measured boot >> +# with -kernel (default: false) > > Missing: (since 7.0) > I agree the "since" clause is missing, but I think this series (at least patches 1-4) should be considered a bug fix (because booting with -kernel will break in 6.2 for older OVMF which doesn't have guest firmware area for hashes). I think it should be added for 6.2. Paolo? If agreed, the hunk should be: --- a/qapi/qom.json +++ b/qapi/qom.json @@ -769,6 +769,10 @@ # @reduced-phys-bits: number of bits in physical addresses that become # unavailable when SEV is enabled # +# @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a +# designated guest firmware page for measured boot +# with -kernel (default: false) (since 6.2) +# # Since: 2.12 ## { 'struct': 'SevGuestProperties', -Dov >> +# >> # Since: 2.12 >> ## >> { 'struct': 'SevGuestProperties', >> @@ -778,7 +782,8 @@ >> '*policy': 'uint32', >> '*handle': 'uint32', >> '*cbitpos': 'uint32', >> - 'reduced-phys-bits': 'uint32' } } >> + 'reduced-phys-bits': 'uint32', >> + '*kernel-hashes': 'bool' } } >> >> ## >> # @ObjectType: > > [...] >
On Mon, Nov 08, 2021 at 08:20:48PM +0200, Dov Murik wrote: > > > On 08/11/2021 17:51, Markus Armbruster wrote: > > Dov Murik <dovmurik@linux.ibm.com> writes: > > > >> Introduce new boolean 'kernel-hashes' option on the sev-guest object. > >> It will be used to to decide whether to add the hashes of > >> kernel/initrd/cmdline to SEV guest memory when booting with -kernel. > >> The default value is 'off'. > >> > >> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > >> --- > >> qapi/qom.json | 7 ++++++- > >> target/i386/sev.c | 20 ++++++++++++++++++++ > >> qemu-options.hx | 6 +++++- > >> 3 files changed, 31 insertions(+), 2 deletions(-) > >> > >> diff --git a/qapi/qom.json b/qapi/qom.json > >> index ccd1167808..4fd5d1716b 100644 > >> --- a/qapi/qom.json > >> +++ b/qapi/qom.json > >> @@ -769,6 +769,10 @@ > >> # @reduced-phys-bits: number of bits in physical addresses that become > >> # unavailable when SEV is enabled > >> # > >> +# @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a > >> +# designated guest firmware page for measured boot > >> +# with -kernel (default: false) > > > > Missing: (since 7.0) > > > > I agree the "since" clause is missing, but I think this series (at least > patches 1-4) should be considered a bug fix (because booting with > -kernel will break in 6.2 for older OVMF which doesn't have guest > firmware area for hashes). > > I think it should be added for 6.2. > > Paolo? > > > If agreed, the hunk should be: Yes, the kernel hashes feature was introduced in this 6.2 dev cycle, and this patch is fixing a significant behavioural problem with it. We need this included in the 6.2 release Regards, Daniel
On Mon, Nov 08, 2021 at 01:48:35PM +0000, Dov Murik wrote: > Introduce new boolean 'kernel-hashes' option on the sev-guest object. > It will be used to to decide whether to add the hashes of > kernel/initrd/cmdline to SEV guest memory when booting with -kernel. > The default value is 'off'. > > Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > --- > qapi/qom.json | 7 ++++++- > target/i386/sev.c | 20 ++++++++++++++++++++ > qemu-options.hx | 6 +++++- > 3 files changed, 31 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> (Assuming the version tag is added as discussed) Regards, Daniel
On 11/11/2021 11:26, Daniel P. Berrangé wrote: > On Mon, Nov 08, 2021 at 08:20:48PM +0200, Dov Murik wrote: >> >> >> On 08/11/2021 17:51, Markus Armbruster wrote: >>> Dov Murik <dovmurik@linux.ibm.com> writes: >>> >>>> Introduce new boolean 'kernel-hashes' option on the sev-guest object. >>>> It will be used to to decide whether to add the hashes of >>>> kernel/initrd/cmdline to SEV guest memory when booting with -kernel. >>>> The default value is 'off'. >>>> >>>> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> >>>> --- >>>> qapi/qom.json | 7 ++++++- >>>> target/i386/sev.c | 20 ++++++++++++++++++++ >>>> qemu-options.hx | 6 +++++- >>>> 3 files changed, 31 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/qapi/qom.json b/qapi/qom.json >>>> index ccd1167808..4fd5d1716b 100644 >>>> --- a/qapi/qom.json >>>> +++ b/qapi/qom.json >>>> @@ -769,6 +769,10 @@ >>>> # @reduced-phys-bits: number of bits in physical addresses that become >>>> # unavailable when SEV is enabled >>>> # >>>> +# @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a >>>> +# designated guest firmware page for measured boot >>>> +# with -kernel (default: false) >>> >>> Missing: (since 7.0) >>> >> >> I agree the "since" clause is missing, but I think this series (at least >> patches 1-4) should be considered a bug fix (because booting with >> -kernel will break in 6.2 for older OVMF which doesn't have guest >> firmware area for hashes). >> >> I think it should be added for 6.2. >> >> Paolo? >> >> >> If agreed, the hunk should be: > > Yes, the kernel hashes feature was introduced in this 6.2 dev > cycle, and this patch is fixing a significant behavioural > problem with it. We need this included in the 6.2 release > Thanks for reviewing. I'll shortly send a v3 with the minor doc/string changes suggested here (patches 1 and 3). -Dov
diff --git a/qapi/qom.json b/qapi/qom.json index ccd1167808..4fd5d1716b 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -769,6 +769,10 @@ # @reduced-phys-bits: number of bits in physical addresses that become # unavailable when SEV is enabled # +# @kernel-hashes: if true, add hashes of kernel/initrd/cmdline to a +# designated guest firmware page for measured boot +# with -kernel (default: false) +# # Since: 2.12 ## { 'struct': 'SevGuestProperties', @@ -778,7 +782,8 @@ '*policy': 'uint32', '*handle': 'uint32', '*cbitpos': 'uint32', - 'reduced-phys-bits': 'uint32' } } + 'reduced-phys-bits': 'uint32', + '*kernel-hashes': 'bool' } } ## # @ObjectType: diff --git a/target/i386/sev.c b/target/i386/sev.c index eede07f11d..cad32812f5 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -62,6 +62,7 @@ struct SevGuestState { char *session_file; uint32_t cbitpos; uint32_t reduced_phys_bits; + bool kernel_hashes; /* runtime state */ uint32_t handle; @@ -327,6 +328,20 @@ sev_guest_set_sev_device(Object *obj, const char *value, Error **errp) sev->sev_device = g_strdup(value); } +static bool sev_guest_get_kernel_hashes(Object *obj, Error **errp) +{ + SevGuestState *sev = SEV_GUEST(obj); + + return sev->kernel_hashes; +} + +static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp) +{ + SevGuestState *sev = SEV_GUEST(obj); + + sev->kernel_hashes = value; +} + static void sev_guest_class_init(ObjectClass *oc, void *data) { @@ -345,6 +360,11 @@ sev_guest_class_init(ObjectClass *oc, void *data) sev_guest_set_session_file); object_class_property_set_description(oc, "session-file", "guest owners session parameters (encoded with base64)"); + object_class_property_add_bool(oc, "kernel-hashes", + sev_guest_get_kernel_hashes, + sev_guest_set_kernel_hashes); + object_class_property_set_description(oc, "kernel-hashes", + "add kernel hashes to guest firmware for measured Linux boot"); } static void diff --git a/qemu-options.hx b/qemu-options.hx index f051536b63..f50fdc3e47 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -5189,7 +5189,7 @@ SRST -object secret,id=sec0,keyid=secmaster0,format=base64,\\ data=$SECRET,iv=$(<iv.b64) - ``-object sev-guest,id=id,cbitpos=cbitpos,reduced-phys-bits=val,[sev-device=string,policy=policy,handle=handle,dh-cert-file=file,session-file=file]`` + ``-object sev-guest,id=id,cbitpos=cbitpos,reduced-phys-bits=val,[sev-device=string,policy=policy,handle=handle,dh-cert-file=file,session-file=file,kernel-hashes=on|off]`` Create a Secure Encrypted Virtualization (SEV) guest object, which can be used to provide the guest memory encryption support on AMD processors. @@ -5229,6 +5229,10 @@ SRST session with the guest owner to negotiate keys used for attestation. The file must be encoded in base64. + The ``kernel-hashes`` adds the hashes of given kernel/initrd/ + cmdline to a designated guest firmware page for measured Linux + boot with -kernel. The default is off. + e.g to launch a SEV guest .. parsed-literal::
Introduce new boolean 'kernel-hashes' option on the sev-guest object. It will be used to to decide whether to add the hashes of kernel/initrd/cmdline to SEV guest memory when booting with -kernel. The default value is 'off'. Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> --- qapi/qom.json | 7 ++++++- target/i386/sev.c | 20 ++++++++++++++++++++ qemu-options.hx | 6 +++++- 3 files changed, 31 insertions(+), 2 deletions(-)