1. Patchew
  2. QEMU
  3. View series

[PATCH 0/2] SEV firmware error list touchups

Connor Kuehl posted 2 patches 3 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210218151633.215374-1-ckuehl@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <ehabkost@redhat.com>
There is a newer version of this series
target/i386/sev.c | 48 ++++++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 23 deletions(-)
[PATCH 0/2] SEV firmware error list touchups
Posted by Connor Kuehl 3 years, 3 months ago 
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
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago 
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?
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Connor Kuehl 3 years, 3 months ago 
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
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Philippe Mathieu-Daudé 3 years, 3 months ago 
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.
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Connor Kuehl 3 years, 2 months ago 
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
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Connor Kuehl 3 years, 2 months ago 
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
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Philippe Mathieu-Daudé 3 years, 1 month ago 
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.
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Connor Kuehl 3 years, 1 month ago 
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
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Eduardo Habkost 3 years, 1 month ago 
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
Re: [PATCH 0/2] SEV firmware error list touchups
Posted by Philippe Mathieu-Daudé 3 years ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.