* [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
@ 2014-07-28 22:11 ` Arianna Avanzini
2014-07-29 11:55 ` Julien Grall
2014-07-28 22:11 ` [PATCH v10 02/12] arch/arm: unmap partially-mapped I/O-memory regions Arianna Avanzini
` (11 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:11 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
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>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Paolo Valente <paolo.valente@unimore.it>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Eric Trudeau <etrudeau@broadcom.com>
Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
---
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 | 56 ++++++++++++++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 143199b..8feb5c5 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;
}
@@ -609,26 +610,42 @@ static int apply_one_level(struct domain *d,
if ( p2m_table(orig_pte) )
return P2M_ONE_DESCEND;
- if ( op == REMOVE &&
- !is_mapping_aligned(*addr, end_gpaddr,
- 0, /* maddr doesn't matter for remove */
- level_size) )
+ if ( op == REMOVE )
{
- /*
- * Removing a mapping from the middle of a superpage. Shatter
- * and descend.
- */
- *flush = true;
- rc = p2m_create_table(d, entry,
- level_shift - PAGE_SHIFT, flush_cache);
- if ( rc < 0 )
- return rc;
-
- p2m->stats.shattered[level]++;
- p2m->stats.mappings[level]--;
- p2m->stats.mappings[level+1] += LPAE_ENTRIES;
-
- return P2M_ONE_DESCEND;
+ if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) )
+ {
+ unsigned long mfn = orig_pte.p2m.base;
+ /*
+ * 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 mfn
+ * is the machine frame number actually mapped to the
+ * guest address: check if the two correspond.
+ */
+ if ( *maddr != pfn_to_paddr(mfn) )
+ printk("p2m_remove dom%d: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected\n",
+ d->domain_id, *addr, pfn_to_paddr(mfn), *maddr);
+ }
+ else
+ {
+ /*
+ * Removing a mapping from the middle of a superpage. Shatter
+ * and descend.
+ */
+ *flush = true;
+ rc = p2m_create_table(d, entry,
+ level_shift - PAGE_SHIFT, flush_cache);
+ if ( rc < 0 )
+ return rc;
+
+ p2m->stats.shattered[level]++;
+ p2m->stats.mappings[level]--;
+ p2m->stats.mappings[level+1] += LPAE_ENTRIES;
+
+ return P2M_ONE_DESCEND;
+ }
}
}
@@ -638,6 +655,7 @@ static int apply_one_level(struct domain *d,
p2m_write_pte(entry, pte, flush_cache);
*addr += level_size;
+ *maddr += level_size;
p2m->stats.mappings[level]--;
--
2.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes
2014-07-28 22:11 ` [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
@ 2014-07-29 11:55 ` Julien Grall
2014-07-29 12:01 ` Ian Campbell
2014-07-29 12:02 ` Andrew Cooper
0 siblings, 2 replies; 34+ messages in thread
From: Julien Grall @ 2014-07-29 11:55 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
Hi Arianna,
On 07/28/2014 11:11 PM, Arianna Avanzini wrote:
> @@ -609,26 +610,42 @@ static int apply_one_level(struct domain *d,
> if ( p2m_table(orig_pte) )
> return P2M_ONE_DESCEND;
>
> - if ( op == REMOVE &&
> - !is_mapping_aligned(*addr, end_gpaddr,
> - 0, /* maddr doesn't matter for remove */
> - level_size) )
> + if ( op == REMOVE )
> {
> - /*
> - * Removing a mapping from the middle of a superpage. Shatter
> - * and descend.
> - */
> - *flush = true;
> - rc = p2m_create_table(d, entry,
> - level_shift - PAGE_SHIFT, flush_cache);
> - if ( rc < 0 )
> - return rc;
> -
> - p2m->stats.shattered[level]++;
> - p2m->stats.mappings[level]--;
> - p2m->stats.mappings[level+1] += LPAE_ENTRIES;
> -
> - return P2M_ONE_DESCEND;
> + if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) )
> + {
You don't cover every case with this solution. This code is only called
when (level < 3). So for single 4k (level 3), Xen will not print a
message when the MFN differ.
I still think that the best place to check it, is after the if ( level < 3 )
The code would look like:
if ( level < 3 )
{
....
}
if ( op == REMOVE && pfn_to_paddr(orig_pte.p2m.base) != *maddr )
printk(XENLOG_ERR "p2m_remove"....);
> + unsigned long mfn = orig_pte.p2m.base;
> + /*
> + * 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 mfn
> + * is the machine frame number actually mapped to the
> + * guest address: check if the two correspond.
> + */
> + if ( *maddr != pfn_to_paddr(mfn) )
> + printk("p2m_remove dom%d: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected\n",
You have to prefix it at least by XENLOG_WARNING or XENLOG_ERR
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes
2014-07-29 11:55 ` Julien Grall
@ 2014-07-29 12:01 ` Ian Campbell
2014-07-29 12:02 ` Andrew Cooper
1 sibling, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2014-07-29 12:01 UTC (permalink / raw)
To: Julien Grall
Cc: paolo.valente, keir, stefano.stabellini, tim, dario.faggioli,
Ian.Jackson, xen-devel, julien.grall, etrudeau, andrew.cooper3,
JBeulich, Arianna Avanzini, viktor.kleinik
On Tue, 2014-07-29 at 12:55 +0100, Julien Grall wrote:
> > - return P2M_ONE_DESCEND;
> > + if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) )
> > + {
>
> You don't cover every case with this solution. This code is only called
> when (level < 3). So for single 4k (level 3), Xen will not print a
> message when the MFN differ.
>
> I still think that the best place to check it, is after the if ( level < 3 )
>
> The code would look like:
>
> if ( level < 3 )
> {
> ....
> }
>
> if ( op == REMOVE && pfn_to_paddr(orig_pte.p2m.base) != *maddr )
> printk(XENLOG_ERR "p2m_remove"....);
I agree, and this suggestion looks correct to me.
Ian.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes
2014-07-29 11:55 ` Julien Grall
2014-07-29 12:01 ` Ian Campbell
@ 2014-07-29 12:02 ` Andrew Cooper
1 sibling, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2014-07-29 12:02 UTC (permalink / raw)
To: Julien Grall, Arianna Avanzini, xen-devel
Cc: julien.grall, paolo.valente, keir, stefano.stabellini, tim,
dario.faggioli, Ian.Jackson, Ian.Campbell, etrudeau, JBeulich,
viktor.kleinik
On 29/07/14 12:55, Julien Grall wrote:
> Hi Arianna,
>
> On 07/28/2014 11:11 PM, Arianna Avanzini wrote:
>> @@ -609,26 +610,42 @@ static int apply_one_level(struct domain *d,
>> if ( p2m_table(orig_pte) )
>> return P2M_ONE_DESCEND;
>>
>> - if ( op == REMOVE &&
>> - !is_mapping_aligned(*addr, end_gpaddr,
>> - 0, /* maddr doesn't matter for remove */
>> - level_size) )
>> + if ( op == REMOVE )
>> {
>> - /*
>> - * Removing a mapping from the middle of a superpage. Shatter
>> - * and descend.
>> - */
>> - *flush = true;
>> - rc = p2m_create_table(d, entry,
>> - level_shift - PAGE_SHIFT, flush_cache);
>> - if ( rc < 0 )
>> - return rc;
>> -
>> - p2m->stats.shattered[level]++;
>> - p2m->stats.mappings[level]--;
>> - p2m->stats.mappings[level+1] += LPAE_ENTRIES;
>> -
>> - return P2M_ONE_DESCEND;
>> + if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) )
>> + {
> You don't cover every case with this solution. This code is only called
> when (level < 3). So for single 4k (level 3), Xen will not print a
> message when the MFN differ.
>
> I still think that the best place to check it, is after the if ( level < 3 )
>
> The code would look like:
>
> if ( level < 3 )
> {
> ....
> }
>
> if ( op == REMOVE && pfn_to_paddr(orig_pte.p2m.base) != *maddr )
> printk(XENLOG_ERR "p2m_remove"....);
>
>> + unsigned long mfn = orig_pte.p2m.base;
>> + /*
>> + * 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 mfn
>> + * is the machine frame number actually mapped to the
>> + * guest address: check if the two correspond.
>> + */
>> + if ( *maddr != pfn_to_paddr(mfn) )
>> + printk("p2m_remove dom%d: mapping at %"PRIpaddr" is of maddr %"PRIpaddr" not %"PRIpaddr" as expected\n",
> You have to prefix it at least by XENLOG_WARNING or XENLOG_ERR
>
> Regards,
>
XENLOG_G_WARNING or XENLOG_G_ERR
Guest printk()s are rate limited differently to core hypervisor
printk()s, and have a separate controllable loglevel.
~Andrew
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 02/12] arch/arm: unmap partially-mapped I/O-memory regions
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-07-28 22:11 ` [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
@ 2014-07-28 22:11 ` Arianna Avanzini
2014-07-29 12:00 ` Julien Grall
2014-07-28 22:12 ` [PATCH v10 03/12] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
` (10 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:11 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
This commit changes the function apply_p2m_changes() to destroy changes
performed while mapping an I/O-memory region, if errors are seen during
the operation. The implemented behaviour is the counterpart of the x86
code, which destroys the whole range: this is useful to avoid that I/O-
memory areas are 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>
---
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 | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8feb5c5..2745279 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -713,7 +713,7 @@ 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 long cur_first_page = ~0,
cur_first_offset = ~0,
cur_second_offset = ~0;
@@ -844,6 +844,10 @@ out:
if (third) unmap_domain_page(third);
if (second) unmap_domain_page(second);
if (first) unmap_domain_page(first);
+ if ( rc < 0 && mattr == MATTR_DEV && ( op == INSERT || op == ALLOCATE ) )
+ apply_p2m_changes(d, REMOVE, start_gpaddr,
+ end_gpaddr, orig_maddr,
+ MATTR_DEV, p2m_invalid);
spin_unlock(&p2m->lock);
--
2.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 02/12] arch/arm: unmap partially-mapped I/O-memory regions
2014-07-28 22:11 ` [PATCH v10 02/12] arch/arm: unmap partially-mapped I/O-memory regions Arianna Avanzini
@ 2014-07-29 12:00 ` Julien Grall
0 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2014-07-29 12:00 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
Hi Arianna,
On 07/28/2014 11:11 PM, Arianna Avanzini wrote:
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 8feb5c5..2745279 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -713,7 +713,7 @@ 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 long cur_first_page = ~0,
> cur_first_offset = ~0,
> cur_second_offset = ~0;
> @@ -844,6 +844,10 @@ out:
> if (third) unmap_domain_page(third);
> if (second) unmap_domain_page(second);
> if (first) unmap_domain_page(first);
> + if ( rc < 0 && mattr == MATTR_DEV && ( op == INSERT || op == ALLOCATE ) )
> + apply_p2m_changes(d, REMOVE, start_gpaddr,
> + end_gpaddr, orig_maddr,
> + MATTR_DEV, p2m_invalid);
I though we agreed to handle remove for any kind of mapping... Why did
you change to only apply for I/O-memory regions?
Hence, testing mattr to check if it's an MMIO regions looks wrong to me.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 03/12] arch/x86: warn if to-be-removed mapping does not exist
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
2014-07-28 22:11 ` [PATCH v10 01/12] arch/arm: add consistency check to REMOVE p2m changes Arianna Avanzini
2014-07-28 22:11 ` [PATCH v10 02/12] arch/arm: unmap partially-mapped I/O-memory regions Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-28 22:12 ` [PATCH v10 04/12] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
` (9 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
---
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 2c7bc0f..297c940 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 0ddbadb..5ab7cb4 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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v10 04/12] arch/x86: cleanup memory_mapping DOMCTL
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (2 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 03/12] arch/x86: warn if to-be-removed mapping does not exist Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-28 22:12 ` [PATCH v10 05/12] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
` (8 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
---
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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v10 05/12] xen/common: add ARM stub for the function memory_type_changed()
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (3 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 04/12] arch/x86: cleanup memory_mapping DOMCTL Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-28 22:12 ` [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
` (7 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
---
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 2745279..ffce7d6 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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (4 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 05/12] xen/common: add ARM stub for the function memory_type_changed() Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-29 7:27 ` Jan Beulich
2014-07-29 12:02 ` Julien Grall
2014-07-28 22:12 ` [PATCH v10 07/12] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
` (6 subsequent siblings)
12 siblings, 2 replies; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
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: 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>
---
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 ffce7d6..753c8d6 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -878,6 +878,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 297c940..116d895 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1722,6 +1722,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 5ab7cb4..cea46e1 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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL
2014-07-28 22:12 ` [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
@ 2014-07-29 7:27 ` Jan Beulich
2014-07-29 12:02 ` Julien Grall
1 sibling, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2014-07-29 7:27 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
andrew.cooper3, dario.faggioli, Ian.Jackson, xen-devel,
julien.grall, etrudeau, tim, viktor.kleinik
>>> On 29.07.14 at 00:12, <avanzini.arianna@gmail.com> wrote:
> 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>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL
2014-07-28 22:12 ` [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
2014-07-29 7:27 ` Jan Beulich
@ 2014-07-29 12:02 ` Julien Grall
1 sibling, 0 replies; 34+ messages in thread
From: Julien Grall @ 2014-07-29 12:02 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
Hi Arianna,
On 07/28/2014 11:12 PM, Arianna Avanzini wrote:
> 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: Julien Grall <julien.grall@linaro.org>
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 07/12] xen/common: move the memory_mapping DOMCTL hypercall to common code
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (5 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 06/12] xen/x86: factor out map and unmap from the memory_mapping DOMCTL Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-30 15:59 ` Julien Grall
2014-07-28 22:12 ` [PATCH v10 08/12] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
` (5 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
---
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 753c8d6..2a77bee 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -868,24 +868,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 cea46e1..2a65848 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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 07/12] xen/common: move the memory_mapping DOMCTL hypercall to common code
2014-07-28 22:12 ` [PATCH v10 07/12] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
@ 2014-07-30 15:59 ` Julien Grall
0 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2014-07-30 15:59 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
Hi Arianna,
On 07/28/2014 11:12 PM, Arianna Avanzini wrote:
> 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>
I've just noticed that this patch won't work correctly on ARM with XSM
enabled.
When flask is used, the error message will be printed no matters how XSM
has been configured. This is because flask_domctl
(xen/xsm/flask/hooks.c) only handle XEN_DOMCTL_memory_mapping for x86
(see #ifdef CONFIG_X86 within the function).
Can you handle this case?
Many thanks,
--
Julien Grall
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 08/12] tools/libxl: parse optional start gfn from the iomem config option
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (6 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 07/12] xen/common: move the memory_mapping DOMCTL hypercall to common code Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-08-04 13:39 ` Julien Grall
2014-07-28 22:12 ` [PATCH v10 09/12] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
` (4 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
---
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 | 6 ++++++
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_types.idl | 4 ++++
tools/libxl/xl_cmdimpl.c | 17 ++++++++++-------
6 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index ff9ea77..5bad491 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -610,12 +610,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 5ae6532..b1a20da 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 0686f96..3bfe65b 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;
@@ -200,6 +202,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 a412f9c..75b5dd0 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 01bce2f..e035e97 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1140,6 +1140,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) {
@@ -1153,13 +1154,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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 08/12] tools/libxl: parse optional start gfn from the iomem config option
2014-07-28 22:12 ` [PATCH v10 08/12] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
@ 2014-08-04 13:39 ` Julien Grall
0 siblings, 0 replies; 34+ messages in thread
From: Julien Grall @ 2014-08-04 13:39 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
Hi Arianna,
On 07/28/2014 11:12 PM, Arianna Avanzini wrote:
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 0686f96..3bfe65b 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;
> +
I've tried to build this series and get the following error in libxl:
libxl_create.c: In function ‘libxl__domain_build_info_setdefault’:
libxl_create.c:195:13: error: declaration of ‘i’ shadows a previous local [-Werror=shadow]
int i;
^
libxl_create.c:106:9: error: shadowed declaration is here [-Werror=shadow]
int i;
^
cc1: all warnings being treated as errors
I think you have to drop the variable i defined around line 195.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 09/12] tools/libxl: handle the iomem parameter with the memory_mapping hcall
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (7 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 08/12] tools/libxl: parse optional start gfn from the iomem config option Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-28 22:12 ` [PATCH v10 10/12] tools/libxl: move PCI-related macros in a header file Arianna Avanzini
` (3 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
---
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 0230c6c..27fe3b6 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1959,6 +1959,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 3bfe65b..1217310 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1158,6 +1158,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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v10 10/12] tools/libxl: move PCI-related macros in a header file
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (8 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 09/12] tools/libxl: handle the iomem parameter with the memory_mapping hcall Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-28 22:12 ` [PATCH v10 11/12] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
` (2 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
Currently, the PCI-related macro used by libxl are in the libxl_pci.c
source file. This commit moves them to a new libxl_pci.h header; such a
change is instrumental to the following commit, as it lets other libxl
source files be able to access the macros by including the header.
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/libxl/libxl_pci.c | 7 +------
tools/libxl/libxl_pci.h | 10 ++++++++++
2 files changed, 11 insertions(+), 6 deletions(-)
create mode 100644 tools/libxl/libxl_pci.h
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2782d0e..21ecab1 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -17,12 +17,7 @@
#include "libxl_osdeps.h" /* must come before any other headers */
#include "libxl_internal.h"
-
-#define PCI_BDF "%04x:%02x:%02x.%01x"
-#define PCI_BDF_SHORT "%02x:%02x.%01x"
-#define PCI_BDF_VDEVFN "%04x:%02x:%02x.%01x@%02x"
-#define PCI_OPTIONS "msitranslate=%d,power_mgmt=%d"
-#define PCI_BDF_XSPATH "%04x-%02x-%02x-%01x"
+#include "libxl_pci.h"
static unsigned int pcidev_encode_bdf(libxl_device_pci *pcidev)
{
diff --git a/tools/libxl/libxl_pci.h b/tools/libxl/libxl_pci.h
new file mode 100644
index 0000000..ed0e45a
--- /dev/null
+++ b/tools/libxl/libxl_pci.h
@@ -0,0 +1,10 @@
+#ifndef LIBXL_PCI_H
+#define LIBXL_PCI_H
+
+#define PCI_BDF "%04x:%02x:%02x.%01x"
+#define PCI_BDF_SHORT "%02x:%02x.%01x"
+#define PCI_BDF_VDEVFN "%04x:%02x:%02x.%01x@%02x"
+#define PCI_OPTIONS "msitranslate=%d,power_mgmt=%d"
+#define PCI_BDF_XSPATH "%04x-%02x-%02x-%01x"
+
+#endif /* LIBXL_PCI_H */
--
2.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH v10 11/12] tools/libxl: explicitly grant access to needed I/O-memory ranges
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (9 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 10/12] tools/libxl: move PCI-related macros in a header file Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-30 11:33 ` Ian Campbell
2014-07-28 22:12 ` [PATCH v10 12/12] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
2014-08-18 19:50 ` [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Andrii Tseglytskyi
12 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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 and
gfx_passthru_primary = 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 for a primary GPU by checking the passthru PCI devices'
class.
This commit is instrumental to the following one, 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>
---
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 | 60 ++++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_pci.c | 26 ++++++++------------
tools/libxl/xl_cmdimpl.c | 1 +
3 files changed, 71 insertions(+), 16 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1217310..b5bc07a 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -19,6 +19,7 @@
#include "libxl_internal.h"
#include "libxl_arch.h"
+#include "libxl_pci.h"
#include <xc_dom.h>
#include <xenguest.h>
@@ -1194,6 +1195,65 @@ 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);
+
+ /*
+ * If VGA passthru is enabled by domain config and one of the
+ * passthru GPUs is of VGA class, be sure that the related
+ * stubdomain and the domain itself can access the VGA
+ * framebuffer.
+ */
+ for (i = 0 ; d_config->b_info.u.hvm.gfx_passthru.val &&
+ i < d_config->num_pcidevs ; i++) {
+ uint64_t vga_iomem_start = 0xa0000 >> XC_PAGE_SHIFT;
+ int 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)
+ 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));
+ goto error_out;
+ }
+ 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));
+ goto error_out;
+ }
+ break;
+ }
+
return;
}
case LIBXL_DOMAIN_TYPE_PV:
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 21ecab1..1524170 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -841,10 +841,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) {
@@ -862,8 +865,7 @@ 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);
@@ -932,10 +934,6 @@ 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)) {
@@ -1161,6 +1159,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;
@@ -1176,8 +1175,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)
@@ -1198,8 +1196,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);
@@ -1249,10 +1247,6 @@ skip1:
}
}
fclose(f);
- break;
- }
- default:
- abort();
}
out:
/* don't do multiple resets while some functions are still passed through */
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e035e97..7f1bb6b 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1422,6 +1422,7 @@ skip_vfb:
if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
d_config->num_pcidevs = 0;
d_config->pcidevs = NULL;
+ fprintf(stderr, "XEN parsing pci list\n");
for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) {
libxl_device_pci *pcidev;
--
2.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 11/12] tools/libxl: explicitly grant access to needed I/O-memory ranges
2014-07-28 22:12 ` [PATCH v10 11/12] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
@ 2014-07-30 11:33 ` Ian Campbell
0 siblings, 0 replies; 34+ messages in thread
From: Ian Campbell @ 2014-07-30 11:33 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
On Tue, 2014-07-29 at 00:12 +0200, Arianna Avanzini wrote:
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 1217310..b5bc07a 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -19,6 +19,7 @@
>
> #include "libxl_internal.h"
> #include "libxl_arch.h"
> +#include "libxl_pci.h"
>
> #include <xc_dom.h>
> #include <xenguest.h>
> @@ -1194,6 +1195,65 @@ 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);
> +
> + /*
> + * If VGA passthru is enabled by domain config and one of the
> + * passthru GPUs is of VGA class, be sure that the related
> + * stubdomain and the domain itself can access the VGA
> + * framebuffer.
> + */
Please move this hole block into a new internal helper (libxl__foo)
function in libxl_pci.c. This will avoid the need to refactor the
headers in the previous patch too.
> + for (i = 0 ; d_config->b_info.u.hvm.gfx_passthru.val &&
> + i < d_config->num_pcidevs ; i++) {
d_config->b_info.u.hvm.gfx_passthru.val (which is static here) in a for
condition is a bit odd. When you refactor this into a function I think
this would quite naturally become a test and return at the head of the
function.
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 21ecab1..1524170 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -841,10 +841,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) {
> @@ -862,8 +865,7 @@ 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:
> + }
If you don't want to bring the following block back a level please still
include a blank line between the } and {.
> @@ -1176,8 +1175,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)
> @@ -1198,8 +1196,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();
Likewise a blank line after this.
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index e035e97..7f1bb6b 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1422,6 +1422,7 @@ skip_vfb:
> if (!xlu_cfg_get_list (config, "pci", &pcis, 0, 0)) {
> d_config->num_pcidevs = 0;
> d_config->pcidevs = NULL;
> + fprintf(stderr, "XEN parsing pci list\n");
Leftover debug?
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v10 12/12] xen/common: do not implicitly permit access to mapped I/O memory
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (10 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 11/12] tools/libxl: explicitly grant access to needed I/O-memory ranges Arianna Avanzini
@ 2014-07-28 22:12 ` Arianna Avanzini
2014-07-29 7:29 ` Jan Beulich
2014-08-18 19:50 ` [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Andrii Tseglytskyi
12 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-07-28 22:12 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
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>
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>
---
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.0.3
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 12/12] xen/common: do not implicitly permit access to mapped I/O memory
2014-07-28 22:12 ` [PATCH v10 12/12] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
@ 2014-07-29 7:29 ` Jan Beulich
0 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2014-07-29 7:29 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Ian.Campbell, paolo.valente, keir, stefano.stabellini,
andrew.cooper3, dario.faggioli, Ian.Jackson, xen-devel,
julien.grall, etrudeau, tim, viktor.kleinik
>>> On 29.07.14 at 00:12, <avanzini.arianna@gmail.com> wrote:
> 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>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-07-28 22:11 [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Arianna Avanzini
` (11 preceding siblings ...)
2014-07-28 22:12 ` [PATCH v10 12/12] xen/common: do not implicitly permit access to mapped I/O memory Arianna Avanzini
@ 2014-08-18 19:50 ` Andrii Tseglytskyi
2014-08-20 18:50 ` Arianna Avanzini
12 siblings, 1 reply; 34+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-18 19:50 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
Hi Arianna,
On Tue, Jul 29, 2014 at 1:11 AM, Arianna Avanzini
<avanzini.arianna@gmail.com> wrote:
> Hello,
>
> here I am finally with the tenth version of my implementation proposal for the
> memory_mapping DOMCTL for the ARM architecture. As I did for the previous
> versions, I'll try to keep this cover letter as short as I can by listing here
> only the most relevant changes, while a more detailed description can be found
> in the description and changelog that comes with each of the patches. More
> detailed information about the patch series' genesis can be found in the last
> full cover letter ([1]).
> Please note that the second and third patches of the v9 series have been merged
> by Ian Campbell ([2]) as they resulted in an interface change that would have
> clashed with other upcoming merges.
>
> Patch 0001 has been fixed according to directives given by Julien Grall and
> Ian Campbell; the main change is in the fact that now the ARM-related code
> handling the removal of an I/O-memory mapping follows a behavior which matches
> the one introduced for x86 by patch 0003: in case the given machine frame
> number is mapped to an unexpected guest frame number, it emits a warning and
> still destroys the mapping.
>
> Patch 0002 has been modified according to suggestions given by Ian Campbell;
> now in case that the insertion of a new I/O-memory mapping fails (even if only
> partially) the whole range is unmapped.
>
> Coding style issues existing in patch 0006 and pointed out by Jan Beulich have
> been hopefully addressed here.
>
> A patch 0010 has been added that moves libxl's PCI-related macros, previously
> kept in the libxl_pci.c file, to a header file.
>
> After some discussion involving Ian Campbell, Jan Beulich and Sander Eikelenboom
> patch 0011 has been changed according to directives from Ian Campbell. Now the
> I/O-memory areas needed to passthrough a VGA device in addition to PCI-related
> ones are made accessible to any guest having gfx_passthru set to true in its
> config and at least one passthru PCI device having a device class of 0x030000
> (which should identify it as a VGA device).
>
> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
> and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on an x86_64
> machine with Linux 3.15.0-rc5 as both dom0 and domU.
> Any feedback about this new version of the patchset is more than welcome,
> Arianna
Do you have any public repo, where can I cherry - pick your latest
patch series ?
This series is very useful for development we do in GlobalLogic - we
have Android as domU, and we need iomem for such devices as GPU.
For now we are using one of your previous series - is works fine for
us. Platform is DRA7 (OMAP5)
Also - I suppose - you are targeting this for Xen 4.5. Am I right ?
Regards,
Andrii
> --
> 2.0.3
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
--
Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-18 19:50 ` [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM Andrii Tseglytskyi
@ 2014-08-20 18:50 ` Arianna Avanzini
2014-08-20 19:15 ` Andrii Tseglytskyi
0 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-08-20 18:50 UTC (permalink / raw)
To: Andrii Tseglytskyi
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
On 18/08/2014 21:50, Andrii Tseglytskyi wrote:
> Hi Arianna,
>
> On Tue, Jul 29, 2014 at 1:11 AM, Arianna Avanzini
> <avanzini.arianna@gmail.com> wrote:
>> Hello,
>>
>> here I am finally with the tenth version of my implementation proposal for the
>> memory_mapping DOMCTL for the ARM architecture. As I did for the previous
>> versions, I'll try to keep this cover letter as short as I can by listing here
>> only the most relevant changes, while a more detailed description can be found
>> in the description and changelog that comes with each of the patches. More
>> detailed information about the patch series' genesis can be found in the last
>> full cover letter ([1]).
>> Please note that the second and third patches of the v9 series have been merged
>> by Ian Campbell ([2]) as they resulted in an interface change that would have
>> clashed with other upcoming merges.
>>
>> Patch 0001 has been fixed according to directives given by Julien Grall and
>> Ian Campbell; the main change is in the fact that now the ARM-related code
>> handling the removal of an I/O-memory mapping follows a behavior which matches
>> the one introduced for x86 by patch 0003: in case the given machine frame
>> number is mapped to an unexpected guest frame number, it emits a warning and
>> still destroys the mapping.
>>
>> Patch 0002 has been modified according to suggestions given by Ian Campbell;
>> now in case that the insertion of a new I/O-memory mapping fails (even if only
>> partially) the whole range is unmapped.
>>
>> Coding style issues existing in patch 0006 and pointed out by Jan Beulich have
>> been hopefully addressed here.
>>
>> A patch 0010 has been added that moves libxl's PCI-related macros, previously
>> kept in the libxl_pci.c file, to a header file.
>>
>> After some discussion involving Ian Campbell, Jan Beulich and Sander Eikelenboom
>> patch 0011 has been changed according to directives from Ian Campbell. Now the
>> I/O-memory areas needed to passthrough a VGA device in addition to PCI-related
>> ones are made accessible to any guest having gfx_passthru set to true in its
>> config and at least one passthru PCI device having a device class of 0x030000
>> (which should identify it as a VGA device).
>>
>> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
>> and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on an x86_64
>> machine with Linux 3.15.0-rc5 as both dom0 and domU.
>> Any feedback about this new version of the patchset is more than welcome,
>> Arianna
>
> Do you have any public repo, where can I cherry - pick your latest
> patch series ?
Unfortunately as of now I don't have any. However (I'm hoping it helps) I will
be sending a new version of the patchset, updated for the latest head of the
development repository, within this week.
> This series is very useful for development we do in GlobalLogic - we
> have Android as domU, and we need iomem for such devices as GPU.
> For now we are using one of your previous series - is works fine for
> us. Platform is DRA7 (OMAP5)
>
Thank you so much for providing feedback. Please do not hesitate to point out
any issue you might have.
> Also - I suppose - you are targeting this for Xen 4.5. Am I right ?
>
Yes, you are right. I hope I'll be able to provide faster response to comments
and reviews in the near future.
Thank you,
Arianna
> Regards,
> Andrii
>
>
>> --
>> 2.0.3
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
>
>
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-20 18:50 ` Arianna Avanzini
@ 2014-08-20 19:15 ` Andrii Tseglytskyi
2014-08-21 15:43 ` Andrii Tseglytskyi
0 siblings, 1 reply; 34+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-20 19:15 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
On Wed, Aug 20, 2014 at 9:50 PM, Arianna Avanzini
<avanzini.arianna@gmail.com> wrote:
> On 18/08/2014 21:50, Andrii Tseglytskyi wrote:
>> Hi Arianna,
>>
>> On Tue, Jul 29, 2014 at 1:11 AM, Arianna Avanzini
>> <avanzini.arianna@gmail.com> wrote:
>>> Hello,
>>>
>>> here I am finally with the tenth version of my implementation proposal for the
>>> memory_mapping DOMCTL for the ARM architecture. As I did for the previous
>>> versions, I'll try to keep this cover letter as short as I can by listing here
>>> only the most relevant changes, while a more detailed description can be found
>>> in the description and changelog that comes with each of the patches. More
>>> detailed information about the patch series' genesis can be found in the last
>>> full cover letter ([1]).
>>> Please note that the second and third patches of the v9 series have been merged
>>> by Ian Campbell ([2]) as they resulted in an interface change that would have
>>> clashed with other upcoming merges.
>>>
>>> Patch 0001 has been fixed according to directives given by Julien Grall and
>>> Ian Campbell; the main change is in the fact that now the ARM-related code
>>> handling the removal of an I/O-memory mapping follows a behavior which matches
>>> the one introduced for x86 by patch 0003: in case the given machine frame
>>> number is mapped to an unexpected guest frame number, it emits a warning and
>>> still destroys the mapping.
>>>
>>> Patch 0002 has been modified according to suggestions given by Ian Campbell;
>>> now in case that the insertion of a new I/O-memory mapping fails (even if only
>>> partially) the whole range is unmapped.
>>>
>>> Coding style issues existing in patch 0006 and pointed out by Jan Beulich have
>>> been hopefully addressed here.
>>>
>>> A patch 0010 has been added that moves libxl's PCI-related macros, previously
>>> kept in the libxl_pci.c file, to a header file.
>>>
>>> After some discussion involving Ian Campbell, Jan Beulich and Sander Eikelenboom
>>> patch 0011 has been changed according to directives from Ian Campbell. Now the
>>> I/O-memory areas needed to passthrough a VGA device in addition to PCI-related
>>> ones are made accessible to any guest having gfx_passthru set to true in its
>>> config and at least one passthru PCI device having a device class of 0x030000
>>> (which should identify it as a VGA device).
>>>
>>> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
>>> and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on an x86_64
>>> machine with Linux 3.15.0-rc5 as both dom0 and domU.
>>> Any feedback about this new version of the patchset is more than welcome,
>>> Arianna
>>
>> Do you have any public repo, where can I cherry - pick your latest
>> patch series ?
>
> Unfortunately as of now I don't have any. However (I'm hoping it helps) I will
> be sending a new version of the patchset, updated for the latest head of the
> development repository, within this week.
>
Great. Thank you.
>> This series is very useful for development we do in GlobalLogic - we
>> have Android as domU, and we need iomem for such devices as GPU.
>> For now we are using one of your previous series - is works fine for
>> us. Platform is DRA7 (OMAP5)
>>
>
> Thank you so much for providing feedback. Please do not hesitate to point out
> any issue you might have.
>
Sure.
Regards,
Andrii
>> Also - I suppose - you are targeting this for Xen 4.5. Am I right ?
>>
>
> Yes, you are right. I hope I'll be able to provide faster response to comments
> and reviews in the near future.
>
> Thank you,
> Arianna
>
>
>
>> Regards,
>> Andrii
>>
>>
>>> --
>>> 2.0.3
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>
>>
>>
>
>
> --
> /*
> * Arianna Avanzini
> * avanzini.arianna@gmail.com
> * 73628@studenti.unimore.it
> */
--
Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-20 19:15 ` Andrii Tseglytskyi
@ 2014-08-21 15:43 ` Andrii Tseglytskyi
2014-08-22 14:25 ` Arianna Avanzini
0 siblings, 1 reply; 34+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-21 15:43 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
Hi Arianna,
Just one more question - how iomem mapping will be handled by XSM framework?
I'm using older revision of your patch series and I need to do the
following change to permit domU working with already mapped iomem,
(only if XSM is enabled and FLASK is enforsed)
commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
Date: Thu Aug 21 18:00:20 2014 +0300
xsm: arm: allow domU to use iomem
Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
diff --git a/tools/flask/policy/policy/modules/xen/xen.te
b/tools/flask/policy/policy/modules/xen/xen.te
index d7147fb..ac4a01d 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -108,6 +108,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)
Is this a point, or I missed something ?
Regards,
Andrii
On Wed, Aug 20, 2014 at 10:15 PM, Andrii Tseglytskyi
<andrii.tseglytskyi@globallogic.com> wrote:
> On Wed, Aug 20, 2014 at 9:50 PM, Arianna Avanzini
> <avanzini.arianna@gmail.com> wrote:
>> On 18/08/2014 21:50, Andrii Tseglytskyi wrote:
>>> Hi Arianna,
>>>
>>> On Tue, Jul 29, 2014 at 1:11 AM, Arianna Avanzini
>>> <avanzini.arianna@gmail.com> wrote:
>>>> Hello,
>>>>
>>>> here I am finally with the tenth version of my implementation proposal for the
>>>> memory_mapping DOMCTL for the ARM architecture. As I did for the previous
>>>> versions, I'll try to keep this cover letter as short as I can by listing here
>>>> only the most relevant changes, while a more detailed description can be found
>>>> in the description and changelog that comes with each of the patches. More
>>>> detailed information about the patch series' genesis can be found in the last
>>>> full cover letter ([1]).
>>>> Please note that the second and third patches of the v9 series have been merged
>>>> by Ian Campbell ([2]) as they resulted in an interface change that would have
>>>> clashed with other upcoming merges.
>>>>
>>>> Patch 0001 has been fixed according to directives given by Julien Grall and
>>>> Ian Campbell; the main change is in the fact that now the ARM-related code
>>>> handling the removal of an I/O-memory mapping follows a behavior which matches
>>>> the one introduced for x86 by patch 0003: in case the given machine frame
>>>> number is mapped to an unexpected guest frame number, it emits a warning and
>>>> still destroys the mapping.
>>>>
>>>> Patch 0002 has been modified according to suggestions given by Ian Campbell;
>>>> now in case that the insertion of a new I/O-memory mapping fails (even if only
>>>> partially) the whole range is unmapped.
>>>>
>>>> Coding style issues existing in patch 0006 and pointed out by Jan Beulich have
>>>> been hopefully addressed here.
>>>>
>>>> A patch 0010 has been added that moves libxl's PCI-related macros, previously
>>>> kept in the libxl_pci.c file, to a header file.
>>>>
>>>> After some discussion involving Ian Campbell, Jan Beulich and Sander Eikelenboom
>>>> patch 0011 has been changed according to directives from Ian Campbell. Now the
>>>> I/O-memory areas needed to passthrough a VGA device in addition to PCI-related
>>>> ones are made accessible to any guest having gfx_passthru set to true in its
>>>> config and at least one passthru PCI device having a device class of 0x030000
>>>> (which should identify it as a VGA device).
>>>>
>>>> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
>>>> and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on an x86_64
>>>> machine with Linux 3.15.0-rc5 as both dom0 and domU.
>>>> Any feedback about this new version of the patchset is more than welcome,
>>>> Arianna
>>>
>>> Do you have any public repo, where can I cherry - pick your latest
>>> patch series ?
>>
>> Unfortunately as of now I don't have any. However (I'm hoping it helps) I will
>> be sending a new version of the patchset, updated for the latest head of the
>> development repository, within this week.
>>
>
> Great. Thank you.
>
>>> This series is very useful for development we do in GlobalLogic - we
>>> have Android as domU, and we need iomem for such devices as GPU.
>>> For now we are using one of your previous series - is works fine for
>>> us. Platform is DRA7 (OMAP5)
>>>
>>
>> Thank you so much for providing feedback. Please do not hesitate to point out
>> any issue you might have.
>>
>
> Sure.
>
> Regards,
> Andrii
>
>>> Also - I suppose - you are targeting this for Xen 4.5. Am I right ?
>>>
>>
>> Yes, you are right. I hope I'll be able to provide faster response to comments
>> and reviews in the near future.
>>
>> Thank you,
>> Arianna
>>
>>
>>
>>> Regards,
>>> Andrii
>>>
>>>
>>>> --
>>>> 2.0.3
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xen.org
>>>> http://lists.xen.org/xen-devel
>>>
>>>
>>>
>>
>>
>> --
>> /*
>> * Arianna Avanzini
>> * avanzini.arianna@gmail.com
>> * 73628@studenti.unimore.it
>> */
>
>
>
> --
>
> Andrii Tseglytskyi | Embedded Dev
> GlobalLogic
> www.globallogic.com
--
Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-21 15:43 ` Andrii Tseglytskyi
@ 2014-08-22 14:25 ` Arianna Avanzini
2014-08-22 18:27 ` Andrii Tseglytskyi
0 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-08-22 14:25 UTC (permalink / raw)
To: Andrii Tseglytskyi
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
> Hi Arianna,
>
> Just one more question - how iomem mapping will be handled by XSM framework?
> I'm using older revision of your patch series and I need to do the
> following change to permit domU working with already mapped iomem,
> (only if XSM is enabled and FLASK is enforsed)
>
> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> Date: Thu Aug 21 18:00:20 2014 +0300
>
> xsm: arm: allow domU to use iomem
>
> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>
> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
> b/tools/flask/policy/policy/modules/xen/xen.te
> index d7147fb..ac4a01d 100644
> --- a/tools/flask/policy/policy/modules/xen/xen.te
> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> @@ -108,6 +108,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)
>
> Is this a point, or I missed something ?
>
I think this is a point, if I understood things well. Do you prefer to submit
your patch personally if/after the memory_mapping patchset is eventually merged,
or do you prefer that I send your patch with the memory_mapping patchset?
Thank you,
Arianna
> Regards,
> Andrii
>
> On Wed, Aug 20, 2014 at 10:15 PM, Andrii Tseglytskyi
> <andrii.tseglytskyi@globallogic.com> wrote:
>> On Wed, Aug 20, 2014 at 9:50 PM, Arianna Avanzini
>> <avanzini.arianna@gmail.com> wrote:
>>> On 18/08/2014 21:50, Andrii Tseglytskyi wrote:
>>>> Hi Arianna,
>>>>
>>>> On Tue, Jul 29, 2014 at 1:11 AM, Arianna Avanzini
>>>> <avanzini.arianna@gmail.com> wrote:
>>>>> Hello,
>>>>>
>>>>> here I am finally with the tenth version of my implementation proposal for the
>>>>> memory_mapping DOMCTL for the ARM architecture. As I did for the previous
>>>>> versions, I'll try to keep this cover letter as short as I can by listing here
>>>>> only the most relevant changes, while a more detailed description can be found
>>>>> in the description and changelog that comes with each of the patches. More
>>>>> detailed information about the patch series' genesis can be found in the last
>>>>> full cover letter ([1]).
>>>>> Please note that the second and third patches of the v9 series have been merged
>>>>> by Ian Campbell ([2]) as they resulted in an interface change that would have
>>>>> clashed with other upcoming merges.
>>>>>
>>>>> Patch 0001 has been fixed according to directives given by Julien Grall and
>>>>> Ian Campbell; the main change is in the fact that now the ARM-related code
>>>>> handling the removal of an I/O-memory mapping follows a behavior which matches
>>>>> the one introduced for x86 by patch 0003: in case the given machine frame
>>>>> number is mapped to an unexpected guest frame number, it emits a warning and
>>>>> still destroys the mapping.
>>>>>
>>>>> Patch 0002 has been modified according to suggestions given by Ian Campbell;
>>>>> now in case that the insertion of a new I/O-memory mapping fails (even if only
>>>>> partially) the whole range is unmapped.
>>>>>
>>>>> Coding style issues existing in patch 0006 and pointed out by Jan Beulich have
>>>>> been hopefully addressed here.
>>>>>
>>>>> A patch 0010 has been added that moves libxl's PCI-related macros, previously
>>>>> kept in the libxl_pci.c file, to a header file.
>>>>>
>>>>> After some discussion involving Ian Campbell, Jan Beulich and Sander Eikelenboom
>>>>> patch 0011 has been changed according to directives from Ian Campbell. Now the
>>>>> I/O-memory areas needed to passthrough a VGA device in addition to PCI-related
>>>>> ones are made accessible to any guest having gfx_passthru set to true in its
>>>>> config and at least one passthru PCI device having a device class of 0x030000
>>>>> (which should identify it as a VGA device).
>>>>>
>>>>> The code has again been tested on a cubieboard2, with Linux v3.15-rc3 as a dom0
>>>>> and ERIKA Enterprise ([3]) as a domU. The x86 bits have been tested on an x86_64
>>>>> machine with Linux 3.15.0-rc5 as both dom0 and domU.
>>>>> Any feedback about this new version of the patchset is more than welcome,
>>>>> Arianna
>>>>
>>>> Do you have any public repo, where can I cherry - pick your latest
>>>> patch series ?
>>>
>>> Unfortunately as of now I don't have any. However (I'm hoping it helps) I will
>>> be sending a new version of the patchset, updated for the latest head of the
>>> development repository, within this week.
>>>
>>
>> Great. Thank you.
>>
>>>> This series is very useful for development we do in GlobalLogic - we
>>>> have Android as domU, and we need iomem for such devices as GPU.
>>>> For now we are using one of your previous series - is works fine for
>>>> us. Platform is DRA7 (OMAP5)
>>>>
>>>
>>> Thank you so much for providing feedback. Please do not hesitate to point out
>>> any issue you might have.
>>>
>>
>> Sure.
>>
>> Regards,
>> Andrii
>>
>>>> Also - I suppose - you are targeting this for Xen 4.5. Am I right ?
>>>>
>>>
>>> Yes, you are right. I hope I'll be able to provide faster response to comments
>>> and reviews in the near future.
>>>
>>> Thank you,
>>> Arianna
>>>
>>>
>>>
>>>> Regards,
>>>> Andrii
>>>>
>>>>
>>>>> --
>>>>> 2.0.3
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Xen-devel mailing list
>>>>> Xen-devel@lists.xen.org
>>>>> http://lists.xen.org/xen-devel
>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> /*
>>> * Arianna Avanzini
>>> * avanzini.arianna@gmail.com
>>> * 73628@studenti.unimore.it
>>> */
>>
>>
>>
>> --
>>
>> Andrii Tseglytskyi | Embedded Dev
>> GlobalLogic
>> www.globallogic.com
>
>
>
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-22 14:25 ` Arianna Avanzini
@ 2014-08-22 18:27 ` Andrii Tseglytskyi
2014-08-22 18:36 ` Arianna Avanzini
0 siblings, 1 reply; 34+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-22 18:27 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
Hi Arianna,
On Fri, Aug 22, 2014 at 5:25 PM, Arianna Avanzini
<avanzini.arianna@gmail.com> wrote:
> On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
>> Hi Arianna,
>>
>> Just one more question - how iomem mapping will be handled by XSM framework?
>> I'm using older revision of your patch series and I need to do the
>> following change to permit domU working with already mapped iomem,
>> (only if XSM is enabled and FLASK is enforsed)
>>
>> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
>> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>> Date: Thu Aug 21 18:00:20 2014 +0300
>>
>> xsm: arm: allow domU to use iomem
>>
>> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>
>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
>> b/tools/flask/policy/policy/modules/xen/xen.te
>> index d7147fb..ac4a01d 100644
>> --- a/tools/flask/policy/policy/modules/xen/xen.te
>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
>> @@ -108,6 +108,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)
>>
>> Is this a point, or I missed something ?
>>
>
> I think this is a point, if I understood things well. Do you prefer to submit
> your patch personally if/after the memory_mapping patchset is eventually merged,
> or do you prefer that I send your patch with the memory_mapping patchset?
>
It would be great if you include this patch to your patch series, keep
my authority and add your sign-off-by.
Do you agree?
--
Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-22 18:27 ` Andrii Tseglytskyi
@ 2014-08-22 18:36 ` Arianna Avanzini
2014-08-22 19:31 ` Andrii Tseglytskyi
0 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-08-22 18:36 UTC (permalink / raw)
To: Andrii Tseglytskyi
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
On 22/08/2014 20:27, Andrii Tseglytskyi wrote:
> Hi Arianna,
>
> On Fri, Aug 22, 2014 at 5:25 PM, Arianna Avanzini
> <avanzini.arianna@gmail.com> wrote:
>> On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
>>> Hi Arianna,
>>>
>>> Just one more question - how iomem mapping will be handled by XSM framework?
>>> I'm using older revision of your patch series and I need to do the
>>> following change to permit domU working with already mapped iomem,
>>> (only if XSM is enabled and FLASK is enforsed)
>>>
>>> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
>>> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>> Date: Thu Aug 21 18:00:20 2014 +0300
>>>
>>> xsm: arm: allow domU to use iomem
>>>
>>> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>
>>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
>>> b/tools/flask/policy/policy/modules/xen/xen.te
>>> index d7147fb..ac4a01d 100644
>>> --- a/tools/flask/policy/policy/modules/xen/xen.te
>>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
>>> @@ -108,6 +108,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)
>>>
>>> Is this a point, or I missed something ?
>>>
>>
>> I think this is a point, if I understood things well. Do you prefer to submit
>> your patch personally if/after the memory_mapping patchset is eventually merged,
>> or do you prefer that I send your patch with the memory_mapping patchset?
>>
>
> It would be great if you include this patch to your patch series, keep
> my authority and add your sign-off-by.
> Do you agree?
>
Of course, but I think my signed-off-by in this case is not appropriate nor
required.
I'll add it to the upcoming v11 with your signed-off-by and your authority, if
it's OK for you. I proposed you to merge it with the memory_mapping patchset
just because I think it might be simpler to review it this way.
Thank you,
Arianna
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-22 18:36 ` Arianna Avanzini
@ 2014-08-22 19:31 ` Andrii Tseglytskyi
2014-08-22 23:51 ` Stefano Stabellini
0 siblings, 1 reply; 34+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-22 19:31 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
On Fri, Aug 22, 2014 at 9:36 PM, Arianna Avanzini
<avanzini.arianna@gmail.com> wrote:
> On 22/08/2014 20:27, Andrii Tseglytskyi wrote:
>> Hi Arianna,
>>
>> On Fri, Aug 22, 2014 at 5:25 PM, Arianna Avanzini
>> <avanzini.arianna@gmail.com> wrote:
>>> On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
>>>> Hi Arianna,
>>>>
>>>> Just one more question - how iomem mapping will be handled by XSM framework?
>>>> I'm using older revision of your patch series and I need to do the
>>>> following change to permit domU working with already mapped iomem,
>>>> (only if XSM is enabled and FLASK is enforsed)
>>>>
>>>> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
>>>> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>> Date: Thu Aug 21 18:00:20 2014 +0300
>>>>
>>>> xsm: arm: allow domU to use iomem
>>>>
>>>> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
>>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>>
>>>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
>>>> b/tools/flask/policy/policy/modules/xen/xen.te
>>>> index d7147fb..ac4a01d 100644
>>>> --- a/tools/flask/policy/policy/modules/xen/xen.te
>>>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
>>>> @@ -108,6 +108,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)
>>>>
>>>> Is this a point, or I missed something ?
>>>>
>>>
>>> I think this is a point, if I understood things well. Do you prefer to submit
>>> your patch personally if/after the memory_mapping patchset is eventually merged,
>>> or do you prefer that I send your patch with the memory_mapping patchset?
>>>
>>
>> It would be great if you include this patch to your patch series, keep
>> my authority and add your sign-off-by.
>> Do you agree?
>>
>
> Of course, but I think my signed-off-by in this case is not appropriate nor
> required.
> I'll add it to the upcoming v11 with your signed-off-by and your authority, if
> it's OK for you. I proposed you to merge it with the memory_mapping patchset
> just because I think it might be simpler to review it this way.
>
Sure. Completely agree. Let's proceed in this way.
Thank you,
Regards,
Andrii
> Thank you,
> Arianna
>
>
>
> --
> /*
> * Arianna Avanzini
> * avanzini.arianna@gmail.com
> * 73628@studenti.unimore.it
> */
--
Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-22 19:31 ` Andrii Tseglytskyi
@ 2014-08-22 23:51 ` Stefano Stabellini
2014-08-23 12:07 ` Arianna Avanzini
0 siblings, 1 reply; 34+ messages in thread
From: Stefano Stabellini @ 2014-08-22 23:51 UTC (permalink / raw)
To: Andrii Tseglytskyi
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Andrew Cooper, Jan Beulich, Arianna Avanzini,
Viktor Kleinik
On Fri, 22 Aug 2014, Andrii Tseglytskyi wrote:
> On Fri, Aug 22, 2014 at 9:36 PM, Arianna Avanzini
> <avanzini.arianna@gmail.com> wrote:
> > On 22/08/2014 20:27, Andrii Tseglytskyi wrote:
> >> Hi Arianna,
> >>
> >> On Fri, Aug 22, 2014 at 5:25 PM, Arianna Avanzini
> >> <avanzini.arianna@gmail.com> wrote:
> >>> On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
> >>>> Hi Arianna,
> >>>>
> >>>> Just one more question - how iomem mapping will be handled by XSM framework?
> >>>> I'm using older revision of your patch series and I need to do the
> >>>> following change to permit domU working with already mapped iomem,
> >>>> (only if XSM is enabled and FLASK is enforsed)
> >>>>
> >>>> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
> >>>> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> >>>> Date: Thu Aug 21 18:00:20 2014 +0300
> >>>>
> >>>> xsm: arm: allow domU to use iomem
> >>>>
> >>>> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
> >>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
> >>>>
> >>>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
> >>>> b/tools/flask/policy/policy/modules/xen/xen.te
> >>>> index d7147fb..ac4a01d 100644
> >>>> --- a/tools/flask/policy/policy/modules/xen/xen.te
> >>>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
> >>>> @@ -108,6 +108,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)
> >>>>
> >>>> Is this a point, or I missed something ?
> >>>>
> >>>
> >>> I think this is a point, if I understood things well. Do you prefer to submit
> >>> your patch personally if/after the memory_mapping patchset is eventually merged,
> >>> or do you prefer that I send your patch with the memory_mapping patchset?
> >>>
> >>
> >> It would be great if you include this patch to your patch series, keep
> >> my authority and add your sign-off-by.
> >> Do you agree?
> >>
> >
> > Of course, but I think my signed-off-by in this case is not appropriate nor
> > required.
> > I'll add it to the upcoming v11 with your signed-off-by and your authority, if
> > it's OK for you. I proposed you to merge it with the memory_mapping patchset
> > just because I think it might be simpler to review it this way.
> >
>
> Sure. Completely agree. Let's proceed in this way.
Arianna,
keeping Andrii as original Author and having his Signed-off-by line in
the commit message is the right thing to do. However in addition to that
you should also add yours as you are making the patch part of your
series. Basically everybody that touches the patch needs to add her own
Signed-off-by line for copyright reasons.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-22 23:51 ` Stefano Stabellini
@ 2014-08-23 12:07 ` Arianna Avanzini
2014-08-23 17:36 ` Andrii Tseglytskyi
0 siblings, 1 reply; 34+ messages in thread
From: Arianna Avanzini @ 2014-08-23 12:07 UTC (permalink / raw)
To: Stefano Stabellini, Andrii Tseglytskyi
Cc: Julien Grall, paolo.valente, Keir Fraser, Tim Deegan,
Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell, etrudeau,
Jan Beulich, Andrew Cooper, Viktor Kleinik
On 23/08/2014 01:51, Stefano Stabellini wrote:
> On Fri, 22 Aug 2014, Andrii Tseglytskyi wrote:
>> On Fri, Aug 22, 2014 at 9:36 PM, Arianna Avanzini
>> <avanzini.arianna@gmail.com> wrote:
>>> On 22/08/2014 20:27, Andrii Tseglytskyi wrote:
>>>> Hi Arianna,
>>>>
>>>> On Fri, Aug 22, 2014 at 5:25 PM, Arianna Avanzini
>>>> <avanzini.arianna@gmail.com> wrote:
>>>>> On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
>>>>>> Hi Arianna,
>>>>>>
>>>>>> Just one more question - how iomem mapping will be handled by XSM framework?
>>>>>> I'm using older revision of your patch series and I need to do the
>>>>>> following change to permit domU working with already mapped iomem,
>>>>>> (only if XSM is enabled and FLASK is enforsed)
>>>>>>
>>>>>> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
>>>>>> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>>>> Date: Thu Aug 21 18:00:20 2014 +0300
>>>>>>
>>>>>> xsm: arm: allow domU to use iomem
>>>>>>
>>>>>> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
>>>>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>>>>
>>>>>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
>>>>>> b/tools/flask/policy/policy/modules/xen/xen.te
>>>>>> index d7147fb..ac4a01d 100644
>>>>>> --- a/tools/flask/policy/policy/modules/xen/xen.te
>>>>>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
>>>>>> @@ -108,6 +108,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)
>>>>>>
>>>>>> Is this a point, or I missed something ?
>>>>>>
>>>>>
>>>>> I think this is a point, if I understood things well. Do you prefer to submit
>>>>> your patch personally if/after the memory_mapping patchset is eventually merged,
>>>>> or do you prefer that I send your patch with the memory_mapping patchset?
>>>>>
>>>>
>>>> It would be great if you include this patch to your patch series, keep
>>>> my authority and add your sign-off-by.
>>>> Do you agree?
>>>>
>>>
>>> Of course, but I think my signed-off-by in this case is not appropriate nor
>>> required.
>>> I'll add it to the upcoming v11 with your signed-off-by and your authority, if
>>> it's OK for you. I proposed you to merge it with the memory_mapping patchset
>>> just because I think it might be simpler to review it this way.
>>>
>>
>> Sure. Completely agree. Let's proceed in this way.
>
> Arianna,
> keeping Andrii as original Author and having his Signed-off-by line in
> the commit message is the right thing to do. However in addition to that
> you should also add yours as you are making the patch part of your
> series. Basically everybody that touches the patch needs to add her own
> Signed-off-by line for copyright reasons.
>
>
Hello Stefano,
thank you for pointing that out, I really didn't know that my Signed-off-by was
required. If Andrii is OK with it I'll go with his initial suggestion then.
Thank you,
Arianna
--
/*
* Arianna Avanzini
* avanzini.arianna@gmail.com
* 73628@studenti.unimore.it
*/
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v10 00/12] Implement the XEN_DOMCTL_memory_mapping hypercall for ARM
2014-08-23 12:07 ` Arianna Avanzini
@ 2014-08-23 17:36 ` Andrii Tseglytskyi
0 siblings, 0 replies; 34+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-23 17:36 UTC (permalink / raw)
To: Arianna Avanzini
Cc: Julien Grall, paolo.valente, Keir Fraser, Stefano Stabellini,
Tim Deegan, Dario Faggioli, Ian Jackson, xen-devel, Ian Campbell,
etrudeau, Jan Beulich, Andrew Cooper, Viktor Kleinik
I am OK
On Sat, Aug 23, 2014 at 3:07 PM, Arianna Avanzini
<avanzini.arianna@gmail.com> wrote:
> On 23/08/2014 01:51, Stefano Stabellini wrote:
>> On Fri, 22 Aug 2014, Andrii Tseglytskyi wrote:
>>> On Fri, Aug 22, 2014 at 9:36 PM, Arianna Avanzini
>>> <avanzini.arianna@gmail.com> wrote:
>>>> On 22/08/2014 20:27, Andrii Tseglytskyi wrote:
>>>>> Hi Arianna,
>>>>>
>>>>> On Fri, Aug 22, 2014 at 5:25 PM, Arianna Avanzini
>>>>> <avanzini.arianna@gmail.com> wrote:
>>>>>> On 21/08/2014 17:43, Andrii Tseglytskyi wrote:
>>>>>>> Hi Arianna,
>>>>>>>
>>>>>>> Just one more question - how iomem mapping will be handled by XSM framework?
>>>>>>> I'm using older revision of your patch series and I need to do the
>>>>>>> following change to permit domU working with already mapped iomem,
>>>>>>> (only if XSM is enabled and FLASK is enforsed)
>>>>>>>
>>>>>>> commit 3f35dae860bd0f566ca156608ec53e3240aacd5a
>>>>>>> Author: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>>>>> Date: Thu Aug 21 18:00:20 2014 +0300
>>>>>>>
>>>>>>> xsm: arm: allow domU to use iomem
>>>>>>>
>>>>>>> Change-Id: I7eff12b127e0d32d97a67e77dbcca3a8326dfd22
>>>>>>> Signed-off-by: Andrii Tseglytskyi <andrii.tseglytskyi@globallogic.com>
>>>>>>>
>>>>>>> diff --git a/tools/flask/policy/policy/modules/xen/xen.te
>>>>>>> b/tools/flask/policy/policy/modules/xen/xen.te
>>>>>>> index d7147fb..ac4a01d 100644
>>>>>>> --- a/tools/flask/policy/policy/modules/xen/xen.te
>>>>>>> +++ b/tools/flask/policy/policy/modules/xen/xen.te
>>>>>>> @@ -108,6 +108,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)
>>>>>>>
>>>>>>> Is this a point, or I missed something ?
>>>>>>>
>>>>>>
>>>>>> I think this is a point, if I understood things well. Do you prefer to submit
>>>>>> your patch personally if/after the memory_mapping patchset is eventually merged,
>>>>>> or do you prefer that I send your patch with the memory_mapping patchset?
>>>>>>
>>>>>
>>>>> It would be great if you include this patch to your patch series, keep
>>>>> my authority and add your sign-off-by.
>>>>> Do you agree?
>>>>>
>>>>
>>>> Of course, but I think my signed-off-by in this case is not appropriate nor
>>>> required.
>>>> I'll add it to the upcoming v11 with your signed-off-by and your authority, if
>>>> it's OK for you. I proposed you to merge it with the memory_mapping patchset
>>>> just because I think it might be simpler to review it this way.
>>>>
>>>
>>> Sure. Completely agree. Let's proceed in this way.
>>
>> Arianna,
>> keeping Andrii as original Author and having his Signed-off-by line in
>> the commit message is the right thing to do. However in addition to that
>> you should also add yours as you are making the patch part of your
>> series. Basically everybody that touches the patch needs to add her own
>> Signed-off-by line for copyright reasons.
>>
>>
>
> Hello Stefano,
>
> thank you for pointing that out, I really didn't know that my Signed-off-by was
> required. If Andrii is OK with it I'll go with his initial suggestion then.
>
> Thank you,
> Arianna
>
>
> --
> /*
> * Arianna Avanzini
> * avanzini.arianna@gmail.com
> * 73628@studenti.unimore.it
> */
--
Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com
^ permalink raw reply [flat|nested] 34+ messages in thread