Re: [PATCH v4 25/31] i386/sev: Invoke launch_updata_data() for SEV class

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On Thu, May 30, 2024 at 1:17 PM Pankaj Gupta <pankaj.gupta@xxxxxxx> wrote:
>
> Add launch_update_data() in SevCommonStateClass and
> invoke as sev_launch_update_data() for SEV object.
>
> Signed-off-by: Pankaj Gupta <pankaj.gupta@xxxxxxx>
> ---
>  target/i386/sev.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index c5c703bc8d..7a0c2ee10f 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -102,6 +102,7 @@ struct SevCommonStateClass {
>      /* public */
>      int (*launch_start)(SevCommonState *sev_common);
>      void (*launch_finish)(SevCommonState *sev_common);
> +    int (*launch_update_data)(hwaddr gpa, uint8_t *ptr, uint64_t len);

This should receive the SevCommonState, so that
sev_launch_update_data() does not have to grab it from the
MachineState.

Also,

> -        if (sev_snp_enabled()) {
> -            ret = snp_launch_update_data(gpa, ptr, len,
> -                                         KVM_SEV_SNP_PAGE_TYPE_NORMAL);
> -        } else {
> -            ret = sev_launch_update_data(SEV_GUEST(sev_common), ptr, len);
> -        }
> +        ret = klass->launch_update_data(gpa, ptr, len);

this patch should be placed earlier in the series, so that this change
is done before snp_launch_data() is introduced..

That is, the hunk should be just:

     /* if SEV is in update state then encrypt the data else do nothing */
     if (sev_check_state(sev_common, SEV_STATE_LAUNCH_UPDATE)) {
-        int ret = sev_launch_update_data(SEV_GUEST(sev_common), ptr, len);
+        int ret;
+
+        ret = klass->launch_update_data(SEV_GUEST(sev_common), gpa, ptr, len);
         if (ret < 0) {
             error_setg(errp, "SEV: Failed to encrypt pflash rom");
             return ret;

Paolo