From: "Jan Beulich" <JBeulich@suse.com>
To: Yu Zhang <yu.c.zhang@linux.intel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Paul Durrant <paul.durrant@citrix.com>,
zhiyuan.lv@intel.com, xen-devel@lists.xen.org
Subject: Re: [PATCH v9 5/5] x86/ioreq server: Synchronously reset outstanding p2m_ioreq_server entries when an ioreq server unmaps.
Date: Wed, 22 Mar 2017 08:39:13 -0600 [thread overview]
Message-ID: <58D29AA102000078001464F7@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1490064773-26751-6-git-send-email-yu.c.zhang@linux.intel.com>
>>> On 21.03.17 at 03:52, <yu.c.zhang@linux.intel.com> wrote:
> --- a/xen/arch/x86/hvm/dm.c
> +++ b/xen/arch/x86/hvm/dm.c
> @@ -385,16 +385,51 @@ static int dm_op(domid_t domid,
>
> case XEN_DMOP_map_mem_type_to_ioreq_server:
> {
> - const struct xen_dm_op_map_mem_type_to_ioreq_server *data =
> + struct xen_dm_op_map_mem_type_to_ioreq_server *data =
> &op.u.map_mem_type_to_ioreq_server;
> + unsigned long first_gfn = data->opaque;
> + unsigned long last_gfn;
> +
> + const_op = false;
>
> rc = -EOPNOTSUPP;
> /* Only support for HAP enabled hvm. */
> if ( !hap_enabled(d) )
> break;
>
> - rc = hvm_map_mem_type_to_ioreq_server(d, data->id,
> - data->type, data->flags);
> + if ( first_gfn == 0 )
> + rc = hvm_map_mem_type_to_ioreq_server(d, data->id,
> + data->type, data->flags);
> + /*
> + * Iterate p2m table when an ioreq server unmaps from p2m_ioreq_server,
> + * and reset the remaining p2m_ioreq_server entries back to p2m_ram_rw.
> + */
> + if ( (first_gfn > 0) || (data->flags == 0 && rc == 0) )
Instead of putting the rc check on the right side, please do
if ( rc == 0 && (first_gfn > 0) || data->flags == 0) )
That'll require setting rc to zero in an else to the previous if(),
but that's needed anyway afaics in order to not return
-EOPNOTSUPP once no further continuation is necessary.
I further wonder why the if() here needs to look at first_gfn at
all - data->flags is supposed to remain at zero for continuations
(unless we have a misbehaving caller, in which case it'll harm
the guest only afaict). It seems to me, however, that this may
have been discussed once already, a long time ago. I'm sorry
for not remembering the outcome, if so.
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1038,6 +1038,35 @@ void p2m_change_type_range(struct domain *d,
> p2m_unlock(p2m);
> }
>
> +/* Synchronously modify the p2m type for a range of gfns from ot to nt. */
> +void p2m_finish_type_change(struct domain *d,
> + unsigned long first_gfn, unsigned long last_gfn,
I think we'd prefer new functions to properly use gfn_t.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-03-22 14:39 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-21 2:52 [PATCH v9 0/5] x86/ioreq server: Introduce HVMMEM_ioreq_server mem type Yu Zhang
2017-03-21 2:52 ` [PATCH v9 1/5] x86/ioreq server: Release the p2m lock after mmio is handled Yu Zhang
2017-03-29 13:39 ` George Dunlap
2017-03-29 13:50 ` Jan Beulich
2017-03-21 2:52 ` [PATCH v9 2/5] x86/ioreq server: Add DMOP to map guest ram with p2m_ioreq_server to an ioreq server Yu Zhang
2017-03-22 7:49 ` Tian, Kevin
2017-03-22 10:12 ` Yu Zhang
2017-03-24 9:26 ` Tian, Kevin
2017-03-24 12:34 ` Yu Zhang
2017-03-22 14:21 ` Jan Beulich
2017-03-23 3:23 ` Yu Zhang
2017-03-23 8:57 ` Jan Beulich
2017-03-24 9:05 ` Yu Zhang
2017-03-24 10:19 ` Jan Beulich
2017-03-24 12:35 ` Yu Zhang
2017-03-24 13:09 ` Jan Beulich
2017-03-21 2:52 ` [PATCH v9 3/5] x86/ioreq server: Handle read-modify-write cases for p2m_ioreq_server pages Yu Zhang
2017-03-22 14:22 ` Jan Beulich
2017-03-21 2:52 ` [PATCH v9 4/5] x86/ioreq server: Asynchronously reset outstanding p2m_ioreq_server entries Yu Zhang
2017-03-21 10:05 ` Paul Durrant
2017-03-22 8:10 ` Tian, Kevin
2017-03-22 10:12 ` Yu Zhang
2017-03-24 9:37 ` Tian, Kevin
2017-03-24 12:45 ` Yu Zhang
2017-03-22 14:29 ` Jan Beulich
2017-03-23 3:23 ` Yu Zhang
2017-03-23 9:00 ` Jan Beulich
2017-03-24 9:05 ` Yu Zhang
2017-03-24 10:37 ` Jan Beulich
2017-03-24 12:36 ` Yu Zhang
2017-03-21 2:52 ` [PATCH v9 5/5] x86/ioreq server: Synchronously reset outstanding p2m_ioreq_server entries when an ioreq server unmaps Yu Zhang
2017-03-21 10:00 ` Paul Durrant
2017-03-21 11:15 ` Yu Zhang
2017-03-21 13:49 ` Paul Durrant
2017-03-21 14:14 ` Yu Zhang
2017-03-22 8:28 ` Tian, Kevin
2017-03-22 8:54 ` Jan Beulich
2017-03-22 9:02 ` Tian, Kevin
2017-03-22 14:39 ` Jan Beulich [this message]
2017-03-23 3:23 ` Yu Zhang
2017-03-23 9:02 ` Jan Beulich
2017-03-24 9:05 ` Yu Zhang
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=58D29AA102000078001464F7@prv-mh.provo.novell.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=paul.durrant@citrix.com \
--cc=xen-devel@lists.xen.org \
--cc=yu.c.zhang@linux.intel.com \
--cc=zhiyuan.lv@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.