[PATCH 0/2] i386/sev: Two trivial improvements to sev_get_capabilities()

[PATCH 0/2] i386/sev: Two trivial improvements to sev_get_capabilities()

[Michal Privoznik]

I've noticed that recent QEMU + libvirt (current HEADs, roughly) behave
a bit different than expected. The problem is in recent change to
'query-sev-capabilities' command (well, sev_get_capabilities() in fact)
which libvirt uses (see patch 2/2). The first one is trivial.

Michal Privoznik (2):
  i386/sev: Fix error message in sev_get_capabilities()
  i386/sev: Fallback to the default SEV device if none provided in
    sev_get_capabilities()

 target/i386/sev.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.44.2

[PATCH 1/2] i386/sev: Fix error message in sev_get_capabilities()

[Michal Privoznik]

When a custom path is provided to sev-guest object and opening
the path fails an error message is reported. But the error
message still mentions DEFAULT_SEV_DEVICE ("/dev/sev") instead of
the custom path.

Fixes: 16dcf200dc951c1cde3e5b442457db5f690b8cf0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 target/i386/sev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 30b83f1d77..8c350d42b0 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -595,7 +595,7 @@ static SevCapability *sev_get_capabilities(Error **errp)
     fd = open(sev_device, O_RDWR);
     if (fd < 0) {
         error_setg_errno(errp, errno, "SEV: Failed to open %s",
-                         DEFAULT_SEV_DEVICE);
+                         sev_device);
         g_free(sev_device);
         return NULL;
     }
-- 
2.44.2

Re: [PATCH 1/2] i386/sev: Fix error message in sev_get_capabilities()

[Philippe Mathieu-Daudé]

On 24/6/24 10:52, Michal Privoznik wrote:
> When a custom path is provided to sev-guest object and opening
> the path fails an error message is reported. But the error
> message still mentions DEFAULT_SEV_DEVICE ("/dev/sev") instead of
> the custom path.
> 
> Fixes: 16dcf200dc951c1cde3e5b442457db5f690b8cf0
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   target/i386/sev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 2/2] i386/sev: Fallback to the default SEV device if none provided in sev_get_capabilities()

[Michal Privoznik]

When management tools (e.g. libvirt) query QEMU capabilities,
they start QEMU with a minimalistic configuration and issue
various commands on monitor. One of the command issued is/might
be "query-sev-capabilities" to learn values like cbitpos or
reduced-phys-bits. But as of v9.0.0-1145-g16dcf200dc the monitor
command returns an error instead.

This creates a chicken-egg problem because in order to query
those aforementioned values QEMU needs to be started with a
'sev-guest' object. But to start QEMU with the values must be
known.

I think it's safe to assume that the default path ("/dev/sev")
provides the same data as user provided one. So fall back to it.

Fixes: 16dcf200dc951c1cde3e5b442457db5f690b8cf0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 target/i386/sev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 8c350d42b0..5bb1ceb3ad 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -585,13 +585,13 @@ static SevCapability *sev_get_capabilities(Error **errp)
     }
 
     sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
-    if (!sev_common) {
-        error_setg(errp, "SEV is not configured");
-        return NULL;
+    if (sev_common) {
+        sev_device = object_property_get_str(OBJECT(sev_common), "sev-device",
+                                             &error_abort);
+    } else {
+        sev_device = g_strdup(DEFAULT_SEV_DEVICE);
     }
 
-    sev_device = object_property_get_str(OBJECT(sev_common), "sev-device",
-                                         &error_abort);
     fd = open(sev_device, O_RDWR);
     if (fd < 0) {
         error_setg_errno(errp, errno, "SEV: Failed to open %s",
-- 
2.44.2

Re: [PATCH 0/2] i386/sev: Two trivial improvements to sev_get_capabilities()

[Michal Prívozník]

On 6/24/24 10:52, Michal Privoznik wrote:
> I've noticed that recent QEMU + libvirt (current HEADs, roughly) behave
> a bit different than expected. The problem is in recent change to
> 'query-sev-capabilities' command (well, sev_get_capabilities() in fact)
> which libvirt uses (see patch 2/2). The first one is trivial.
> 
> Michal Privoznik (2):
>   i386/sev: Fix error message in sev_get_capabilities()
>   i386/sev: Fallback to the default SEV device if none provided in
>     sev_get_capabilities()
> 
>  target/i386/sev.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

Polite ping. Patch 2/2 is not reviewed and it causes problems to libvirt.

Michal

Re: [PATCH 0/2] i386/sev: Two trivial improvements to sev_get_capabilities()

[Paolo Bonzini]

Queued, thanks.

Paolo