| Message ID | 20210218151633.215374-1-ckuehl@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | SEV firmware error list touchups | expand |
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?
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
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.
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
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
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.
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
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.
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