On 23.09.2019 16:43, Jan Beulich wrote:
> 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 <jbeul...@suse.com>
>
I agree, thanks for the review and the help with this patch
Alex
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
