[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>
  • Date: Mon, 23 Sep 2019 13:52:53 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=bitdefender.com; dmarc=pass action=none header.from=bitdefender.com; dkim=pass header.d=bitdefender.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=8q+8RTFhgrDaT7GapWBzD9QmJ9JPwfYKOJ1ECUvW9Pg=; b=UfhvkfBgj6oWjbGdKxnzbFL3DiCNXqU8uFsakYul5fPH+1/1f/smZohg9oJa+CxKCht9h6ujpya+1iv3mtPE/tw4DSf2Ruoo2sHWYytVWHLPBfY0Mn4UE/5SSPaMat7z0AwvWereWvodiRSk4Ql/t4peQLaVFMAAMJqgQsgkGU9qW2ivz7gbnpWC7SWpjpTMj951uQ3cdLbk+YeKI0OyRz3359TKsIMKHPqVCIZUhn6KtmlzqyZR44kEaWw6Iv5+yVFq4TW0I5taxXFFSjdj5OdNNHtQkqhS3ymq/W9N780pFEW5uMoVDzstAGAAmit3kgUQOi4afpmoN1J62OuhRg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Toppn3S3lTQ+Mnqxm7YpybydrJRzRt+ld6fHRELDvZdUAnJg9nYN8Q8bB/7cnoZyR0oUrevxg1D17Y8MWXu45dv1+IuY6/rTw7iyhe3hJfCyk2rVFEK+XfttGOuGRe45pKnaMIF8ZZc7fYerUIUATUgZBW9fcP7EjMtOiqfiFAGGn8/NYY/On23iXUcx/+AK6Ggb/AcnqP7qoRfHDL6EUAQde/0dU+wft+ecBV3G+resv8Sd5ZXPYmtKx69ii+ZPIXJzTeDvDvrjqjgaJV425Uhybe2/gJUvGJDAHxfw8+XLpy+Nn4GEvJ+NNLjssCraLPr8SE8+Vu8vOOmyZVIMNQ==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=aisaila@xxxxxxxxxxxxxxx;
  • Cc: Petre Ovidiu PIRCALABU <ppircalabu@xxxxxxxxxxxxxxx>, "tamas@xxxxxxxxxxxxx" <tamas@xxxxxxxxxxxxx>, "wl@xxxxxxx" <wl@xxxxxxx>, Razvan COJOCARU <rcojocaru@xxxxxxxxxxxxxxx>, "george.dunlap@xxxxxxxxxxxxx" <george.dunlap@xxxxxxxxxxxxx>, "andrew.cooper3@xxxxxxxxxx" <andrew.cooper3@xxxxxxxxxx>, "paul.durrant@xxxxxxxxxx" <paul.durrant@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "roger.pau@xxxxxxxxxx" <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 23 Sep 2019 13:53:08 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHVcgc/RjZRGBRqJUWCjIFeFraK9Kc5RdcAgAACfIA=
  • Thread-topic: [Xen-devel] [PATCH v13] x86/emulate: Send vm_event from emulate


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 <jbeulich@xxxxxxxx>
> 

I agree, thanks for the review and the help with this patch

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.