On Sat, Jan 6, 2024 at 12:43 AM Alex Bennée <alex.ben...@linaro.org> wrote:
> Hyman Huang <yong.hu...@smartx.com> writes: > > > The incorrect error message was produced as a result of > > the return number being disregarded on the sev_kvm_init > > failure path. > > > > For instance, when a user's failure to launch a SEV guest > > is caused by an incorrect IOCTL, the following message is > > reported: > > > > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 > > kvm: failed to initialize kvm: Operation not permitted > > > > While the error message's accurate output should be: > > > > kvm: sev_kvm_init: failed to initialize ret=-25 fw_error=0 > > kvm: failed to initialize kvm: Inappropriate ioctl for device > > > > Fix this by returning the return number directly on the > > failure path. > > > > Signed-off-by: Hyman Huang <yong.hu...@smartx.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 9a71246682..4a69ca457c 100644 > > --- a/target/i386/sev.c > > +++ b/target/i386/sev.c > > @@ -1019,7 +1019,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, > Error **errp) > > err: > > sev_guest = NULL; > > ram_block_discard_disable(false); > > - return -1; > > + return ret; > > I don't think this will be correct in all cases because there are other > legs (e.g. if (host_cbitpos != sev->cbitpos)) where ret may be the > successful setting of ram_block_discard_disable(true). > Indeed, the other failed paths are overlooked by me. I'll try a commit aiming to sort the error message on all failure paths in the next version. > You might want to explore if the function can be re-written with > explicit return's and utilise autofree to do the clean-up of dynamic > objects. > > I think this entails setting sev_guest at the end of the function just > before the return 0. > > I'm not sure if there is a clean way to handle > ram_block_discard_disable(false); cleanly for all the failure legs > though. Maybe someone with more familiarity with the code has some ideas? > > > > } > > > > int > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro > -- Best regards