Re: [Xen-devel] [PATCH v13] x86/emulate: Send vm_event from emulate

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.09.2019 14:05, Alexandru Stefan ISAILA wrote:
> @@ -599,8 +600,15 @@ static void *hvmemul_map_linear_addr(
>              err = NULL;
>              goto out;
>  
> -        case HVMTRANS_gfn_paged_out:
> +        case HVMTRANS_need_retry:
> +            /*
> +             * hvm_translate_get_page() does not return HVMTRANS_need_retry.
> +             * It can dropped if future work requires this.
> +             */

To me, "it" in this comment can only refer to something mentioned in
the prior sentence. But to be honest I'd drop the 2nd sentence
altogether, adding "currently" to the 1st one. (Same further down
then.)

> +            ASSERT_UNREACHABLE();
> +            /* fall through */
>          case HVMTRANS_gfn_shared:
> +        case HVMTRANS_gfn_paged_out:
>              err = ERR_PTR(~X86EMUL_RETRY);
>              goto out;

It also escapes me why you felt like moving the
"case HVMTRANS_gfn_paged_out:" line.

> @@ -1852,19 +1870,27 @@ static int hvmemul_rep_movs(
>  
>      xfree(buf);
>  
> -    if ( rc == HVMTRANS_gfn_paged_out )
> -        return X86EMUL_RETRY;
> -    if ( rc == HVMTRANS_gfn_shared )
> -        return X86EMUL_RETRY;
> -    if ( rc != HVMTRANS_okay )
> +    switch ( rc )
>      {
> -        gdprintk(XENLOG_WARNING, "Failed memory-to-memory REP MOVS: sgpa=%"
> -                 PRIpaddr" dgpa=%"PRIpaddr" reps=%lu bytes_per_rep=%u\n",
> -                 sgpa, dgpa, *reps, bytes_per_rep);
> -        return X86EMUL_UNHANDLEABLE;
> +    case HVMTRANS_need_retry:
> +        /*
> +         * hvm_copy_to_guest_phys() does not return HVMTRANS_need_retry.
> +         * It can dropped if future work requires this.
> +         */

Unlike in its rep_stos counterpart, here the return value may come
from hvm_copy_from_guest_phys() or hvm_copy_to_guest_phys(), and I
think the comment should not say otherwise.

With these changes (which are of enough of a cosmetic nature that
they could probably be taken care of while committing, provided
you agree), non-monitor-specific parts
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel