xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: aisaila@bitdefender.com
Cc: Tamas K Lengyel <tamas@tklengyel.com>,
	Wei Liu <wei.liu2@citrix.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v4 2/2] x86/emulate: Send vm_event from emulate
Date: Fri, 31 May 2019 03:16:27 -0600	[thread overview]
Message-ID: <5CF0F0EB0200007800233D64@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <b6f3254b-2bce-ecd6-cddb-10131f075fb6@bitdefender.com>

>>> On 30.05.19 at 10:59, <aisaila@bitdefender.com> wrote:
>>  
>>> +        return false;
>>> +
>>> +    rc = hvmemul_linear_to_phys(gla, &gpa, bytes, &reps, pfec, &ctxt);
>> 
>> As said before - I don't think it's a good idea to do the page walk
>> twice: This and the pre-existing one can easily return different
>> results.
> 
> What preexisting page walk are you talking about here?

Well, I'm afraid I don't know what to say (in a polite way). I'm sure
you're understanding the code you try to add to, so it would seem
natural to me that you can answer this question all by yourself:
Since you don't remove any linear->phys translations in your patch,
and since there necessarily is one prior to your patch, you adding
(another) one means there are now two of them.

So to answer your question directly: hvmemul_map_linear_addr()
calls hvm_translate_get_page(), and hvmemul_insn_fetch() ->
__hvmemul_read() -> linear_read() -> hvm_copy_from_guest_linear()
-> __hvm_copy() calls hvm_translate_get_page() as well.

As an aside - while I'm advocating the stripping of reply quoting that's
not needed as context, you surely went too far here: You've left in
place neither an indication when the mails were sent context of which
is still present above, nor enough context to re-construct what
function your additions go into. IOW I had to search the earlier parts
of this thread to gather enough context to actually be able to reply.

> I don't think 
> there is a way to get the gpa by passing it from somewhere.

Possibly, but that's also not what I did suggest. Instead what I
think you need to look into is how to restructure things, perhaps
just as to the place where you insert your new code.

Jan



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

WARNING: multiple messages have this Message-ID (diff)
From: "Jan Beulich" <JBeulich@suse.com>
To: <aisaila@bitdefender.com>
Cc: Tamas K Lengyel <tamas@tklengyel.com>,
	Wei Liu <wei.liu2@citrix.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v4 2/2] x86/emulate: Send vm_event from emulate
Date: Fri, 31 May 2019 03:16:27 -0600	[thread overview]
Message-ID: <5CF0F0EB0200007800233D64@prv1-mh.provo.novell.com> (raw)
Message-ID: <20190531091627.Miyh-Y6SmU8MKCM-iSAjJeKC-y6vY4o7OEliuRLT3QM@z> (raw)
In-Reply-To: <b6f3254b-2bce-ecd6-cddb-10131f075fb6@bitdefender.com>

>>> On 30.05.19 at 10:59, <aisaila@bitdefender.com> wrote:
>>  
>>> +        return false;
>>> +
>>> +    rc = hvmemul_linear_to_phys(gla, &gpa, bytes, &reps, pfec, &ctxt);
>> 
>> As said before - I don't think it's a good idea to do the page walk
>> twice: This and the pre-existing one can easily return different
>> results.
> 
> What preexisting page walk are you talking about here?

Well, I'm afraid I don't know what to say (in a polite way). I'm sure
you're understanding the code you try to add to, so it would seem
natural to me that you can answer this question all by yourself:
Since you don't remove any linear->phys translations in your patch,
and since there necessarily is one prior to your patch, you adding
(another) one means there are now two of them.

So to answer your question directly: hvmemul_map_linear_addr()
calls hvm_translate_get_page(), and hvmemul_insn_fetch() ->
__hvmemul_read() -> linear_read() -> hvm_copy_from_guest_linear()
-> __hvm_copy() calls hvm_translate_get_page() as well.

As an aside - while I'm advocating the stripping of reply quoting that's
not needed as context, you surely went too far here: You've left in
place neither an indication when the mails were sent context of which
is still present above, nor enough context to re-construct what
function your additions go into. IOW I had to search the earlier parts
of this thread to gather enough context to actually be able to reply.

> I don't think 
> there is a way to get the gpa by passing it from somewhere.

Possibly, but that's also not what I did suggest. Instead what I
think you need to look into is how to restructure things, perhaps
just as to the place where you insert your new code.

Jan



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

  parent reply	other threads:[~2019-05-31  9:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-20 12:55 [PATCH v4 1/2] x86/emulate: Move hvmemul_linear_to_phys Alexandru Stefan ISAILA
2019-05-20 12:55 ` [Xen-devel] " Alexandru Stefan ISAILA
2019-05-20 12:55 ` [PATCH v4 2/2] x86/emulate: Send vm_event from emulate Alexandru Stefan ISAILA
2019-05-20 12:55   ` [Xen-devel] " Alexandru Stefan ISAILA
2019-05-22  9:56   ` Jan Beulich
2019-05-22  9:56     ` [Xen-devel] " Jan Beulich
2019-05-22 12:59     ` Alexandru Stefan ISAILA
2019-05-22 12:59       ` [Xen-devel] " Alexandru Stefan ISAILA
2019-05-22 13:34       ` Jan Beulich
2019-05-22 13:34         ` [Xen-devel] " Jan Beulich
2019-05-22 13:50         ` Alexandru Stefan ISAILA
2019-05-22 13:50           ` [Xen-devel] " Alexandru Stefan ISAILA
2019-05-22 13:57           ` Jan Beulich
2019-05-22 13:57             ` [Xen-devel] " Jan Beulich
2019-05-30  8:59     ` Alexandru Stefan ISAILA
2019-05-30  8:59       ` [Xen-devel] " Alexandru Stefan ISAILA
2019-05-31  9:16       ` Jan Beulich [this message]
2019-05-31  9:16         ` Jan Beulich
2019-05-22 13:13 ` [PATCH v4 1/2] x86/emulate: Move hvmemul_linear_to_phys Paul Durrant
2019-05-22 13:13   ` [Xen-devel] " Paul Durrant
2019-05-22 13:55   ` George Dunlap
2019-05-22 13:55     ` [Xen-devel] " George Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5CF0F0EB0200007800233D64@prv1-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=aisaila@bitdefender.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=paul.durrant@citrix.com \
    --cc=rcojocaru@bitdefender.com \
    --cc=roger.pau@citrix.com \
    --cc=tamas@tklengyel.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).