* [PATCH 1/2] implement is_hardware_domain using hardware_domain global @ 2014-04-14 21:23 Daniel De Graaf 2014-04-14 21:23 ` [PATCH v4 2/2] allow hardware domain != dom0 Daniel De Graaf 2014-04-15 8:56 ` [PATCH 1/2] implement is_hardware_domain using hardware_domain global Ian Campbell 0 siblings, 2 replies; 9+ messages in thread From: Daniel De Graaf @ 2014-04-14 21:23 UTC (permalink / raw) To: xen-devel Cc: Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson, Stefano Stabellini, Jan Beulich, Daniel De Graaf This requires setting the hardware_domain variable earlier in domain_create so that functions called from it (primarily in arch_domain_create) observe the correct results when they call is_hardware_domain. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Keir Fraser <keir@xen.org> Cc: Stefano Stabellini <stefano.stabellini@citrix.com> Cc: Tim Deegan <tim@xen.org> --- xen/arch/arm/setup.c | 2 +- xen/arch/x86/setup.c | 2 +- xen/common/domain.c | 5 ++++- xen/include/xen/sched.h | 4 ++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 358eafb..6b77a4c 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -761,7 +761,7 @@ void __init start_xen(unsigned long boot_phys_offset, do_initcalls(); /* Create initial domain 0. */ - hardware_domain = dom0 = domain_create(0, 0, 0); + dom0 = domain_create(0, 0, 0); if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) panic("Error creating domain 0"); diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 11c95fc..2e30701 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1339,7 +1339,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) panic("Could not protect TXT memory regions"); /* Create initial domain 0. */ - hardware_domain = dom0 = domain_create(0, DOMCRF_s3_integrity, 0); + dom0 = domain_create(0, DOMCRF_s3_integrity, 0); if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) panic("Error creating domain 0"); diff --git a/xen/common/domain.c b/xen/common/domain.c index c4720a9..3c05711 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -237,10 +237,11 @@ struct domain *domain_create( else if ( domcr_flags & DOMCRF_pvh ) d->guest_type = guest_type_pvh; - if ( is_hardware_domain(d) ) + if ( domid == 0 ) { d->is_pinned = opt_dom0_vcpus_pin; d->disable_migrate = 1; + hardware_domain = d; } rangeset_domain_initialise(d); @@ -319,6 +320,8 @@ struct domain *domain_create( fail: d->is_dying = DOMDYING_dead; + if ( hardware_domain == d ) + hardware_domain = NULL; atomic_set(&d->refcnt, DOMAIN_DESTROYED); xfree(d->mem_event); xfree(d->pbuf); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index b080c9e..734f7a9 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -795,9 +795,9 @@ void watchdog_domain_destroy(struct domain *d); * Use this check when the following are both true: * - Using this feature or interface requires full access to the hardware * (that is, this would not be suitable for a driver domain) - * - There is never a reason to deny dom0 access to this + * - There is never a reason to deny the hardware domain access to this */ -#define is_hardware_domain(_d) ((_d)->domain_id == 0) +#define is_hardware_domain(_d) ((_d) == hardware_domain) /* This check is for functionality specific to a control domain */ #define is_control_domain(_d) ((_d)->is_privileged) -- 1.9.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/2] allow hardware domain != dom0 2014-04-14 21:23 [PATCH 1/2] implement is_hardware_domain using hardware_domain global Daniel De Graaf @ 2014-04-14 21:23 ` Daniel De Graaf 2014-04-15 7:53 ` Jan Beulich 2014-04-15 14:45 ` Andrew Cooper 2014-04-15 8:56 ` [PATCH 1/2] implement is_hardware_domain using hardware_domain global Ian Campbell 1 sibling, 2 replies; 9+ messages in thread From: Daniel De Graaf @ 2014-04-14 21:23 UTC (permalink / raw) To: xen-devel Cc: Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson, Jan Beulich, Daniel De Graaf This adds a hypervisor command line option "hardware_dom=" which takes a domain ID. When the domain with this ID is created, it will be used as the hardware domain. This is intended to be used when domain 0 is a dedicated stub domain for domain building, allowing the hardware domain to be de-privileged and act only as a driver domain. Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Keir Fraser <keir@xen.org> Cc: Tim Deegan <tim@xen.org> --- docs/misc/xen-command-line.markdown | 10 ++++++ xen/arch/x86/domain_build.c | 4 ++- xen/common/domain.c | 62 +++++++++++++++++++++++++++++++++++-- xen/common/rangeset.c | 40 ++++++++++++++++++++++++ xen/include/xen/rangeset.h | 3 ++ xen/include/xen/sched.h | 6 ++++ xen/include/xsm/dummy.h | 6 ++++ xen/include/xsm/xsm.h | 6 ++++ xen/xsm/dummy.c | 2 ++ xen/xsm/flask/hooks.c | 6 ++++ xen/xsm/flask/policy/access_vectors | 2 ++ 11 files changed, 143 insertions(+), 4 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 87de2dc..6f5064f 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -590,6 +590,16 @@ Paging (HAP). Flag to enable 2 MB host page table support for Hardware Assisted Paging (HAP). +### hardware\_dom +> `= <domid>` + +> Default: `0` + +Enable late hardware domain creation using the specified domain ID. This is +intended to be used when domain 0 is a stub domain which builds a disaggregated +system including a hardware domain with the specified domain ID. This option is +supported only when compiled with CONFIG\_XSM on x86. + ### hpetbroadcast > `= <boolean>` diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c index f75f6e7..10fcd43 100644 --- a/xen/arch/x86/domain_build.c +++ b/xen/arch/x86/domain_build.c @@ -1150,7 +1150,9 @@ int __init construct_dom0( printk(" Xen warning: dom0 kernel broken ELF: %s\n", elf_check_broken(&elf)); - iommu_hwdom_init(hardware_domain); + if ( d->domain_id == hardware_domid ) + iommu_hwdom_init(d); + return 0; out: diff --git a/xen/common/domain.c b/xen/common/domain.c index 3c05711..11c905a 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -61,6 +61,11 @@ struct domain *domain_list; struct domain *hardware_domain __read_mostly; +#ifdef CONFIG_LATE_HWDOM +domid_t hardware_domid __read_mostly; +integer_param("hardware_dom", hardware_domid); +#endif + struct vcpu *idle_vcpu[NR_CPUS] __read_mostly; vcpu_info_t dummy_vcpu_info; @@ -178,6 +183,53 @@ struct vcpu *alloc_vcpu( return v; } +static int late_hwdom_init(struct domain *d) +{ +#ifdef CONFIG_LATE_HWDOM + struct domain *dom0; + int rv; + + if ( d != hardware_domain || d->domain_id == 0 ) + return 0; + + rv = xsm_init_hardware_domain(XSM_HOOK, d); + if ( rv ) + return rv; + + printk("Initialising hardware domain %d\n", hardware_domid); + + dom0 = rcu_lock_domain_by_id(0); + ASSERT(dom0 != NULL); + /* + * Hardware resource ranges for domain 0 have been set up from + * various sources intended to restrict the hardware domain's + * access. Apply these ranges to the actual hardware domain. + * + * Because the lists are being swapped, a side effect of this + * operation is that Domain 0's rangesets are cleared. Since + * domain 0 should not be accessing the hardware when it constructs + * a hardware domain, this should not be a problem. Both lists + * may be modified after this hypercall returns if a more complex + * device model is desired. + * + * Since late hardware domain initialization is only supported on + * x86, the reference to arch.ioport_caps does not need its own + * preprocessor conditional. + */ + rangeset_swap(d->irq_caps, dom0->irq_caps); + rangeset_swap(d->iomem_caps, dom0->iomem_caps); + rangeset_swap(d->arch.ioport_caps, dom0->arch.ioport_caps); + + rcu_unlock_domain(dom0); + + iommu_hwdom_init(d); + + return rv; +#else + return 0; +#endif +} + static unsigned int __read_mostly extra_dom0_irqs = 256; static unsigned int __read_mostly extra_domU_irqs = 32; static void __init parse_extra_guest_irqs(const char *s) @@ -192,7 +244,7 @@ custom_param("extra_guest_irqs", parse_extra_guest_irqs); struct domain *domain_create( domid_t domid, unsigned int domcr_flags, uint32_t ssidref) { - struct domain *d, **pd; + struct domain *d, **pd, *old_hwdom = NULL; enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2, INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 }; int err, init_status = 0; @@ -237,10 +289,11 @@ struct domain *domain_create( else if ( domcr_flags & DOMCRF_pvh ) d->guest_type = guest_type_pvh; - if ( domid == 0 ) + if ( domid == 0 || domid == hardware_domid ) { d->is_pinned = opt_dom0_vcpus_pin; d->disable_migrate = 1; + old_hwdom = hardware_domain; hardware_domain = d; } @@ -316,12 +369,15 @@ struct domain *domain_create( spin_unlock(&domlist_update_lock); } + if ( (err = late_hwdom_init(d)) != 0 ) + goto fail; + return d; fail: d->is_dying = DOMDYING_dead; if ( hardware_domain == d ) - hardware_domain = NULL; + hardware_domain = old_hwdom; atomic_set(&d->refcnt, DOMAIN_DESTROYED); xfree(d->mem_event); xfree(d->pbuf); diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c index f09c0c4..52fae1f 100644 --- a/xen/common/rangeset.c +++ b/xen/common/rangeset.c @@ -438,3 +438,43 @@ void rangeset_domain_printk( spin_unlock(&d->rangesets_lock); } + +void rangeset_swap(struct rangeset *a, struct rangeset *b) +{ + struct list_head tmp; + if (&a < &b) + { + spin_lock(&a->lock); + spin_lock(&b->lock); + } + else + { + spin_lock(&b->lock); + spin_lock(&a->lock); + } + memcpy(&tmp, &a->range_list, sizeof(tmp)); + memcpy(&a->range_list, &b->range_list, sizeof(tmp)); + memcpy(&b->range_list, &tmp, sizeof(tmp)); + if ( a->range_list.next == &b->range_list ) + { + a->range_list.next = &a->range_list; + a->range_list.prev = &a->range_list; + } + else + { + a->range_list.next->prev = &a->range_list; + a->range_list.prev->next = &a->range_list; + } + if ( b->range_list.next == &a->range_list ) + { + b->range_list.next = &b->range_list; + b->range_list.prev = &b->range_list; + } + else + { + b->range_list.next->prev = &b->range_list; + b->range_list.prev->next = &b->range_list; + } + spin_unlock(&a->lock); + spin_unlock(&b->lock); +} diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h index 1e16a6b..805ebde 100644 --- a/xen/include/xen/rangeset.h +++ b/xen/include/xen/rangeset.h @@ -73,4 +73,7 @@ void rangeset_printk( void rangeset_domain_printk( struct domain *d); +/* swap contents */ +void rangeset_swap(struct rangeset *a, struct rangeset *b); + #endif /* __XEN_RANGESET_H__ */ diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 734f7a9..44851ae 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -46,6 +46,12 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_compat_t); /* A global pointer to the hardware domain (usually DOM0). */ extern struct domain *hardware_domain; +#ifdef CONFIG_LATE_HWDOM +extern domid_t hardware_domid; +#else +#define hardware_domid 0 +#endif + #ifndef CONFIG_COMPAT #define BITS_PER_EVTCHN_WORD(d) BITS_PER_XEN_ULONG #else diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index e722155..8ca1117 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -299,6 +299,12 @@ static XSM_INLINE char *xsm_show_security_evtchn(struct domain *d, const struct return NULL; } +static XSM_INLINE int xsm_init_hardware_domain(XSM_DEFAULT_ARG struct domain *d) +{ + XSM_ASSERT_ACTION(XSM_HOOK); + return xsm_default_action(action, current->domain, d); +} + static XSM_INLINE int xsm_get_pod_target(XSM_DEFAULT_ARG struct domain *d) { XSM_ASSERT_ACTION(XSM_PRIV); diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 2cd3a3b..ef1c584 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -82,6 +82,7 @@ struct xsm_operations { int (*alloc_security_evtchn) (struct evtchn *chn); void (*free_security_evtchn) (struct evtchn *chn); char *(*show_security_evtchn) (struct domain *d, const struct evtchn *chn); + int (*init_hardware_domain) (struct domain *d); int (*get_pod_target) (struct domain *d); int (*set_pod_target) (struct domain *d); @@ -311,6 +312,11 @@ static inline char *xsm_show_security_evtchn (struct domain *d, const struct evt return xsm_ops->show_security_evtchn(d, chn); } +static inline int xsm_init_hardware_domain (xsm_default_t def, struct domain *d) +{ + return xsm_ops->init_hardware_domain(d); +} + static inline int xsm_get_pod_target (xsm_default_t def, struct domain *d) { return xsm_ops->get_pod_target(d); diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index b79e10f..c2804f2 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -58,6 +58,8 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, alloc_security_evtchn); set_to_dummy_if_null(ops, free_security_evtchn); set_to_dummy_if_null(ops, show_security_evtchn); + set_to_dummy_if_null(ops, init_hardware_domain); + set_to_dummy_if_null(ops, get_pod_target); set_to_dummy_if_null(ops, set_pod_target); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 4ce31c9..f1a4a2d 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -327,6 +327,11 @@ static char *flask_show_security_evtchn(struct domain *d, const struct evtchn *c return ctx; } +static int flask_init_hardware_domain(struct domain *d) +{ + return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CREATE_HARDWARE_DOMAIN); +} + static int flask_grant_mapref(struct domain *d1, struct domain *d2, uint32_t flags) { @@ -1500,6 +1505,7 @@ static struct xsm_operations flask_ops = { .alloc_security_evtchn = flask_alloc_security_evtchn, .free_security_evtchn = flask_free_security_evtchn, .show_security_evtchn = flask_show_security_evtchn, + .init_hardware_domain = flask_init_hardware_domain, .get_pod_target = flask_get_pod_target, .set_pod_target = flask_set_pod_target, diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index a0ed13d..32371a9 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -198,6 +198,8 @@ class domain2 set_max_evtchn # XEN_DOMCTL_cacheflush cacheflush +# Creation of the hardware domain when it is not dom0 + create_hardware_domain } # Similar to class domain, but primarily contains domctls related to HVM domains -- 1.9.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] allow hardware domain != dom0 2014-04-14 21:23 ` [PATCH v4 2/2] allow hardware domain != dom0 Daniel De Graaf @ 2014-04-15 7:53 ` Jan Beulich 2014-04-15 14:45 ` Andrew Cooper 1 sibling, 0 replies; 9+ messages in thread From: Jan Beulich @ 2014-04-15 7:53 UTC (permalink / raw) To: Daniel De Graaf Cc: Keir Fraser, Tim Deegan, Ian Jackson, Ian Campbell, xen-devel >>> On 14.04.14 at 23:23, <dgdegra@tycho.nsa.gov> wrote: Much better! > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -590,6 +590,16 @@ Paging (HAP). > Flag to enable 2 MB host page table support for Hardware Assisted > Paging (HAP). > > +### hardware\_dom > +> `= <domid>` > + > +> Default: `0` > + > +Enable late hardware domain creation using the specified domain ID. This is > +intended to be used when domain 0 is a stub domain which builds a disaggregated > +system including a hardware domain with the specified domain ID. This option is > +supported only when compiled with CONFIG\_XSM on x86. I think we ought to name the build option (XSM_ENABLE=y) here rather than the internally used manifest constant to propagate the requested setting. > +static int late_hwdom_init(struct domain *d) > +{ > +#ifdef CONFIG_LATE_HWDOM > + struct domain *dom0; > + int rv; > + > + if ( d != hardware_domain || d->domain_id == 0 ) > + return 0; > + > + rv = xsm_init_hardware_domain(XSM_HOOK, d); > + if ( rv ) > + return rv; > + > + printk("Initialising hardware domain %d\n", hardware_domid); > + > + dom0 = rcu_lock_domain_by_id(0); > + ASSERT(dom0 != NULL); > + /* > + * Hardware resource ranges for domain 0 have been set up from > + * various sources intended to restrict the hardware domain's > + * access. Apply these ranges to the actual hardware domain. > + * > + * Because the lists are being swapped, a side effect of this > + * operation is that Domain 0's rangesets are cleared. Since > + * domain 0 should not be accessing the hardware when it constructs > + * a hardware domain, this should not be a problem. Both lists > + * may be modified after this hypercall returns if a more complex > + * device model is desired. > + * > + * Since late hardware domain initialization is only supported on > + * x86, the reference to arch.ioport_caps does not need its own > + * preprocessor conditional. This is only "for now" - please either say so, or even better drop this part of the comment and add the conditional regardless of it being redundant right now. > +void rangeset_swap(struct rangeset *a, struct rangeset *b) > +{ > + struct list_head tmp; > + if (&a < &b) Coding style. Also I pretty certain you don't want to compare the addresses of the pointers here, but the pointer themselves. > @@ -73,4 +73,7 @@ void rangeset_printk( > void rangeset_domain_printk( > struct domain *d); > > +/* swap contents */ > +void rangeset_swap(struct rangeset *a, struct rangeset *b); > + Please put this above the printing ones. Jan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] allow hardware domain != dom0 2014-04-14 21:23 ` [PATCH v4 2/2] allow hardware domain != dom0 Daniel De Graaf 2014-04-15 7:53 ` Jan Beulich @ 2014-04-15 14:45 ` Andrew Cooper 2014-04-15 22:07 ` Daniel De Graaf 1 sibling, 1 reply; 9+ messages in thread From: Andrew Cooper @ 2014-04-15 14:45 UTC (permalink / raw) To: Daniel De Graaf Cc: Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson, xen-devel, Jan Beulich On 14/04/14 22:23, Daniel De Graaf wrote: > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 3c05711..11c905a 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -61,6 +61,11 @@ struct domain *domain_list; > > struct domain *hardware_domain __read_mostly; > > +#ifdef CONFIG_LATE_HWDOM > +domid_t hardware_domid __read_mostly; > +integer_param("hardware_dom", hardware_domid); > +#endif > + Is it worth putting a custom_param() in here which clamps hardware_domid below FIRST_RESERVED_DOMID, or allow anyone specifying hardware_dom=0xffff to keep all the broken pieces they find? Aliasing the magic domids is sure to break things. ~Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] allow hardware domain != dom0 2014-04-15 14:45 ` Andrew Cooper @ 2014-04-15 22:07 ` Daniel De Graaf 2014-04-15 23:37 ` Andrew Cooper 0 siblings, 1 reply; 9+ messages in thread From: Daniel De Graaf @ 2014-04-15 22:07 UTC (permalink / raw) To: Andrew Cooper Cc: Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson, xen-devel, Jan Beulich On 04/15/2014 10:45 AM, Andrew Cooper wrote: > On 14/04/14 22:23, Daniel De Graaf wrote: >> diff --git a/xen/common/domain.c b/xen/common/domain.c >> index 3c05711..11c905a 100644 >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -61,6 +61,11 @@ struct domain *domain_list; >> >> struct domain *hardware_domain __read_mostly; >> >> +#ifdef CONFIG_LATE_HWDOM >> +domid_t hardware_domid __read_mostly; >> +integer_param("hardware_dom", hardware_domid); >> +#endif >> + > > Is it worth putting a custom_param() in here which clamps hardware_domid > below FIRST_RESERVED_DOMID, or allow anyone specifying > hardware_dom=0xffff to keep all the broken pieces they find? Aliasing > the magic domids is sure to break things. > > ~Andrew I'm currently of the opinion that a custom_param is overkill to prevent users from deliberately doing bad things, but could easily write one if others think that it would be helpful. -- Daniel De Graaf National Security Agency ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] allow hardware domain != dom0 2014-04-15 22:07 ` Daniel De Graaf @ 2014-04-15 23:37 ` Andrew Cooper 2014-04-16 9:08 ` Jan Beulich 0 siblings, 1 reply; 9+ messages in thread From: Andrew Cooper @ 2014-04-15 23:37 UTC (permalink / raw) To: Daniel De Graaf Cc: Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson, xen-devel, Jan Beulich On 15/04/2014 23:07, Daniel De Graaf wrote: > On 04/15/2014 10:45 AM, Andrew Cooper wrote: >> On 14/04/14 22:23, Daniel De Graaf wrote: >>> diff --git a/xen/common/domain.c b/xen/common/domain.c >>> index 3c05711..11c905a 100644 >>> --- a/xen/common/domain.c >>> +++ b/xen/common/domain.c >>> @@ -61,6 +61,11 @@ struct domain *domain_list; >>> >>> struct domain *hardware_domain __read_mostly; >>> >>> +#ifdef CONFIG_LATE_HWDOM >>> +domid_t hardware_domid __read_mostly; >>> +integer_param("hardware_dom", hardware_domid); >>> +#endif >>> + >> >> Is it worth putting a custom_param() in here which clamps hardware_domid >> below FIRST_RESERVED_DOMID, or allow anyone specifying >> hardware_dom=0xffff to keep all the broken pieces they find? Aliasing >> the magic domids is sure to break things. >> >> ~Andrew > > I'm currently of the opinion that a custom_param is overkill to prevent > users from deliberately doing bad things, but could easily write one if > others think that it would be helpful. > I suppose the answer depends on how subtle the breakage will be if the user gets it accidentally wrong. Given that slightly over half the domid space is reserved and domid_t signed value, I can forsee subtle bugs if a domid_t ever used as an array index, as well as very unsubtle breakage if the user aliases one of the magic domids. On the balance, a custom_param() is probably worthwhile for peace of mind. ~Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] allow hardware domain != dom0 2014-04-15 23:37 ` Andrew Cooper @ 2014-04-16 9:08 ` Jan Beulich 2014-04-16 9:41 ` Andrew Cooper 0 siblings, 1 reply; 9+ messages in thread From: Jan Beulich @ 2014-04-16 9:08 UTC (permalink / raw) To: Andrew Cooper, Daniel De Graaf Cc: Keir Fraser, Tim Deegan, Ian Jackson, Ian Campbell, xen-devel >>> On 16.04.14 at 01:37, <andrew.cooper3@citrix.com> wrote: > On 15/04/2014 23:07, Daniel De Graaf wrote: >> On 04/15/2014 10:45 AM, Andrew Cooper wrote: >>> On 14/04/14 22:23, Daniel De Graaf wrote: >>>> diff --git a/xen/common/domain.c b/xen/common/domain.c >>>> index 3c05711..11c905a 100644 >>>> --- a/xen/common/domain.c >>>> +++ b/xen/common/domain.c >>>> @@ -61,6 +61,11 @@ struct domain *domain_list; >>>> >>>> struct domain *hardware_domain __read_mostly; >>>> >>>> +#ifdef CONFIG_LATE_HWDOM >>>> +domid_t hardware_domid __read_mostly; >>>> +integer_param("hardware_dom", hardware_domid); >>>> +#endif >>>> + >>> >>> Is it worth putting a custom_param() in here which clamps hardware_domid >>> below FIRST_RESERVED_DOMID, or allow anyone specifying >>> hardware_dom=0xffff to keep all the broken pieces they find? Aliasing >>> the magic domids is sure to break things. >>> >>> ~Andrew >> >> I'm currently of the opinion that a custom_param is overkill to prevent >> users from deliberately doing bad things, but could easily write one if >> others think that it would be helpful. >> > > I suppose the answer depends on how subtle the breakage will be if the > user gets it accidentally wrong. Given that slightly over half the > domid space is reserved and domid_t signed value, I can forsee subtle > bugs if a domid_t ever used as an array index, as well as very unsubtle > breakage if the user aliases one of the magic domids. > > On the balance, a custom_param() is probably worthwhile for peace of mind. Why can't we just BUG_ON() or panic() the first time domain_create() gets to see the out of range domain ID? Jan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] allow hardware domain != dom0 2014-04-16 9:08 ` Jan Beulich @ 2014-04-16 9:41 ` Andrew Cooper 0 siblings, 0 replies; 9+ messages in thread From: Andrew Cooper @ 2014-04-16 9:41 UTC (permalink / raw) To: Jan Beulich Cc: Keir Fraser, Ian Campbell, Ian Jackson, Tim Deegan, xen-devel, Daniel De Graaf On 16/04/14 10:08, Jan Beulich wrote: >>>> On 16.04.14 at 01:37, <andrew.cooper3@citrix.com> wrote: >> On 15/04/2014 23:07, Daniel De Graaf wrote: >>> On 04/15/2014 10:45 AM, Andrew Cooper wrote: >>>> On 14/04/14 22:23, Daniel De Graaf wrote: >>>>> diff --git a/xen/common/domain.c b/xen/common/domain.c >>>>> index 3c05711..11c905a 100644 >>>>> --- a/xen/common/domain.c >>>>> +++ b/xen/common/domain.c >>>>> @@ -61,6 +61,11 @@ struct domain *domain_list; >>>>> >>>>> struct domain *hardware_domain __read_mostly; >>>>> >>>>> +#ifdef CONFIG_LATE_HWDOM >>>>> +domid_t hardware_domid __read_mostly; >>>>> +integer_param("hardware_dom", hardware_domid); >>>>> +#endif >>>>> + >>>> Is it worth putting a custom_param() in here which clamps hardware_domid >>>> below FIRST_RESERVED_DOMID, or allow anyone specifying >>>> hardware_dom=0xffff to keep all the broken pieces they find? Aliasing >>>> the magic domids is sure to break things. >>>> >>>> ~Andrew >>> I'm currently of the opinion that a custom_param is overkill to prevent >>> users from deliberately doing bad things, but could easily write one if >>> others think that it would be helpful. >>> >> I suppose the answer depends on how subtle the breakage will be if the >> user gets it accidentally wrong. Given that slightly over half the >> domid space is reserved and domid_t signed value, I can forsee subtle >> bugs if a domid_t ever used as an array index, as well as very unsubtle >> breakage if the user aliases one of the magic domids. >> >> On the balance, a custom_param() is probably worthwhile for peace of mind. > Why can't we just BUG_ON() or panic() the first time domain_create() > gets to see the out of range domain ID? > > Jan > That would also work. ~Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] implement is_hardware_domain using hardware_domain global 2014-04-14 21:23 [PATCH 1/2] implement is_hardware_domain using hardware_domain global Daniel De Graaf 2014-04-14 21:23 ` [PATCH v4 2/2] allow hardware domain != dom0 Daniel De Graaf @ 2014-04-15 8:56 ` Ian Campbell 1 sibling, 0 replies; 9+ messages in thread From: Ian Campbell @ 2014-04-15 8:56 UTC (permalink / raw) To: Daniel De Graaf Cc: Keir Fraser, Ian Jackson, Tim Deegan, xen-devel, Stefano Stabellini, Jan Beulich On Mon, 2014-04-14 at 17:23 -0400, Daniel De Graaf wrote: > This requires setting the hardware_domain variable earlier in > domain_create so that functions called from it (primarily in > arch_domain_create) observe the correct results when they call > is_hardware_domain. > > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> For the ARM side: Acked-by: Ian Campbell <ian.campbell@citrix.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-04-16 9:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-14 21:23 [PATCH 1/2] implement is_hardware_domain using hardware_domain global Daniel De Graaf 2014-04-14 21:23 ` [PATCH v4 2/2] allow hardware domain != dom0 Daniel De Graaf 2014-04-15 7:53 ` Jan Beulich 2014-04-15 14:45 ` Andrew Cooper 2014-04-15 22:07 ` Daniel De Graaf 2014-04-15 23:37 ` Andrew Cooper 2014-04-16 9:08 ` Jan Beulich 2014-04-16 9:41 ` Andrew Cooper 2014-04-15 8:56 ` [PATCH 1/2] implement is_hardware_domain using hardware_domain global Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.