xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Arianna Avanzini <avanzini.arianna@gmail.com>
To: xen-devel@lists.xen.org
Cc: Ian.Campbell@eu.citrix.com, paolo.valente@unimore.it,
	keir@xen.org, stefano.stabellini@eu.citrix.com,
	Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com,
	tim@xen.org, julien.grall@citrix.com, etrudeau@broadcom.com,
	andrew.cooper3@citrix.com, JBeulich@suse.com,
	avanzini.arianna@gmail.com, viktor.kleinik@globallogic.com
Subject: [PATCH v7 02/10] arch/arm: add consistency check to REMOVE p2m changes
Date: Mon,  5 May 2014 17:54:06 +0200	[thread overview]
Message-ID: <1399305254-3695-3-git-send-email-avanzini.arianna@gmail.com> (raw)
In-Reply-To: <1399305254-3695-1-git-send-email-avanzini.arianna@gmail.com>

Currently, the REMOVE case of the switch in apply_p2m_changes()
does not perform any consistency check on the mapping to be removed.
More in detail, the code does not check if the guest address to be
unmapped is actually mapped to the machine address given as a
parameter.
This commit attempts to add the above-described consistency check
to the REMOVE path of apply_p2m_changes(). This is instrumental to
one of the following commits which implements the possibility to
trigger the removal of p2m ranges via the memory_mapping DOMCTL
for ARM.

Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>

---

    v7:
        - Silently ignore the fact that, when removing a mapping, the specified
          gfn is not mapped at all.
        - Remove spurious spacing change.

    v6:
        - Don't update "count" on REMOVE as it is only used inside the
          RELINQUISH case of the switch in apply_p2m_changes().
        - Return with an error if removal of a page fails instead of just
          skipping the page.

    v5:
        - Do not use a temporary variable to hold the machine address:
          use the "maddr" function parameter itself.
        - Increment the machine address also when first and second level
          mappings are not valid.
        - Get the actual machine frame number mapped to the guest frame
          number given as parameter to the function directly in the
          REMOVE case of the switch construct, as it might not be valid
          in other cases and its value might be uncorrectly used in the
          future.
        - Remove useless and/or harmful ASSERT; check however if the
          mapping is valid and skip the page if it is not.

    v4:
        - Remove useless and slow lookup and use already-available
          data from pte instead.
        - Correctly increment the local variable used to keep the
          machine address whose mapping is currently being removed.
        - Return with an error upon finding a mismatch between the
          actual machine address mapped to the guest address and
          the machine address passed as parameter, instead of just
          skipping the page.

---
 xen/arch/arm/p2m.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 403fd89..17b0635 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -406,12 +406,26 @@ static int apply_p2m_changes(struct domain *d,
                 {
                     pte = mfn_to_p2m_entry(maddr >> PAGE_SHIFT, mattr, t);
                     write_pte(&third[third_table_offset(addr)], pte);
-                    maddr += PAGE_SIZE;
                 }
                 break;
-            case RELINQUISH:
             case REMOVE:
                 {
+                    unsigned long mfn = pte.p2m.base;
+
+                    /*
+                     * Ensure that the guest address given as argument to
+                     * this function is actually mapped to the specified
+                     * machine address. maddr here is the machine address
+                     * given to the function, while mfn is the machine
+                     * frame number actually mapped to the guest address:
+                     * check if the two correspond.
+                     */
+                    if ( !pte.p2m.valid || maddr != pfn_to_paddr(mfn) )
+                        return -EINVAL;
+                }
+                /* fall through */
+            case RELINQUISH:
+                {
                     if ( !pte.p2m.valid )
                     {
                         count++;
@@ -450,6 +464,7 @@ static int apply_p2m_changes(struct domain *d,
 
         /* Got the next page */
         addr += PAGE_SIZE;
+        maddr += PAGE_SIZE;
     }
 
     if ( flush )
-- 
1.9.2

  parent reply	other threads:[~2014-05-05 15:54 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-05 15:54 [PATCH v7 00/10] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-05-05 15:54 ` [PATCH v7 01/10] arch/arm: domain build: let dom0 access I/O memory of mapped devices Arianna Avanzini
2014-05-05 15:54 ` Arianna Avanzini [this message]
2014-05-06 16:51   ` [PATCH v7 02/10] arch/arm: add consistency check to REMOVE p2m changes Julien Grall
2014-05-06 16:52     ` Julien Grall
2014-05-05 15:54 ` [PATCH v7 03/10] arch/arm: let map_mmio_regions() take pfn as parameters Arianna Avanzini
2014-05-05 15:54 ` [PATCH v7 04/10] arch/arm: let map_mmio_regions() use start and count Arianna Avanzini
2014-05-05 18:55   ` Julien Grall
2014-05-07 11:03   ` Ian Campbell
2014-05-19 13:47   ` Julien Grall
2014-05-05 15:54 ` [PATCH v7 05/10] arch/x86: check if mapping exists before memory_mapping removes it Arianna Avanzini
2014-05-06  8:25   ` Jan Beulich
2014-05-05 15:54 ` [PATCH v7 06/10] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
2014-05-06  8:35   ` Jan Beulich
2014-05-05 15:54 ` [PATCH v7 07/10] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
2014-05-06  8:40   ` Jan Beulich
2014-05-07 11:09     ` Ian Campbell
2014-05-10  0:26       ` Arianna Avanzini
2014-05-12  8:29         ` Jan Beulich
2014-05-07 11:10     ` Ian Campbell
2014-05-06 16:54   ` Julien Grall
2014-05-10  1:20     ` Arianna Avanzini
2014-05-10  9:03       ` Julien Grall
2014-05-05 15:54 ` [PATCH v7 08/10] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
2014-05-05 15:54 ` [PATCH v7 09/10] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
2014-05-06  8:44   ` Jan Beulich
2014-05-07 11:16   ` Ian Campbell
2014-05-05 15:54 ` [PATCH v7 10/10] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2014-05-06  9:06   ` Jan Beulich
2014-05-10  1:10     ` Arianna Avanzini
2014-05-12  8:35       ` Jan Beulich
2014-05-25 17:14     ` Julien Grall
2014-05-26  9:03       ` Jan Beulich
2014-05-26 10:14         ` Jan Beulich
2014-05-26 10:53           ` Julien Grall
2014-05-26 11:14             ` Jan Beulich
2014-05-26 11:24               ` Julien Grall
2014-05-26 11:37                 ` Jan Beulich
2014-05-26 11:42                   ` Julien Grall
2014-05-26 11:51                     ` Jan Beulich
2014-05-26 12:15                       ` Julien Grall
2014-05-26 13:22                         ` Jan Beulich
2014-05-26 14:26                           ` Julien Grall
2014-05-26 15:00                             ` Jan Beulich
2014-05-06  8:21 ` [PATCH v7 00/10] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Jan Beulich

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=1399305254-3695-3-git-send-email-avanzini.arianna@gmail.com \
    --to=avanzini.arianna@gmail.com \
    --cc=Ian.Campbell@eu.citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=etrudeau@broadcom.com \
    --cc=julien.grall@citrix.com \
    --cc=keir@xen.org \
    --cc=paolo.valente@unimore.it \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=viktor.kleinik@globallogic.com \
    --cc=xen-devel@lists.xen.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).