* [PATCH v12 01/14] arch/arm: add consistency check to REMOVE p2m changes
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
` (13 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
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 adds the above-described consistency check to the REMOVE
path of apply_p2m_changes() and lets a warning be emitted when trying
to remove a non-existent mapping. 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>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v11:
- Handle also level 3 mappings by performing the check outside
the ( level < 3 ) branch.
- Use XENLOG_G_WARNING as a loglevel to allow for appropriate rate-
limiting of messages.
v10:
- Emit a warning and still unmap the mapping when failing to remove
a mapping.
- Correctly place the check for an unexpected mapping in the REMOVE
case of apply_one_level().
- Drop the check for non-present entries which is redundant.
- Print the domain id when emitting a warning message.
v9:
- Don't return with an error when failing to remove a mapping, but
simply keep unmapping.
- Don't force assignment to the flush variable, as it is already
set before the switch.
- Change warning message to be more appropriate and clear; use the
correct format for paddr_t and gdprintk(), which is more restricted
than regular printk()s.
- Adapt to rework of p2m-related functions for ARM.
v8:
- Re-add erroneously-removed increments to the maddr variable.
- When failing to remove a mapping, add previously-mapped PT entry,
unlock the p2m_lock and flush TLBs if necessary.
- Emit an error message when failing to remove a mapping.
- Remove tentative phrases from commit description.
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 | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 143199b..8f83d17 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -601,6 +601,7 @@ static int apply_one_level(struct domain *d,
{
/* Progress up to next boundary */
*addr = (*addr + level_size) & level_mask;
+ *maddr = (*maddr + level_size) & level_mask;
return P2M_ONE_PROGRESS_NOP;
}
@@ -632,12 +633,29 @@ static int apply_one_level(struct domain *d,
}
}
+ /*
+ * Ensure that the guest address addr currently being
+ * handled (that is in the range given as argument to
+ * this function) is actually mapped to the corresponding
+ * machine address in the specified range. maddr here is
+ * the machine address given to the function, while
+ * orig_pte.p2m.base is the machine frame number actually
+ * mapped to the guest address: check if the two correspond.
+ */
+ if ( op == REMOVE &&
+ pfn_to_paddr(orig_pte.p2m.base) != *maddr )
+ printk(XENLOG_G_WARNING
+ "p2m_remove dom%d: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected\n",
+ d->domain_id, *addr, pfn_to_paddr(orig_pte.p2m.base),
+ *maddr);
+
*flush = true;
memset(&pte, 0x00, sizeof(pte));
p2m_write_pte(entry, pte, flush_cache);
*addr += level_size;
+ *maddr += level_size;
p2m->stats.mappings[level]--;
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 02/14] arch/arm: unmap partially-mapped memory regions
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-09-01 17:53 ` Julien Grall
2014-08-30 16:29 ` [PATCH v12 03/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
` (12 subsequent siblings)
14 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This commit modifies the function apply_p2m_changes() so that it
destroys changes performed while mapping a memory region, if errors are
seen during the operation. The implemented behaviour includes destroying
only mappings created during the latest invocation of apply_p2m_changes().
This is useful to avoid that memory areas remain partially accessible
to guests.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v12:
- Unmap only the memory area actually affected by the current
invocation of apply_p2m_changes().
- Use the correct mattr instead of always using MATTR_DEV when
unmapping a partially-mapped memory region.
v11:
- Handle partially-mapped memory regions regardless of their being
I/O-memory regions or not.
v10:
- Recursively call apply_p2m_changes() on the whole I/O-memory range
when unmapping a partially-mapped I/O-memory region.
v9:
- Let apply_p2m_ranges() unwind its own progress instead of relying on
the caller to unmap partially-mapped I/O-memory regions.
- Adapt to rework of p2m-related functions for ARM.
---
xen/arch/arm/p2m.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8f83d17..96d7dd8 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -440,6 +440,14 @@ static bool_t is_mapping_aligned(const paddr_t start_gpaddr,
#define P2M_ONE_PROGRESS_NOP 0x1
#define P2M_ONE_PROGRESS 0x10
+/* Helpers to lookup the properties of each level */
+const paddr_t level_sizes[] =
+ { ZEROETH_SIZE, FIRST_SIZE, SECOND_SIZE, THIRD_SIZE };
+const paddr_t level_masks[] =
+ { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
+const paddr_t level_shifts[] =
+ { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
+
/*
* 0 == (P2M_ONE_DESCEND) continue to descend the tree
* +ve == (P2M_ONE_PROGRESS_*) handled at this level, continue, flush,
@@ -460,13 +468,6 @@ static int apply_one_level(struct domain *d,
int mattr,
p2m_type_t t)
{
- /* Helpers to lookup the properties of each level */
- const paddr_t level_sizes[] =
- { ZEROETH_SIZE, FIRST_SIZE, SECOND_SIZE, THIRD_SIZE };
- const paddr_t level_masks[] =
- { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
- const paddr_t level_shifts[] =
- { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
const paddr_t level_size = level_sizes[level];
const paddr_t level_mask = level_masks[level];
const paddr_t level_shift = level_shifts[level];
@@ -713,7 +714,8 @@ static int apply_p2m_changes(struct domain *d,
int rc, ret;
struct p2m_domain *p2m = &d->arch.p2m;
lpae_t *first = NULL, *second = NULL, *third = NULL;
- paddr_t addr;
+ paddr_t addr, orig_maddr = maddr;
+ unsigned int level = 0;
unsigned long cur_first_page = ~0,
cur_first_offset = ~0,
cur_second_offset = ~0;
@@ -769,8 +771,9 @@ static int apply_p2m_changes(struct domain *d,
* current hardware doesn't support super page mappings at
* level 0 anyway */
+ level = 1;
ret = apply_one_level(d, &first[first_table_offset(addr)],
- 1, flush_pt, op,
+ level, flush_pt, op,
start_gpaddr, end_gpaddr,
&addr, &maddr, &flush,
mattr, t);
@@ -790,8 +793,9 @@ static int apply_p2m_changes(struct domain *d,
}
/* else: second already valid */
+ level = 2;
ret = apply_one_level(d,&second[second_table_offset(addr)],
- 2, flush_pt, op,
+ level, flush_pt, op,
start_gpaddr, end_gpaddr,
&addr, &maddr, &flush,
mattr, t);
@@ -809,8 +813,9 @@ static int apply_p2m_changes(struct domain *d,
cur_second_offset = second_table_offset(addr);
}
+ level = 3;
ret = apply_one_level(d, &third[third_table_offset(addr)],
- 3, flush_pt, op,
+ level, flush_pt, op,
start_gpaddr, end_gpaddr,
&addr, &maddr, &flush,
mattr, t);
@@ -844,6 +849,20 @@ out:
if (third) unmap_domain_page(third);
if (second) unmap_domain_page(second);
if (first) unmap_domain_page(first);
+ if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) &&
+ addr != start_gpaddr )
+ {
+ BUG_ON(addr == end_gpaddr);
+ /*
+ * addr keeps the address of the last successfully-inserted mapping,
+ * while apply_p2m_changes() considers an address range which is
+ * exclusive of end_gpaddr: add level_size to addr to obtain the
+ * right end of the range
+ */
+ apply_p2m_changes(d, REMOVE,
+ start_gpaddr, addr + level_sizes[level], orig_maddr,
+ mattr, p2m_invalid);
+ }
spin_unlock(&p2m->lock);
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v12 02/14] arch/arm: unmap partially-mapped memory regions
2014-08-30 16:29 ` [PATCH v12 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
@ 2014-09-01 17:53 ` Julien Grall
2014-09-01 20:13 ` Arianna Avanzini
2014-09-01 23:47 ` [PATCH] " Arianna Avanzini
0 siblings, 2 replies; 41+ messages in thread
From: Julien Grall @ 2014-09-01 17:53 UTC (permalink / raw)
To: Arianna Avanzini, xen-devel
Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
dario.faggioli, Ian.Jackson, Ian.Campbell, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
Hi Arianna,
On 30/08/14 12:29, Arianna Avanzini wrote:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 8f83d17..96d7dd8 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -440,6 +440,14 @@ static bool_t is_mapping_aligned(const paddr_t start_gpaddr,
> #define P2M_ONE_PROGRESS_NOP 0x1
> #define P2M_ONE_PROGRESS 0x10
>
> +/* Helpers to lookup the properties of each level */
> +const paddr_t level_sizes[] =
> + { ZEROETH_SIZE, FIRST_SIZE, SECOND_SIZE, THIRD_SIZE };
> +const paddr_t level_masks[] =
> + { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
> +const paddr_t level_shifts[] =
> + { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
> +
If you made those variables global, you should add static.
It doesn't seem that the other patch of this series depends on this
patch. Assuming there is no other modifications in series, I think you
can resend it stand-alone.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v12 02/14] arch/arm: unmap partially-mapped memory regions
2014-09-01 17:53 ` Julien Grall
@ 2014-09-01 20:13 ` Arianna Avanzini
2014-09-01 23:47 ` [PATCH] " Arianna Avanzini
1 sibling, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-01 20:13 UTC (permalink / raw)
To: Julien Grall
Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
dario.faggioli, Ian.Jackson, xen-devel, Ian.Campbell, etrudeau,
JBeulich, andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Mon, Sep 01, 2014 at 01:53:53PM -0400, Julien Grall wrote:
> Hi Arianna,
Hello!
>
> On 30/08/14 12:29, Arianna Avanzini wrote:
> >diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> >index 8f83d17..96d7dd8 100644
> >--- a/xen/arch/arm/p2m.c
> >+++ b/xen/arch/arm/p2m.c
> >@@ -440,6 +440,14 @@ static bool_t is_mapping_aligned(const paddr_t start_gpaddr,
> > #define P2M_ONE_PROGRESS_NOP 0x1
> > #define P2M_ONE_PROGRESS 0x10
> >
> >+/* Helpers to lookup the properties of each level */
> >+const paddr_t level_sizes[] =
> >+ { ZEROETH_SIZE, FIRST_SIZE, SECOND_SIZE, THIRD_SIZE };
> >+const paddr_t level_masks[] =
> >+ { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
> >+const paddr_t level_shifts[] =
> >+ { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
> >+
>
> If you made those variables global, you should add static.
Right, thank you for pointing that out.
>
> It doesn't seem that the other patch of this series depends on this patch.
> Assuming there is no other modifications in series, I think you can resend
> it stand-alone.
I will, thank you for the suggestion. I will add a changelog with respect
to the v12 version. In case other comments arise before I repost I will
obviously resend all affected patches.
Thank you,
Arianna
>
> Regards,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH] arch/arm: unmap partially-mapped memory regions
2014-09-01 17:53 ` Julien Grall
2014-09-01 20:13 ` Arianna Avanzini
@ 2014-09-01 23:47 ` Arianna Avanzini
1 sibling, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-01 23:47 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This commit modifies the function apply_p2m_changes() so that it
destroys changes performed while mapping a memory region, if errors are
seen during the operation. The implemented behaviour includes destroying
only mappings created during the latest invocation of apply_p2m_changes().
This is useful to avoid that memory areas remain partially accessible
to guests.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
With respect to patch 0002 of the v12 memory_mapping series ([1]):
- Add static qualifier to constants that change scope in the
context of this patch, as suggested by Julien Grall.
Previous history of this patch within the patchset ([1]):
v12:
- Unmap only the memory area actually affected by the current
invocation of apply_p2m_changes().
- Use the correct mattr instead of always using MATTR_DEV when
unmapping a partially-mapped memory region.
v11:
- Handle partially-mapped memory regions regardless of their being
I/O-memory regions or not.
v10:
- Recursively call apply_p2m_changes() on the whole I/O-memory range
when unmapping a partially-mapped I/O-memory region.
v9:
- Let apply_p2m_ranges() unwind its own progress instead of relying on
the caller to unmap partially-mapped I/O-memory regions.
- Adapt to rework of p2m-related functions for ARM.
[1] http://markmail.org/message/yxuie76e7antewyb
---
xen/arch/arm/p2m.c | 41 ++++++++++++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8f83d17..ede839d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -440,6 +440,14 @@ static bool_t is_mapping_aligned(const paddr_t start_gpaddr,
#define P2M_ONE_PROGRESS_NOP 0x1
#define P2M_ONE_PROGRESS 0x10
+/* Helpers to lookup the properties of each level */
+static const paddr_t level_sizes[] =
+ { ZEROETH_SIZE, FIRST_SIZE, SECOND_SIZE, THIRD_SIZE };
+static const paddr_t level_masks[] =
+ { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
+static const paddr_t level_shifts[] =
+ { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
+
/*
* 0 == (P2M_ONE_DESCEND) continue to descend the tree
* +ve == (P2M_ONE_PROGRESS_*) handled at this level, continue, flush,
@@ -460,13 +468,6 @@ static int apply_one_level(struct domain *d,
int mattr,
p2m_type_t t)
{
- /* Helpers to lookup the properties of each level */
- const paddr_t level_sizes[] =
- { ZEROETH_SIZE, FIRST_SIZE, SECOND_SIZE, THIRD_SIZE };
- const paddr_t level_masks[] =
- { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
- const paddr_t level_shifts[] =
- { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
const paddr_t level_size = level_sizes[level];
const paddr_t level_mask = level_masks[level];
const paddr_t level_shift = level_shifts[level];
@@ -713,7 +714,8 @@ static int apply_p2m_changes(struct domain *d,
int rc, ret;
struct p2m_domain *p2m = &d->arch.p2m;
lpae_t *first = NULL, *second = NULL, *third = NULL;
- paddr_t addr;
+ paddr_t addr, orig_maddr = maddr;
+ unsigned int level = 0;
unsigned long cur_first_page = ~0,
cur_first_offset = ~0,
cur_second_offset = ~0;
@@ -769,8 +771,9 @@ static int apply_p2m_changes(struct domain *d,
* current hardware doesn't support super page mappings at
* level 0 anyway */
+ level = 1;
ret = apply_one_level(d, &first[first_table_offset(addr)],
- 1, flush_pt, op,
+ level, flush_pt, op,
start_gpaddr, end_gpaddr,
&addr, &maddr, &flush,
mattr, t);
@@ -790,8 +793,9 @@ static int apply_p2m_changes(struct domain *d,
}
/* else: second already valid */
+ level = 2;
ret = apply_one_level(d,&second[second_table_offset(addr)],
- 2, flush_pt, op,
+ level, flush_pt, op,
start_gpaddr, end_gpaddr,
&addr, &maddr, &flush,
mattr, t);
@@ -809,8 +813,9 @@ static int apply_p2m_changes(struct domain *d,
cur_second_offset = second_table_offset(addr);
}
+ level = 3;
ret = apply_one_level(d, &third[third_table_offset(addr)],
- 3, flush_pt, op,
+ level, flush_pt, op,
start_gpaddr, end_gpaddr,
&addr, &maddr, &flush,
mattr, t);
@@ -844,6 +849,20 @@ out:
if (third) unmap_domain_page(third);
if (second) unmap_domain_page(second);
if (first) unmap_domain_page(first);
+ if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) &&
+ addr != start_gpaddr )
+ {
+ BUG_ON(addr == end_gpaddr);
+ /*
+ * addr keeps the address of the last successfully-inserted mapping,
+ * while apply_p2m_changes() considers an address range which is
+ * exclusive of end_gpaddr: add level_size to addr to obtain the
+ * right end of the range
+ */
+ apply_p2m_changes(d, REMOVE,
+ start_gpaddr, addr + level_sizes[level], orig_maddr,
+ mattr, p2m_invalid);
+ }
spin_unlock(&p2m->lock);
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v12 03/14] arch/x86: warn if to-be-removed mapping does not exist
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 01/14] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 02/14] arch/arm: unmap partially-mapped memory regions Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 04/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
` (11 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
Currently, when a memory mapping is removed with the memory_mapping
DOMCTL, no check is performed on the existence of such a mapping.
This commit adds such a consistency check to the code performing the
unmap, emitting a warning message if the check fails.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v8:
- Fix commit description and subject to better fit the contents of
the patch.
- Remove tentative phrases from commit description.
v7:
- Do not fail with an error when attempting to remove a non-existent
mapping: just emit a warning.
---
xen/arch/x86/domctl.c | 4 ++--
xen/arch/x86/mm/p2m.c | 12 ++++++++----
xen/common/memory.c | 2 +-
xen/include/asm-x86/p2m.h | 2 +-
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index d1517c4..1ead690 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -679,7 +679,7 @@ long arch_do_domctl(
"memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n",
d->domain_id, gfn + i, mfn + i, ret);
while ( i-- )
- clear_mmio_p2m_entry(d, gfn + i);
+ clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
is_hardware_domain(current->domain) )
printk(XENLOG_ERR
@@ -699,7 +699,7 @@ long arch_do_domctl(
if ( paging_mode_translate(d) )
for ( i = 0; i < nr_mfns; i++ )
{
- ret = clear_mmio_p2m_entry(d, gfn + i);
+ ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
if ( ret )
tmp_rc = ret;
}
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index c2e89e1..2586a3c 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -859,10 +859,10 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
}
/* Returns: 0 for success, -errno for failure */
-int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn)
+int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
{
int rc = -EINVAL;
- mfn_t mfn;
+ mfn_t actual_mfn;
p2m_access_t a;
p2m_type_t t;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
@@ -871,15 +871,19 @@ int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn)
return -EIO;
gfn_lock(p2m, gfn, 0);
- mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
+ actual_mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
/* Do not use mfn_valid() here as it will usually fail for MMIO pages. */
- if ( (INVALID_MFN == mfn_x(mfn)) || (t != p2m_mmio_direct) )
+ if ( (INVALID_MFN == mfn_x(actual_mfn)) || (t != p2m_mmio_direct) )
{
gdprintk(XENLOG_ERR,
"gfn_to_mfn failed! gfn=%08lx type:%d\n", gfn, t);
goto out;
}
+ if ( mfn_x(mfn) != mfn_x(actual_mfn) )
+ gdprintk(XENLOG_WARNING,
+ "no mapping between mfn %08lx and gfn %08lx\n",
+ mfn_x(mfn), gfn);
rc = p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_4K, p2m_invalid,
p2m->default_access);
diff --git a/xen/common/memory.c b/xen/common/memory.c
index c2dd31b..2433111 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -206,7 +206,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
}
if ( p2mt == p2m_mmio_direct )
{
- clear_mmio_p2m_entry(d, gmfn);
+ clear_mmio_p2m_entry(d, gmfn, _mfn(mfn));
put_gfn(d, gmfn);
return 1;
}
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 3975e32..d19f50c 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -534,7 +534,7 @@ int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start,
/* Set mmio addresses in the p2m table (for pass-through) */
int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
-int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn);
+int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
/* Add foreign mapping to the guest's p2m table. */
int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 04/14] arch/x86: cleanup memory_mapping DOMCTL
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (2 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 03/14] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 05/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
` (10 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This commit lets the end mfn be computed only once while handling a
XEN_DOMCTL_memory_mapping hypercall. Also, the name of the tmp_rc
local variable is changed to rc.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
xen/arch/x86/domctl.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 1ead690..9cdbc3d 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -646,19 +646,20 @@ long arch_do_domctl(
unsigned long gfn = domctl->u.memory_mapping.first_gfn;
unsigned long mfn = domctl->u.memory_mapping.first_mfn;
unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
+ unsigned long mfn_end = mfn + nr_mfns - 1;
int add = domctl->u.memory_mapping.add_mapping;
ret = -EINVAL;
- if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
- ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
+ if ( mfn_end < mfn || /* wrap? */
+ ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
(gfn + nr_mfns - 1) < gfn ) /* wrap? */
break;
ret = -EPERM;
- if ( !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
+ if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
break;
- ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add);
+ ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
if ( ret )
break;
@@ -668,7 +669,7 @@ long arch_do_domctl(
"memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
+ ret = iomem_permit_access(d, mfn, mfn_end);
if ( !ret && paging_mode_translate(d) )
{
for ( i = 0; !ret && i < nr_mfns; i++ )
@@ -680,17 +681,17 @@ long arch_do_domctl(
d->domain_id, gfn + i, mfn + i, ret);
while ( i-- )
clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
- if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
+ if ( iomem_deny_access(d, mfn, mfn_end) &&
is_hardware_domain(current->domain) )
printk(XENLOG_ERR
"memory_map: failed to deny dom%d access to [%lx,%lx]\n",
- d->domain_id, mfn, mfn + nr_mfns - 1);
+ d->domain_id, mfn, mfn_end);
}
}
}
else
{
- int tmp_rc = 0;
+ int rc = 0;
printk(XENLOG_G_INFO
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
@@ -701,16 +702,16 @@ long arch_do_domctl(
{
ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
if ( ret )
- tmp_rc = ret;
+ rc = ret;
}
- ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+ ret = iomem_deny_access(d, mfn, mfn_end);
if ( !ret )
- ret = tmp_rc;
+ ret = rc;
if ( ret && is_hardware_domain(current->domain) )
printk(XENLOG_ERR
"memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
- ret, tmp_rc ? "removing" : "denying", d->domain_id,
- mfn, mfn + nr_mfns - 1);
+ ret, rc ? "removing" : "denying", d->domain_id,
+ mfn, mfn_end);
}
/* Do this unconditionally to cover errors on above failure paths. */
memory_type_changed(d);
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 05/14] xen/common: add ARM stub for the function memory_type_changed()
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (3 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 04/14] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
` (9 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
MTRR-related code is not available for the ARM architecture. Given
that the memory_type_changed() function would be called also from
common code, its invocation is currently ifdef'd out to be only
compiled in on an x86 machine. This commit adds an empty stub for ARM.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v9:
- Don't expose the memory_type_changed() function to common code,
just add an empty stub in arch/arm/p2m.c and in the related header
for ARM.
---
xen/arch/arm/p2m.c | 4 ++++
xen/common/domctl.c | 2 --
xen/include/asm-arm/p2m.h | 2 ++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 96d7dd8..3f0dc0d 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -47,6 +47,10 @@ void p2m_dump_info(struct domain *d)
spin_unlock(&p2m->lock);
}
+void memory_type_changed(struct domain *d)
+{
+}
+
void dump_p2m_lookup(struct domain *d, paddr_t addr)
{
struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index c326aba..24102c0 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -897,10 +897,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
else
ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
-#ifdef CONFIG_X86
if ( !ret )
memory_type_changed(d);
-#endif
}
break;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 06c93a0..13fea36 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -5,6 +5,8 @@
struct domain;
+extern void memory_type_changed(struct domain *);
+
/* Per-p2m-table state */
struct p2m_domain {
/* Lock that protects updates to the p2m */
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (4 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 05/14] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
` (8 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This commit factors out from the XEN_DOMCTL_memory_mapping hypercall
implementation, currently available only for x86, the operations
related to memory ranges map and unmap. The code is factored out
into two {map|unmap}_mmio_regions() functions for x86, that will match
the corresponding pair of ARM functions when the DOMCTL will be moved
to common code in the following commit. This commit also adds an
unmap_mmio_regions() function for ARM so that the following transition
to common code is cleaner.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v10:
- Address coding-style issues in unmap_mmio_regions() for x86.
v9:
- Don't ignore an error if it doesn't happen while unmapping the last
I/O-memory mapping in unmap_mmio_regions() for x86.
- Fix wrong argument in the call to unmap_mmio_regions() performed on
failure by map_mmio_regions() for x86.
- Explicitly specify nr_mfns in the commit message emitted when the
function map_mmio_regions() fails in the DOMCTL handling code.
- Change "nr_mfns" to "nr" in the arch-specific map/unmap functions
for x86.
v8:
- Latch the ret variable in {map|unmap}_mmio_regions() to the
first or last errors encountered during mapping or unmapping
operations.
- Separate cleanup-related changes from refactoring ones.
- Use correct count in unmap_mmio_regions().
v7:
- Use unmap_mmio_regions() for the error recovery path in
map_mmio_regions().
- Fix wrong use of memory-handling functions.
- Use mfn_t as last parameter to {map|unmap}_mmio_regions().
- Don't split format strings.
- Remove spurious changes, such as those to already-existing format
strings that are not necessary.
v6:
- Fix uncorrect usage of [mg]fn_end which, in the initial checks,
was subtracted 1 twice.
- Pass p2m_invalid as last parameter to unmap_mmio_regions() for ARM
as it is not (and currently must not be) used.
- Remove useless comment about the handling of errors in the
remove path of the hypercall.
- Replace stray hard tab with spaces.
v5:
- Let the unmap_mmio_regions() function for x86 return a proper
error code upon failure.
- Restore correct handling of errors in the remove path of the
hypercall, assigning to the "ret" local variable the error
code returned by the unmap_mmio_regions() function only if
iomem_deny_access() didn't return with an error.
- Compute gfn_end - 1 and mfn_end - 1 only once in the DOMCTL.
- Use a local variable to keep the return value of the function
unmap_mmio_regions() instead of re-using the "add" variable.
- Add a comment to make hopefully clearer how error values of
functions are handled in the remove path of the DOMCTL.
v4:
- Fix type and signedness of local variables used as indexes in
map_mmio_regions() and unmap_mmio_regions() for x86.
- Clear p2m entries in map_mmio_regions() for x86 only if
set_mmio_p2m_entry() returned with an error.
- Make ranges inclusive of the end address in map_mmio_regions()
and unmap_mmio_regions() for x86.
- Turn hard tabs into spaces.
v3:
- Add map_mmio_regions() and unmap_mmio_regions() functions for x86;
- Use pfn as parameters to the unmap_mmio_regions() function.
- Compute gfn + nr_mfns and mfn + nr_mfns only once.
v2:
- Use the already-defined PADDR_BITS constant in the new DOMCTL.
- Use paddr_t as arguments to the map_mmio_regions() function.
- Page-align addresses given as arguments to map_mmio_regions() and
unmap_mmio_regions().
---
xen/arch/arm/p2m.c | 12 ++++++++++++
xen/arch/x86/domctl.c | 19 +++++--------------
xen/arch/x86/mm/p2m.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/p2m.h | 4 ++++
xen/include/asm-x86/p2m.h | 12 ++++++++++++
5 files changed, 78 insertions(+), 14 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 3f0dc0d..6f32e81 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -893,6 +893,18 @@ int map_mmio_regions(struct domain *d,
MATTR_DEV, p2m_mmio_direct);
}
+int unmap_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr_mfns,
+ unsigned long mfn)
+{
+ return apply_p2m_changes(d, REMOVE,
+ pfn_to_paddr(start_gfn),
+ pfn_to_paddr(start_gfn + nr_mfns),
+ pfn_to_paddr(mfn),
+ MATTR_DEV, p2m_invalid);
+}
+
int guest_physmap_add_entry(struct domain *d,
unsigned long gpfn,
unsigned long mfn,
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 9cdbc3d..caa3be6 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -670,17 +670,14 @@ long arch_do_domctl(
d->domain_id, gfn, mfn, nr_mfns);
ret = iomem_permit_access(d, mfn, mfn_end);
- if ( !ret && paging_mode_translate(d) )
+ if ( !ret )
{
- for ( i = 0; !ret && i < nr_mfns; i++ )
- ret = set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
+ ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
if ( ret )
{
printk(XENLOG_G_WARNING
- "memory_map:fail: dom%d gfn=%lx mfn=%lx ret:%ld\n",
- d->domain_id, gfn + i, mfn + i, ret);
- while ( i-- )
- clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
+ "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
+ d->domain_id, gfn, mfn, nr_mfns, ret);
if ( iomem_deny_access(d, mfn, mfn_end) &&
is_hardware_domain(current->domain) )
printk(XENLOG_ERR
@@ -697,13 +694,7 @@ long arch_do_domctl(
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- if ( paging_mode_translate(d) )
- for ( i = 0; i < nr_mfns; i++ )
- {
- ret = clear_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i));
- if ( ret )
- rc = ret;
- }
+ rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
ret = iomem_deny_access(d, mfn, mfn_end);
if ( !ret )
ret = rc;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 2586a3c..32776c3 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1725,6 +1725,51 @@ unsigned long paging_gva_to_gfn(struct vcpu *v,
return hostmode->gva_to_gfn(v, hostp2m, va, pfec);
}
+int map_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr,
+ unsigned long mfn)
+{
+ int ret = 0;
+ unsigned long i;
+
+ if ( !paging_mode_translate(d) )
+ return 0;
+
+ for ( i = 0; !ret && i < nr; i++ )
+ {
+ ret = set_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+ if ( ret )
+ {
+ unmap_mmio_regions(d, start_gfn, i, mfn);
+ break;
+ }
+ }
+
+ return ret;
+}
+
+int unmap_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr,
+ unsigned long mfn)
+{
+ int err = 0;
+ unsigned long i;
+
+ if ( !paging_mode_translate(d) )
+ return 0;
+
+ for ( i = 0; i < nr; i++ )
+ {
+ int ret = clear_mmio_p2m_entry(d, start_gfn + i, _mfn(mfn + i));
+ if ( ret )
+ err = ret;
+ }
+
+ return err;
+}
+
/*** Audit ***/
#if P2M_AUDIT
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 13fea36..648144f 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -108,6 +108,10 @@ int map_mmio_regions(struct domain *d,
unsigned long start_gfn,
unsigned long nr_mfns,
unsigned long mfn);
+int unmap_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr_mfns,
+ unsigned long mfn);
int guest_physmap_add_entry(struct domain *d,
unsigned long gfn,
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index d19f50c..2a128ed 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -32,6 +32,18 @@
#include <asm/mem_sharing.h>
#include <asm/page.h> /* for pagetable_t */
+/* Map MMIO regions in the p2m: start_gfn and nr describe the range in
+ * the guest physical address space to map, starting from the machine
+ * frame number mfn. */
+int map_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr,
+ unsigned long mfn);
+int unmap_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr,
+ unsigned long mfn);
+
extern bool_t opt_hap_1gb, opt_hap_2mb;
/*
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (5 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 06/14] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 08/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
` (7 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This commit moves to common code the implementation of the memory_mapping
DOMCTL, currently available only for the x86 architecture. It also adds
a definition for the PADDR_BITS constant for ARM, that is to be used in
common code and currently not available for the ARM architecture.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Tested-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v9:
- Don't expose the type mfn_t to common code; let the x86-specific map/
unmap functions perform the type cast and use unsigned long in common
code.
v7:
- Change the name of tmp_rc to rc. Also, do not uselessly initialize it.
- ifdef out the invocation of memory_type_changed() to be called only
if the architecture is x86 instead of adding an useless empty stub
for ARM.
v6:
- Add an empty definition of the memory_type_changed() function for ARM.
v5:
- Rename new header to p2m-common.h.
v4:
- Use a define for paddr_bits instead of a new variable.
- Define prototypes for common functions map_mmio_regions() and
unmap_mmio_regions() only once in a common header.
v3:
- Add a paddr_bits variable for ARM.
v2:
- Move code to xen/arm/domctl.c.
---
xen/arch/arm/p2m.c | 8 +++---
xen/arch/x86/domctl.c | 68 --------------------------------------------
xen/common/domctl.c | 68 ++++++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/p2m.h | 15 +++-------
xen/include/asm-x86/p2m.h | 13 +--------
xen/include/xen/p2m-common.h | 16 +++++++++++
6 files changed, 93 insertions(+), 95 deletions(-)
create mode 100644 xen/include/xen/p2m-common.h
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 6f32e81..e83fff1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -883,24 +883,24 @@ int p2m_populate_ram(struct domain *d,
int map_mmio_regions(struct domain *d,
unsigned long start_gfn,
- unsigned long nr_mfns,
+ unsigned long nr,
unsigned long mfn)
{
return apply_p2m_changes(d, INSERT,
pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr_mfns),
+ pfn_to_paddr(start_gfn + nr),
pfn_to_paddr(mfn),
MATTR_DEV, p2m_mmio_direct);
}
int unmap_mmio_regions(struct domain *d,
unsigned long start_gfn,
- unsigned long nr_mfns,
+ unsigned long nr,
unsigned long mfn)
{
return apply_p2m_changes(d, REMOVE,
pfn_to_paddr(start_gfn),
- pfn_to_paddr(start_gfn + nr_mfns),
+ pfn_to_paddr(start_gfn + nr),
pfn_to_paddr(mfn),
MATTR_DEV, p2m_invalid);
}
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index caa3be6..7a5de43 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -641,74 +641,6 @@ long arch_do_domctl(
}
break;
- case XEN_DOMCTL_memory_mapping:
- {
- unsigned long gfn = domctl->u.memory_mapping.first_gfn;
- unsigned long mfn = domctl->u.memory_mapping.first_mfn;
- unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
- unsigned long mfn_end = mfn + nr_mfns - 1;
- int add = domctl->u.memory_mapping.add_mapping;
-
- ret = -EINVAL;
- if ( mfn_end < mfn || /* wrap? */
- ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
- (gfn + nr_mfns - 1) < gfn ) /* wrap? */
- break;
-
- ret = -EPERM;
- if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
- break;
-
- ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
- if ( ret )
- break;
-
- if ( add )
- {
- printk(XENLOG_G_INFO
- "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
- d->domain_id, gfn, mfn, nr_mfns);
-
- ret = iomem_permit_access(d, mfn, mfn_end);
- if ( !ret )
- {
- ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
- if ( ret )
- {
- printk(XENLOG_G_WARNING
- "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
- d->domain_id, gfn, mfn, nr_mfns, ret);
- if ( iomem_deny_access(d, mfn, mfn_end) &&
- is_hardware_domain(current->domain) )
- printk(XENLOG_ERR
- "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
- d->domain_id, mfn, mfn_end);
- }
- }
- }
- else
- {
- int rc = 0;
-
- printk(XENLOG_G_INFO
- "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
- d->domain_id, gfn, mfn, nr_mfns);
-
- rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
- ret = iomem_deny_access(d, mfn, mfn_end);
- if ( !ret )
- ret = rc;
- if ( ret && is_hardware_domain(current->domain) )
- printk(XENLOG_ERR
- "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
- ret, rc ? "removing" : "denying", d->domain_id,
- mfn, mfn_end);
- }
- /* Do this unconditionally to cover errors on above failure paths. */
- memory_type_changed(d);
- }
- break;
-
case XEN_DOMCTL_ioport_mapping:
{
struct hvm_iommu *hd;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 24102c0..80b7800 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -902,6 +902,74 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
}
break;
+ case XEN_DOMCTL_memory_mapping:
+ {
+ unsigned long gfn = op->u.memory_mapping.first_gfn;
+ unsigned long mfn = op->u.memory_mapping.first_mfn;
+ unsigned long nr_mfns = op->u.memory_mapping.nr_mfns;
+ unsigned long mfn_end = mfn + nr_mfns - 1;
+ int add = op->u.memory_mapping.add_mapping;
+
+ ret = -EINVAL;
+ if ( mfn_end < mfn || /* wrap? */
+ ((mfn | mfn_end) >> (paddr_bits - PAGE_SHIFT)) ||
+ (gfn + nr_mfns - 1) < gfn ) /* wrap? */
+ break;
+
+ ret = -EPERM;
+ if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+ break;
+
+ ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
+ if ( ret )
+ break;
+
+ if ( add )
+ {
+ printk(XENLOG_G_INFO
+ "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+ d->domain_id, gfn, mfn, nr_mfns);
+
+ ret = iomem_permit_access(d, mfn, mfn_end);
+ if ( !ret )
+ {
+ ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
+ if ( ret )
+ {
+ printk(XENLOG_G_WARNING
+ "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
+ d->domain_id, gfn, mfn, nr_mfns, ret);
+ if ( iomem_deny_access(d, mfn, mfn_end) &&
+ is_hardware_domain(current->domain) )
+ printk(XENLOG_ERR
+ "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
+ d->domain_id, mfn, mfn_end);
+ }
+ }
+ }
+ else
+ {
+ int rc = 0;
+
+ printk(XENLOG_G_INFO
+ "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+ d->domain_id, gfn, mfn, nr_mfns);
+
+ rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
+ ret = iomem_deny_access(d, mfn, mfn_end);
+ if ( !ret )
+ ret = rc;
+ if ( ret && is_hardware_domain(current->domain) )
+ printk(XENLOG_ERR
+ "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
+ ret, rc ? "removing" : "denying", d->domain_id,
+ mfn, mfn_end);
+ }
+ /* Do this unconditionally to cover errors on above failure paths. */
+ memory_type_changed(d);
+ }
+ break;
+
case XEN_DOMCTL_settimeoffset:
{
domain_set_time_offset(d, op->u.settimeoffset.time_offset_seconds);
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 648144f..08ce07b 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -3,6 +3,10 @@
#include <xen/mm.h>
+#include <xen/p2m-common.h>
+
+#define paddr_bits PADDR_BITS
+
struct domain;
extern void memory_type_changed(struct domain *);
@@ -101,17 +105,6 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn);
/* Setup p2m RAM mapping for domain d from start-end. */
int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
-/* Map MMIO regions in the p2m: start_gfn and nr_mfns describe the range
- * in the guest physical address space to map, starting from the machine
- * frame number mfn. */
-int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
- unsigned long nr_mfns,
- unsigned long mfn);
-int unmap_mmio_regions(struct domain *d,
- unsigned long start_gfn,
- unsigned long nr_mfns,
- unsigned long mfn);
int guest_physmap_add_entry(struct domain *d,
unsigned long gfn,
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 2a128ed..39f235d 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -29,21 +29,10 @@
#include <xen/config.h>
#include <xen/paging.h>
+#include <xen/p2m-common.h>
#include <asm/mem_sharing.h>
#include <asm/page.h> /* for pagetable_t */
-/* Map MMIO regions in the p2m: start_gfn and nr describe the range in
- * the guest physical address space to map, starting from the machine
- * frame number mfn. */
-int map_mmio_regions(struct domain *d,
- unsigned long start_gfn,
- unsigned long nr,
- unsigned long mfn);
-int unmap_mmio_regions(struct domain *d,
- unsigned long start_gfn,
- unsigned long nr,
- unsigned long mfn);
-
extern bool_t opt_hap_1gb, opt_hap_2mb;
/*
diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
new file mode 100644
index 0000000..9f1b771
--- /dev/null
+++ b/xen/include/xen/p2m-common.h
@@ -0,0 +1,16 @@
+#ifndef _XEN_P2M_COMMON_H
+#define _XEN_P2M_COMMON_H
+
+/* Map MMIO regions in the p2m: start_gfn and nr describe the range in
+ * * the guest physical address space to map, starting from the machine
+ * * frame number mfn. */
+int map_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr,
+ unsigned long mfn);
+int unmap_mmio_regions(struct domain *d,
+ unsigned long start_gfn,
+ unsigned long nr,
+ unsigned long mfn);
+
+#endif /* _XEN_P2M_COMMON_H */
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 08/14] tools/libxl: parse optional start gfn from the iomem config option
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (6 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 07/14] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
` (6 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
Currently, the "iomem" domU config option allows to specify a machine
address range to be mapped to the domU. However, there is no way to
specify the guest address range used for the mapping. This commit
extends the iomem option handling code to parse an additional, optional
parameter: this parameter, if given, specifies the start guest address
used for the mapping; if no start guest address is given, a 1:1 mapping
is performed as default.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v11:
- Avoid shadowed declaration warning by moving to function scope an
index variable.
v7:
- Use LIBXL_INVALID_GFN when needed.
- Add to the manpage a note about the mapping defaulting to 1:1 when
the gfn parameter is not specified, and about the option being for
auto-translated guests.
- Remove spurious change.
v6:
- Document what happens if the new "gfn" parameter is omitted
while specifying the "iomem" option in the domain configuration.
- Add LIBXL_INVALID_GFN macro with value "~(uint64_t)0".
- Fix typo in the comment to the LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN
macro.
v5:
- Initialize the gfn field of the iomem structure with the
value "(uint64_t)-1".
- Defer the assignment of the value of "start" to "gfn", if
"gfn" has been initialized to the default value, in libxl.
- Use a local variable to keep the return value of sscanf()
for better code readability.
v4:
- Add definition of a LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN macro
to indicate the availability of the new option.
- Simplify the code parsing the new optional parameter by using
the switch construct on the return value of the sscanf() function
instead of calling the function twice.
- Add a short paragraph to the manpage about the use of the iomem
option to passthrough devices protected by an IOMMU.
- Add comments to the fields of the "iomem" structure.
---
docs/man/xl.cfg.pod.5 | 18 +++++++++++++-----
tools/libxl/libxl.h | 10 ++++++++++
tools/libxl/libxl_create.c | 8 ++++++--
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_types.idl | 4 ++++
tools/libxl/xl_cmdimpl.c | 17 ++++++++++-------
6 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index f1fc906..517ae2f 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -670,12 +670,20 @@ is given in hexadecimal and may either a span e.g. C<2f8-2ff>
It is recommended to use this option only for trusted VMs under
administrator control.
-=item B<iomem=[ "IOMEM_START,NUM_PAGES", "IOMEM_START,NUM_PAGES", ... ]>
+=item B<iomem=[ "IOMEM_START,NUM_PAGES[@GFN]", "IOMEM_START,NUM_PAGES[@GFN]", ... ]>
-Allow guest to access specific hardware I/O memory pages. B<IOMEM_START>
-is a physical page number. B<NUM_PAGES> is the number
-of pages beginning with B<START_PAGE> to allow access. Both values
-must be given in hexadecimal.
+Allow auto-translated domains to access specific hardware I/O memory pages.
+
+B<IOMEM_START> is a physical page number. B<NUM_PAGES> is the number of pages
+beginning with B<START_PAGE> to allow access. B<GFN> specifies the guest frame
+number where the mapping will start in the domU's address space. If B<GFN> is
+not given, the mapping will be performed using B<IOMEM_START> as a start in the
+domU's address space, therefore performing an 1:1 mapping as default.
+All of these values must be given in hexadecimal.
+
+Note that the IOMMU won't be updated with the mappings specified with this
+option. This option therefore should not be used to passthrough any
+IOMMU-protected device.
It is recommended to use this option only for trusted VMs under
administrator control.
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index ded4ce5..460207b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -128,6 +128,16 @@
#define LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE 1
/*
+ * LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN indicates that it is possible
+ * to specify the start guest frame number used to map a range of I/O
+ * memory machine frame numbers via the 'gfn' field (of type uint64)
+ * of the 'iomem' structure. An array of iomem structures is embedded
+ * in libxl_domain_build_info and used to map the indicated memory
+ * ranges during domain build.
+ */
+#define LIBXL_HAVE_BUILDINFO_IOMEM_START_GFN 1
+
+/*
* libxl ABI compatibility
*
* The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index fc332ef..bc44ef5 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -102,6 +102,8 @@ static int sched_params_valid(libxl__gc *gc,
int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info)
{
+ int i;
+
if (b_info->type != LIBXL_DOMAIN_TYPE_HVM &&
b_info->type != LIBXL_DOMAIN_TYPE_PV)
return ERROR_INVAL;
@@ -189,8 +191,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
/* In libxl internals, we want to deal with vcpu_hard_affinity only! */
if (b_info->cpumap.size && !b_info->num_vcpu_hard_affinity) {
- int i;
-
b_info->vcpu_hard_affinity = libxl__calloc(gc, b_info->max_vcpus,
sizeof(libxl_bitmap));
for (i = 0; i < b_info->max_vcpus; i++) {
@@ -215,6 +215,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->disable_migrate, false);
+ for (i = 0 ; i < b_info->num_iomem; i++)
+ if (b_info->iomem[i].gfn == LIBXL_INVALID_GFN)
+ b_info->iomem[i].gfn = b_info->iomem[i].start;
+
if (!b_info->event_channels)
b_info->event_channels = 1023;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index beb052e..04c9378 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -91,6 +91,7 @@
#define LIBXL_PV_EXTRA_MEMORY 1024
#define LIBXL_HVM_EXTRA_MEMORY 2048
#define LIBXL_MIN_DOM0_MEM (128*1024)
+#define LIBXL_INVALID_GFN (~(uint64_t)0)
/* use 0 as the domid of the toolstack domain for now */
#define LIBXL_TOOLSTACK_DOMID 0
#define QEMU_SIGNATURE "DeviceModelRecord0002"
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 08a7927..931c9e9 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -185,8 +185,12 @@ libxl_ioport_range = Struct("ioport_range", [
])
libxl_iomem_range = Struct("iomem_range", [
+ # start host frame number to be mapped to the guest
("start", uint64),
+ # number of frames to be mapped
("number", uint64),
+ # guest frame number used as a start for the mapping
+ ("gfn", uint64, {'init_val': "LIBXL_INVALID_GFN"}),
])
libxl_vga_interface_info = Struct("vga_interface_info", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 409a795..e6b9615 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1199,6 +1199,7 @@ static void parse_config_data(const char *config_source,
}
if (!xlu_cfg_get_list(config, "iomem", &iomem, &num_iomem, 0)) {
+ int ret;
b_info->num_iomem = num_iomem;
b_info->iomem = calloc(num_iomem, sizeof(*b_info->iomem));
if (b_info->iomem == NULL) {
@@ -1212,13 +1213,15 @@ static void parse_config_data(const char *config_source,
"xl: Unable to get element %d in iomem list\n", i);
exit(1);
}
- if(sscanf(buf, "%" SCNx64",%" SCNx64,
- &b_info->iomem[i].start,
- &b_info->iomem[i].number)
- != 2) {
- fprintf(stderr,
- "xl: Invalid argument parsing iomem: %s\n", buf);
- exit(1);
+ libxl_iomem_range_init(&b_info->iomem[i]);
+ ret = sscanf(buf, "%" SCNx64",%" SCNx64"@%" SCNx64,
+ &b_info->iomem[i].start,
+ &b_info->iomem[i].number,
+ &b_info->iomem[i].gfn);
+ if (ret < 2) {
+ fprintf(stderr,
+ "xl: Invalid argument parsing iomem: %s\n", buf);
+ exit(1);
}
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (7 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 08/14] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-08-30 16:29 ` [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
` (5 subsequent siblings)
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
Currently, the configuration-parsing code concerning the handling of the
iomem parameter only invokes the XEN_DOMCTL_iomem_permission hypercall.
This commit lets the XEN_DOMCTL_memory_mapping hypercall be invoked
after XEN_DOMCTL_iomem_permission when the iomem parameter is parsed
from a domU configuration file, so that the address range can be mapped
to the address space of the domU. The hypercall is invoked only in case
of domains using an auto-translated physmap.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v9:
- Enforce checks on return value of xc_domain_getinfo() as it could
return information concerning the wrong domain.
- Don't use PERROR for xc_core_* functions.
- Avoid emitting an error message if xc_core_auto_translated_physmap()
fails.
v8:
- Fix wrong error message emitted when the memory_mapping DOMCTL is
called by a domain that is not auto-translated.
v7:
- Move the check for an auto-translated physmap to xc to avoid having
to expose the information to libxl.
v4:
- Let the hypercall be called only in case the guest uses an
auto-translated physmap.
v3:
- Add ifdefs to let the hypercall be called only by ARM or x86
HVM guests, with a whitelist approach.
- Remove the NOTE comment.
v2:
- Add a comment explaining outstanding issues.
---
tools/libxc/xc_domain.c | 10 ++++++++++
tools/libxl/libxl_create.c | 11 +++++++++++
2 files changed, 21 insertions(+)
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index c67ac9a..1eba393 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1960,6 +1960,16 @@ int xc_domain_memory_mapping(
uint32_t add_mapping)
{
DECLARE_DOMCTL;
+ xc_dominfo_t info;
+
+ if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
+ info.domid != domid )
+ {
+ PERROR("Could not get info for domain");
+ return -EINVAL;
+ }
+ if ( !xc_core_arch_auto_translated_physmap(&info) )
+ return 0;
domctl.cmd = XEN_DOMCTL_memory_mapping;
domctl.domain = domid;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bc44ef5..ee328e9 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1180,6 +1180,17 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
"failed give dom%d access to iomem range %"PRIx64"-%"PRIx64,
domid, io->start, io->start + io->number - 1);
ret = ERROR_FAIL;
+ continue;
+ }
+ ret = xc_domain_memory_mapping(CTX->xch, domid,
+ io->gfn, io->start,
+ io->number, 1);
+ if (ret < 0) {
+ LOGE(ERROR,
+ "failed to map to dom%d iomem range %"PRIx64"-%"PRIx64
+ " to guest address %"PRIx64,
+ domid, io->start, io->start + io->number - 1, io->gfn);
+ ret = ERROR_FAIL;
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (8 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 09/14] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-09-03 11:22 ` Ian Campbell
2014-09-03 14:41 ` Daniel De Graaf
2014-08-30 16:29 ` [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory Arianna Avanzini
` (4 subsequent siblings)
14 siblings, 2 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
Currently, FLASK only handles the memory_mapping hypercall for the
x86 architecture. In case the architecture is ARM, an error message
will be printed regardless of how XSM has been configured. This
commit lets the memory_mapping case be handled also for architectures
other than x86.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
xen/xsm/flask/hooks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f2f59ea..a0e4ae0 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -570,6 +570,7 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_scheduler_op:
case XEN_DOMCTL_irq_permission:
case XEN_DOMCTL_iomem_permission:
+ case XEN_DOMCTL_memory_mapping:
case XEN_DOMCTL_set_target:
#ifdef CONFIG_X86
/* These have individual XSM hooks (arch/x86/domctl.c) */
@@ -577,7 +578,6 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_ioport_permission:
case XEN_DOMCTL_bind_pt_irq:
case XEN_DOMCTL_unbind_pt_irq:
- case XEN_DOMCTL_memory_mapping:
case XEN_DOMCTL_ioport_mapping:
case XEN_DOMCTL_mem_event_op:
/* These have individual XSM hooks (drivers/passthrough/iommu.c) */
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory
2014-08-30 16:29 ` [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
@ 2014-09-03 11:22 ` Ian Campbell
2014-09-03 11:30 ` Ian Campbell
2014-09-03 14:41 ` Daniel De Graaf
1 sibling, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-03 11:22 UTC (permalink / raw)
To: Arianna Avanzini
Cc: paolo.valente, keir, stefano.stabellini, Ian.Jackson,
dario.faggioli, tim, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> Currently, FLASK only handles the memory_mapping hypercall for the
> x86 architecture. In case the architecture is ARM, an error message
> will be printed regardless of how XSM has been configured. This
> commit lets the memory_mapping case be handled also for architectures
> other than x86.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Reviewed-by: Julien Grall <julien.grall@citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: 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>
> Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> ---
> xen/xsm/flask/hooks.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index f2f59ea..a0e4ae0 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -570,6 +570,7 @@ static int flask_domctl(struct domain *d, int cmd)
> case XEN_DOMCTL_scheduler_op:
> case XEN_DOMCTL_irq_permission:
> case XEN_DOMCTL_iomem_permission:
> + case XEN_DOMCTL_memory_mapping:
> case XEN_DOMCTL_set_target:
> #ifdef CONFIG_X86
> /* These have individual XSM hooks (arch/x86/domctl.c) */
> @@ -577,7 +578,6 @@ static int flask_domctl(struct domain *d, int cmd)
> case XEN_DOMCTL_ioport_permission:
> case XEN_DOMCTL_bind_pt_irq:
> case XEN_DOMCTL_unbind_pt_irq:
> - case XEN_DOMCTL_memory_mapping:
> case XEN_DOMCTL_ioport_mapping:
> case XEN_DOMCTL_mem_event_op:
> /* These have individual XSM hooks (drivers/passthrough/iommu.c) */
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory
2014-09-03 11:22 ` Ian Campbell
@ 2014-09-03 11:30 ` Ian Campbell
0 siblings, 0 replies; 41+ messages in thread
From: Ian Campbell @ 2014-09-03 11:30 UTC (permalink / raw)
To: Arianna Avanzini
Cc: paolo.valente, keir, stefano.stabellini, tim, dario.faggioli,
Ian.Jackson, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Wed, 2014-09-03 at 12:22 +0100, Ian Campbell wrote:
> On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> > Currently, FLASK only handles the memory_mapping hypercall for the
> > x86 architecture. In case the architecture is ARM, an error message
> > will be printed regardless of how XSM has been configured. This
> > commit lets the memory_mapping case be handled also for architectures
> > other than x86.
> >
> > Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> > Reviewed-by: Julien Grall <julien.grall@citrix.com>
> > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > Cc: Paolo Valente <paolo.valente@unimore.it>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Although since patches further down the series require a resend I'm also
going to ask that you rephrase the commit message in terms of what it
does "Handle XEN_DOMCTL_memory_mapping for all architectures" rather
than its side effect (removing an error message). With some sort of
change along those lines you may keep the acked by.
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory
2014-08-30 16:29 ` [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
2014-09-03 11:22 ` Ian Campbell
@ 2014-09-03 14:41 ` Daniel De Graaf
2014-09-04 11:49 ` [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures Arianna Avanzini
1 sibling, 1 reply; 41+ messages in thread
From: Daniel De Graaf @ 2014-09-03 14:41 UTC (permalink / raw)
To: Arianna Avanzini, xen-devel
Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
dario.faggioli, Ian.Jackson, Ian.Campbell, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On 08/30/2014 12:29 PM, Arianna Avanzini wrote:
> Currently, FLASK only handles the memory_mapping hypercall for the
> x86 architecture. In case the architecture is ARM, an error message
> will be printed regardless of how XSM has been configured. This
> commit lets the memory_mapping case be handled also for architectures
> other than x86.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Reviewed-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
--
Daniel De Graaf
National Security Agency
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-03 14:41 ` Daniel De Graaf
@ 2014-09-04 11:49 ` Arianna Avanzini
2014-09-08 10:11 ` Ian Campbell
0 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-04 11:49 UTC (permalink / raw)
To: xen-devel, dgdegra
Cc: Ian.Campbell, paolo.valente, stefano.stabellini, dario.faggioli,
julien.grall, etrudeau, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
Currently, FLASK only handles the memory_mapping hypercall for the
x86 architecture. As the DOMCTL's hook now is in common code and
no more specific to x86, this commit lets the DOMCTL be handled also
for other architectures.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
With respect to patch 0010 of the v12 memory_mapping patchset ([1]):
- Reworded commit message so that it explains what the patch does and
not the side effects of its absence.
[1] http://markmail.org/thread/cx2q7vhlwuzssmzp
---
xen/xsm/flask/hooks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index f2f59ea..a0e4ae0 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -570,6 +570,7 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_scheduler_op:
case XEN_DOMCTL_irq_permission:
case XEN_DOMCTL_iomem_permission:
+ case XEN_DOMCTL_memory_mapping:
case XEN_DOMCTL_set_target:
#ifdef CONFIG_X86
/* These have individual XSM hooks (arch/x86/domctl.c) */
@@ -577,7 +578,6 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_ioport_permission:
case XEN_DOMCTL_bind_pt_irq:
case XEN_DOMCTL_unbind_pt_irq:
- case XEN_DOMCTL_memory_mapping:
case XEN_DOMCTL_ioport_mapping:
case XEN_DOMCTL_mem_event_op:
/* These have individual XSM hooks (drivers/passthrough/iommu.c) */
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-04 11:49 ` [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures Arianna Avanzini
@ 2014-09-08 10:11 ` Ian Campbell
2014-09-08 12:24 ` Arianna Avanzini
0 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-08 10:11 UTC (permalink / raw)
To: Arianna Avanzini
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> - Reworded commit message so that it explains what the patch does and
> not the side effects of its absence.
Thanks.
Am I correct in assuming that this is replacing the previous #10/14 in
v12 but that a v13 is not on its way. IOW I can do the switcharoo during
commit (assuming the rest is acked, I didn't check yet)?
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-08 10:11 ` Ian Campbell
@ 2014-09-08 12:24 ` Arianna Avanzini
2014-09-08 12:38 ` Ian Campbell
0 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-08 12:24 UTC (permalink / raw)
To: Ian Campbell
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > - Reworded commit message so that it explains what the patch does and
> > not the side effects of its absence.
>
> Thanks.
>
> Am I correct in assuming that this is replacing the previous #10/14 in
> v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> commit (assuming the rest is acked, I didn't check yet)?
Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
comments about patch #11, and I'll resend patches from #12 to #14 (with
your comments addressed) within today.
Thanks for your feedback,
Arianna
>
> Ian.
>
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-08 12:24 ` Arianna Avanzini
@ 2014-09-08 12:38 ` Ian Campbell
2014-09-08 12:50 ` Arianna Avanzini
0 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-08 12:38 UTC (permalink / raw)
To: avanzini.arianna
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Mon, 2014-09-08 at 14:24 +0200, Arianna Avanzini wrote:
> On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> > On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > > - Reworded commit message so that it explains what the patch does and
> > > not the side effects of its absence.
> >
> > Thanks.
> >
> > Am I correct in assuming that this is replacing the previous #10/14 in
> > v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> > commit (assuming the rest is acked, I didn't check yet)?
>
> Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
> have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
> comments about patch #11, and I'll resend patches from #12 to #14 (with
> your comments addressed) within today.
Thanks, it looks to me like #12-#14 can (once resent) go ahead without
#11 for the time being, is that right?
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-08 12:38 ` Ian Campbell
@ 2014-09-08 12:50 ` Arianna Avanzini
2014-09-09 12:35 ` Ian Campbell
0 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-08 12:50 UTC (permalink / raw)
To: Ian Campbell
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Mon, Sep 08, 2014 at 01:38:43PM +0100, Ian Campbell wrote:
> On Mon, 2014-09-08 at 14:24 +0200, Arianna Avanzini wrote:
> > On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> > > On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > > > - Reworded commit message so that it explains what the patch does and
> > > > not the side effects of its absence.
> > >
> > > Thanks.
> > >
> > > Am I correct in assuming that this is replacing the previous #10/14 in
> > > v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> > > commit (assuming the rest is acked, I didn't check yet)?
> >
> > Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
> > have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
> > comments about patch #11, and I'll resend patches from #12 to #14 (with
> > your comments addressed) within today.
>
> Thanks, it looks to me like #12-#14 can (once resent) go ahead without
> #11 for the time being, is that right?
Yes exactly, that's what I thought too.
Thank you,
Arianna
>
> Ian.
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-08 12:50 ` Arianna Avanzini
@ 2014-09-09 12:35 ` Ian Campbell
2014-09-09 13:13 ` Arianna Avanzini
0 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-09 12:35 UTC (permalink / raw)
To: avanzini.arianna
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Mon, 2014-09-08 at 14:50 +0200, Arianna Avanzini wrote:
> On Mon, Sep 08, 2014 at 01:38:43PM +0100, Ian Campbell wrote:
> > On Mon, 2014-09-08 at 14:24 +0200, Arianna Avanzini wrote:
> > > On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> > > > On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > > > > - Reworded commit message so that it explains what the patch does and
> > > > > not the side effects of its absence.
> > > >
> > > > Thanks.
> > > >
> > > > Am I correct in assuming that this is replacing the previous #10/14 in
> > > > v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> > > > commit (assuming the rest is acked, I didn't check yet)?
> > >
> > > Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
> > > have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
> > > comments about patch #11, and I'll resend patches from #12 to #14 (with
> > > your comments addressed) within today.
> >
> > Thanks, it looks to me like #12-#14 can (once resent) go ahead without
> > #11 for the time being, is that right?
>
> Yes exactly, that's what I thought too.
I've now applied this new replacement #10 plus the little three patch
series you sent corresponding to #12-#14 in this one.
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-09 12:35 ` Ian Campbell
@ 2014-09-09 13:13 ` Arianna Avanzini
2014-09-09 14:45 ` Ian Campbell
0 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-09 13:13 UTC (permalink / raw)
To: Ian Campbell
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Tue, Sep 09, 2014 at 01:35:29PM +0100, Ian Campbell wrote:
> On Mon, 2014-09-08 at 14:50 +0200, Arianna Avanzini wrote:
> > On Mon, Sep 08, 2014 at 01:38:43PM +0100, Ian Campbell wrote:
> > > On Mon, 2014-09-08 at 14:24 +0200, Arianna Avanzini wrote:
> > > > On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> > > > > On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > > > > > - Reworded commit message so that it explains what the patch does and
> > > > > > not the side effects of its absence.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > Am I correct in assuming that this is replacing the previous #10/14 in
> > > > > v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> > > > > commit (assuming the rest is acked, I didn't check yet)?
> > > >
> > > > Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
> > > > have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
> > > > comments about patch #11, and I'll resend patches from #12 to #14 (with
> > > > your comments addressed) within today.
> > >
> > > Thanks, it looks to me like #12-#14 can (once resent) go ahead without
> > > #11 for the time being, is that right?
> >
> > Yes exactly, that's what I thought too.
>
> I've now applied this new replacement #10 plus the little three patch
> series you sent corresponding to #12-#14 in this one.
>
Thank you for taking the time to review them!
Arianna
> Ian.
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-09 13:13 ` Arianna Avanzini
@ 2014-09-09 14:45 ` Ian Campbell
2014-09-10 20:07 ` Arianna Avanzini
0 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-09 14:45 UTC (permalink / raw)
To: avanzini.arianna
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Tue, 2014-09-09 at 15:13 +0200, Arianna Avanzini wrote:
> On Tue, Sep 09, 2014 at 01:35:29PM +0100, Ian Campbell wrote:
> > On Mon, 2014-09-08 at 14:50 +0200, Arianna Avanzini wrote:
> > > On Mon, Sep 08, 2014 at 01:38:43PM +0100, Ian Campbell wrote:
> > > > On Mon, 2014-09-08 at 14:24 +0200, Arianna Avanzini wrote:
> > > > > On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> > > > > > On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > > > > > > - Reworded commit message so that it explains what the patch does and
> > > > > > > not the side effects of its absence.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > Am I correct in assuming that this is replacing the previous #10/14 in
> > > > > > v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> > > > > > commit (assuming the rest is acked, I didn't check yet)?
> > > > >
> > > > > Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
> > > > > have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
> > > > > comments about patch #11, and I'll resend patches from #12 to #14 (with
> > > > > your comments addressed) within today.
> > > >
> > > > Thanks, it looks to me like #12-#14 can (once resent) go ahead without
> > > > #11 for the time being, is that right?
> > >
> > > Yes exactly, that's what I thought too.
> >
> > I've now applied this new replacement #10 plus the little three patch
> > series you sent corresponding to #12-#14 in this one.
> >
>
> Thank you for taking the time to review them!
Thanks for the contribution (and commitment!), I hadn't realised when I
committed that batch that it was effectively everything but some
xsm/flask loose ends, congrats!
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] xsm/flask: handle XEN_DOMCTL_memory_mapping for all architectures
2014-09-09 14:45 ` Ian Campbell
@ 2014-09-10 20:07 ` Arianna Avanzini
0 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-10 20:07 UTC (permalink / raw)
To: Ian Campbell
Cc: paolo.valente, stefano.stabellini, dario.faggioli, xen-devel,
julien.grall, etrudeau, dgdegra, viktor.kleinik,
andrii.tseglytskyi
On Tue, Sep 09, 2014 at 03:45:05PM +0100, Ian Campbell wrote:
> On Tue, 2014-09-09 at 15:13 +0200, Arianna Avanzini wrote:
> > On Tue, Sep 09, 2014 at 01:35:29PM +0100, Ian Campbell wrote:
> > > On Mon, 2014-09-08 at 14:50 +0200, Arianna Avanzini wrote:
> > > > On Mon, Sep 08, 2014 at 01:38:43PM +0100, Ian Campbell wrote:
> > > > > On Mon, 2014-09-08 at 14:24 +0200, Arianna Avanzini wrote:
> > > > > > On Mon, Sep 08, 2014 at 11:11:25AM +0100, Ian Campbell wrote:
> > > > > > > On Thu, 2014-09-04 at 13:49 +0200, Arianna Avanzini wrote:
> > > > > > > > - Reworded commit message so that it explains what the patch does and
> > > > > > > > not the side effects of its absence.
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > > > Am I correct in assuming that this is replacing the previous #10/14 in
> > > > > > > v12 but that a v13 is not on its way. IOW I can do the switcharoo during
> > > > > > > commit (assuming the rest is acked, I didn't check yet)?
> > > > > >
> > > > > > Yes, this patch is replacing #10/14. Unfortunately patches #11 to #14 still
> > > > > > have to be acked. I was waiting for Andrii's feedback on Daniel De Graaf's
> > > > > > comments about patch #11, and I'll resend patches from #12 to #14 (with
> > > > > > your comments addressed) within today.
> > > > >
> > > > > Thanks, it looks to me like #12-#14 can (once resent) go ahead without
> > > > > #11 for the time being, is that right?
> > > >
> > > > Yes exactly, that's what I thought too.
> > >
> > > I've now applied this new replacement #10 plus the little three patch
> > > series you sent corresponding to #12-#14 in this one.
> > >
> >
> > Thank you for taking the time to review them!
>
> Thanks for the contribution (and commitment!), I hadn't realised when I
> committed that batch that it was effectively everything but some
> xsm/flask loose ends, congrats!
>
Thank you! I'll follow the discussion on Andrii's patch to see what is to be
done at the end.
Arianna
> Ian.
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (9 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 10/14] xsm/flask: avoid spurious error messages when mapping I/O-memory Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-09-03 11:21 ` Ian Campbell
2014-08-30 16:29 ` [PATCH v12 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
` (3 subsequent siblings)
14 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
From: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
This commit allows the domU to access previously-mapped I/O-memory
even if XSM is enabled and FLASK is enforced.
Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
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>
---
tools/flask/policy/policy/modules/xen/xen.te | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
index bb59fe8..34b5bfa 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -107,6 +107,7 @@ admin_device(dom0_t, device_t)
admin_device(dom0_t, irq_t)
admin_device(dom0_t, ioport_t)
admin_device(dom0_t, iomem_t)
+admin_device(domU_t, iomem_t)
domain_comms(dom0_t, dom0_t)
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory
2014-08-30 16:29 ` [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory Arianna Avanzini
@ 2014-09-03 11:21 ` Ian Campbell
2014-09-03 14:45 ` Daniel De Graaf
0 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-03 11:21 UTC (permalink / raw)
To: Arianna Avanzini, Daniel De Graaf
Cc: paolo.valente, keir, stefano.stabellini, Ian.Jackson,
dario.faggioli, tim, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> From: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>
> This commit allows the domU to access previously-mapped I/O-memory
> even if XSM is enabled and FLASK is enforced.
CCing Daniel (XSM maintainer).
I think this is probably OK, but I'm no XSM expert.
(If I were writing the ocmmit message I would have said something like
"Update the example XSM policy to allow...")
>
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 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>
> ---
> tools/flask/policy/policy/modules/xen/xen.te | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
> index bb59fe8..34b5bfa 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -107,6 +107,7 @@ admin_device(dom0_t, device_t)
> admin_device(dom0_t, irq_t)
> admin_device(dom0_t, ioport_t)
> admin_device(dom0_t, iomem_t)
> +admin_device(domU_t, iomem_t)
>
> domain_comms(dom0_t, dom0_t)
>
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory
2014-09-03 11:21 ` Ian Campbell
@ 2014-09-03 14:45 ` Daniel De Graaf
2014-09-05 23:25 ` Arianna Avanzini
0 siblings, 1 reply; 41+ messages in thread
From: Daniel De Graaf @ 2014-09-03 14:45 UTC (permalink / raw)
To: Arianna Avanzini
Cc: julien.grall, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, xen-devel, Ian Campbell,
etrudeau, JBeulich, andrew.cooper3, viktor.kleinik,
andrii.tseglytskyi
On 09/03/2014 07:21 AM, Ian Campbell wrote:
> On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
>> From: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>
>> This commit allows the domU to access previously-mapped I/O-memory
>> even if XSM is enabled and FLASK is enforced.
>
> CCing Daniel (XSM maintainer).
>
> I think this is probably OK, but I'm no XSM expert.
>
> (If I were writing the ocmmit message I would have said something like
> "Update the example XSM policy to allow...")
The message Ian suggests is a bit clearer as to the effect of the patch.
Regarding the patch: at minimum, a domU should only need the permissions
defined by "use_device(domU_t, iomem_t)" to access mapped memory. However,
it is preferred to label the IO memory being used instead of allowing access
to the default/fallback iomem_t.
The intention for handing pass-through devices with FLASK is to label the
device (either using the tool flask-label-pci or manually in the policy;
example lines for the latter are present and commented out). The example
policy defines the type nic_dev_t as a device that is usable by domU_t, and
docs/misc/xsm-flask.txt has an example of flask-label-pci's use.
If you are actually only passing IO memory and not a PCI device, labeling
the IO memory range would be the preferred solution. If you cannot label
it statically, a tool similar to flask-label-pci for memory will be needed -
something like "flask-label-resource <type> <start>-<end> <label>". This
may be more common on ARM than on x86; I am not familiar with pass-through
on ARM, and the only non-PCI device on x86 that I have used pass-through on
is the TPM, which has a well-defined IO memory range.
--
Daniel De Graaf
National Security Agency
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory
2014-09-03 14:45 ` Daniel De Graaf
@ 2014-09-05 23:25 ` Arianna Avanzini
0 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-05 23:25 UTC (permalink / raw)
To: Daniel De Graaf
Cc: julien.grall, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, xen-devel, Ian Campbell,
etrudeau, JBeulich, andrew.cooper3, viktor.kleinik,
andrii.tseglytskyi
Hello,
thank you for your thorough explanation.
On Wed, Sep 03, 2014 at 10:45:18AM -0400, Daniel De Graaf wrote:
> On 09/03/2014 07:21 AM, Ian Campbell wrote:
> >On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> >>From: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> >>
> >>This commit allows the domU to access previously-mapped I/O-memory
> >>even if XSM is enabled and FLASK is enforced.
> >
> >CCing Daniel (XSM maintainer).
> >
> >I think this is probably OK, but I'm no XSM expert.
> >
> >(If I were writing the ocmmit message I would have said something like
> >"Update the example XSM policy to allow...")
>
> The message Ian suggests is a bit clearer as to the effect of the patch.
>
Thanks to both of you; as I took the liberty of writing the commit message for
Andrii's patch I will certainly fix my mistakes according to your suggestions.
> Regarding the patch: at minimum, a domU should only need the permissions
> defined by "use_device(domU_t, iomem_t)" to access mapped memory. However,
> it is preferred to label the IO memory being used instead of allowing access
> to the default/fallback iomem_t.
>
> The intention for handing pass-through devices with FLASK is to label the
> device (either using the tool flask-label-pci or manually in the policy;
> example lines for the latter are present and commented out). The example
> policy defines the type nic_dev_t as a device that is usable by domU_t, and
> docs/misc/xsm-flask.txt has an example of flask-label-pci's use.
>
> If you are actually only passing IO memory and not a PCI device, labeling
> the IO memory range would be the preferred solution. If you cannot label
> it statically, a tool similar to flask-label-pci for memory will be needed -
> something like "flask-label-resource <type> <start>-<end> <label>". This
> may be more common on ARM than on x86; I am not familiar with pass-through
> on ARM, and the only non-PCI device on x86 that I have used pass-through on
> is the TPM, which has a well-defined IO memory range.
>
When using the iomem option to make an I/O-memory range available to a domU
the mapping to be performed is explicitly defined by domain configuration,
so labeling it should be possible, if I understood things correctly.
> --
> Daniel De Graaf
> National Security Agency
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v12 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (10 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 11/14] flask/policy: allow domU to use previously-mapped I/O-memory Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-09-03 11:26 ` Ian Campbell
2014-08-30 16:29 ` [PATCH v12 13/14] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
` (2 subsequent siblings)
14 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This commit changes the existing libxl code to be sure to grant access
permission to PCI-related I/O memory ranges, while setting up passthrough
of PCI devices specified in the domain's configuration, and to VGA-related
memory ranges, while setting up VGA passthrough (if gfx_passthru = 1 in
the domain's configuration).
As for the latter, the newly-added code does not replace any existing one,
but instead matches the calls to xc_domain_memory_mapping() performed by
QEMU on the path that is executed if gfx passthru is enabled and follows
the registration of a new VGA controller (in register_vga_regions(),
defined in hw/pt-graphics.c). In fact, VGA needs some extra memory
ranges to be mapped with respect to PCI; QEMU expects that access to those
memory ranges is implicitly granted when he calls the hypervisor with the
function xc_domain_memory_mapping(): this commit calls iomem_permission
for it when needed by checking the passthru PCI device's class.
NOTE: the code added by this commit still does not verify if the passthru
of the framebuffer area is being performed for the primary GPU, but
only replicates the behavior of QEMU which is limited to performing
the passthru for all PCI devices of VGA class.
This commit is instrumental to the last one in the series, which will
separate the functions of the iomem_permission and memory_mapping DOMCTLs,
so that requesting an I/O-memory range will not imply that access to such
a range is implicitly granted.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v12:
- Fix commit description and add a paragraph about outstanding issues.
v11:
- Move the VGA-related hunk to a libxl helper; evaluate gfx_passthru.val
only once as it is static in the scope of the helper.
- Remove leftover debug fprintf().
- Improve code readability by separating blocks of code.
v10:
- Use a class-based check on the PCI device to determine if VGA-related
I/O-memory ranges must be made accessible to a guest that needs gfx
passthrough.
v9:
- Allow a domain access to the VGA framebuffer only if the user has
signaled that one of the passthru GPUs is primary via domain config.
v8:
- Explain better in the commit description VGA-related code additions.
- Fix v6 changelog which, being too generic, ended up to uncorrectly
state that one of the requests had been addressed.
- Remove tentative phrases from commit description.
v7:
- Let libxl not handle I/O ports and I/O memory differently when access
to a PCI device is allowed to a domain.
- Change the construct used by libxl during PCI-related initialization
from a switch to an if to better suit the new execution flow.
---
tools/libxl/libxl_create.c | 9 +++++
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_pci.c | 91 ++++++++++++++++++++++++++++++++++++--------
3 files changed, 86 insertions(+), 16 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index ee328e9..8062e4d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1216,6 +1216,15 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl__spawn_stub_dm(egc, &dcs->dmss);
else
libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+
+ /*
+ * Handle the domain's (and the related stubdomain's) access to
+ * the VGA framebuffer.
+ */
+ ret = libxl__grant_vga_iomem_permission(gc, domid, d_config);
+ if ( ret )
+ goto error_out;
+
return;
}
case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 04c9378..e27861b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -951,6 +951,8 @@ _hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
_hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
libxl_domain_sched_params *scparams);
+_hidden int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
+ libxl_domain_config *const d_config);
typedef struct {
uint32_t store_port;
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2782d0e..4fc9218 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -846,10 +846,13 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl_domain_type type = libxl__domain_type(gc, domid);
int rc, hvm = 0;
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
+ if (type == LIBXL_DOMAIN_TYPE_INVALID)
+ return ERROR_FAIL;
+
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
hvm = 1;
if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0) {
@@ -867,8 +870,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
}
if ( rc )
return ERROR_FAIL;
- break;
- case LIBXL_DOMAIN_TYPE_PV:
+ }
+
{
char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
pcidev->bus, pcidev->dev, pcidev->func);
@@ -937,11 +940,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
return ERROR_FAIL;
}
}
- break;
- }
- case LIBXL_DOMAIN_TYPE_INVALID:
- return ERROR_FAIL;
}
+
out:
if (!libxl_is_stubdom(ctx, domid, NULL)) {
rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev));
@@ -1166,6 +1166,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
{
libxl_ctx *ctx = libxl__gc_owner(gc);
libxl_device_pci *assigned;
+ libxl_domain_type type = libxl__domain_type(gc, domid);
int hvm = 0, rc, num;
int stubdomid = 0;
@@ -1181,8 +1182,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
}
rc = ERROR_FAIL;
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
hvm = 1;
if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0)
@@ -1203,8 +1203,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
rc = ERROR_FAIL;
goto out_fail;
}
- break;
- case LIBXL_DOMAIN_TYPE_PV:
+ } else if (type != LIBXL_DOMAIN_TYPE_PV)
+ abort();
{
char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
pcidev->bus, pcidev->dev, pcidev->func);
@@ -1254,10 +1254,6 @@ skip1:
}
}
fclose(f);
- break;
- }
- default:
- abort();
}
out:
/* don't do multiple resets while some functions are still passed through */
@@ -1435,6 +1431,69 @@ int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid)
return 0;
}
+int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
+ libxl_domain_config *const d_config)
+{
+ int i, ret;
+
+ if (!d_config->b_info.u.hvm.gfx_passthru.val)
+ return 0;
+
+ for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+ uint64_t vga_iomem_start = 0xa0000 >> XC_PAGE_SHIFT;
+ uint32_t stubdom_domid;
+ libxl_device_pci *pcidev = &d_config->pcidevs[i];
+ char *pci_device_class_path =
+ libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/class",
+ pcidev->domain, pcidev->bus, pcidev->dev,
+ pcidev->func);
+ int read_items;
+ unsigned long pci_device_class;
+
+ FILE *f = fopen(pci_device_class_path, "r");
+ if (!f) {
+ LOGE(ERROR,
+ "pci device "PCI_BDF" does not have class attribute",
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ continue;
+ }
+ read_items = fscanf(f, "0x%lx\n", &pci_device_class);
+ fclose(f);
+ if (read_items != 1) {
+ LOGE(ERROR,
+ "cannot read class of pci device "PCI_BDF,
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ continue;
+ }
+ if (pci_device_class != 0x030000) /* VGA class */
+ continue;
+
+ stubdom_domid = libxl_get_stubdom_id(CTX, domid);
+ ret = xc_domain_iomem_permission(CTX->xch, stubdom_domid,
+ vga_iomem_start, 0x20, 1);
+ if (ret < 0) {
+ LOGE(ERROR,
+ "failed to give stubdom%d access to iomem range "
+ "%"PRIx64"-%"PRIx64" for VGA passthru",
+ stubdom_domid,
+ vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+ return ret;
+ }
+ ret = xc_domain_iomem_permission(CTX->xch, domid,
+ vga_iomem_start, 0x20, 1);
+ if (ret < 0) {
+ LOGE(ERROR,
+ "failed to give dom%d access to iomem range "
+ "%"PRIx64"-%"PRIx64" for VGA passthru",
+ domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+ return ret;
+ }
+ break;
+ }
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v12 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges
2014-08-30 16:29 ` [PATCH v12 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
@ 2014-09-03 11:26 ` Ian Campbell
2014-09-08 15:05 ` [PATCH 0/3] Separate the functions of the memory_mapping and iomem_permission DOMCTLs Arianna Avanzini
0 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-03 11:26 UTC (permalink / raw)
To: Arianna Avanzini
Cc: paolo.valente, keir, stefano.stabellini, Ian.Jackson,
dario.faggioli, tim, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> + if (!d_config->b_info.u.hvm.gfx_passthru.val)
You have to use libxl_defbool_val() to read a defbool, val is not a bool
in the normal sense and can't be treated as such.
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 0/3] Separate the functions of the memory_mapping and iomem_permission DOMCTLs
2014-09-03 11:26 ` Ian Campbell
@ 2014-09-08 15:05 ` Arianna Avanzini
2014-09-08 15:05 ` [PATCH 1/3] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
` (2 more replies)
0 siblings, 3 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-08 15:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, stefano.stabellini, dario.faggioli,
ian.jackson, julien.grall, etrudeau, JBeulich, avanzini.arianna,
viktor.kleinik, andrii.tseglytskyi
Hello,
this patchset originates from the twelfth version of the series "Implement the
XEN_DOMCTL_memory_mapping hypercall for ARM" ([1]). It includes only the last
three patches, whose aim was to separate the functions of the memory_mapping
and iomem_permission DOMCTLs. The memory_mapping DOMCTL's behavior, in fact,
currently defaults to implicitly granting to a domain access permission to an
I/O-memory range that was to be mapped in its guest address space, while a
specific hypercall for granting access permissions to I/O-memory ranges exists.
With respect to v12 of the original patchset, the main change concerns the use
of libxl_defbool_val() to access the value of the gfx_passthru configuration
option, as indicated by Ian Campbell. Also, the description of commit 0002 has
been fixed in its lack of clarity pointed out by Ian Campbell.
The patches have been tested on a cubieboard2, with Linux v3.15 as a dom0
and both ERIKA Enterprise ([2]) and Linux v3.15 as a domU. It has also been
tested on an x86_64 machine with Linux v3.15 as both dom0 and domU.
Any feedback is, as usual, welcome.
Thank you,
Arianna
[1] http://markmail.org/thread/cx2q7vhlwuzssmzp
[2] http://erika.tuxfamily.org/drupal/
Arianna Avanzini (3):
tools/libxl: explicitly grant access to needed I/O-memory ranges
tools/libxl: cleanup the do_pci_add() function
xen/common: do not implicitly permit access to mapped I/O memory
tools/libxl/libxl_create.c | 9 ++
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_pci.c | 213 +++++++++++++++++++++++++++----------------
xen/common/domctl.c | 36 ++------
4 files changed, 157 insertions(+), 103 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 1/3] tools/libxl: explicitly grant access to needed I/O-memory ranges
2014-09-08 15:05 ` [PATCH 0/3] Separate the functions of the memory_mapping and iomem_permission DOMCTLs Arianna Avanzini
@ 2014-09-08 15:05 ` Arianna Avanzini
2014-09-08 15:05 ` [PATCH 2/3] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
2014-09-08 15:05 ` [PATCH 3/3] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-08 15:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, stefano.stabellini, dario.faggioli,
ian.jackson, julien.grall, etrudeau, JBeulich, avanzini.arianna,
viktor.kleinik, andrii.tseglytskyi
This commit changes the existing libxl code to be sure to grant access
permission to PCI-related I/O memory ranges, while setting up passthrough
of PCI devices specified in the domain's configuration, and to VGA-related
memory ranges, while setting up VGA passthrough (if gfx_passthru = 1 in
the domain's configuration).
As for the latter, the newly-added code does not replace any existing one,
but instead matches the calls to xc_domain_memory_mapping() performed by
QEMU on the path that is executed if gfx passthru is enabled and follows
the registration of a new VGA controller (in register_vga_regions(),
defined in hw/pt-graphics.c). In fact, VGA needs some extra memory
ranges to be mapped with respect to PCI; QEMU expects that access to those
memory ranges is implicitly granted when he calls the hypervisor with the
function xc_domain_memory_mapping(): this commit calls iomem_permission
for it when needed by checking the passthru PCI device's class.
NOTE: the code added by this commit still does not verify if the passthru
of the framebuffer area is being performed for the primary GPU, but
only replicates the behavior of QEMU which is limited to performing
the passthru for all PCI devices of VGA class.
This commit is instrumental to the last one in the series, which will
separate the functions of the iomem_permission and memory_mapping DOMCTLs,
so that requesting an I/O-memory range will not imply that access to such
a range is implicitly granted.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
With respect to the v12 memory_mapping series ([1]):
- Use libxl_defbool_val() to access the value of the gfx_passthru
configuration option.
v12:
- Fix commit description and add a paragraph about outstanding issues.
v11:
- Move the VGA-related hunk to a libxl helper; evaluate gfx_passthru.val
only once as it is static in the scope of the helper.
- Remove leftover debug fprintf().
- Improve code readability by separating blocks of code.
v10:
- Use a class-based check on the PCI device to determine if VGA-related
I/O-memory ranges must be made accessible to a guest that needs gfx
passthrough.
v9:
- Allow a domain access to the VGA framebuffer only if the user has
signaled that one of the passthru GPUs is primary via domain config.
v8:
- Explain better in the commit description VGA-related code additions.
- Fix v6 changelog which, being too generic, ended up to uncorrectly
state that one of the requests had been addressed.
- Remove tentative phrases from commit description.
v7:
- Let libxl not handle I/O ports and I/O memory differently when access
to a PCI device is allowed to a domain.
- Change the construct used by libxl during PCI-related initialization
from a switch to an if to better suit the new execution flow.
[1] http://markmail.org/thread/cx2q7vhlwuzssmzp
---
tools/libxl/libxl_create.c | 9 +++++
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_pci.c | 91 ++++++++++++++++++++++++++++++++++++--------
3 files changed, 86 insertions(+), 16 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index ee328e9..8062e4d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1216,6 +1216,15 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl__spawn_stub_dm(egc, &dcs->dmss);
else
libxl__spawn_local_dm(egc, &dcs->dmss.dm);
+
+ /*
+ * Handle the domain's (and the related stubdomain's) access to
+ * the VGA framebuffer.
+ */
+ ret = libxl__grant_vga_iomem_permission(gc, domid, d_config);
+ if ( ret )
+ goto error_out;
+
return;
}
case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 04c9378..e27861b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -951,6 +951,8 @@ _hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
_hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
libxl_domain_sched_params *scparams);
+_hidden int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
+ libxl_domain_config *const d_config);
typedef struct {
uint32_t store_port;
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2782d0e..f8c980b 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -846,10 +846,13 @@ static int qemu_pci_add_xenstore(libxl__gc *gc, uint32_t domid,
static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
+ libxl_domain_type type = libxl__domain_type(gc, domid);
int rc, hvm = 0;
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
+ if (type == LIBXL_DOMAIN_TYPE_INVALID)
+ return ERROR_FAIL;
+
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
hvm = 1;
if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0) {
@@ -867,8 +870,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
}
if ( rc )
return ERROR_FAIL;
- break;
- case LIBXL_DOMAIN_TYPE_PV:
+ }
+
{
char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
pcidev->bus, pcidev->dev, pcidev->func);
@@ -937,11 +940,8 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
return ERROR_FAIL;
}
}
- break;
- }
- case LIBXL_DOMAIN_TYPE_INVALID:
- return ERROR_FAIL;
}
+
out:
if (!libxl_is_stubdom(ctx, domid, NULL)) {
rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev));
@@ -1166,6 +1166,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
{
libxl_ctx *ctx = libxl__gc_owner(gc);
libxl_device_pci *assigned;
+ libxl_domain_type type = libxl__domain_type(gc, domid);
int hvm = 0, rc, num;
int stubdomid = 0;
@@ -1181,8 +1182,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
}
rc = ERROR_FAIL;
- switch (libxl__domain_type(gc, domid)) {
- case LIBXL_DOMAIN_TYPE_HVM:
+ if (type == LIBXL_DOMAIN_TYPE_HVM) {
hvm = 1;
if (libxl__wait_for_device_model_deprecated(gc, domid, "running",
NULL, NULL, NULL) < 0)
@@ -1203,8 +1203,8 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
rc = ERROR_FAIL;
goto out_fail;
}
- break;
- case LIBXL_DOMAIN_TYPE_PV:
+ } else if (type != LIBXL_DOMAIN_TYPE_PV)
+ abort();
{
char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
pcidev->bus, pcidev->dev, pcidev->func);
@@ -1254,10 +1254,6 @@ skip1:
}
}
fclose(f);
- break;
- }
- default:
- abort();
}
out:
/* don't do multiple resets while some functions are still passed through */
@@ -1435,6 +1431,69 @@ int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid)
return 0;
}
+int libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,
+ libxl_domain_config *const d_config)
+{
+ int i, ret;
+
+ if (!libxl_defbool_val(d_config->b_info.u.hvm.gfx_passthru))
+ return 0;
+
+ for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+ uint64_t vga_iomem_start = 0xa0000 >> XC_PAGE_SHIFT;
+ uint32_t stubdom_domid;
+ libxl_device_pci *pcidev = &d_config->pcidevs[i];
+ char *pci_device_class_path =
+ libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/class",
+ pcidev->domain, pcidev->bus, pcidev->dev,
+ pcidev->func);
+ int read_items;
+ unsigned long pci_device_class;
+
+ FILE *f = fopen(pci_device_class_path, "r");
+ if (!f) {
+ LOGE(ERROR,
+ "pci device "PCI_BDF" does not have class attribute",
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ continue;
+ }
+ read_items = fscanf(f, "0x%lx\n", &pci_device_class);
+ fclose(f);
+ if (read_items != 1) {
+ LOGE(ERROR,
+ "cannot read class of pci device "PCI_BDF,
+ pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
+ continue;
+ }
+ if (pci_device_class != 0x030000) /* VGA class */
+ continue;
+
+ stubdom_domid = libxl_get_stubdom_id(CTX, domid);
+ ret = xc_domain_iomem_permission(CTX->xch, stubdom_domid,
+ vga_iomem_start, 0x20, 1);
+ if (ret < 0) {
+ LOGE(ERROR,
+ "failed to give stubdom%d access to iomem range "
+ "%"PRIx64"-%"PRIx64" for VGA passthru",
+ stubdom_domid,
+ vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+ return ret;
+ }
+ ret = xc_domain_iomem_permission(CTX->xch, domid,
+ vga_iomem_start, 0x20, 1);
+ if (ret < 0) {
+ LOGE(ERROR,
+ "failed to give dom%d access to iomem range "
+ "%"PRIx64"-%"PRIx64" for VGA passthru",
+ domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
+ return ret;
+ }
+ break;
+ }
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 2/3] tools/libxl: cleanup the do_pci_add() function
2014-09-08 15:05 ` [PATCH 0/3] Separate the functions of the memory_mapping and iomem_permission DOMCTLs Arianna Avanzini
2014-09-08 15:05 ` [PATCH 1/3] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
@ 2014-09-08 15:05 ` Arianna Avanzini
2014-09-08 15:05 ` [PATCH 3/3] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-08 15:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, stefano.stabellini, dario.faggioli,
ian.jackson, julien.grall, etrudeau, JBeulich, avanzini.arianna,
viktor.kleinik, andrii.tseglytskyi
This function modifies the do_pci_add() function in libxl_pci.c
by unindenting a code block whose condition was removed in the
previous commit. The block was left as is to facilitate functional
review of the previous commit; this commit cleans it up.
This commit introduces no functional change.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
With respect to the v12 memory_mapping series ([1]):
- Fix commit description using the correct terminology to describe
the cleanup being performed.
[1] http://markmail.org/thread/cx2q7vhlwuzssmzp
---
tools/libxl/libxl_pci.c | 122 ++++++++++++++++++++++++------------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index f8c980b..cedb758 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -847,7 +847,10 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
{
libxl_ctx *ctx = libxl__gc_owner(gc);
libxl_domain_type type = libxl__domain_type(gc, domid);
- int rc, hvm = 0;
+ char *sysfs_path;
+ FILE *f;
+ unsigned long long start, end, flags, size;
+ int irq, i, rc, hvm = 0;
if (type == LIBXL_DOMAIN_TYPE_INVALID)
return ERROR_FAIL;
@@ -872,73 +875,70 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
return ERROR_FAIL;
}
- {
- char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
- pcidev->bus, pcidev->dev, pcidev->func);
- FILE *f = fopen(sysfs_path, "r");
- unsigned long long start = 0, end = 0, flags = 0, size = 0;
- int irq = 0;
- int i;
+ sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func);
+ f = fopen(sysfs_path, "r");
+ start = end = flags = size = 0;
+ irq = 0;
- if (f == NULL) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
- return ERROR_FAIL;
- }
- for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
- if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
- continue;
- size = end - start + 1;
- if (start) {
- if (flags & PCI_BAR_IO) {
- rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
- fclose(f);
- return ERROR_FAIL;
- }
- } else {
- rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
- (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
- fclose(f);
- return ERROR_FAIL;
- }
+ if (f == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
+ return ERROR_FAIL;
+ }
+ for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
+ if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
+ continue;
+ size = end - start + 1;
+ if (start) {
+ if (flags & PCI_BAR_IO) {
+ rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
+ fclose(f);
+ return ERROR_FAIL;
+ }
+ } else {
+ rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
+ (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
+ fclose(f);
+ return ERROR_FAIL;
}
}
}
- fclose(f);
- sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
- pcidev->bus, pcidev->dev, pcidev->func);
- f = fopen(sysfs_path, "r");
- if (f == NULL) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
- goto out;
+ }
+ fclose(f);
+ sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func);
+ f = fopen(sysfs_path, "r");
+ if (f == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
+ goto out;
+ }
+ if ((fscanf(f, "%u", &irq) == 1) && irq) {
+ rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_physdev_map_pirq irq=%d", irq);
+ fclose(f);
+ return ERROR_FAIL;
}
- if ((fscanf(f, "%u", &irq) == 1) && irq) {
- rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_physdev_map_pirq irq=%d", irq);
- fclose(f);
- return ERROR_FAIL;
- }
- rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_irq_permission irq=%d", irq);
- fclose(f);
- return ERROR_FAIL;
- }
+ rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_irq_permission irq=%d", irq);
+ fclose(f);
+ return ERROR_FAIL;
}
- fclose(f);
+ }
+ fclose(f);
- /* Don't restrict writes to the PCI config space from this VM */
- if (pcidev->permissive) {
- if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
- pcidev) < 0 ) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "Setting permissive for device");
- return ERROR_FAIL;
- }
+ /* Don't restrict writes to the PCI config space from this VM */
+ if (pcidev->permissive) {
+ if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
+ pcidev) < 0 ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Setting permissive for device");
+ return ERROR_FAIL;
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 3/3] xen/common: do not implicitly permit access to mapped I/O memory
2014-09-08 15:05 ` [PATCH 0/3] Separate the functions of the memory_mapping and iomem_permission DOMCTLs Arianna Avanzini
2014-09-08 15:05 ` [PATCH 1/3] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
2014-09-08 15:05 ` [PATCH 2/3] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
@ 2014-09-08 15:05 ` Arianna Avanzini
2 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-08 15:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, stefano.stabellini, dario.faggioli,
ian.jackson, julien.grall, etrudeau, JBeulich, avanzini.arianna,
viktor.kleinik, andrii.tseglytskyi
Currently, the XEN_DOMCTL_memory_mapping hypercall implicitly grants
to a domain access permission to the I/O memory areas mapped in its
guest address space. This conflicts with the presence of a specific
hypercall (XEN_DOMCTL_iomem_permission) used to grant such a permission
to a domain.
This commit separates the functions of the two hypercalls by having only
the latter be able to permit I/O memory access to a domain, and the
former just performing the mapping after a permissions check on both the
granting and the grantee domains.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v8:
- Drop iomem_permission-related changes.
- Conservatively check both the granting and the grantee domains'
permissions in the memory_mapping DOMCTL.
- Remove tentative phrases from commit description.
v7:
- Let iomem_permission check if the calling domain is allowed to access
memory ranges to be mapped to a domain. Remove such a check from the
memory_mapping hypercall.
---
xen/common/domctl.c | 36 ++++++++++--------------------------
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 80b7800..04ecd53 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -917,7 +917,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
break;
ret = -EPERM;
- if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+ if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ||
+ !iomem_access_permitted(d, mfn, mfn_end) )
break;
ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
@@ -930,40 +931,23 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
"memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- ret = iomem_permit_access(d, mfn, mfn_end);
- if ( !ret )
- {
- ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
- if ( ret )
- {
- printk(XENLOG_G_WARNING
- "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
- d->domain_id, gfn, mfn, nr_mfns, ret);
- if ( iomem_deny_access(d, mfn, mfn_end) &&
- is_hardware_domain(current->domain) )
- printk(XENLOG_ERR
- "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
- d->domain_id, mfn, mfn_end);
- }
- }
+ ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
+ if ( ret )
+ printk(XENLOG_G_WARNING
+ "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
+ d->domain_id, gfn, mfn, nr_mfns, ret);
}
else
{
- int rc = 0;
-
printk(XENLOG_G_INFO
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
- ret = iomem_deny_access(d, mfn, mfn_end);
- if ( !ret )
- ret = rc;
+ ret = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
if ( ret && is_hardware_domain(current->domain) )
printk(XENLOG_ERR
- "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
- ret, rc ? "removing" : "denying", d->domain_id,
- mfn, mfn_end);
+ "memory_map: error %ld removing dom%d access to [%lx,%lx]\n",
+ ret, d->domain_id, mfn, mfn_end);
}
/* Do this unconditionally to cover errors on above failure paths. */
memory_type_changed(d);
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v12 13/14] tools/libxl: cleanup the do_pci_add() function
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (11 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 12/14] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-09-03 11:27 ` Ian Campbell
2014-08-30 16:29 ` [PATCH v12 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2014-09-03 12:15 ` [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Ian Campbell
14 siblings, 1 reply; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
This function modifies the do_pci_add() function in libxl_pci.c
by bringing back a code block whose condition was removed in the
previous commit. The block was left as is to facilitate functional
review of the previous commit; this commit cleans it up.
This commit introduces no functional change.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
tools/libxl/libxl_pci.c | 122 ++++++++++++++++++++++++------------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4fc9218..ef8cdaa 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -847,7 +847,10 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
{
libxl_ctx *ctx = libxl__gc_owner(gc);
libxl_domain_type type = libxl__domain_type(gc, domid);
- int rc, hvm = 0;
+ char *sysfs_path;
+ FILE *f;
+ unsigned long long start, end, flags, size;
+ int irq, i, rc, hvm = 0;
if (type == LIBXL_DOMAIN_TYPE_INVALID)
return ERROR_FAIL;
@@ -872,73 +875,70 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, i
return ERROR_FAIL;
}
- {
- char *sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
- pcidev->bus, pcidev->dev, pcidev->func);
- FILE *f = fopen(sysfs_path, "r");
- unsigned long long start = 0, end = 0, flags = 0, size = 0;
- int irq = 0;
- int i;
+ sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func);
+ f = fopen(sysfs_path, "r");
+ start = end = flags = size = 0;
+ irq = 0;
- if (f == NULL) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
- return ERROR_FAIL;
- }
- for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
- if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
- continue;
- size = end - start + 1;
- if (start) {
- if (flags & PCI_BAR_IO) {
- rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
- fclose(f);
- return ERROR_FAIL;
- }
- } else {
- rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
- (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
- fclose(f);
- return ERROR_FAIL;
- }
+ if (f == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
+ return ERROR_FAIL;
+ }
+ for (i = 0; i < PROC_PCI_NUM_RESOURCES; i++) {
+ if (fscanf(f, "0x%llx 0x%llx 0x%llx\n", &start, &end, &flags) != 3)
+ continue;
+ size = end - start + 1;
+ if (start) {
+ if (flags & PCI_BAR_IO) {
+ rc = xc_domain_ioport_permission(ctx->xch, domid, start, size, 1);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_ioport_permission error 0x%llx/0x%llx", start, size);
+ fclose(f);
+ return ERROR_FAIL;
+ }
+ } else {
+ rc = xc_domain_iomem_permission(ctx->xch, domid, start>>XC_PAGE_SHIFT,
+ (size+(XC_PAGE_SIZE-1))>>XC_PAGE_SHIFT, 1);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_iomem_permission error 0x%llx/0x%llx", start, size);
+ fclose(f);
+ return ERROR_FAIL;
}
}
}
- fclose(f);
- sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
- pcidev->bus, pcidev->dev, pcidev->func);
- f = fopen(sysfs_path, "r");
- if (f == NULL) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
- goto out;
+ }
+ fclose(f);
+ sysfs_path = libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/irq", pcidev->domain,
+ pcidev->bus, pcidev->dev, pcidev->func);
+ f = fopen(sysfs_path, "r");
+ if (f == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't open %s", sysfs_path);
+ goto out;
+ }
+ if ((fscanf(f, "%u", &irq) == 1) && irq) {
+ rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_physdev_map_pirq irq=%d", irq);
+ fclose(f);
+ return ERROR_FAIL;
}
- if ((fscanf(f, "%u", &irq) == 1) && irq) {
- rc = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_physdev_map_pirq irq=%d", irq);
- fclose(f);
- return ERROR_FAIL;
- }
- rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
- if (rc < 0) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_irq_permission irq=%d", irq);
- fclose(f);
- return ERROR_FAIL;
- }
+ rc = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
+ if (rc < 0) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error: xc_domain_irq_permission irq=%d", irq);
+ fclose(f);
+ return ERROR_FAIL;
}
- fclose(f);
+ }
+ fclose(f);
- /* Don't restrict writes to the PCI config space from this VM */
- if (pcidev->permissive) {
- if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
- pcidev) < 0 ) {
- LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
- "Setting permissive for device");
- return ERROR_FAIL;
- }
+ /* Don't restrict writes to the PCI config space from this VM */
+ if (pcidev->permissive) {
+ if ( sysfs_write_bdf(gc, SYSFS_PCIBACK_DRIVER"/permissive",
+ pcidev) < 0 ) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Setting permissive for device");
+ return ERROR_FAIL;
}
}
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v12 13/14] tools/libxl: cleanup the do_pci_add() function
2014-08-30 16:29 ` [PATCH v12 13/14] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
@ 2014-09-03 11:27 ` Ian Campbell
0 siblings, 0 replies; 41+ messages in thread
From: Ian Campbell @ 2014-09-03 11:27 UTC (permalink / raw)
To: Arianna Avanzini
Cc: paolo.valente, keir, stefano.stabellini, Ian.Jackson,
dario.faggioli, tim, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> This function modifies the do_pci_add() function in libxl_pci.c
> by bringing back a code block whose condition was removed in the
> previous commit. The block was left as is to facilitate functional
> review of the previous commit; this commit cleans it up.
> This commit introduces no functional change.
By "bring back" I think you mean reindent (as opposed to
reinstating/recovering some deleted code). Assuming so:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v12 14/14] xen/common: do not implicitly permit access to mapped I/O memory
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (12 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 13/14] tools/libxl: cleanup the do_pci_add() function Arianna Avanzini
@ 2014-08-30 16:29 ` Arianna Avanzini
2014-09-03 12:15 ` [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Ian Campbell
14 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-08-30 16:29 UTC (permalink / raw)
To: xen-devel
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
Ian.Jackson, dario.faggioli, tim, julien.grall, etrudeau,
andrew.cooper3, JBeulich, avanzini.arianna, viktor.kleinik,
andrii.tseglytskyi
Currently, the XEN_DOMCTL_memory_mapping hypercall implicitly grants
to a domain access permission to the I/O memory areas mapped in its
guest address space. This conflicts with the presence of a specific
hypercall (XEN_DOMCTL_iomem_permission) used to grant such a permission
to a domain.
This commit separates the functions of the two hypercalls by having only
the latter be able to permit I/O memory access to a domain, and the
former just performing the mapping after a permissions check on both the
granting and the grantee domains.
Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.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>
Cc: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
---
v8:
- Drop iomem_permission-related changes.
- Conservatively check both the granting and the grantee domains'
permissions in the memory_mapping DOMCTL.
- Remove tentative phrases from commit description.
v7:
- Let iomem_permission check if the calling domain is allowed to access
memory ranges to be mapped to a domain. Remove such a check from the
memory_mapping hypercall.
---
xen/common/domctl.c | 36 ++++++++++--------------------------
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 80b7800..04ecd53 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -917,7 +917,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
break;
ret = -EPERM;
- if ( !iomem_access_permitted(current->domain, mfn, mfn_end) )
+ if ( !iomem_access_permitted(current->domain, mfn, mfn_end) ||
+ !iomem_access_permitted(d, mfn, mfn_end) )
break;
ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn_end, add);
@@ -930,40 +931,23 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
"memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- ret = iomem_permit_access(d, mfn, mfn_end);
- if ( !ret )
- {
- ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
- if ( ret )
- {
- printk(XENLOG_G_WARNING
- "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
- d->domain_id, gfn, mfn, nr_mfns, ret);
- if ( iomem_deny_access(d, mfn, mfn_end) &&
- is_hardware_domain(current->domain) )
- printk(XENLOG_ERR
- "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
- d->domain_id, mfn, mfn_end);
- }
- }
+ ret = map_mmio_regions(d, gfn, nr_mfns, mfn);
+ if ( ret )
+ printk(XENLOG_G_WARNING
+ "memory_map:fail: dom%d gfn=%lx mfn=%lx nr=%lx ret:%ld\n",
+ d->domain_id, gfn, mfn, nr_mfns, ret);
}
else
{
- int rc = 0;
-
printk(XENLOG_G_INFO
"memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
d->domain_id, gfn, mfn, nr_mfns);
- rc = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
- ret = iomem_deny_access(d, mfn, mfn_end);
- if ( !ret )
- ret = rc;
+ ret = unmap_mmio_regions(d, gfn, nr_mfns, mfn);
if ( ret && is_hardware_domain(current->domain) )
printk(XENLOG_ERR
- "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
- ret, rc ? "removing" : "denying", d->domain_id,
- mfn, mfn_end);
+ "memory_map: error %ld removing dom%d access to [%lx,%lx]\n",
+ ret, d->domain_id, mfn, mfn_end);
}
/* Do this unconditionally to cover errors on above failure paths. */
memory_type_changed(d);
--
2.1.0
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-30 16:29 [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (13 preceding siblings ...)
2014-08-30 16:29 ` [PATCH v12 14/14] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
@ 2014-09-03 12:15 ` Ian Campbell
2014-09-03 13:55 ` Arianna Avanzini
14 siblings, 1 reply; 41+ messages in thread
From: Ian Campbell @ 2014-09-03 12:15 UTC (permalink / raw)
To: Arianna Avanzini
Cc: paolo.valente, keir, stefano.stabellini, Ian.Jackson,
dario.faggioli, tim, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> here's a twelfth version of my implementation proposal for the memory_mapping
I've applied up to and including #9, including picking up the resent
copy of #2.
Ian.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-09-03 12:15 ` [PATCH v12 00/14] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Ian Campbell
@ 2014-09-03 13:55 ` Arianna Avanzini
0 siblings, 0 replies; 41+ messages in thread
From: Arianna Avanzini @ 2014-09-03 13:55 UTC (permalink / raw)
To: Ian Campbell
Cc: paolo.valente, keir, stefano.stabellini, Ian.Jackson,
dario.faggioli, tim, xen-devel, julien.grall, etrudeau, JBeulich,
andrew.cooper3, viktor.kleinik, andrii.tseglytskyi
On Wed, Sep 03, 2014 at 01:15:29PM +0100, Ian Campbell wrote:
> On Sat, 2014-08-30 at 18:29 +0200, Arianna Avanzini wrote:
> > here's a twelfth version of my implementation proposal for the memory_mapping
>
> I've applied up to and including #9, including picking up the resent
> copy of #2.
Thank you, I'll wait for feedback from Daniel De Graaf on patch #11 and I'll
resend the patches from #10 to #14 (with your comments addressed).
>
> Ian.
>
>
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 41+ messages in thread