These three patches finish up the XSM IS_PRIV reworking, in addition to the patches posted by Ian and Lars fixing things for ARM. I did not include a patch removing the rcu_lock_target_domain_by_id function because it''s still referenced in ARM at the moment, but it looks like those locations are being changed (or will be changed in the future). [PATCH 1/3] hvm: wire up domctl and xsm hypercalls - Have tested simple operations like xl list/pause/unpause from HVM. Things like domain creation may need some of the PVH work that also applies to HVM (and domain creation in particular probably shouldn''t be done from HVM anyway). [PATCH 2/3] xen/arch/x86: complete XSM hooks on irq/pirq mappings - This should compliment Ian''s work on MSI interrupts, although it doesn''t look to conflict directly [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get,set}_param
These hypercalls are usable by HVM guests. Once connected, simple functions of the Xen toolstack can be run from an HVM domain if that domain is permitted access (which is currently only possible via XSM). Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Keir Fraser <keir@xen.org> Cc: Jan Beulich <jbeulich@suse.com> --- xen/arch/x86/hvm/hvm.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 374a740..7eece1f 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3237,8 +3237,10 @@ static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = { HYPERCALL(event_channel_op), HYPERCALL(sched_op), HYPERCALL(set_timer_op), + HYPERCALL(xsm_op), HYPERCALL(hvm_op), HYPERCALL(sysctl), + HYPERCALL(domctl), HYPERCALL(tmem_op) }; @@ -3254,8 +3256,10 @@ static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = { HYPERCALL(event_channel_op), COMPAT_CALL(sched_op), COMPAT_CALL(set_timer_op), + HYPERCALL(xsm_op), HYPERCALL(hvm_op), HYPERCALL(sysctl), + HYPERCALL(domctl), HYPERCALL(tmem_op) }; -- 1.8.0.2
Daniel De Graaf
2013-Jan-17 18:55 UTC
[PATCH 2/3] xen/arch/x86: complete XSM hooks on irq/pirq mappings
Manipulation of a domain''s pirq namespace was not fully protected by XSM hooks because the XSM hooks for IRQs needed a physical IRQ. Since this may not apply to HVM domains, a complete solution needs to split the XSM hook for this operation, using one hook for the PIRQ manipulation and one for controlling access to the hardware IRQ. This reworking has the advantage of providing the same MSI data to remove_irq that is provided to add_irq, allowing the PCI device to be determined in both functions. It also eliminates the last callers of rcu_lock_target_domain_by_id in x86 and common code in preparation for this function''s removal. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Keir Fraser <keir@xen.org> Cc: Jan Beulich <jbeulich@suse.com> --- xen/arch/x86/irq.c | 13 +++++++++---- xen/arch/x86/physdev.c | 18 +++++++++++------- xen/include/xsm/dummy.h | 16 ++++++++++++++-- xen/include/xsm/xsm.h | 24 ++++++++++++++++++------ xen/xsm/dummy.c | 2 ++ xen/xsm/flask/hooks.c | 37 +++++++++++++++++++++---------------- xen/xsm/flask/policy/access_vectors | 5 ++--- 7 files changed, 77 insertions(+), 38 deletions(-) diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index 095c17d..068c5a0 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -1874,7 +1874,7 @@ int map_domain_pirq( return 0; } - ret = xsm_map_domain_pirq(XSM_HOOK, d, irq, data); + ret = xsm_map_domain_irq(XSM_HOOK, d, irq, data); if ( ret ) { dprintk(XENLOG_G_ERR, "dom%d: could not permit access to irq %d mapping to pirq %d\n", @@ -1978,14 +1978,19 @@ int unmap_domain_pirq(struct domain *d, int pirq) goto done; } + desc = irq_to_desc(irq); + msi_desc = desc->msi_desc; + + ret = xsm_unmap_domain_irq(XSM_HOOK, d, irq, msi_desc); + if ( ret ) + goto done; + forced_unbind = pirq_guest_force_unbind(d, info); if ( forced_unbind ) dprintk(XENLOG_G_WARNING, "dom%d: forcing unbind of pirq %d\n", d->domain_id, pirq); - desc = irq_to_desc(irq); - - if ( (msi_desc = desc->msi_desc) != NULL ) + if ( msi_desc != NULL ) pci_disable_msi(msi_desc); spin_lock_irqsave(&desc->lock, flags); diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c index b45e18a..d9ed5df 100644 --- a/xen/arch/x86/physdev.c +++ b/xen/arch/x86/physdev.c @@ -105,7 +105,11 @@ int physdev_map_pirq(domid_t domid, int type, int *index, int *pirq_p, return physdev_hvm_map_pirq(d, type, index, pirq_p); } - ret = rcu_lock_target_domain_by_id(domid, &d); + d = rcu_lock_domain_by_any_id(domid); + if ( d == NULL ) + return -ESRCH; + + ret = xsm_map_domain_pirq(XSM_TARGET, d); if ( ret ) return ret; @@ -218,9 +222,13 @@ int physdev_unmap_pirq(domid_t domid, int pirq) struct domain *d; int ret; - ret = rcu_lock_target_domain_by_id(domid, &d); + d = rcu_lock_domain_by_any_id(domid); + if ( d == NULL ) + return -ESRCH; + + ret = xsm_unmap_domain_pirq(XSM_TARGET, d); if ( ret ) - return ret; + goto free_domain; if ( is_hvm_domain(d) ) { @@ -232,10 +240,6 @@ int physdev_unmap_pirq(domid_t domid, int pirq) goto free_domain; } - ret = xsm_unmap_domain_pirq(XSM_TARGET, d, domain_pirq_to_irq(d, pirq)); - if ( ret ) - goto free_domain; - spin_lock(&pcidevs_lock); spin_lock(&d->event_lock); ret = unmap_domain_pirq(d, pirq); diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 1ca82b0..aa4c7d2 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -405,18 +405,30 @@ static XSM_INLINE char *xsm_show_irq_sid(int irq) return NULL; } -static XSM_INLINE int xsm_map_domain_pirq(XSM_DEFAULT_ARG struct domain *d, int irq, void *data) +static XSM_INLINE int xsm_map_domain_pirq(XSM_DEFAULT_ARG struct domain *d) +{ + XSM_ASSERT_ACTION(XSM_TARGET); + return xsm_default_action(action, current->domain, d); +} + +static XSM_INLINE int xsm_map_domain_irq(XSM_DEFAULT_ARG struct domain *d, int irq, void *data) { XSM_ASSERT_ACTION(XSM_HOOK); return xsm_default_action(action, current->domain, d); } -static XSM_INLINE int xsm_unmap_domain_pirq(XSM_DEFAULT_ARG struct domain *d, int irq) +static XSM_INLINE int xsm_unmap_domain_pirq(XSM_DEFAULT_ARG struct domain *d) { XSM_ASSERT_ACTION(XSM_TARGET); return xsm_default_action(action, current->domain, d); } +static XSM_INLINE int xsm_unmap_domain_irq(XSM_DEFAULT_ARG struct domain *d, int irq, void *data) +{ + XSM_ASSERT_ACTION(XSM_HOOK); + return xsm_default_action(action, current->domain, d); +} + static XSM_INLINE int xsm_irq_permission(XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow) { XSM_ASSERT_ACTION(XSM_HOOK); diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 8947372..c77cb92 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -100,8 +100,10 @@ struct xsm_operations { int (*schedop_shutdown) (struct domain *d1, struct domain *d2); char *(*show_irq_sid) (int irq); - int (*map_domain_pirq) (struct domain *d, int irq, void *data); - int (*unmap_domain_pirq) (struct domain *d, int irq); + int (*map_domain_pirq) (struct domain *d); + int (*map_domain_irq) (struct domain *d, int irq, void *data); + int (*unmap_domain_pirq) (struct domain *d); + int (*unmap_domain_irq) (struct domain *d, int irq, void *data); int (*irq_permission) (struct domain *d, int pirq, uint8_t allow); int (*iomem_permission) (struct domain *d, uint64_t s, uint64_t e, uint8_t allow); int (*iomem_mapping) (struct domain *d, uint64_t s, uint64_t e, uint8_t allow); @@ -365,14 +367,24 @@ static inline char *xsm_show_irq_sid (int irq) return xsm_ops->show_irq_sid(irq); } -static inline int xsm_map_domain_pirq (xsm_default_t def, struct domain *d, int irq, void *data) +static inline int xsm_map_domain_pirq (xsm_default_t def, struct domain *d) { - return xsm_ops->map_domain_pirq(d, irq, data); + return xsm_ops->map_domain_pirq(d); } -static inline int xsm_unmap_domain_pirq (xsm_default_t def, struct domain *d, int irq) +static inline int xsm_map_domain_irq (xsm_default_t def, struct domain *d, int irq, void *data) { - return xsm_ops->unmap_domain_pirq(d, irq); + return xsm_ops->map_domain_irq(d, irq, data); +} + +static inline int xsm_unmap_domain_pirq (xsm_default_t def, struct domain *d) +{ + return xsm_ops->unmap_domain_pirq(d); +} + +static inline int xsm_unmap_domain_irq (xsm_default_t def, struct domain *d, int irq, void *data) +{ + return xsm_ops->unmap_domain_irq(d, irq, data); } static inline int xsm_irq_permission (xsm_default_t def, struct domain *d, int pirq, uint8_t allow) diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 529a724..5a9371f 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -76,7 +76,9 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, show_irq_sid); set_to_dummy_if_null(ops, map_domain_pirq); + set_to_dummy_if_null(ops, map_domain_irq); set_to_dummy_if_null(ops, unmap_domain_pirq); + set_to_dummy_if_null(ops, unmap_domain_irq); set_to_dummy_if_null(ops, irq_permission); set_to_dummy_if_null(ops, iomem_permission); set_to_dummy_if_null(ops, iomem_mapping); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index ba67502..ba136ba 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -818,18 +818,18 @@ static char *flask_show_irq_sid (int irq) return ctx; } -static int flask_map_domain_pirq (struct domain *d, int irq, void *data) +static int flask_map_domain_pirq (struct domain *d) +{ + return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD); +} + +static int flask_map_domain_irq (struct domain *d, int irq, void *data) { u32 sid, dsid; int rc = -EPERM; struct msi_info *msi = data; struct avc_audit_data ad; - rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__ADD); - - if ( rc ) - return rc; - if ( irq >= nr_static_irqs && msi ) { u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn; AVC_AUDIT_DATA_INIT(&ad, DEV); @@ -851,22 +851,25 @@ static int flask_map_domain_pirq (struct domain *d, int irq, void *data) return rc; } -static int flask_unmap_domain_pirq (struct domain *d, int irq) +static int flask_unmap_domain_pirq (struct domain *d) +{ + return current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE); +} + +static int flask_unmap_domain_irq (struct domain *d, int irq, void *data) { u32 sid; int rc = -EPERM; + struct msi_info *msi = data; struct avc_audit_data ad; - rc = current_has_perm(d, SECCLASS_RESOURCE, RESOURCE__REMOVE); - if ( rc ) - return rc; - - if ( irq < nr_static_irqs ) { - rc = get_irq_sid(irq, &sid, &ad); + if ( irq >= nr_static_irqs && msi ) { + u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn; + AVC_AUDIT_DATA_INIT(&ad, DEV); + ad.device = machine_bdf; + rc = security_device_sid(machine_bdf, &sid); } else { - /* It is currently not possible to check the specific MSI IRQ being - * removed, since we do not have the msi_info like map_domain_pirq */ - return 0; + rc = get_irq_sid(irq, &sid, &ad); } if ( rc ) return rc; @@ -1481,7 +1484,9 @@ static struct xsm_operations flask_ops = { .show_irq_sid = flask_show_irq_sid, .map_domain_pirq = flask_map_domain_pirq, + .map_domain_irq = flask_map_domain_irq, .unmap_domain_pirq = flask_unmap_domain_pirq, + .unmap_domain_irq = flask_unmap_domain_irq, .irq_permission = flask_irq_permission, .iomem_permission = flask_iomem_permission, .iomem_mapping = flask_iomem_mapping, diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index 2fdaede..36cbacf 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -368,12 +368,11 @@ class resource # target = resource''s security label # also checked when using some core Xen devices (target xen_t) use -# PHYSDEVOP_map_pirq and ioapic writes for dom0 +# PHYSDEVOP_map_pirq and ioapic writes for dom0, when acting on real IRQs # For GSI interrupts, the IRQ''s label is indexed by the IRQ number # For MSI interrupts, the label of the PCI device is used add_irq -# PHYSDEVOP_unmap_pirq: -# This is currently only checked for GSI interrupts +# PHYSDEVOP_unmap_pirq (same as map, and only for real IRQs) remove_irq # XEN_DOMCTL_ioport_permission, XEN_DOMCTL_ioport_mapping add_ioport -- 1.8.0.2
Daniel De Graaf
2013-Jan-17 18:55 UTC
[PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
This hook is not x86-specific; move it out of CONFIG_X86. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Stefano Stabellini <stefano.stabellini@citrix.com> Cc: Tim Deegan <tim@xen.org> --- xen/arch/arm/hvm.c | 11 ++++++++--- xen/include/xsm/dummy.h | 10 +++++----- xen/include/xsm/xsm.h | 13 +++++++------ xen/xsm/dummy.c | 2 +- xen/xsm/flask/hooks.c | 46 +++++++++++++++++++++++----------------------- 5 files changed, 44 insertions(+), 38 deletions(-) diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c index 40f519e..63ac793 100644 --- a/xen/arch/arm/hvm.c +++ b/xen/arch/arm/hvm.c @@ -30,9 +30,13 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( a.index >= HVM_NR_PARAMS ) return -EINVAL; - rc = rcu_lock_target_domain_by_id(a.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; + + rc = xsm_hvm_param(XSM_TARGET, d, op); + if ( rc ) + goto param_fail; if ( op == HVMOP_set_param ) { @@ -44,6 +48,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; } + param_fail: rcu_unlock_domain(d); break; } diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index aa4c7d2..8c746a4 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -455,16 +455,16 @@ static XSM_INLINE int xsm_pci_config_permission(XSM_DEFAULT_ARG struct domain *d return xsm_default_action(action, current->domain, d); } -#ifdef CONFIG_X86 -static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op) +static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op) { - XSM_ASSERT_ACTION(XSM_HOOK); + XSM_ASSERT_ACTION(XSM_TARGET); return xsm_default_action(action, current->domain, d); } -static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op) +#ifdef CONFIG_X86 +static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op) { - XSM_ASSERT_ACTION(XSM_TARGET); + XSM_ASSERT_ACTION(XSM_HOOK); return xsm_default_action(action, current->domain, d); } diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index c77cb92..c73c872 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -128,9 +128,10 @@ struct xsm_operations { long (*do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op); + int (*hvm_param) (struct domain *d, unsigned long op); + #ifdef CONFIG_X86 int (*shadow_control) (struct domain *d, uint32_t op); - int (*hvm_param) (struct domain *d, unsigned long op); int (*hvm_set_pci_intx_level) (struct domain *d); int (*hvm_set_isa_irq_level) (struct domain *d); int (*hvm_set_pci_link_route) (struct domain *d); @@ -482,15 +483,15 @@ static inline long xsm_do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op) return xsm_ops->do_xsm_op(op); } -#ifdef CONFIG_X86 -static inline int xsm_shadow_control (xsm_default_t def, struct domain *d, uint32_t op) +static inline int xsm_hvm_param (xsm_default_t def, struct domain *d, unsigned long op) { - return xsm_ops->shadow_control(d, op); + return xsm_ops->hvm_param(d, op); } -static inline int xsm_hvm_param (xsm_default_t def, struct domain *d, unsigned long op) +#ifdef CONFIG_X86 +static inline int xsm_shadow_control (xsm_default_t def, struct domain *d, uint32_t op) { - return xsm_ops->hvm_param(d, op); + return xsm_ops->shadow_control(d, op); } static inline int xsm_hvm_set_pci_intx_level (xsm_default_t def, struct domain *d) diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 5a9371f..cae42f5 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -100,12 +100,12 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, page_offline); set_to_dummy_if_null(ops, tmem_op); set_to_dummy_if_null(ops, tmem_control); + set_to_dummy_if_null(ops, hvm_param); set_to_dummy_if_null(ops, do_xsm_op); #ifdef CONFIG_X86 set_to_dummy_if_null(ops, shadow_control); - set_to_dummy_if_null(ops, hvm_param); set_to_dummy_if_null(ops, hvm_set_pci_intx_level); set_to_dummy_if_null(ops, hvm_set_isa_irq_level); set_to_dummy_if_null(ops, hvm_set_pci_link_route); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index ba136ba..eb1d764 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -1058,6 +1058,28 @@ static inline int flask_tmem_control(void) return domain_has_xen(current->domain, XEN__TMEM_CONTROL); } +static int flask_hvm_param(struct domain *d, unsigned long op) +{ + u32 perm; + + switch ( op ) + { + case HVMOP_set_param: + perm = HVM__SETPARAM; + break; + case HVMOP_get_param: + perm = HVM__GETPARAM; + break; + case HVMOP_track_dirty_vram: + perm = HVM__TRACKDIRTYVRAM; + break; + default: + perm = HVM__HVMCTL; + } + + return current_has_perm(d, SECCLASS_HVM, perm); +} + #ifdef CONFIG_X86 static int flask_shadow_control(struct domain *d, uint32_t op) { @@ -1138,28 +1160,6 @@ static int flask_ioport_mapping(struct domain *d, uint32_t start, uint32_t end, return flask_ioport_permission(d, start, end, access); } -static int flask_hvm_param(struct domain *d, unsigned long op) -{ - u32 perm; - - switch ( op ) - { - case HVMOP_set_param: - perm = HVM__SETPARAM; - break; - case HVMOP_get_param: - perm = HVM__GETPARAM; - break; - case HVMOP_track_dirty_vram: - perm = HVM__TRACKDIRTYVRAM; - break; - default: - perm = HVM__HVMCTL; - } - - return current_has_perm(d, SECCLASS_HVM, perm); -} - static int flask_hvm_set_pci_intx_level(struct domain *d) { return current_has_perm(d, SECCLASS_HVM, HVM__PCILEVEL); @@ -1503,12 +1503,12 @@ static struct xsm_operations flask_ops = { .page_offline = flask_page_offline, .tmem_op = flask_tmem_op, .tmem_control = flask_tmem_control, + .hvm_param = flask_hvm_param, .do_xsm_op = do_flask_op, #ifdef CONFIG_X86 .shadow_control = flask_shadow_control, - .hvm_param = flask_hvm_param, .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level, .hvm_set_isa_irq_level = flask_hvm_set_isa_irq_level, .hvm_set_pci_link_route = flask_hvm_set_pci_link_route, -- 1.8.0.2
Ian Campbell
2013-Jan-18 09:19 UTC
Re: [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote:> This hook is not x86-specific; move it out of CONFIG_X86. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > Cc: Tim Deegan <tim@xen.org>Looks good to me (other than diff choosing the most confusing possible way to represent the movement of xsm_hvm_param in xsm.h!) Acked-by: Ian Campbell <ian.campbell@citrix.com>
On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote:> These three patches finish up the XSM IS_PRIV reworking, in addition to > the patches posted by Ian and Lars fixing things for ARM. I did not > include a patch removing the rcu_lock_target_domain_by_id function > because it''s still referenced in ARM at the moment, but it looks like > those locations are being changed (or will be changed in the future).Am I right that the appropriate transformation is: rc = rcu_lock_target_domain_by_id(domid, &d); becomes: d = rcu_lock_domain_by_any_id(a.domid); if ( d == NULL ) return -ESRCH; rc = xsm_SOMETHING(XSM_TARGET, d, PARAMS); if ( rc ) ... unlock .. return or goto err Ian.
Keir Fraser
2013-Jan-23 09:20 UTC
Re: [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
On 17/01/2013 18:55, "Daniel De Graaf" <dgdegra@tycho.nsa.gov> wrote:> This hook is not x86-specific; move it out of CONFIG_X86. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > Cc: Tim Deegan <tim@xen.org>Doesn''t apply to tip of xen-unstable. I''ve committed the other two patches in this series. -- Keir
Daniel De Graaf
2013-Jan-23 14:24 UTC
[PATCH v2 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
This hook is not x86-specific; move it out of CONFIG_X86. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Acked-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/hvm.c | 11 ++++++++--- xen/include/xsm/dummy.h | 10 +++++----- xen/include/xsm/xsm.h | 13 +++++++------ xen/xsm/dummy.c | 2 +- xen/xsm/flask/hooks.c | 46 +++++++++++++++++++++++----------------------- 5 files changed, 44 insertions(+), 38 deletions(-) diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c index 40f519e..63ac793 100644 --- a/xen/arch/arm/hvm.c +++ b/xen/arch/arm/hvm.c @@ -30,9 +30,13 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) if ( a.index >= HVM_NR_PARAMS ) return -EINVAL; - rc = rcu_lock_target_domain_by_id(a.domid, &d); - if ( rc != 0 ) - return rc; + d = rcu_lock_domain_by_any_id(a.domid); + if ( d == NULL ) + return -ESRCH; + + rc = xsm_hvm_param(XSM_TARGET, d, op); + if ( rc ) + goto param_fail; if ( op == HVMOP_set_param ) { @@ -44,6 +48,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg) rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0; } + param_fail: rcu_unlock_domain(d); break; } diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index 19bbe19..025936a 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -467,16 +467,16 @@ static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1, return xsm_default_action(action, d1, d2); } -#ifdef CONFIG_X86 -static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op) +static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op) { - XSM_ASSERT_ACTION(XSM_HOOK); + XSM_ASSERT_ACTION(XSM_TARGET); return xsm_default_action(action, current->domain, d); } -static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op) +#ifdef CONFIG_X86 +static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op) { - XSM_ASSERT_ACTION(XSM_TARGET); + XSM_ASSERT_ACTION(XSM_HOOK); return xsm_default_action(action, current->domain, d); } diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 2399da0..cba744c 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -129,9 +129,10 @@ struct xsm_operations { long (*do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op); + int (*hvm_param) (struct domain *d, unsigned long op); + #ifdef CONFIG_X86 int (*shadow_control) (struct domain *d, uint32_t op); - int (*hvm_param) (struct domain *d, unsigned long op); int (*hvm_set_pci_intx_level) (struct domain *d); int (*hvm_set_isa_irq_level) (struct domain *d); int (*hvm_set_pci_link_route) (struct domain *d); @@ -487,15 +488,15 @@ static inline long xsm_do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op) return xsm_ops->do_xsm_op(op); } -#ifdef CONFIG_X86 -static inline int xsm_shadow_control (xsm_default_t def, struct domain *d, uint32_t op) +static inline int xsm_hvm_param (xsm_default_t def, struct domain *d, unsigned long op) { - return xsm_ops->shadow_control(d, op); + return xsm_ops->hvm_param(d, op); } -static inline int xsm_hvm_param (xsm_default_t def, struct domain *d, unsigned long op) +#ifdef CONFIG_X86 +static inline int xsm_shadow_control (xsm_default_t def, struct domain *d, uint32_t op) { - return xsm_ops->hvm_param(d, op); + return xsm_ops->shadow_control(d, op); } static inline int xsm_hvm_set_pci_intx_level (xsm_default_t def, struct domain *d) diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index dcd3e31..6f1e0b4 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -100,6 +100,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, page_offline); set_to_dummy_if_null(ops, tmem_op); set_to_dummy_if_null(ops, tmem_control); + set_to_dummy_if_null(ops, hvm_param); set_to_dummy_if_null(ops, do_xsm_op); @@ -108,7 +109,6 @@ void xsm_fixup_ops (struct xsm_operations *ops) #ifdef CONFIG_X86 set_to_dummy_if_null(ops, shadow_control); - set_to_dummy_if_null(ops, hvm_param); set_to_dummy_if_null(ops, hvm_set_pci_intx_level); set_to_dummy_if_null(ops, hvm_set_isa_irq_level); set_to_dummy_if_null(ops, hvm_set_pci_link_route); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 5869588..85d009c 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -1068,6 +1068,28 @@ static int flask_remove_from_physmap(struct domain *d1, struct domain *d2) return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); } +static int flask_hvm_param(struct domain *d, unsigned long op) +{ + u32 perm; + + switch ( op ) + { + case HVMOP_set_param: + perm = HVM__SETPARAM; + break; + case HVMOP_get_param: + perm = HVM__GETPARAM; + break; + case HVMOP_track_dirty_vram: + perm = HVM__TRACKDIRTYVRAM; + break; + default: + perm = HVM__HVMCTL; + } + + return current_has_perm(d, SECCLASS_HVM, perm); +} + #ifdef CONFIG_X86 static int flask_shadow_control(struct domain *d, uint32_t op) { @@ -1148,28 +1170,6 @@ static int flask_ioport_mapping(struct domain *d, uint32_t start, uint32_t end, return flask_ioport_permission(d, start, end, access); } -static int flask_hvm_param(struct domain *d, unsigned long op) -{ - u32 perm; - - switch ( op ) - { - case HVMOP_set_param: - perm = HVM__SETPARAM; - break; - case HVMOP_get_param: - perm = HVM__GETPARAM; - break; - case HVMOP_track_dirty_vram: - perm = HVM__TRACKDIRTYVRAM; - break; - default: - perm = HVM__HVMCTL; - } - - return current_has_perm(d, SECCLASS_HVM, perm); -} - static int flask_hvm_set_pci_intx_level(struct domain *d) { return current_has_perm(d, SECCLASS_HVM, HVM__PCILEVEL); @@ -1503,6 +1503,7 @@ static struct xsm_operations flask_ops = { .page_offline = flask_page_offline, .tmem_op = flask_tmem_op, .tmem_control = flask_tmem_control, + .hvm_param = flask_hvm_param, .do_xsm_op = do_flask_op, @@ -1511,7 +1512,6 @@ static struct xsm_operations flask_ops = { #ifdef CONFIG_X86 .shadow_control = flask_shadow_control, - .hvm_param = flask_hvm_param, .hvm_set_pci_intx_level = flask_hvm_set_pci_intx_level, .hvm_set_isa_irq_level = flask_hvm_set_isa_irq_level, .hvm_set_pci_link_route = flask_hvm_set_pci_link_route, -- 1.8.0.2
Daniel De Graaf
2013-Jan-23 14:37 UTC
Re: [PATCH 0/3] XSM PIRQ and rcu_lock_target cleanup
On 01/18/2013 04:23 AM, Ian Campbell wrote:> On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote: >> These three patches finish up the XSM IS_PRIV reworking, in addition to >> the patches posted by Ian and Lars fixing things for ARM. I did not >> include a patch removing the rcu_lock_target_domain_by_id function >> because it''s still referenced in ARM at the moment, but it looks like >> those locations are being changed (or will be changed in the future). > > Am I right that the appropriate transformation is: > rc = rcu_lock_target_domain_by_id(domid, &d); > becomes: > d = rcu_lock_domain_by_any_id(a.domid); > if ( d == NULL ) > return -ESRCH; > > rc = xsm_SOMETHING(XSM_TARGET, d, PARAMS); > if ( rc ) > ... unlock .. return or goto err > > Ian. >Yes, this is correct. Looking at today''s xen-unstable, only the XENMAPSPACE_gmfn_foreign mapping needs a new ARM-only XSM hook. -- Daniel De Graaf National Security Agency
On Wed, 2013-01-23 at 14:37 +0000, Daniel De Graaf wrote:> On 01/18/2013 04:23 AM, Ian Campbell wrote: > > On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote: > >> These three patches finish up the XSM IS_PRIV reworking, in addition to > >> the patches posted by Ian and Lars fixing things for ARM. I did not > >> include a patch removing the rcu_lock_target_domain_by_id function > >> because it''s still referenced in ARM at the moment, but it looks like > >> those locations are being changed (or will be changed in the future). > > > > Am I right that the appropriate transformation is: > > rc = rcu_lock_target_domain_by_id(domid, &d); > > becomes: > > d = rcu_lock_domain_by_any_id(a.domid); > > if ( d == NULL ) > > return -ESRCH; > > > > rc = xsm_SOMETHING(XSM_TARGET, d, PARAMS); > > if ( rc ) > > ... unlock .. return or goto err > > > > Ian. > > > > Yes, this is correct. Looking at today''s xen-unstable, only the > XENMAPSPACE_gmfn_foreign mapping needs a new ARM-only XSM hook.Does it? Even on x86 xsm only has a single xsm_add_to_physmap covering all XENMAPSPACE cases and ARM matches this. Ian.
Daniel De Graaf
2013-Jan-23 14:51 UTC
Re: [PATCH 0/3] XSM PIRQ and rcu_lock_target cleanup
On 01/23/2013 09:43 AM, Ian Campbell wrote:> On Wed, 2013-01-23 at 14:37 +0000, Daniel De Graaf wrote: >> On 01/18/2013 04:23 AM, Ian Campbell wrote: >>> On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote: >>>> These three patches finish up the XSM IS_PRIV reworking, in addition to >>>> the patches posted by Ian and Lars fixing things for ARM. I did not >>>> include a patch removing the rcu_lock_target_domain_by_id function >>>> because it''s still referenced in ARM at the moment, but it looks like >>>> those locations are being changed (or will be changed in the future). >>> >>> Am I right that the appropriate transformation is: >>> rc = rcu_lock_target_domain_by_id(domid, &d); >>> becomes: >>> d = rcu_lock_domain_by_any_id(a.domid); >>> if ( d == NULL ) >>> return -ESRCH; >>> >>> rc = xsm_SOMETHING(XSM_TARGET, d, PARAMS); >>> if ( rc ) >>> ... unlock .. return or goto err >>> >>> Ian. >>> >> >> Yes, this is correct. Looking at today''s xen-unstable, only the >> XENMAPSPACE_gmfn_foreign mapping needs a new ARM-only XSM hook. > > Does it? Even on x86 xsm only has a single xsm_add_to_physmap covering > all XENMAPSPACE cases and ARM matches this. > > Ian. >Since x86 doesn''t implement XENMAPSPACE_gmfn_foreign, it doesn''t need another check. The additional check would ensure that the domain doing the mapping has access to the foreign pages, while the existing checks verify that it has the right to change the physmap being modified. -- Daniel De Graaf National Security Agency
On Wed, 2013-01-23 at 14:51 +0000, Daniel De Graaf wrote:> On 01/23/2013 09:43 AM, Ian Campbell wrote: > > On Wed, 2013-01-23 at 14:37 +0000, Daniel De Graaf wrote: > >> On 01/18/2013 04:23 AM, Ian Campbell wrote: > >>> On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote: > >>>> These three patches finish up the XSM IS_PRIV reworking, in addition to > >>>> the patches posted by Ian and Lars fixing things for ARM. I did not > >>>> include a patch removing the rcu_lock_target_domain_by_id function > >>>> because it''s still referenced in ARM at the moment, but it looks like > >>>> those locations are being changed (or will be changed in the future). > >>> > >>> Am I right that the appropriate transformation is: > >>> rc = rcu_lock_target_domain_by_id(domid, &d); > >>> becomes: > >>> d = rcu_lock_domain_by_any_id(a.domid); > >>> if ( d == NULL ) > >>> return -ESRCH; > >>> > >>> rc = xsm_SOMETHING(XSM_TARGET, d, PARAMS); > >>> if ( rc ) > >>> ... unlock .. return or goto err > >>> > >>> Ian. > >>> > >> > >> Yes, this is correct. Looking at today''s xen-unstable, only the > >> XENMAPSPACE_gmfn_foreign mapping needs a new ARM-only XSM hook. > > > > Does it? Even on x86 xsm only has a single xsm_add_to_physmap covering > > all XENMAPSPACE cases and ARM matches this. > > > > Ian. > > > > Since x86 doesn''t implement XENMAPSPACE_gmfn_foreign, it doesn''t need > another check. The additional check would ensure that the domain doing > the mapping has access to the foreign pages, while the existing checks > verify that it has the right to change the physmap being modified.Ah, right yes. x86 will need this too when PVH lands. Ian.
Ian Campbell
2013-Jan-31 09:16 UTC
Re: [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
On Fri, 2013-01-18 at 09:19 +0000, Ian Campbell wrote:> On Thu, 2013-01-17 at 18:55 +0000, Daniel De Graaf wrote: > > This hook is not x86-specific; move it out of CONFIG_X86. > > > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Cc: Ian Campbell <ian.campbell@citrix.com> > > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > > Cc: Tim Deegan <tim@xen.org> > > Looks good to me (other than diff choosing the most confusing possible > way to represent the movement of xsm_hvm_param in xsm.h!) > > Acked-by: Ian Campbell <ian.campbell@citrix.com>Looks like I should have actually built this one before acking... 8<-------------------------------- From f2ec1f2cba7eab8d6c6c93378625379abe0e29bc Mon Sep 17 00:00:00 2001 From: Ian Campbell <ian.campbell@citrix.com> Date: Thu, 31 Jan 2013 09:14:40 +0000 Subject: [PATCH] xen: arm: fix build of hvm.c Add include of xsm/xsm.h to fix: hvm.c: In function ''do_hvm_op'': hvm.c:37:9: error: implicit declaration of function ''xsm_hvm_param'' [-Werror=implicit-function-declaration] hvm.c:37:9: error: nested extern declaration of ''xsm_hvm_param'' [-Werror=nested-externs] hvm.c:37:28: error: ''XSM_TARGET'' undeclared (first use in this function) hvm.c:37:28: note: each undeclared identifier is reported only once for each function it appears in cc1: all warnings being treated as errors Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Stefano Stabellini <stefano.stabellini@citrix.com> Cc: Tim Deegan <tim@xen.org> --- xen/arch/arm/hvm.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c index 63ac793..471c4cd 100644 --- a/xen/arch/arm/hvm.c +++ b/xen/arch/arm/hvm.c @@ -5,6 +5,8 @@ #include <xen/guest_access.h> #include <xen/sched.h> +#include <xsm/xsm.h> + #include <public/xen.h> #include <public/hvm/params.h> #include <public/hvm/hvm_op.h> -- 1.7.9.1
Ian Campbell
2013-Feb-05 10:59 UTC
Re: [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
Stefano, Tim, can you (n)ack this please. On Thu, 2013-01-31 at 09:16 +0000, Ian Campbell wrote:> 8<-------------------------------- > > From f2ec1f2cba7eab8d6c6c93378625379abe0e29bc Mon Sep 17 00:00:00 2001 > From: Ian Campbell <ian.campbell@citrix.com> > Date: Thu, 31 Jan 2013 09:14:40 +0000 > Subject: [PATCH] xen: arm: fix build of hvm.c > > Add include of xsm/xsm.h to fix: > > hvm.c: In function ''do_hvm_op'': hvm.c:37:9: error: implicit declaration of function ''xsm_hvm_param'' [-Werror=implicit-function-declaration] > hvm.c:37:9: error: nested extern declaration of ''xsm_hvm_param'' [-Werror=nested-externs] > hvm.c:37:28: error: ''XSM_TARGET'' undeclared (first use in this function) > hvm.c:37:28: note: each undeclared identifier is reported only once for each function it appears in > cc1: all warnings being treated as errors > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > Cc: Tim Deegan <tim@xen.org> > --- > xen/arch/arm/hvm.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > index 63ac793..471c4cd 100644 > --- a/xen/arch/arm/hvm.c > +++ b/xen/arch/arm/hvm.c > @@ -5,6 +5,8 @@ > #include <xen/guest_access.h> > #include <xen/sched.h> > > +#include <xsm/xsm.h> > + > #include <public/xen.h> > #include <public/hvm/params.h> > #include <public/hvm/hvm_op.h>
Stefano Stabellini
2013-Feb-05 11:31 UTC
Re: [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
On Tue, 5 Feb 2013, Ian Campbell wrote:> Stefano, Tim, can you (n)ack this please.The patch seems simple enough. Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> On Thu, 2013-01-31 at 09:16 +0000, Ian Campbell wrote: > > 8<-------------------------------- > > > > From f2ec1f2cba7eab8d6c6c93378625379abe0e29bc Mon Sep 17 00:00:00 2001 > > From: Ian Campbell <ian.campbell@citrix.com> > > Date: Thu, 31 Jan 2013 09:14:40 +0000 > > Subject: [PATCH] xen: arm: fix build of hvm.c > > > > Add include of xsm/xsm.h to fix: > > > > hvm.c: In function ''do_hvm_op'': hvm.c:37:9: error: implicit declaration of function ''xsm_hvm_param'' [-Werror=implicit-function-declaration] > > hvm.c:37:9: error: nested extern declaration of ''xsm_hvm_param'' [-Werror=nested-externs] > > hvm.c:37:28: error: ''XSM_TARGET'' undeclared (first use in this function) > > hvm.c:37:28: note: each undeclared identifier is reported only once for each function it appears in > > cc1: all warnings being treated as errors > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > > Cc: Tim Deegan <tim@xen.org> > > --- > > xen/arch/arm/hvm.c | 2 ++ > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > > index 63ac793..471c4cd 100644 > > --- a/xen/arch/arm/hvm.c > > +++ b/xen/arch/arm/hvm.c > > @@ -5,6 +5,8 @@ > > #include <xen/guest_access.h> > > #include <xen/sched.h> > > > > +#include <xsm/xsm.h> > > + > > #include <public/xen.h> > > #include <public/hvm/params.h> > > #include <public/hvm/hvm_op.h> > > >
Ian Campbell
2013-Feb-05 11:46 UTC
Re: [PATCH 3/3] xen/arch/arm: add XSM hook to HVMOP_{get, set}_param
On Tue, 2013-02-05 at 11:31 +0000, Stefano Stabellini wrote:> On Tue, 5 Feb 2013, Ian Campbell wrote: > > Stefano, Tim, can you (n)ack this please. > > The patch seems simple enough. > > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Applied, thanks.