| Message ID | 20211002125317.3418648-6-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/i386/sev: Housekeeping SEV + measured Linux SEV guest | expand |
On 02/10/21 14:53, Philippe Mathieu-Daudé wrote: > If the management layer tries to inject a secret, it gets an empty > response in case the binary built without SEV: > > { "execute": "sev-inject-launch-secret", > "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 } > } > { > "return": { > } > } > > Make it clearer by returning an error, mentioning the feature is > disabled: > > { "execute": "sev-inject-launch-secret", > "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 } > } > { > "error": { > "class": "GenericError", > "desc": "this feature or command is not currently supported" > } > } > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Reviewed-by: Connor Kuehl <ckuehl@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > target/i386/monitor.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/target/i386/monitor.c b/target/i386/monitor.c > index 196c1c9e77f..a9f85acd473 100644 > --- a/target/i386/monitor.c > +++ b/target/i386/monitor.c > @@ -28,6 +28,7 @@ > #include "monitor/hmp-target.h" > #include "monitor/hmp.h" > #include "qapi/qmp/qdict.h" > +#include "qapi/qmp/qerror.h" > #include "sysemu/kvm.h" > #include "sysemu/sev.h" > #include "qapi/error.h" > @@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr, > bool has_gpa, uint64_t gpa, > Error **errp) > { > + if (!sev_enabled()) { > + error_setg(errp, QERR_UNSUPPORTED); > + return; > + } > if (!has_gpa) { > uint8_t *data; > struct sev_secret_area *area; > This should be done in the sev_inject_launch_secret stub instead, I think. Or if you do it here, you can remove the "if (!sev_guest)" conditional in the non-stub version. Paolo
On 10/4/21 10:11, Paolo Bonzini wrote: > On 02/10/21 14:53, Philippe Mathieu-Daudé wrote: >> If the management layer tries to inject a secret, it gets an empty >> response in case the binary built without SEV: >> >> { "execute": "sev-inject-launch-secret", >> "arguments": { "packet-header": "mypkt", "secret": "mypass", >> "gpa": 4294959104 } >> } >> { >> "return": { >> } >> } >> >> Make it clearer by returning an error, mentioning the feature is >> disabled: >> >> { "execute": "sev-inject-launch-secret", >> "arguments": { "packet-header": "mypkt", "secret": "mypass", >> "gpa": 4294959104 } >> } >> { >> "error": { >> "class": "GenericError", >> "desc": "this feature or command is not currently supported" >> } >> } >> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Reviewed-by: Connor Kuehl <ckuehl@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> target/i386/monitor.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/target/i386/monitor.c b/target/i386/monitor.c >> index 196c1c9e77f..a9f85acd473 100644 >> --- a/target/i386/monitor.c >> +++ b/target/i386/monitor.c >> @@ -28,6 +28,7 @@ >> #include "monitor/hmp-target.h" >> #include "monitor/hmp.h" >> #include "qapi/qmp/qdict.h" >> +#include "qapi/qmp/qerror.h" >> #include "sysemu/kvm.h" >> #include "sysemu/sev.h" >> #include "qapi/error.h" >> @@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char >> *packet_hdr, >> bool has_gpa, uint64_t gpa, >> Error **errp) >> { >> + if (!sev_enabled()) { >> + error_setg(errp, QERR_UNSUPPORTED); >> + return; >> + } >> if (!has_gpa) { >> uint8_t *data; >> struct sev_secret_area *area; >> > > This should be done in the sev_inject_launch_secret stub instead, I > think. Or if you do it here, you can remove the "if (!sev_guest)" > conditional in the non-stub version. This part is not related to SEV builtin; what we want to avoid here is management layer to try to inject secret while the guest hasn't been started with SEV (IOW 'no memory encryption requested for KVM). Maybe this error message is more explicit? error_setg(errp, "Guest is not using memory encryption"); Or: error_setg(errp, "Guest is not using SEV");
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote: > On 10/4/21 10:11, Paolo Bonzini wrote: > > On 02/10/21 14:53, Philippe Mathieu-Daudé wrote: > >> If the management layer tries to inject a secret, it gets an empty > >> response in case the binary built without SEV: > >> > >> { "execute": "sev-inject-launch-secret", > >> "arguments": { "packet-header": "mypkt", "secret": "mypass", > >> "gpa": 4294959104 } > >> } > >> { > >> "return": { > >> } > >> } > >> > >> Make it clearer by returning an error, mentioning the feature is > >> disabled: > >> > >> { "execute": "sev-inject-launch-secret", > >> "arguments": { "packet-header": "mypkt", "secret": "mypass", > >> "gpa": 4294959104 } > >> } > >> { > >> "error": { > >> "class": "GenericError", > >> "desc": "this feature or command is not currently supported" > >> } > >> } > >> > >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >> Reviewed-by: Connor Kuehl <ckuehl@redhat.com> > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> --- > >> target/i386/monitor.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/target/i386/monitor.c b/target/i386/monitor.c > >> index 196c1c9e77f..a9f85acd473 100644 > >> --- a/target/i386/monitor.c > >> +++ b/target/i386/monitor.c > >> @@ -28,6 +28,7 @@ > >> #include "monitor/hmp-target.h" > >> #include "monitor/hmp.h" > >> #include "qapi/qmp/qdict.h" > >> +#include "qapi/qmp/qerror.h" > >> #include "sysemu/kvm.h" > >> #include "sysemu/sev.h" > >> #include "qapi/error.h" > >> @@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char > >> *packet_hdr, > >> bool has_gpa, uint64_t gpa, > >> Error **errp) > >> { > >> + if (!sev_enabled()) { > >> + error_setg(errp, QERR_UNSUPPORTED); > >> + return; > >> + } > >> if (!has_gpa) { > >> uint8_t *data; > >> struct sev_secret_area *area; > >> > > > > This should be done in the sev_inject_launch_secret stub instead, I > > think. Or if you do it here, you can remove the "if (!sev_guest)" > > conditional in the non-stub version. > > This part is not related to SEV builtin; what we want to avoid here > is management layer to try to inject secret while the guest hasn't > been started with SEV (IOW 'no memory encryption requested for KVM). > > Maybe this error message is more explicit? > > error_setg(errp, "Guest is not using memory encryption"); > > Or: > > error_setg(errp, "Guest is not using SEV"); This is better; there's a separate feature called memory encryption, so we don't want to confuse things. Dave >
diff --git a/target/i386/monitor.c b/target/i386/monitor.c index 196c1c9e77f..a9f85acd473 100644 --- a/target/i386/monitor.c +++ b/target/i386/monitor.c @@ -28,6 +28,7 @@ #include "monitor/hmp-target.h" #include "monitor/hmp.h" #include "qapi/qmp/qdict.h" +#include "qapi/qmp/qerror.h" #include "sysemu/kvm.h" #include "sysemu/sev.h" #include "qapi/error.h" @@ -743,6 +744,10 @@ void qmp_sev_inject_launch_secret(const char *packet_hdr, bool has_gpa, uint64_t gpa, Error **errp) { + if (!sev_enabled()) { + error_setg(errp, QERR_UNSUPPORTED); + return; + } if (!has_gpa) { uint8_t *data; struct sev_secret_area *area;
If the management layer tries to inject a secret, it gets an empty response in case the binary built without SEV: { "execute": "sev-inject-launch-secret", "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 } } { "return": { } } Make it clearer by returning an error, mentioning the feature is disabled: { "execute": "sev-inject-launch-secret", "arguments": { "packet-header": "mypkt", "secret": "mypass", "gpa": 4294959104 } } { "error": { "class": "GenericError", "desc": "this feature or command is not currently supported" } } Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Connor Kuehl <ckuehl@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- target/i386/monitor.c | 5 +++++ 1 file changed, 5 insertions(+)