All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] SEV firmware error list touchups
@ 2021-02-18 15:16 Connor Kuehl
  2021-02-18 15:16 ` [PATCH 1/2] sev: use explicit indices for mapping firmware error codes to strings Connor Kuehl
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Connor Kuehl @ 2021-02-18 15:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

Connor Kuehl (2):
  sev: use explicit indices for mapping firmware error codes to strings
  sev: add missing firmware error conditions

 target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

-- 
2.29.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] sev: use explicit indices for mapping firmware error codes to strings
  2021-02-18 15:16 [PATCH 0/2] SEV firmware error list touchups Connor Kuehl
@ 2021-02-18 15:16 ` Connor Kuehl
  2021-02-18 15:16 ` [PATCH 2/2] sev: add missing firmware error conditions Connor Kuehl
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Connor Kuehl @ 2021-02-18 15:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

This can help lower any margin for error when making future additions to
the list, especially if they're made out of order.

While doing so, make capitalization of ASID consistent with its usage in
the SEV firmware spec (Asid -> ASID).

Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
---
 target/i386/sev.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 0f414df02f..0de690058e 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -88,29 +88,29 @@ static SevGuestState *sev_guest;
 static Error *sev_mig_blocker;
 
 static const char *const sev_fw_errlist[] = {
-    "",
-    "Platform state is invalid",
-    "Guest state is invalid",
-    "Platform configuration is invalid",
-    "Buffer too small",
-    "Platform is already owned",
-    "Certificate is invalid",
-    "Policy is not allowed",
-    "Guest is not active",
-    "Invalid address",
-    "Bad signature",
-    "Bad measurement",
-    "Asid is already owned",
-    "Invalid ASID",
-    "WBINVD is required",
-    "DF_FLUSH is required",
-    "Guest handle is invalid",
-    "Invalid command",
-    "Guest is active",
-    "Hardware error",
-    "Hardware unsafe",
-    "Feature not supported",
-    "Invalid parameter"
+    [SEV_RET_SUCCESS]                = "",
+    [SEV_RET_INVALID_PLATFORM_STATE] = "Platform state is invalid",
+    [SEV_RET_INVALID_GUEST_STATE]    = "Guest state is invalid",
+    [SEV_RET_INAVLID_CONFIG]         = "Platform configuration is invalid",
+    [SEV_RET_INVALID_LEN]            = "Buffer too small",
+    [SEV_RET_ALREADY_OWNED]          = "Platform is already owned",
+    [SEV_RET_INVALID_CERTIFICATE]    = "Certificate is invalid",
+    [SEV_RET_POLICY_FAILURE]         = "Policy is not allowed",
+    [SEV_RET_INACTIVE]               = "Guest is not active",
+    [SEV_RET_INVALID_ADDRESS]        = "Invalid address",
+    [SEV_RET_BAD_SIGNATURE]          = "Bad signature",
+    [SEV_RET_BAD_MEASUREMENT]        = "Bad measurement",
+    [SEV_RET_ASID_OWNED]             = "ASID is already owned",
+    [SEV_RET_INVALID_ASID]           = "Invalid ASID",
+    [SEV_RET_WBINVD_REQUIRED]        = "WBINVD is required",
+    [SEV_RET_DFFLUSH_REQUIRED]       = "DF_FLUSH is required",
+    [SEV_RET_INVALID_GUEST]          = "Guest handle is invalid",
+    [SEV_RET_INVALID_COMMAND]        = "Invalid command",
+    [SEV_RET_ACTIVE]                 = "Guest is active",
+    [SEV_RET_HWSEV_RET_PLATFORM]     = "Hardware error",
+    [SEV_RET_HWSEV_RET_UNSAFE]       = "Hardware unsafe",
+    [SEV_RET_UNSUPPORTED]            = "Feature not supported",
+    [SEV_RET_INVALID_PARAM]          = "Invalid parameter",
 };
 
 #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
-- 
2.29.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] sev: add missing firmware error conditions
  2021-02-18 15:16 [PATCH 0/2] SEV firmware error list touchups Connor Kuehl
  2021-02-18 15:16 ` [PATCH 1/2] sev: use explicit indices for mapping firmware error codes to strings Connor Kuehl
@ 2021-02-18 15:16 ` Connor Kuehl
  2021-02-18 15:48 ` [PATCH 0/2] SEV firmware error list touchups Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Connor Kuehl @ 2021-02-18 15:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

The SEV userspace header[1] exports a couple of other error conditions that
aren't listed in QEMU's SEV implementation, so let's just round out the
list.

[1] linux-headers/linux/psp-sev.h

Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
---
 target/i386/sev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 0de690058e..37690ae809 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -111,6 +111,8 @@ static const char *const sev_fw_errlist[] = {
     [SEV_RET_HWSEV_RET_UNSAFE]       = "Hardware unsafe",
     [SEV_RET_UNSUPPORTED]            = "Feature not supported",
     [SEV_RET_INVALID_PARAM]          = "Invalid parameter",
+    [SEV_RET_RESOURCE_LIMIT]         = "Required firmware resource depleted",
+    [SEV_RET_SECURE_DATA_INVALID]    = "Part-specific integrity check failure",
 };
 
 #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
-- 
2.29.2



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-02-18 15:16 [PATCH 0/2] SEV firmware error list touchups Connor Kuehl
  2021-02-18 15:16 ` [PATCH 1/2] sev: use explicit indices for mapping firmware error codes to strings Connor Kuehl
  2021-02-18 15:16 ` [PATCH 2/2] sev: add missing firmware error conditions Connor Kuehl
@ 2021-02-18 15:48 ` Philippe Mathieu-Daudé
  2021-02-19 14:46   ` Connor Kuehl
  2021-03-08 16:41 ` Connor Kuehl
  2021-03-15 14:08 ` Connor Kuehl
  4 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-18 15:48 UTC (permalink / raw)
  To: Connor Kuehl, qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

On 2/18/21 4:16 PM, Connor Kuehl wrote:
> Connor Kuehl (2):
>   sev: use explicit indices for mapping firmware error codes to strings
>   sev: add missing firmware error conditions
> 
>  target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 23 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


To avoid this problem in future (new error code added on the Linux
kernel side) would it be acceptable to add a 3rd patch as:

-- >8 --
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 0f414df02f3..e086d3198e8 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -155,9 +155,12 @@ sev_platform_ioctl(int fd, int cmd, void *data, int
*error)
 static const char *
 fw_error_to_str(int code)
 {
+    QEMU_BUILD_BUG_ON(SEV_RET_SECURE_DATA_INVALID + 1 == SEV_RET_MAX);
+
     if (code < 0 || code >= SEV_FW_MAX_ERROR) {
         return "unknown error";
     }
+    assert(sev_fw_errlist[code]);

     return sev_fw_errlist[code];
 }
---

which triggers a build error if scripts/update-linux-headers.sh
added another sev_ret_code entry?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-02-18 15:48 ` [PATCH 0/2] SEV firmware error list touchups Philippe Mathieu-Daudé
@ 2021-02-19 14:46   ` Connor Kuehl
  2021-02-19 17:57     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 12+ messages in thread
From: Connor Kuehl @ 2021-02-19 14:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

On 2/18/21 9:48 AM, Philippe Mathieu-Daudé wrote:
> On 2/18/21 4:16 PM, Connor Kuehl wrote:
>> Connor Kuehl (2):
>>    sev: use explicit indices for mapping firmware error codes to strings
>>    sev: add missing firmware error conditions
>>
>>   target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>>   1 file changed, 25 insertions(+), 23 deletions(-)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thank you! :-)

> To avoid this problem in future (new error code added on the Linux
> kernel side) would it be acceptable to add a 3rd patch as:
> 
> -- >8 --
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 0f414df02f3..e086d3198e8 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -155,9 +155,12 @@ sev_platform_ioctl(int fd, int cmd, void *data, int
> *error)
>   static const char *
>   fw_error_to_str(int code)
>   {
> +    QEMU_BUILD_BUG_ON(SEV_RET_SECURE_DATA_INVALID + 1 == SEV_RET_MAX);
> +
>       if (code < 0 || code >= SEV_FW_MAX_ERROR) {
>           return "unknown error";
>       }
> +    assert(sev_fw_errlist[code]);
> 
>       return sev_fw_errlist[code];
>   }
> ---
> 
> which triggers a build error if scripts/update-linux-headers.sh
> added another sev_ret_code entry?
> 

I like this a lot. Should I send a v2 of the series with a third patch 
like this or shall I wait to see if these patches get applied then send 
something like this as a follow up patch?

Thank you,

Connor



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-02-19 14:46   ` Connor Kuehl
@ 2021-02-19 17:57     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-19 17:57 UTC (permalink / raw)
  To: Connor Kuehl, qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

On 2/19/21 3:46 PM, Connor Kuehl wrote:
> On 2/18/21 9:48 AM, Philippe Mathieu-Daudé wrote:
>> On 2/18/21 4:16 PM, Connor Kuehl wrote:
>>> Connor Kuehl (2):
>>>    sev: use explicit indices for mapping firmware error codes to strings
>>>    sev: add missing firmware error conditions
>>>
>>>   target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>>>   1 file changed, 25 insertions(+), 23 deletions(-)
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Thank you! :-)
> 
>> To avoid this problem in future (new error code added on the Linux
>> kernel side) would it be acceptable to add a 3rd patch as:
>>
>> -- >8 --
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index 0f414df02f3..e086d3198e8 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -155,9 +155,12 @@ sev_platform_ioctl(int fd, int cmd, void *data, int
>> *error)
>>   static const char *
>>   fw_error_to_str(int code)
>>   {
>> +    QEMU_BUILD_BUG_ON(SEV_RET_SECURE_DATA_INVALID + 1 == SEV_RET_MAX);
>> +
>>       if (code < 0 || code >= SEV_FW_MAX_ERROR) {
>>           return "unknown error";
>>       }
>> +    assert(sev_fw_errlist[code]);
>>
>>       return sev_fw_errlist[code];
>>   }
>> ---
>>
>> which triggers a build error if scripts/update-linux-headers.sh
>> added another sev_ret_code entry?
>>
> 
> I like this a lot. Should I send a v2 of the series with a third patch
> like this or shall I wait to see if these patches get applied then send
> something like this as a follow up patch?

Since I've the patch locally I'll simply send it.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-02-18 15:16 [PATCH 0/2] SEV firmware error list touchups Connor Kuehl
                   ` (2 preceding siblings ...)
  2021-02-18 15:48 ` [PATCH 0/2] SEV firmware error list touchups Philippe Mathieu-Daudé
@ 2021-03-08 16:41 ` Connor Kuehl
  2021-03-15 14:08 ` Connor Kuehl
  4 siblings, 0 replies; 12+ messages in thread
From: Connor Kuehl @ 2021-03-08 16:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

On 2/18/21 9:16 AM, Connor Kuehl wrote:
> Connor Kuehl (2):
>    sev: use explicit indices for mapping firmware error codes to strings
>    sev: add missing firmware error conditions
> 
>   target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>   1 file changed, 25 insertions(+), 23 deletions(-)
> 

Ping. Also, I am not sure whose tree these are best suited for.

Connor



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-02-18 15:16 [PATCH 0/2] SEV firmware error list touchups Connor Kuehl
                   ` (3 preceding siblings ...)
  2021-03-08 16:41 ` Connor Kuehl
@ 2021-03-15 14:08 ` Connor Kuehl
  2021-03-22 10:18   ` Philippe Mathieu-Daudé
  4 siblings, 1 reply; 12+ messages in thread
From: Connor Kuehl @ 2021-03-15 14:08 UTC (permalink / raw)
  To: qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

On 2/18/21 9:16 AM, Connor Kuehl wrote:
> Connor Kuehl (2):
>    sev: use explicit indices for mapping firmware error codes to strings
>    sev: add missing firmware error conditions
> 
>   target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>   1 file changed, 25 insertions(+), 23 deletions(-)
> 

Eduardo, Paolo, Richard: ping



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-03-15 14:08 ` Connor Kuehl
@ 2021-03-22 10:18   ` Philippe Mathieu-Daudé
  2021-03-22 14:09     ` Connor Kuehl
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-03-22 10:18 UTC (permalink / raw)
  To: Connor Kuehl, qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

Hi Connor,

On 3/15/21 3:08 PM, Connor Kuehl wrote:
> On 2/18/21 9:16 AM, Connor Kuehl wrote:
>> Connor Kuehl (2):
>>    sev: use explicit indices for mapping firmware error codes to strings
>>    sev: add missing firmware error conditions
>>
>>   target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>>   1 file changed, 25 insertions(+), 23 deletions(-)
>>
> 
> Eduardo, Paolo, Richard: ping

Looks too late for 6.0 now.

Can you repost/ping after QEMU 6.0 is release?

Thanks,

Phil.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-03-22 10:18   ` Philippe Mathieu-Daudé
@ 2021-03-22 14:09     ` Connor Kuehl
  2021-03-22 16:43       ` Eduardo Habkost
  0 siblings, 1 reply; 12+ messages in thread
From: Connor Kuehl @ 2021-03-22 14:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: pbonzini, richard.henderson, ehabkost, brijesh.singh, thomas.lendacky

On 3/22/21 5:18 AM, Philippe Mathieu-Daudé wrote:
> Hi Connor,
> 
> On 3/15/21 3:08 PM, Connor Kuehl wrote:
>> On 2/18/21 9:16 AM, Connor Kuehl wrote:
>>> Connor Kuehl (2):
>>>     sev: use explicit indices for mapping firmware error codes to strings
>>>     sev: add missing firmware error conditions
>>>
>>>    target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
>>>    1 file changed, 25 insertions(+), 23 deletions(-)
>>>
>>
>> Eduardo, Paolo, Richard: ping
> 
> Looks too late for 6.0 now.
> 
> Can you repost/ping after QEMU 6.0 is release?

Sure.

Connor



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-03-22 14:09     ` Connor Kuehl
@ 2021-03-22 16:43       ` Eduardo Habkost
  2021-05-11  7:36         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 12+ messages in thread
From: Eduardo Habkost @ 2021-03-22 16:43 UTC (permalink / raw)
  To: Connor Kuehl
  Cc: thomas.lendacky, brijesh.singh, richard.henderson, qemu-devel,
	pbonzini, Philippe Mathieu-Daudé

On Mon, Mar 22, 2021 at 09:09:44AM -0500, Connor Kuehl wrote:
> On 3/22/21 5:18 AM, Philippe Mathieu-Daudé wrote:
> > Hi Connor,
> > 
> > On 3/15/21 3:08 PM, Connor Kuehl wrote:
> > > On 2/18/21 9:16 AM, Connor Kuehl wrote:
> > > > Connor Kuehl (2):
> > > >     sev: use explicit indices for mapping firmware error codes to strings
> > > >     sev: add missing firmware error conditions
> > > > 
> > > >    target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
> > > >    1 file changed, 25 insertions(+), 23 deletions(-)
> > > > 
> > > 
> > > Eduardo, Paolo, Richard: ping
> > 
> > Looks too late for 6.0 now.
> > 
> > Can you repost/ping after QEMU 6.0 is release?
> 
> Sure.

My apologies for not replying before and not reviewing or merging
this in time for 6.0.  I'm seriously behind on all my upstream
maintainer work.

-- 
Eduardo



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] SEV firmware error list touchups
  2021-03-22 16:43       ` Eduardo Habkost
@ 2021-05-11  7:36         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-11  7:36 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Tom Lendacky, Brijesh Singh, Connor Kuehl, Richard Henderson,
	QEMU Developers, Paolo Bonzini

On Mon, Mar 22, 2021 at 5:43 PM Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Mar 22, 2021 at 09:09:44AM -0500, Connor Kuehl wrote:
> > On 3/22/21 5:18 AM, Philippe Mathieu-Daudé wrote:
> > > On 3/15/21 3:08 PM, Connor Kuehl wrote:
> > > > On 2/18/21 9:16 AM, Connor Kuehl wrote:
> > > > > Connor Kuehl (2):
> > > > >     sev: use explicit indices for mapping firmware error codes to strings
> > > > >     sev: add missing firmware error conditions
> > > > >
> > > > >    target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
> > > > >    1 file changed, 25 insertions(+), 23 deletions(-)
> > > > >
> > > >
> > > > Eduardo, Paolo, Richard: ping
> > >
> > > Looks too late for 6.0 now.
> > >
> > > Can you repost/ping after QEMU 6.0 is release?
> >
> > Sure.
>
> My apologies for not replying before and not reviewing or merging
> this in time for 6.0.

FYI there is a RESEND:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg803017.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-05-11  7:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 15:16 [PATCH 0/2] SEV firmware error list touchups Connor Kuehl
2021-02-18 15:16 ` [PATCH 1/2] sev: use explicit indices for mapping firmware error codes to strings Connor Kuehl
2021-02-18 15:16 ` [PATCH 2/2] sev: add missing firmware error conditions Connor Kuehl
2021-02-18 15:48 ` [PATCH 0/2] SEV firmware error list touchups Philippe Mathieu-Daudé
2021-02-19 14:46   ` Connor Kuehl
2021-02-19 17:57     ` Philippe Mathieu-Daudé
2021-03-08 16:41 ` Connor Kuehl
2021-03-15 14:08 ` Connor Kuehl
2021-03-22 10:18   ` Philippe Mathieu-Daudé
2021-03-22 14:09     ` Connor Kuehl
2021-03-22 16:43       ` Eduardo Habkost
2021-05-11  7:36         ` Philippe Mathieu-Daudé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.