* [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function
@ 2013-03-16 0:26 Mukesh Rathor
2013-03-21 15:04 ` Tim Deegan
0 siblings, 1 reply; 3+ messages in thread
From: Mukesh Rathor @ 2013-03-16 0:26 UTC (permalink / raw)
To: Xen-devel@lists.xensource.com
In this patch, XEN_DOMCTL_memory_mapping code is put into a function so
it can be shared later for PVH. There is no change in it's
functionality.
Changes in V2:
- Remove PHYSDEVOP_map_iomem sub hypercall, and the code supporting it
as the IO region is mapped transparently now.
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
xen/arch/x86/domctl.c | 119 ++++++++++++++++++++++++---------------------
xen/include/xen/domain.h | 2 +
2 files changed, 65 insertions(+), 56 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 31937e0..ecc8240 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -46,6 +46,68 @@ static int gdbsx_guest_mem_io(
return (iop->remain ? -EFAULT : 0);
}
+long domctl_memory_mapping(struct domain *d, unsigned long gfn,
+ unsigned long mfn, unsigned long nr_mfns,
+ int add_map)
+{
+ int i;
+ long ret = -EINVAL;
+
+ if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
+ ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
+ (gfn + nr_mfns - 1) < gfn ) /* wrap? */
+ return ret;
+
+ ret = xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add_map);
+ if ( ret )
+ return ret;
+
+ if ( add_map )
+ {
+ 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 + nr_mfns - 1);
+ if ( !ret && paging_mode_translate(d) )
+ {
+ for ( i = 0; !ret && i < nr_mfns; i++ )
+ if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
+ ret = -EIO;
+ if ( ret )
+ {
+ printk(XENLOG_G_WARNING
+ "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
+ d->domain_id, gfn + i, mfn + i);
+ while ( i-- )
+ clear_mmio_p2m_entry(d, gfn + i);
+ if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
+ IS_PRIV(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);
+ }
+ }
+ } else {
+ printk(XENLOG_G_INFO
+ "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++ )
+ add_map |= !clear_mmio_p2m_entry(d, gfn + i);
+ ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+ if ( !ret && add_map )
+ ret = -EIO;
+ if ( ret && IS_PRIV(current->domain) )
+ printk(XENLOG_ERR
+ "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
+ ret, add_map ? "removing" : "denying", d->domain_id,
+ mfn, mfn + nr_mfns - 1);
+ }
+ return ret;
+}
+
long arch_do_domctl(
struct xen_domctl *domctl, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
@@ -628,68 +690,13 @@ long arch_do_domctl(
unsigned long mfn = domctl->u.memory_mapping.first_mfn;
unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
int add = domctl->u.memory_mapping.add_mapping;
- unsigned long i;
-
- ret = -EINVAL;
- if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
- ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
- (gfn + nr_mfns - 1) < gfn ) /* wrap? */
- break;
ret = -EPERM;
if ( !IS_PRIV(current->domain) &&
!iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
break;
- ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, 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 + nr_mfns - 1);
- if ( !ret && paging_mode_translate(d) )
- {
- for ( i = 0; !ret && i < nr_mfns; i++ )
- if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
- ret = -EIO;
- if ( ret )
- {
- printk(XENLOG_G_WARNING
- "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
- d->domain_id, gfn + i, mfn + i);
- while ( i-- )
- clear_mmio_p2m_entry(d, gfn + i);
- if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
- IS_PRIV(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);
- }
- }
- }
- else
- {
- printk(XENLOG_G_INFO
- "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++ )
- add |= !clear_mmio_p2m_entry(d, gfn + i);
- ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
- if ( !ret && add )
- ret = -EIO;
- if ( ret && IS_PRIV(current->domain) )
- printk(XENLOG_ERR
- "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
- ret, add ? "removing" : "denying", d->domain_id,
- mfn, mfn + nr_mfns - 1);
- }
+ ret = domctl_memory_mapping(d, gfn, mfn, nr_mfns, add);
}
break;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index d4ac50f..a7b4c34 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -86,4 +86,6 @@ extern unsigned int xen_processor_pmbits;
extern bool_t opt_dom0_vcpus_pin;
+extern long domctl_memory_mapping(struct domain *d, unsigned long gfn,
+ unsigned long mfn, unsigned long nr_mfns, int add_map);
#endif /* __XEN_DOMAIN_H__ */
--
1.7.2.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function
2013-03-16 0:26 [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function Mukesh Rathor
@ 2013-03-21 15:04 ` Tim Deegan
2013-03-22 21:02 ` Mukesh Rathor
0 siblings, 1 reply; 3+ messages in thread
From: Tim Deegan @ 2013-03-21 15:04 UTC (permalink / raw)
To: Mukesh Rathor; +Cc: Xen-devel@lists.xensource.com
At 17:26 -0700 on 15 Mar (1363368383), Mukesh Rathor wrote:
> In this patch, XEN_DOMCTL_memory_mapping code is put into a function so
> it can be shared later for PVH. There is no change in it's
> functionality.
>
> Changes in V2:
> - Remove PHYSDEVOP_map_iomem sub hypercall, and the code supporting it
> as the IO region is mapped transparently now.
>
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
> xen/arch/x86/domctl.c | 119 ++++++++++++++++++++++++---------------------
> xen/include/xen/domain.h | 2 +
> 2 files changed, 65 insertions(+), 56 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 31937e0..ecc8240 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -46,6 +46,68 @@ static int gdbsx_guest_mem_io(
> return (iop->remain ? -EFAULT : 0);
> }
>
> +long domctl_memory_mapping(struct domain *d, unsigned long gfn,
> + unsigned long mfn, unsigned long nr_mfns,
> + int add_map)
> +{
> + int i;
The loop variable at the source was 'unsigned long', and the loop limit
is an unsigned long, so 'int' doesn't seem right.
> @@ -628,68 +690,13 @@ long arch_do_domctl(
> unsigned long mfn = domctl->u.memory_mapping.first_mfn;
> unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
> int add = domctl->u.memory_mapping.add_mapping;
> - unsigned long i;
> -
> - ret = -EINVAL;
> - if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
> - ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
> - (gfn + nr_mfns - 1) < gfn ) /* wrap? */
> - break;
>
> ret = -EPERM;
> if ( !IS_PRIV(current->domain) &&
> !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
> break;
Why is this privilege check not being moved along with the rest of it?
Execially given that the two other IS_PRIV()s below are taken?
Tim.
> - ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, 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 + nr_mfns - 1);
> - if ( !ret && paging_mode_translate(d) )
> - {
> - for ( i = 0; !ret && i < nr_mfns; i++ )
> - if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
> - ret = -EIO;
> - if ( ret )
> - {
> - printk(XENLOG_G_WARNING
> - "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
> - d->domain_id, gfn + i, mfn + i);
> - while ( i-- )
> - clear_mmio_p2m_entry(d, gfn + i);
> - if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
> - IS_PRIV(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);
> - }
> - }
> - }
> - else
> - {
> - printk(XENLOG_G_INFO
> - "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++ )
> - add |= !clear_mmio_p2m_entry(d, gfn + i);
> - ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
> - if ( !ret && add )
> - ret = -EIO;
> - if ( ret && IS_PRIV(current->domain) )
> - printk(XENLOG_ERR
> - "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
> - ret, add ? "removing" : "denying", d->domain_id,
> - mfn, mfn + nr_mfns - 1);
> - }
> + ret = domctl_memory_mapping(d, gfn, mfn, nr_mfns, add);
> }
> break;
>
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index d4ac50f..a7b4c34 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -86,4 +86,6 @@ extern unsigned int xen_processor_pmbits;
>
> extern bool_t opt_dom0_vcpus_pin;
>
> +extern long domctl_memory_mapping(struct domain *d, unsigned long gfn,
> + unsigned long mfn, unsigned long nr_mfns, int add_map);
> #endif /* __XEN_DOMAIN_H__ */
> --
> 1.7.2.3
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function
2013-03-21 15:04 ` Tim Deegan
@ 2013-03-22 21:02 ` Mukesh Rathor
0 siblings, 0 replies; 3+ messages in thread
From: Mukesh Rathor @ 2013-03-22 21:02 UTC (permalink / raw)
To: Tim Deegan; +Cc: Xen-devel@lists.xensource.com
On Thu, 21 Mar 2013 15:04:33 +0000
Tim Deegan <tim@xen.org> wrote:
> At 17:26 -0700 on 15 Mar (1363368383), Mukesh Rathor wrote:
> > In this patch, XEN_DOMCTL_memory_mapping code is put into a
> > function so it can be shared later for PVH. There is no change in
> > it's functionality.
> >
>limit is an unsigned long, so 'int' doesn't seem right.
>
> > @@ -628,68 +690,13 @@ long arch_do_domctl(
> > unsigned long mfn = domctl->u.memory_mapping.first_mfn;
> > unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
> > int add = domctl->u.memory_mapping.add_mapping;
> > - unsigned long i;
> > -
> > - ret = -EINVAL;
> > - if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
> > - ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits -
> > PAGE_SHIFT)) ||
> > - (gfn + nr_mfns - 1) < gfn ) /* wrap? */
> > - break;
> >
> > ret = -EPERM;
> > if ( !IS_PRIV(current->domain) &&
> > !iomem_access_permitted(current->domain, mfn, mfn +
> > nr_mfns - 1) ) break;
>
> Why is this privilege check not being moved along with the rest of it?
> Execially given that the two other IS_PRIV()s below are taken?
Because, the other caller is construct_dom0, so not really needed. But
I'll move it anyways.
thanks,
Mukesh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-03-22 21:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-16 0:26 [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function Mukesh Rathor
2013-03-21 15:04 ` Tim Deegan
2013-03-22 21:02 ` Mukesh Rathor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).