| Message ID | 20220311061123.1883189-3-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Static shared memory on dom0less system | expand |
On Fri, 11 Mar 2022, Penny Zheng wrote: > From: Penny Zheng <penzhe01@a011292.shanghai.arm.com> > > In case to own statically shared pages when owner domain is not > explicitly defined, this commits propose a special domain DOMID_SHARED, > and we assign it 0x7FF5, as one of the system domains. > > Statically shared memory reuses the same way of initialization with static > memory, hence this commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap > related codes, and this option depends on static memory(CONFIG_STATIC_MEMORY). Why does it depend on CONFIG_STATIC_MEMORY? This is a genuine question, I am not trying to scope-creep the series. Is there an actual technical dependency on CONFIG_STATIC_MEMORY? If not, it would be super useful to be able to share memory statically even between normal dom0less guests (of course it would be responsibility of the user to provide the right addresses and avoid mapping clashes.) I know that some of our users have requested this feature in the past. > We intends to do shared domain creation after setup_virt_paging so shared > domain could successfully do p2m initialization. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > --- > xen/arch/arm/Kconfig | 7 +++++++ > xen/arch/arm/domain.c | 12 ++++++++++-- > xen/arch/arm/include/asm/domain.h | 6 ++++++ > xen/arch/arm/setup.c | 22 ++++++++++++++++++++++ > xen/common/domain.c | 11 +++++++---- > xen/common/page_alloc.c | 5 +++++ > xen/common/vsprintf.c | 9 +++++---- > xen/include/public/xen.h | 6 ++++++ > xen/include/xen/sched.h | 2 ++ > 9 files changed, 70 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index ecfa6822e4..c54accefb1 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -106,6 +106,13 @@ config TEE > > source "arch/arm/tee/Kconfig" > > +config STATIC_SHM > + bool "Statically shared memory on a dom0less system" if UNSUPPORTED > + depends on STATIC_MEMORY > + default n > + help > + This option enables statically shared memory on a dom0less system. > + > endmenu > > menu "ARM errata workaround via the alternative framework" > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 8110c1df86..1ff1df5d3f 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -44,6 +44,10 @@ > > DEFINE_PER_CPU(struct vcpu *, curr_vcpu); > > +#ifdef CONFIG_STATIC_SHM > +struct domain *__read_mostly dom_shared; > +#endif This one should probably go to xen/common/domain.c to stay close to the other special domains. > static void do_idle(void) > { > unsigned int cpu = smp_processor_id(); > @@ -703,7 +707,7 @@ int arch_domain_create(struct domain *d, > if ( is_idle_domain(d) ) > return 0; > > - ASSERT(config != NULL); > + ASSERT(is_shared_domain(d) ? config == NULL : config != NULL); > > #ifdef CONFIG_IOREQ_SERVER > ioreq_domain_init(d); > @@ -712,12 +716,16 @@ int arch_domain_create(struct domain *d, > d->arch.directmap = flags & CDF_directmap; > > /* p2m_init relies on some value initialized by the IOMMU subsystem */ > - if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 ) > + if ( (rc = iommu_domain_init(d, is_shared_domain(d) ? 0 : config->iommu_opts)) != 0 ) > goto fail; > > if ( (rc = p2m_init(d)) != 0 ) > goto fail; > > + /* DOMID_shared is sufficiently constructed after p2m initialization. */ > + if ( is_shared_domain(d) ) > + return 0; > + > rc = -ENOMEM; > if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL ) > goto fail; > diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h > index c56f6e4398..ea7a7219a3 100644 > --- a/xen/arch/arm/include/asm/domain.h > +++ b/xen/arch/arm/include/asm/domain.h > @@ -31,6 +31,12 @@ enum domain_type { > > #define is_domain_direct_mapped(d) (d)->arch.directmap > > +#ifdef CONFIG_STATIC_SHM > +extern struct domain *dom_shared; > +#else > +#define dom_shared NULL > +#endif I think this should probably go to xen/include/xen/mm.h to stay close to the others (dom_xen, dom_io and dom_cow). > /* > * Is the domain using the host memory layout? > * > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index d5d0792ed4..f6a3b04958 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -855,6 +855,20 @@ static bool __init is_dom0less_mode(void) > return ( !dom0found && domUfound ); > } > > +#ifdef CONFIG_STATIC_SHM > +static void __init setup_shared_domain(void) > +{ > + /* > + * Initialise our DOMID_SHARED domain. > + * This domain owns statically shared pages when owner domain is not > + * explicitly defined. > + */ > + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); > + if ( IS_ERR(dom_shared) ) > + panic("Failed to create d[SHARED]: %ld\n", PTR_ERR(dom_shared)); > +} > +#endif > + > size_t __read_mostly dcache_line_bytes; > > /* C entry point for boot CPU */ > @@ -1022,6 +1036,14 @@ void __init start_xen(unsigned long boot_phys_offset, > apply_alternatives_all(); > enable_errata_workarounds(); > > +#ifdef CONFIG_STATIC_SHM > + /* > + * This needs to be called **after** setup_virt_paging so shared > + * domains could successfully do p2m initialization. ^ domain I take you are talking about DOMID_SHARED rather than any domain sharing memory statically. Maybe it clearer if you say "so DOMID_SHARED could successfully do p2m initialization". > + */ > + setup_shared_domain(); > +#endif > + > /* Create initial domain 0. */ > if ( !is_dom0less_mode() ) > create_dom0(); > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 3742322d22..5cdd0b9f5b 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -643,11 +643,14 @@ struct domain *domain_create(domid_t domid, > > rangeset_domain_initialise(d); > > - /* DOMID_{XEN,IO,etc} (other than IDLE) are sufficiently constructed. */ > - if ( is_system_domain(d) && !is_idle_domain(d) ) > + /* > + * DOMID_{XEN,IO,etc} (other than IDLE and DOMID_shared) are > + * sufficiently constructed. > + */ > + if ( is_system_domain(d) && !is_idle_domain(d) && !is_shared_domain(d) ) > return d; > > - if ( !is_idle_domain(d) ) > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > { > if ( !is_hardware_domain(d) ) > d->nr_pirqs = nr_static_irqs + extra_domU_irqs; > @@ -663,7 +666,7 @@ struct domain *domain_create(domid_t domid, > goto fail; > init_status |= INIT_arch; > > - if ( !is_idle_domain(d) ) > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > { > watchdog_domain_init(d); > init_status |= INIT_watchdog; > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c > index f8749b0787..e5e357969d 100644 > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -2616,6 +2616,11 @@ struct domain *get_pg_owner(domid_t domid) > > switch ( domid ) > { > +#ifdef CONFIG_STATIC_SHM > + case DOMID_SHARED: > + pg_owner = rcu_lock_domain(dom_shared); > + break; > +#endif > case DOMID_IO: > pg_owner = rcu_lock_domain(dom_io); > break; > diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c > index b278961cc3..a22854001b 100644 > --- a/xen/common/vsprintf.c > +++ b/xen/common/vsprintf.c > @@ -359,10 +359,11 @@ static char *print_domain(char *str, const char *end, const struct domain *d) > > switch ( d->domain_id ) > { > - case DOMID_IO: name = "[IO]"; break; > - case DOMID_XEN: name = "[XEN]"; break; > - case DOMID_COW: name = "[COW]"; break; > - case DOMID_IDLE: name = "[IDLE]"; break; > + case DOMID_IO: name = "[IO]"; break; > + case DOMID_XEN: name = "[XEN]"; break; > + case DOMID_COW: name = "[COW]"; break; > + case DOMID_IDLE: name = "[IDLE]"; break; > + case DOMID_SHARED: name = "[SHARED]"; break; > /* > * In principle, we could ASSERT_UNREACHABLE() in the default case. > * However, this path is used to print out crash information, which > diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h > index e373592c33..2e00741f09 100644 > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -612,6 +612,12 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t); > /* DOMID_INVALID is used to identify pages with unknown owner. */ > #define DOMID_INVALID xen_mk_uint(0x7FF4) > > +/* > + * DOMID_SHARED is used as the owner of statically shared pages, when > + * owner is not explicitly defined. > + */ > +#define DOMID_SHARED xen_mk_uint(0x7FF5) > + > /* Idle domain. */ > #define DOMID_IDLE xen_mk_uint(0x7FFF) > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index 24a9a87f83..2fb236f4ea 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -618,6 +618,8 @@ static inline bool is_system_domain(const struct domain *d) > return d->domain_id >= DOMID_FIRST_RESERVED; > } > > +#define is_shared_domain(d) ((d)->domain_id == DOMID_SHARED) > + > #define DOMAIN_DESTROYED (1u << 31) /* assumes atomic_t is >= 32 bits */ > #define put_domain(_d) \ > if ( atomic_dec_and_test(&(_d)->refcnt) ) domain_destroy(_d) > -- > 2.25.1 >
Hi Stefano > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: Friday, March 18, 2022 9:59 AM > To: Penny Zheng <Penny.Zheng@arm.com> > Cc: xen-devel@lists.xenproject.org; nd <nd@arm.com>; Penny Zheng > <penzhe01@a011292.shanghai.arm.com>; Stefano Stabellini > <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis > <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Andrew Cooper > <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; > Jan Beulich <jbeulich@suse.com>; Wei Liu <wl@xen.org> > Subject: Re: [PATCH v1 02/13] xen/arm: introduce a special domain > DOMID_SHARED > > On Fri, 11 Mar 2022, Penny Zheng wrote: > > From: Penny Zheng <penzhe01@a011292.shanghai.arm.com> > > > > In case to own statically shared pages when owner domain is not > > explicitly defined, this commits propose a special domain > > DOMID_SHARED, and we assign it 0x7FF5, as one of the system domains. > > > > Statically shared memory reuses the same way of initialization with > > static memory, hence this commits proposes a new Kconfig > > CONFIG_STATIC_SHM to wrap related codes, and this option depends on > static memory(CONFIG_STATIC_MEMORY). > > Why does it depend on CONFIG_STATIC_MEMORY? This is a genuine question, > I am not trying to scope-creep the series. Is there an actual technical > dependency on CONFIG_STATIC_MEMORY? If not, it would be super useful to > be able to share memory statically even between normal dom0less guests (of > course it would be responsibility of the user to provide the right addresses and > avoid mapping clashes.) I know that some of our users have requested this > feature in the past. > I may find a proper way to rephrase here. My poor English writing skill... When I implemented domain on static allocation, statically configured guest RAM is treated as static memory in Xen and I introduced a few helpers to initialize/allocate/free static memory, like acquire_staticmem_pages, etc, and all these helpers are guarded with CONFIG_STATIC_MEMORY. I want to reuse these helpers on static shared memory, so CONFIG_STATIC_SHM depends on CONFIG_STATIC_MEMORY. So I'm not restricting sharing static memory between domain on static allocation, current Implementation is also useful to normal dom0less guests. > > > We intends to do shared domain creation after setup_virt_paging so > > shared domain could successfully do p2m initialization. > > > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > > --- > > xen/arch/arm/Kconfig | 7 +++++++ > > xen/arch/arm/domain.c | 12 ++++++++++-- > > xen/arch/arm/include/asm/domain.h | 6 ++++++ > > xen/arch/arm/setup.c | 22 ++++++++++++++++++++++ > > xen/common/domain.c | 11 +++++++---- > > xen/common/page_alloc.c | 5 +++++ > > xen/common/vsprintf.c | 9 +++++---- > > xen/include/public/xen.h | 6 ++++++ > > xen/include/xen/sched.h | 2 ++ > > 9 files changed, 70 insertions(+), 10 deletions(-) > > > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index > > ecfa6822e4..c54accefb1 100644 > > --- a/xen/arch/arm/Kconfig > > +++ b/xen/arch/arm/Kconfig > > @@ -106,6 +106,13 @@ config TEE > > > > source "arch/arm/tee/Kconfig" > > > > +config STATIC_SHM > > + bool "Statically shared memory on a dom0less system" if UNSUPPORTED > > + depends on STATIC_MEMORY > > + default n > > + help > > + This option enables statically shared memory on a dom0less system. > > + > > endmenu > > > > menu "ARM errata workaround via the alternative framework" > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index > > 8110c1df86..1ff1df5d3f 100644 > > --- a/xen/arch/arm/domain.c > > +++ b/xen/arch/arm/domain.c > > @@ -44,6 +44,10 @@ > > > > DEFINE_PER_CPU(struct vcpu *, curr_vcpu); > > > > +#ifdef CONFIG_STATIC_SHM > > +struct domain *__read_mostly dom_shared; #endif > > This one should probably go to xen/common/domain.c to stay close to the > other special domains. > Ack. Thx > > > static void do_idle(void) > > { > > unsigned int cpu = smp_processor_id(); @@ -703,7 +707,7 @@ int > > arch_domain_create(struct domain *d, > > if ( is_idle_domain(d) ) > > return 0; > > > > - ASSERT(config != NULL); > > + ASSERT(is_shared_domain(d) ? config == NULL : config != NULL); > > > > #ifdef CONFIG_IOREQ_SERVER > > ioreq_domain_init(d); > > @@ -712,12 +716,16 @@ int arch_domain_create(struct domain *d, > > d->arch.directmap = flags & CDF_directmap; > > > > /* p2m_init relies on some value initialized by the IOMMU subsystem */ > > - if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 ) > > + if ( (rc = iommu_domain_init(d, is_shared_domain(d) ? 0 : > > + config->iommu_opts)) != 0 ) > > goto fail; > > > > if ( (rc = p2m_init(d)) != 0 ) > > goto fail; > > > > + /* DOMID_shared is sufficiently constructed after p2m initialization. */ > > + if ( is_shared_domain(d) ) > > + return 0; > > + > > rc = -ENOMEM; > > if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL ) > > goto fail; > > diff --git a/xen/arch/arm/include/asm/domain.h > > b/xen/arch/arm/include/asm/domain.h > > index c56f6e4398..ea7a7219a3 100644 > > --- a/xen/arch/arm/include/asm/domain.h > > +++ b/xen/arch/arm/include/asm/domain.h > > @@ -31,6 +31,12 @@ enum domain_type { > > > > #define is_domain_direct_mapped(d) (d)->arch.directmap > > > > +#ifdef CONFIG_STATIC_SHM > > +extern struct domain *dom_shared; > > +#else > > +#define dom_shared NULL > > +#endif > > I think this should probably go to xen/include/xen/mm.h to stay close to the > others (dom_xen, dom_io and dom_cow). > Ack, thx > > > /* > > * Is the domain using the host memory layout? > > * > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index > > d5d0792ed4..f6a3b04958 100644 > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -855,6 +855,20 @@ static bool __init is_dom0less_mode(void) > > return ( !dom0found && domUfound ); } > > > > +#ifdef CONFIG_STATIC_SHM > > +static void __init setup_shared_domain(void) { > > + /* > > + * Initialise our DOMID_SHARED domain. > > + * This domain owns statically shared pages when owner domain is not > > + * explicitly defined. > > + */ > > + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); > > + if ( IS_ERR(dom_shared) ) > > + panic("Failed to create d[SHARED]: %ld\n", > > +PTR_ERR(dom_shared)); } #endif > > + > > size_t __read_mostly dcache_line_bytes; > > > > /* C entry point for boot CPU */ > > @@ -1022,6 +1036,14 @@ void __init start_xen(unsigned long > boot_phys_offset, > > apply_alternatives_all(); > > enable_errata_workarounds(); > > > > +#ifdef CONFIG_STATIC_SHM > > + /* > > + * This needs to be called **after** setup_virt_paging so shared > > + * domains could successfully do p2m initialization. > ^ domain > > I take you are talking about DOMID_SHARED rather than any domain sharing > memory statically. Maybe it clearer if you say "so DOMID_SHARED could > successfully do p2m initialization". > Ack, thx. > > > + */ > > + setup_shared_domain(); > > +#endif > > + > > /* Create initial domain 0. */ > > if ( !is_dom0less_mode() ) > > create_dom0(); > > diff --git a/xen/common/domain.c b/xen/common/domain.c index > > 3742322d22..5cdd0b9f5b 100644 > > --- a/xen/common/domain.c > > +++ b/xen/common/domain.c > > @@ -643,11 +643,14 @@ struct domain *domain_create(domid_t domid, > > > > rangeset_domain_initialise(d); > > > > - /* DOMID_{XEN,IO,etc} (other than IDLE) are sufficiently constructed. */ > > - if ( is_system_domain(d) && !is_idle_domain(d) ) > > + /* > > + * DOMID_{XEN,IO,etc} (other than IDLE and DOMID_shared) are > > + * sufficiently constructed. > > + */ > > + if ( is_system_domain(d) && !is_idle_domain(d) && > > + !is_shared_domain(d) ) > > return d; > > > > - if ( !is_idle_domain(d) ) > > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > > { > > if ( !is_hardware_domain(d) ) > > d->nr_pirqs = nr_static_irqs + extra_domU_irqs; @@ -663,7 > > +666,7 @@ struct domain *domain_create(domid_t domid, > > goto fail; > > init_status |= INIT_arch; > > > > - if ( !is_idle_domain(d) ) > > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > > { > > watchdog_domain_init(d); > > init_status |= INIT_watchdog; diff --git > > a/xen/common/page_alloc.c b/xen/common/page_alloc.c index > > f8749b0787..e5e357969d 100644 > > --- a/xen/common/page_alloc.c > > +++ b/xen/common/page_alloc.c > > @@ -2616,6 +2616,11 @@ struct domain *get_pg_owner(domid_t domid) > > > > switch ( domid ) > > { > > +#ifdef CONFIG_STATIC_SHM > > + case DOMID_SHARED: > > + pg_owner = rcu_lock_domain(dom_shared); > > + break; > > +#endif > > case DOMID_IO: > > pg_owner = rcu_lock_domain(dom_io); > > break; > > diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c index > > b278961cc3..a22854001b 100644 > > --- a/xen/common/vsprintf.c > > +++ b/xen/common/vsprintf.c > > @@ -359,10 +359,11 @@ static char *print_domain(char *str, const char > > *end, const struct domain *d) > > > > switch ( d->domain_id ) > > { > > - case DOMID_IO: name = "[IO]"; break; > > - case DOMID_XEN: name = "[XEN]"; break; > > - case DOMID_COW: name = "[COW]"; break; > > - case DOMID_IDLE: name = "[IDLE]"; break; > > + case DOMID_IO: name = "[IO]"; break; > > + case DOMID_XEN: name = "[XEN]"; break; > > + case DOMID_COW: name = "[COW]"; break; > > + case DOMID_IDLE: name = "[IDLE]"; break; > > + case DOMID_SHARED: name = "[SHARED]"; break; > > /* > > * In principle, we could ASSERT_UNREACHABLE() in the default case. > > * However, this path is used to print out crash information, > > which diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h > > index e373592c33..2e00741f09 100644 > > --- a/xen/include/public/xen.h > > +++ b/xen/include/public/xen.h > > @@ -612,6 +612,12 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t); > > /* DOMID_INVALID is used to identify pages with unknown owner. */ > > #define DOMID_INVALID xen_mk_uint(0x7FF4) > > > > +/* > > + * DOMID_SHARED is used as the owner of statically shared pages, when > > + * owner is not explicitly defined. > > + */ > > +#define DOMID_SHARED xen_mk_uint(0x7FF5) > > + > > /* Idle domain. */ > > #define DOMID_IDLE xen_mk_uint(0x7FFF) > > > > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index > > 24a9a87f83..2fb236f4ea 100644 > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -618,6 +618,8 @@ static inline bool is_system_domain(const struct > domain *d) > > return d->domain_id >= DOMID_FIRST_RESERVED; } > > > > +#define is_shared_domain(d) ((d)->domain_id == DOMID_SHARED) > > + > > #define DOMAIN_DESTROYED (1u << 31) /* assumes atomic_t is >= 32 bits > > */ #define put_domain(_d) \ > > if ( atomic_dec_and_test(&(_d)->refcnt) ) domain_destroy(_d) > > -- > > 2.25.1 > > Cheers, --- Penny Zheng
On 11.03.2022 07:11, Penny Zheng wrote: > In case to own statically shared pages when owner domain is not > explicitly defined, this commits propose a special domain DOMID_SHARED, > and we assign it 0x7FF5, as one of the system domains. > > Statically shared memory reuses the same way of initialization with static > memory, hence this commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap > related codes, and this option depends on static memory(CONFIG_STATIC_MEMORY). > > We intends to do shared domain creation after setup_virt_paging so shared > domain could successfully do p2m initialization. There's nothing said here, in the earlier patch, or in the cover letter about the security aspects of this. There is a reason we haven't been allowing arbitrary, un-supervised sharing of memory between domains. It wants clarifying why e.g. grants aren't an option to achieve what you need, and how you mean to establish which domains are / aren't permitted to access any individual page owned by this domain. > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -106,6 +106,13 @@ config TEE > > source "arch/arm/tee/Kconfig" > > +config STATIC_SHM > + bool "Statically shared memory on a dom0less system" if UNSUPPORTED > + depends on STATIC_MEMORY > + default n Nit: "default n" is redundant and hence would imo better be omitted. > @@ -712,12 +716,16 @@ int arch_domain_create(struct domain *d, > d->arch.directmap = flags & CDF_directmap; > > /* p2m_init relies on some value initialized by the IOMMU subsystem */ > - if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 ) > + if ( (rc = iommu_domain_init(d, is_shared_domain(d) ? 0 : config->iommu_opts)) != 0 ) Nit: Overlong line. > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -855,6 +855,20 @@ static bool __init is_dom0less_mode(void) > return ( !dom0found && domUfound ); > } > > +#ifdef CONFIG_STATIC_SHM > +static void __init setup_shared_domain(void) > +{ > + /* > + * Initialise our DOMID_SHARED domain. > + * This domain owns statically shared pages when owner domain is not > + * explicitly defined. > + */ > + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); > + if ( IS_ERR(dom_shared) ) > + panic("Failed to create d[SHARED]: %ld\n", PTR_ERR(dom_shared)); I don't think this should be a panic - the system ought to be able to come up fine, just without actually using this domain. After all this is an optional feature which may not actually be used. Also, along the lines of what Stefano has said, this setting up of the domain would also better live next to where the other special domains are set up. And even if it was to remain here, ... > @@ -1022,6 +1036,14 @@ void __init start_xen(unsigned long boot_phys_offset, > apply_alternatives_all(); > enable_errata_workarounds(); > > +#ifdef CONFIG_STATIC_SHM > + /* > + * This needs to be called **after** setup_virt_paging so shared > + * domains could successfully do p2m initialization. > + */ > + setup_shared_domain(); > +#endif ... the #ifdef-ary here should be avoided by moving the other #ifdef inside the function body. > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -643,11 +643,14 @@ struct domain *domain_create(domid_t domid, > > rangeset_domain_initialise(d); > > - /* DOMID_{XEN,IO,etc} (other than IDLE) are sufficiently constructed. */ > - if ( is_system_domain(d) && !is_idle_domain(d) ) > + /* > + * DOMID_{XEN,IO,etc} (other than IDLE and DOMID_shared) are > + * sufficiently constructed. > + */ > + if ( is_system_domain(d) && !is_idle_domain(d) && !is_shared_domain(d) ) > return d; > > - if ( !is_idle_domain(d) ) > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > { > if ( !is_hardware_domain(d) ) > d->nr_pirqs = nr_static_irqs + extra_domU_irqs; > @@ -663,7 +666,7 @@ struct domain *domain_create(domid_t domid, > goto fail; > init_status |= INIT_arch; > > - if ( !is_idle_domain(d) ) > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > { > watchdog_domain_init(d); > init_status |= INIT_watchdog; All of these extra is_shared_domain() are quite ugly to see added. First and foremost going this route doesn't scale very well - consider how the code will look like when two more special domains with special needs would be added. I think you want to abstract this some by introducing one (or a small set of) new is_...() or e.g. needs_...() predicates. Further (there's no particularly good place to mention this) I'm afraid I don't view "shared" as a good name: It's not the domain which is shared, but it's the domain to hold shared memory. For this my first consideration would be to see whether an existing special domain can be re-used; after all the set of reserved domain IDs is a very limited one, and hence each value taken from there should come with a very good reason. We did such re-use e.g. when introducing quarantining for PCI devices, by associating them with DOM_IO rather than inventing a new DOM_QUARANTINE. If there are good reasons speaking against such re-use, then I'd like to ask to consider e.g. DOMID_SHM / DOMID_SHMEM plus associated predicate. > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -2616,6 +2616,11 @@ struct domain *get_pg_owner(domid_t domid) > > switch ( domid ) > { > +#ifdef CONFIG_STATIC_SHM > + case DOMID_SHARED: > + pg_owner = rcu_lock_domain(dom_shared); > + break; > +#endif Please can you avoid #ifdef in cases like this one, by instead using case DOMID_SHMEM: pg_owner = dom_shared ? rcu_lock_domain(dom_shared) : NULL; break; > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -618,6 +618,8 @@ static inline bool is_system_domain(const struct domain *d) > return d->domain_id >= DOMID_FIRST_RESERVED; > } > > +#define is_shared_domain(d) ((d)->domain_id == DOMID_SHARED) Would this better evaluate to "false" when !STATIC_SHM, such that the compiler can eliminate respective conditionals and/or code? Jan
On Fri, 18 Mar 2022, Jan Beulich wrote: > On 11.03.2022 07:11, Penny Zheng wrote: > > In case to own statically shared pages when owner domain is not > > explicitly defined, this commits propose a special domain DOMID_SHARED, > > and we assign it 0x7FF5, as one of the system domains. > > > > Statically shared memory reuses the same way of initialization with static > > memory, hence this commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap > > related codes, and this option depends on static memory(CONFIG_STATIC_MEMORY). > > > > We intends to do shared domain creation after setup_virt_paging so shared > > domain could successfully do p2m initialization. > > There's nothing said here, in the earlier patch, or in the cover letter > about the security aspects of this. There is a reason we haven't been > allowing arbitrary, un-supervised sharing of memory between domains. It > wants clarifying why e.g. grants aren't an option to achieve what you > need, and how you mean to establish which domains are / aren't permitted > to access any individual page owned by this domain. I'll let Penny write a full reply but I'll chime in to try to help with the explanation. This is not arbitrary un-supervised sharing of memory between domains, which indeed is concerning. This is statically-configured, supervised by the system configurator, sharing of memory between domains. And in fact safety (which is just a different aspect of security) is one of the primary goals for this work. In safety-critical environments, it is not considered safe to dynamically change important configurations at runtime. Everything should be statically defined and statically verified. In this case, if the system configuration knows a priori that there are only 2 VM and they need to communication over shared memory, it is safer to pre-configure the shared memory at build time rather than let the VMs attempt to share memory at runtime. It is faster too. The only way to trigger this static shared memory configuration should be via device tree, which is at the same level as the XSM rules themselves. Hopefully I made things clearer and not murkier :-)
On Fri, 18 Mar 2022, Penny Zheng wrote: > > On Fri, 11 Mar 2022, Penny Zheng wrote: > > > From: Penny Zheng <penzhe01@a011292.shanghai.arm.com> > > > > > > In case to own statically shared pages when owner domain is not > > > explicitly defined, this commits propose a special domain > > > DOMID_SHARED, and we assign it 0x7FF5, as one of the system domains. > > > > > > Statically shared memory reuses the same way of initialization with > > > static memory, hence this commits proposes a new Kconfig > > > CONFIG_STATIC_SHM to wrap related codes, and this option depends on > > static memory(CONFIG_STATIC_MEMORY). > > > > Why does it depend on CONFIG_STATIC_MEMORY? This is a genuine question, > > I am not trying to scope-creep the series. Is there an actual technical > > dependency on CONFIG_STATIC_MEMORY? If not, it would be super useful to > > be able to share memory statically even between normal dom0less guests (of > > course it would be responsibility of the user to provide the right addresses and > > avoid mapping clashes.) I know that some of our users have requested this > > feature in the past. > > > > I may find a proper way to rephrase here. My poor English writing skill... > When I implemented domain on static allocation, statically configured guest RAM is > treated as static memory in Xen and I introduced a few helpers to initialize/allocate/free > static memory, like acquire_staticmem_pages, etc, and all these helpers are guarded with > CONFIG_STATIC_MEMORY. > I want to reuse these helpers on static shared memory, so CONFIG_STATIC_SHM depends > on CONFIG_STATIC_MEMORY. > > So I'm not restricting sharing static memory between domain on static allocation, current > Implementation is also useful to normal dom0less guests. Ah, excellent! That makes sense.
On Fri, 18 Mar 2022, Jan Beulich wrote: > Further (there's no particularly good place to mention this) I'm > afraid I don't view "shared" as a good name: It's not the domain > which is shared, but it's the domain to hold shared memory. To be honest I thought the same thing when reading this patch but couldn't easily come up with a better name. > For this > my first consideration would be to see whether an existing special > domain can be re-used; after all the set of reserved domain IDs is > a very limited one, and hence each value taken from there should come > with a very good reason. We did such re-use e.g. when introducing > quarantining for PCI devices, by associating them with DOM_IO rather > than inventing a new DOM_QUARANTINE. If there are good reasons > speaking against such re-use, then I'd like to ask to consider e.g. > DOMID_SHM / DOMID_SHMEM plus associated predicate. From my point of view is totally fine to reuse one of the existing special domains if we can. DOMID_IO seems to be the closest match but its current definition doesn't cover what Penny is trying to achieve. I am happy to change its definition and make it wider to support this use-case too. It is not trivial to come up with an appropriate description though. I took a stab at it but I couldn't come up with anything better: * DOMID_IO is used for mapping memory and MMIO regions when no explicit * Domain need to be specified. * * For instance, DOMID_IO is the owner of memory pre-shared among * multiple domains at boot time, when no explicit owner is specified. * * Also, DOMID_IO is used to restrict page-table updates to mapping I/O * memory. Although no Foreign Domain need be specified to map I/O * pages, DOMID_IO is useful to ensure that no mappings to the OS's own * heap are accidentally installed. (e.g., in Linux this could cause * havoc as reference counts aren't adjusted on the I/O-mapping code * path). This only makes sense as HYPERVISOR_mmu_update()'s and * HYPERVISOR_update_va_mapping_otherdomain()'s "foreigndom" argument. * For HYPERVISOR_mmu_update() context it can be specified by any * calling domain, otherwise it's only permitted if the caller is * privileged.
On 18.03.2022 22:50, Stefano Stabellini wrote: > On Fri, 18 Mar 2022, Jan Beulich wrote: >> On 11.03.2022 07:11, Penny Zheng wrote: >>> In case to own statically shared pages when owner domain is not >>> explicitly defined, this commits propose a special domain DOMID_SHARED, >>> and we assign it 0x7FF5, as one of the system domains. >>> >>> Statically shared memory reuses the same way of initialization with static >>> memory, hence this commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap >>> related codes, and this option depends on static memory(CONFIG_STATIC_MEMORY). >>> >>> We intends to do shared domain creation after setup_virt_paging so shared >>> domain could successfully do p2m initialization. >> >> There's nothing said here, in the earlier patch, or in the cover letter >> about the security aspects of this. There is a reason we haven't been >> allowing arbitrary, un-supervised sharing of memory between domains. It >> wants clarifying why e.g. grants aren't an option to achieve what you >> need, and how you mean to establish which domains are / aren't permitted >> to access any individual page owned by this domain. > > > I'll let Penny write a full reply but I'll chime in to try to help with > the explanation. > > This is not arbitrary un-supervised sharing of memory between domains, > which indeed is concerning. > > This is statically-configured, supervised by the system configurator, > sharing of memory between domains. > > And in fact safety (which is just a different aspect of security) is one > of the primary goals for this work. > > In safety-critical environments, it is not considered safe to > dynamically change important configurations at runtime. Everything > should be statically defined and statically verified. > > In this case, if the system configuration knows a priori that there are > only 2 VM and they need to communication over shared memory, it is safer > to pre-configure the shared memory at build time rather than let the VMs > attempt to share memory at runtime. It is faster too. > > The only way to trigger this static shared memory configuration should > be via device tree, which is at the same level as the XSM rules > themselves. > > Hopefully I made things clearer and not murkier :-) It adds some helpful background, yes, but at the same time it doesn't address the security concern at all: How are access permissions managed when the owning domain is a special one? I haven't spotted any recording of the domains which are actually permitted to map / access the pages in questions. (But of course I also only looked at non-Arm-specific code. I'd expect such code not to live in arch- specific files.) Jan
On Mon, 21 Mar 2022, Jan Beulich wrote: > On 18.03.2022 22:50, Stefano Stabellini wrote: > > On Fri, 18 Mar 2022, Jan Beulich wrote: > >> On 11.03.2022 07:11, Penny Zheng wrote: > >>> In case to own statically shared pages when owner domain is not > >>> explicitly defined, this commits propose a special domain DOMID_SHARED, > >>> and we assign it 0x7FF5, as one of the system domains. > >>> > >>> Statically shared memory reuses the same way of initialization with static > >>> memory, hence this commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap > >>> related codes, and this option depends on static memory(CONFIG_STATIC_MEMORY). > >>> > >>> We intends to do shared domain creation after setup_virt_paging so shared > >>> domain could successfully do p2m initialization. > >> > >> There's nothing said here, in the earlier patch, or in the cover letter > >> about the security aspects of this. There is a reason we haven't been > >> allowing arbitrary, un-supervised sharing of memory between domains. It > >> wants clarifying why e.g. grants aren't an option to achieve what you > >> need, and how you mean to establish which domains are / aren't permitted > >> to access any individual page owned by this domain. > > > > > > I'll let Penny write a full reply but I'll chime in to try to help with > > the explanation. > > > > This is not arbitrary un-supervised sharing of memory between domains, > > which indeed is concerning. > > > > This is statically-configured, supervised by the system configurator, > > sharing of memory between domains. > > > > And in fact safety (which is just a different aspect of security) is one > > of the primary goals for this work. > > > > In safety-critical environments, it is not considered safe to > > dynamically change important configurations at runtime. Everything > > should be statically defined and statically verified. > > > > In this case, if the system configuration knows a priori that there are > > only 2 VM and they need to communication over shared memory, it is safer > > to pre-configure the shared memory at build time rather than let the VMs > > attempt to share memory at runtime. It is faster too. > > > > The only way to trigger this static shared memory configuration should > > be via device tree, which is at the same level as the XSM rules > > themselves. > > > > Hopefully I made things clearer and not murkier :-) > > It adds some helpful background, yes, but at the same time it doesn't > address the security concern at all: How are access permissions > managed when the owning domain is a special one? I haven't spotted > any recording of the domains which are actually permitted to map / > access the pages in questions. (But of course I also only looked at > non-Arm-specific code. I'd expect such code not to live in arch- > specific files.) All this static memory sharing is statically done at __init time only. It should not be possible to trigger any further memory sharing at runtime (if there is, that would be a bug). In the Arm patches, all the related functions are marked as __init and only called at boot time. They map the memory owned by DOMID_SHARED at given guest pseudo-physical addresses, also specified in device tree. There are no new interfaces for the guest to map this memory because it is already "pre-mapped".
Hi Stefano, On 21/03/2022 20:03, Stefano Stabellini wrote: > On Mon, 21 Mar 2022, Jan Beulich wrote: >> On 18.03.2022 22:50, Stefano Stabellini wrote: >>> On Fri, 18 Mar 2022, Jan Beulich wrote: >>>> On 11.03.2022 07:11, Penny Zheng wrote: >>>>> In case to own statically shared pages when owner domain is not >>>>> explicitly defined, this commits propose a special domain DOMID_SHARED, >>>>> and we assign it 0x7FF5, as one of the system domains. >>>>> >>>>> Statically shared memory reuses the same way of initialization with static >>>>> memory, hence this commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap >>>>> related codes, and this option depends on static memory(CONFIG_STATIC_MEMORY). >>>>> >>>>> We intends to do shared domain creation after setup_virt_paging so shared >>>>> domain could successfully do p2m initialization. >>>> >>>> There's nothing said here, in the earlier patch, or in the cover letter >>>> about the security aspects of this. There is a reason we haven't been >>>> allowing arbitrary, un-supervised sharing of memory between domains. It >>>> wants clarifying why e.g. grants aren't an option to achieve what you >>>> need, and how you mean to establish which domains are / aren't permitted >>>> to access any individual page owned by this domain. >>> >>> >>> I'll let Penny write a full reply but I'll chime in to try to help with >>> the explanation. >>> >>> This is not arbitrary un-supervised sharing of memory between domains, >>> which indeed is concerning. >>> >>> This is statically-configured, supervised by the system configurator, >>> sharing of memory between domains. >>> >>> And in fact safety (which is just a different aspect of security) is one >>> of the primary goals for this work. >>> >>> In safety-critical environments, it is not considered safe to >>> dynamically change important configurations at runtime. Everything >>> should be statically defined and statically verified. >>> >>> In this case, if the system configuration knows a priori that there are >>> only 2 VM and they need to communication over shared memory, it is safer >>> to pre-configure the shared memory at build time rather than let the VMs >>> attempt to share memory at runtime. It is faster too. >>> >>> The only way to trigger this static shared memory configuration should >>> be via device tree, which is at the same level as the XSM rules >>> themselves. >>> >>> Hopefully I made things clearer and not murkier :-) >> >> It adds some helpful background, yes, but at the same time it doesn't >> address the security concern at all: How are access permissions >> managed when the owning domain is a special one? I haven't spotted >> any recording of the domains which are actually permitted to map / >> access the pages in questions. (But of course I also only looked at >> non-Arm-specific code. I'd expect such code not to live in arch- >> specific files.) > > All this static memory sharing is statically done at __init time only. > It should not be possible to trigger any further memory sharing at > runtime (if there is, that would be a bug). Looking at the code, get_pg_owner() will be able to handle DOMID_SHARED. So anyone that is permitted to access DOMID_SHARED will be able to map any memory region at runtime. > There are no new interfaces for the guest to map this memory because it > is already "pre-mapped". It can via XENMAPSPACE_gmfn_foreign (assuming proper permission). Cheers,
Hi Julien and Stefano > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: Saturday, April 9, 2022 5:12 PM > To: Stefano Stabellini <sstabellini@kernel.org>; Jan Beulich > <jbeulich@suse.com> > Cc: Penny Zheng <Penny.Zheng@arm.com>; nd <nd@arm.com>; Penny Zheng > <penzhe01@a011292.shanghai.arm.com>; Bertrand Marquis > <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Andrew Cooper > <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; > Wei Liu <wl@xen.org>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v1 02/13] xen/arm: introduce a special domain > DOMID_SHARED > > Hi Stefano, > > On 21/03/2022 20:03, Stefano Stabellini wrote: > > On Mon, 21 Mar 2022, Jan Beulich wrote: > >> On 18.03.2022 22:50, Stefano Stabellini wrote: > >>> On Fri, 18 Mar 2022, Jan Beulich wrote: > >>>> On 11.03.2022 07:11, Penny Zheng wrote: > >>>>> In case to own statically shared pages when owner domain is not > >>>>> explicitly defined, this commits propose a special domain > >>>>> DOMID_SHARED, and we assign it 0x7FF5, as one of the system domains. > >>>>> > >>>>> Statically shared memory reuses the same way of initialization > >>>>> with static memory, hence this commits proposes a new Kconfig > >>>>> CONFIG_STATIC_SHM to wrap related codes, and this option depends > on static memory(CONFIG_STATIC_MEMORY). > >>>>> > >>>>> We intends to do shared domain creation after setup_virt_paging so > >>>>> shared domain could successfully do p2m initialization. > >>>> > >>>> There's nothing said here, in the earlier patch, or in the cover > >>>> letter about the security aspects of this. There is a reason we > >>>> haven't been allowing arbitrary, un-supervised sharing of memory > >>>> between domains. It wants clarifying why e.g. grants aren't an > >>>> option to achieve what you need, and how you mean to establish > >>>> which domains are / aren't permitted to access any individual page > owned by this domain. > >>> > >>> > >>> I'll let Penny write a full reply but I'll chime in to try to help > >>> with the explanation. > >>> > >>> This is not arbitrary un-supervised sharing of memory between > >>> domains, which indeed is concerning. > >>> > >>> This is statically-configured, supervised by the system > >>> configurator, sharing of memory between domains. > >>> > >>> And in fact safety (which is just a different aspect of security) is > >>> one of the primary goals for this work. > >>> > >>> In safety-critical environments, it is not considered safe to > >>> dynamically change important configurations at runtime. Everything > >>> should be statically defined and statically verified. > >>> > >>> In this case, if the system configuration knows a priori that there > >>> are only 2 VM and they need to communication over shared memory, it > >>> is safer to pre-configure the shared memory at build time rather > >>> than let the VMs attempt to share memory at runtime. It is faster too. > >>> > >>> The only way to trigger this static shared memory configuration > >>> should be via device tree, which is at the same level as the XSM > >>> rules themselves. > >>> > >>> Hopefully I made things clearer and not murkier :-) > >> > >> It adds some helpful background, yes, but at the same time it doesn't > >> address the security concern at all: How are access permissions > >> managed when the owning domain is a special one? I haven't spotted > >> any recording of the domains which are actually permitted to map / > >> access the pages in questions. (But of course I also only looked at > >> non-Arm-specific code. I'd expect such code not to live in arch- > >> specific files.) > > > > All this static memory sharing is statically done at __init time only. > > It should not be possible to trigger any further memory sharing at > > runtime (if there is, that would be a bug). > > Looking at the code, get_pg_owner() will be able to handle DOMID_SHARED. > So anyone that is permitted to access DOMID_SHARED will be able to map any > memory region at runtime. > > > There are no new interfaces for the guest to map this memory because > > it is already "pre-mapped". > > It can via XENMAPSPACE_gmfn_foreign (assuming proper permission). > Correct me if I'm wrong: The existing XENMAPSPACE_gmfn_foreign only allows privileged Dom0 to map memory pages from one foreign DomU to itself. So It can happen that Dom0 is using XENMAPSPACE_gmfn_foreign to (maliciously?) access shared memory owned by DOMID_SHARED, and for now only Dom0 could. So, maybe we should enhance the check of xsm_map_gmfn_foreign() to forbid the access to DOMID_SHARED, hmm, but static shared memory region could actually be owned by any arbitrary domain. So, how about we add restriction on the page itself? Pages of static shared memory region are static memory(with PGC_reserved flag on), so maybe we could restrict XENMAPSPACE_gmfn_foreign to access any PGC_reserved pages? > Cheers, > > -- Cheers, -- Penny Zheng > Julien Grall
Hi jan > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: Friday, March 18, 2022 4:53 PM > To: Penny Zheng <Penny.Zheng@arm.com> > Cc: nd <nd@arm.com>; Penny Zheng > <penzhe01@a011292.shanghai.arm.com>; Stefano Stabellini > <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis > <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com>; Andrew Cooper > <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; > Wei Liu <wl@xen.org>; xen-devel@lists.xenproject.org > Subject: Re: [PATCH v1 02/13] xen/arm: introduce a special domain > DOMID_SHARED > > On 11.03.2022 07:11, Penny Zheng wrote: > > In case to own statically shared pages when owner domain is not > > explicitly defined, this commits propose a special domain > > DOMID_SHARED, and we assign it 0x7FF5, as one of the system domains. > > > > Statically shared memory reuses the same way of initialization with > > static memory, hence this commits proposes a new Kconfig > > CONFIG_STATIC_SHM to wrap related codes, and this option depends on > static memory(CONFIG_STATIC_MEMORY). > > > > We intends to do shared domain creation after setup_virt_paging so > > shared domain could successfully do p2m initialization. > > There's nothing said here, in the earlier patch, or in the cover letter about the > security aspects of this. There is a reason we haven't been allowing arbitrary, > un-supervised sharing of memory between domains. It wants clarifying why e.g. > grants aren't an option to achieve what you need, and how you mean to > establish which domains are / aren't permitted to access any individual page > owned by this domain. > I'll add the security aspects what Stefano explains in the cover letter next serie for better understanding. > > --- a/xen/arch/arm/Kconfig > > +++ b/xen/arch/arm/Kconfig > > @@ -106,6 +106,13 @@ config TEE > > > > source "arch/arm/tee/Kconfig" > > > > +config STATIC_SHM > > + bool "Statically shared memory on a dom0less system" if UNSUPPORTED > > + depends on STATIC_MEMORY > > + default n > > Nit: "default n" is redundant and hence would imo better be omitted. > > > @@ -712,12 +716,16 @@ int arch_domain_create(struct domain *d, > > d->arch.directmap = flags & CDF_directmap; > > > > /* p2m_init relies on some value initialized by the IOMMU subsystem */ > > - if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 ) > > + if ( (rc = iommu_domain_init(d, is_shared_domain(d) ? 0 : > > + config->iommu_opts)) != 0 ) > > Nit: Overlong line. > > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -855,6 +855,20 @@ static bool __init is_dom0less_mode(void) > > return ( !dom0found && domUfound ); } > > > > +#ifdef CONFIG_STATIC_SHM > > +static void __init setup_shared_domain(void) { > > + /* > > + * Initialise our DOMID_SHARED domain. > > + * This domain owns statically shared pages when owner domain is not > > + * explicitly defined. > > + */ > > + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); > > + if ( IS_ERR(dom_shared) ) > > + panic("Failed to create d[SHARED]: %ld\n", > > +PTR_ERR(dom_shared)); > > I don't think this should be a panic - the system ought to be able to come up > fine, just without actually using this domain. After all this is an optional > feature which may not actually be used. > > Also, along the lines of what Stefano has said, this setting up of the domain > would also better live next to where the other special domains are set up. And > even if it was to remain here, ... > The reason why I place the setting up here is that DOMID_SHARED needs to map pre-configured static shared memory in its p2m table, so it must be set up after system P2M initialization(setup_virt_paging()). setup_system_domains() is called before system P2M initialization on xen/arch/arm/setup.c, which can't meet the requirement. > > @@ -1022,6 +1036,14 @@ void __init start_xen(unsigned long > boot_phys_offset, > > apply_alternatives_all(); > > enable_errata_workarounds(); > > > > +#ifdef CONFIG_STATIC_SHM > > + /* > > + * This needs to be called **after** setup_virt_paging so shared > > + * domains could successfully do p2m initialization. > > + */ > > + setup_shared_domain(); > > +#endif > > ... the #ifdef-ary here should be avoided by moving the other #ifdef inside the > function body. > > > --- a/xen/common/domain.c > > +++ b/xen/common/domain.c > > @@ -643,11 +643,14 @@ struct domain *domain_create(domid_t domid, > > > > rangeset_domain_initialise(d); > > > > - /* DOMID_{XEN,IO,etc} (other than IDLE) are sufficiently constructed. */ > > - if ( is_system_domain(d) && !is_idle_domain(d) ) > > + /* > > + * DOMID_{XEN,IO,etc} (other than IDLE and DOMID_shared) are > > + * sufficiently constructed. > > + */ > > + if ( is_system_domain(d) && !is_idle_domain(d) && > > + !is_shared_domain(d) ) > > return d; > > > > - if ( !is_idle_domain(d) ) > > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > > { > > if ( !is_hardware_domain(d) ) > > d->nr_pirqs = nr_static_irqs + extra_domU_irqs; @@ -663,7 > > +666,7 @@ struct domain *domain_create(domid_t domid, > > goto fail; > > init_status |= INIT_arch; > > > > - if ( !is_idle_domain(d) ) > > + if ( !is_idle_domain(d) && !is_shared_domain(d) ) > > { > > watchdog_domain_init(d); > > init_status |= INIT_watchdog; > > All of these extra is_shared_domain() are quite ugly to see added. > First and foremost going this route doesn't scale very well - consider how the > code will look like when two more special domains with special needs would > be added. I think you want to abstract this some by introducing one (or a small > set of) new is_...() or e.g. needs_...() predicates. > Agree. Shared domain needs the p2m initialization(p2m_init), which is in arch_domain_create. So I will introduce a new helper needs_arch_domain_creation() to include these system domains which need to call arch_domain_create to be sufficiently constructed. > Further (there's no particularly good place to mention this) I'm afraid I don't > view "shared" as a good name: It's not the domain which is shared, but it's the > domain to hold shared memory. For this my first consideration would be to > see whether an existing special domain can be re-used; after all the set of > reserved domain IDs is a very limited one, and hence each value taken from > there should come with a very good reason. We did such re-use e.g. when > introducing quarantining for PCI devices, by associating them with DOM_IO > rather than inventing a new DOM_QUARANTINE. If there are good reasons > speaking against such re-use, then I'd like to ask to consider e.g. > DOMID_SHM / DOMID_SHMEM plus associated predicate. > I'll take stefano's suggestion to reuse DOMID_IO. > > --- a/xen/common/page_alloc.c > > +++ b/xen/common/page_alloc.c > > @@ -2616,6 +2616,11 @@ struct domain *get_pg_owner(domid_t domid) > > > > switch ( domid ) > > { > > +#ifdef CONFIG_STATIC_SHM > > + case DOMID_SHARED: > > + pg_owner = rcu_lock_domain(dom_shared); > > + break; > > +#endif > > Please can you avoid #ifdef in cases like this one, by instead using > > case DOMID_SHMEM: > pg_owner = dom_shared ? rcu_lock_domain(dom_shared) : NULL; > break; > > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -618,6 +618,8 @@ static inline bool is_system_domain(const struct > domain *d) > > return d->domain_id >= DOMID_FIRST_RESERVED; } > > > > +#define is_shared_domain(d) ((d)->domain_id == DOMID_SHARED) > > Would this better evaluate to "false" when !STATIC_SHM, such that the > compiler can eliminate respective conditionals and/or code? > > Jan
On Fri, 15 Apr 2022, Penny Zheng wrote: > > Hi Stefano, > > > > On 21/03/2022 20:03, Stefano Stabellini wrote: > > > On Mon, 21 Mar 2022, Jan Beulich wrote: > > >> On 18.03.2022 22:50, Stefano Stabellini wrote: > > >>> On Fri, 18 Mar 2022, Jan Beulich wrote: > > >>>> On 11.03.2022 07:11, Penny Zheng wrote: > > >>>>> In case to own statically shared pages when owner domain is not > > >>>>> explicitly defined, this commits propose a special domain > > >>>>> DOMID_SHARED, and we assign it 0x7FF5, as one of the system domains. > > >>>>> > > >>>>> Statically shared memory reuses the same way of initialization > > >>>>> with static memory, hence this commits proposes a new Kconfig > > >>>>> CONFIG_STATIC_SHM to wrap related codes, and this option depends > > on static memory(CONFIG_STATIC_MEMORY). > > >>>>> > > >>>>> We intends to do shared domain creation after setup_virt_paging so > > >>>>> shared domain could successfully do p2m initialization. > > >>>> > > >>>> There's nothing said here, in the earlier patch, or in the cover > > >>>> letter about the security aspects of this. There is a reason we > > >>>> haven't been allowing arbitrary, un-supervised sharing of memory > > >>>> between domains. It wants clarifying why e.g. grants aren't an > > >>>> option to achieve what you need, and how you mean to establish > > >>>> which domains are / aren't permitted to access any individual page > > owned by this domain. > > >>> > > >>> > > >>> I'll let Penny write a full reply but I'll chime in to try to help > > >>> with the explanation. > > >>> > > >>> This is not arbitrary un-supervised sharing of memory between > > >>> domains, which indeed is concerning. > > >>> > > >>> This is statically-configured, supervised by the system > > >>> configurator, sharing of memory between domains. > > >>> > > >>> And in fact safety (which is just a different aspect of security) is > > >>> one of the primary goals for this work. > > >>> > > >>> In safety-critical environments, it is not considered safe to > > >>> dynamically change important configurations at runtime. Everything > > >>> should be statically defined and statically verified. > > >>> > > >>> In this case, if the system configuration knows a priori that there > > >>> are only 2 VM and they need to communication over shared memory, it > > >>> is safer to pre-configure the shared memory at build time rather > > >>> than let the VMs attempt to share memory at runtime. It is faster too. > > >>> > > >>> The only way to trigger this static shared memory configuration > > >>> should be via device tree, which is at the same level as the XSM > > >>> rules themselves. > > >>> > > >>> Hopefully I made things clearer and not murkier :-) > > >> > > >> It adds some helpful background, yes, but at the same time it doesn't > > >> address the security concern at all: How are access permissions > > >> managed when the owning domain is a special one? I haven't spotted > > >> any recording of the domains which are actually permitted to map / > > >> access the pages in questions. (But of course I also only looked at > > >> non-Arm-specific code. I'd expect such code not to live in arch- > > >> specific files.) > > > > > > All this static memory sharing is statically done at __init time only. > > > It should not be possible to trigger any further memory sharing at > > > runtime (if there is, that would be a bug). > > > > Looking at the code, get_pg_owner() will be able to handle DOMID_SHARED. > > So anyone that is permitted to access DOMID_SHARED will be able to map any > > memory region at runtime. > > > > > There are no new interfaces for the guest to map this memory because > > > it is already "pre-mapped". > > > > It can via XENMAPSPACE_gmfn_foreign (assuming proper permission). > > > > Correct me if I'm wrong: > The existing XENMAPSPACE_gmfn_foreign only allows privileged Dom0 to map > memory pages from one foreign DomU to itself. So It can happen that Dom0 is > using XENMAPSPACE_gmfn_foreign to (maliciously?) access shared memory owned > by DOMID_SHARED, and for now only Dom0 could. No, currently there is no protection against dom0 doing malicious operations. Dom0 can poweroff the entire system. If we are certain that only dom0 (and not other domains) can use XENMAPSPACE_gmfn_foreign to access shared memory owned by DOMID_SHARED then we are good. Looking at the code, and also considering that we have agreed to move to DOMID_IO, get_pg_owner can already handle DOMID_IO. Next is the xsm_map_gmfn_foreign(XSM_TARGET, d, od) check, which would fail unless the asking domain is privileged over the target domain. xsm_map_gmfn_foreign would fail for all domains except dom0. So I think we are OK. I don't think we need to do anything else.
Hi Penny, Sorry for the formatting. On Fri, 15 Apr 2022 at 09:53, Penny Zheng <Penny.Zheng@arm.com> wrote: > Hi jan > > > ----- > > > +#ifdef CONFIG_STATIC_SHM > > > +static void __init setup_shared_domain(void) { > > > + /* > > > + * Initialise our DOMID_SHARED domain. > > > + * This domain owns statically shared pages when owner domain is > not > > > + * explicitly defined. > > > + */ > > > + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); > > > + if ( IS_ERR(dom_shared) ) > > > + panic("Failed to create d[SHARED]: %ld\n", > > > +PTR_ERR(dom_shared)); > > > > I don't think this should be a panic - the system ought to be able to > come up > > fine, just without actually using this domain. After all this is an > optional > > feature which may not actually be used. > > > > Also, along the lines of what Stefano has said, this setting up of the > domain > > would also better live next to where the other special domains are set > up. And > > even if it was to remain here, ... > > > > The reason why I place the setting up here is that DOMID_SHARED needs to > map > pre-configured static shared memory in its p2m table, so it must be set up > after system P2M initialization(setup_virt_paging()). > setup_system_domains() > is called before system P2M initialization on xen/arch/arm/setup.c, which > can't meet the requirement. AFAIU, DOM_SHARED (or whatever you want to call it) will never run and the GFN will always be equal to the MFN. So it sounds to me that creating a P2M is a bit over the top. Instead, we should have a special case in get_page_from_gfn like we did for DOM_XEN. Cheers,
Hi, On Fri, 15 Apr 2022 at 22:19, Stefano Stabellini <sstabellini@kernel.org> wrote: > On Fri, 15 Apr 2022, Penny Zheng wrote: > > > Hi Stefano, > > > > > > On 21/03/2022 20:03, Stefano Stabellini wrote: > > > > On Mon, 21 Mar 2022, Jan Beulich wrote: > > > >> On 18.03.2022 22:50, Stefano Stabellini wrote: > > > >>> On Fri, 18 Mar 2022, Jan Beulich wrote: > > > >>>> On 11.03.2022 07:11, Penny Zheng wrote: > > > >>>>> In case to own statically shared pages when owner domain is not > > > >>>>> explicitly defined, this commits propose a special domain > > > >>>>> DOMID_SHARED, and we assign it 0x7FF5, as one of the system > domains. > > > >>>>> > > > >>>>> Statically shared memory reuses the same way of initialization > > > >>>>> with static memory, hence this commits proposes a new Kconfig > > > >>>>> CONFIG_STATIC_SHM to wrap related codes, and this option depends > > > on static memory(CONFIG_STATIC_MEMORY). > > > >>>>> > > > >>>>> We intends to do shared domain creation after setup_virt_paging > so > > > >>>>> shared domain could successfully do p2m initialization. > > > >>>> > > > >>>> There's nothing said here, in the earlier patch, or in the cover > > > >>>> letter about the security aspects of this. There is a reason we > > > >>>> haven't been allowing arbitrary, un-supervised sharing of memory > > > >>>> between domains. It wants clarifying why e.g. grants aren't an > > > >>>> option to achieve what you need, and how you mean to establish > > > >>>> which domains are / aren't permitted to access any individual page > > > owned by this domain. > > > >>> > > > >>> > > > >>> I'll let Penny write a full reply but I'll chime in to try to help > > > >>> with the explanation. > > > >>> > > > >>> This is not arbitrary un-supervised sharing of memory between > > > >>> domains, which indeed is concerning. > > > >>> > > > >>> This is statically-configured, supervised by the system > > > >>> configurator, sharing of memory between domains. > > > >>> > > > >>> And in fact safety (which is just a different aspect of security) > is > > > >>> one of the primary goals for this work. > > > >>> > > > >>> In safety-critical environments, it is not considered safe to > > > >>> dynamically change important configurations at runtime. Everything > > > >>> should be statically defined and statically verified. > > > >>> > > > >>> In this case, if the system configuration knows a priori that there > > > >>> are only 2 VM and they need to communication over shared memory, it > > > >>> is safer to pre-configure the shared memory at build time rather > > > >>> than let the VMs attempt to share memory at runtime. It is faster > too. > > > >>> > > > >>> The only way to trigger this static shared memory configuration > > > >>> should be via device tree, which is at the same level as the XSM > > > >>> rules themselves. > > > >>> > > > >>> Hopefully I made things clearer and not murkier :-) > > > >> > > > >> It adds some helpful background, yes, but at the same time it > doesn't > > > >> address the security concern at all: How are access permissions > > > >> managed when the owning domain is a special one? I haven't spotted > > > >> any recording of the domains which are actually permitted to map / > > > >> access the pages in questions. (But of course I also only looked at > > > >> non-Arm-specific code. I'd expect such code not to live in arch- > > > >> specific files.) > > > > > > > > All this static memory sharing is statically done at __init time > only. > > > > It should not be possible to trigger any further memory sharing at > > > > runtime (if there is, that would be a bug). > > > > > > Looking at the code, get_pg_owner() will be able to handle > DOMID_SHARED. > > > So anyone that is permitted to access DOMID_SHARED will be able to map > any > > > memory region at runtime. > > > > > > > There are no new interfaces for the guest to map this memory because > > > > it is already "pre-mapped". > > > > > > It can via XENMAPSPACE_gmfn_foreign (assuming proper permission). > > > > > > > Correct me if I'm wrong: > > The existing XENMAPSPACE_gmfn_foreign only allows privileged Dom0 to map > > memory pages from one foreign DomU to itself. So It can happen that Dom0 > is > > using XENMAPSPACE_gmfn_foreign to (maliciously?) access shared memory > owned > > by DOMID_SHARED, and for now only Dom0 could. > > No, currently there is no protection against dom0 doing malicious > operations. Dom0 can poweroff the entire system. This is the classic argument… Yes, on a default setup, dom0 is fully trusted today. However there are way to reduce the trust via XSM. For new interfaces we should also try to avoid considering dom0 as fully trusted whenever it is possible. This is one of the example where I think this should be done. The more if we use DOMID_IO as we may add more than shared pages there… > > If we are certain that only dom0 (and not other domains) can use > XENMAPSPACE_gmfn_foreign to access shared memory owned by DOMID_SHARED > then we are good. Looking at the code, and also considering that we have > agreed to move to DOMID_IO, get_pg_owner can already handle DOMID_IO. AFAIK, dom0 cannot map DOMID_IO page for now because get_page_from_gfn would not work. This would change with this approach. > > Next is the xsm_map_gmfn_foreign(XSM_TARGET, d, od) check, which would > fail unless the asking domain is privileged over the target domain. > xsm_map_gmfn_foreign would fail for all domains except dom0. This depends your XSM policy. In this case, think we need to prevent runtime mapping via the hypercall interface. We can relax it afterwards if we have use cases for it. > > So I think we are OK. I don't think we need to do anything else. Cheers,
On 15.04.2022 11:52, Penny Zheng wrote: >> From: Jan Beulich <jbeulich@suse.com> >> Sent: Friday, March 18, 2022 4:53 PM >> >> On 11.03.2022 07:11, Penny Zheng wrote: >>> --- a/xen/arch/arm/setup.c >>> +++ b/xen/arch/arm/setup.c >>> @@ -855,6 +855,20 @@ static bool __init is_dom0less_mode(void) >>> return ( !dom0found && domUfound ); } >>> >>> +#ifdef CONFIG_STATIC_SHM >>> +static void __init setup_shared_domain(void) { >>> + /* >>> + * Initialise our DOMID_SHARED domain. >>> + * This domain owns statically shared pages when owner domain is not >>> + * explicitly defined. >>> + */ >>> + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); >>> + if ( IS_ERR(dom_shared) ) >>> + panic("Failed to create d[SHARED]: %ld\n", >>> +PTR_ERR(dom_shared)); >> >> I don't think this should be a panic - the system ought to be able to come up >> fine, just without actually using this domain. After all this is an optional >> feature which may not actually be used. >> >> Also, along the lines of what Stefano has said, this setting up of the domain >> would also better live next to where the other special domains are set up. And >> even if it was to remain here, ... >> > > The reason why I place the setting up here is that DOMID_SHARED needs to map > pre-configured static shared memory in its p2m table, so it must be set up > after system P2M initialization(setup_virt_paging()). setup_system_domains() > is called before system P2M initialization on xen/arch/arm/setup.c, which > can't meet the requirement. While possibly moot with the plan to use DomIO (and my hope that you don't mean to move DomIO's creation), I'd like to point out that there can't be "too early" setting up of something. If it happens earlier than where you need it, all you need to do is arrange for the further setup you mean to add to be invoked separately, whenever it's time to do so. Jan
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index ecfa6822e4..c54accefb1 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -106,6 +106,13 @@ config TEE source "arch/arm/tee/Kconfig" +config STATIC_SHM + bool "Statically shared memory on a dom0less system" if UNSUPPORTED + depends on STATIC_MEMORY + default n + help + This option enables statically shared memory on a dom0less system. + endmenu menu "ARM errata workaround via the alternative framework" diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 8110c1df86..1ff1df5d3f 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -44,6 +44,10 @@ DEFINE_PER_CPU(struct vcpu *, curr_vcpu); +#ifdef CONFIG_STATIC_SHM +struct domain *__read_mostly dom_shared; +#endif + static void do_idle(void) { unsigned int cpu = smp_processor_id(); @@ -703,7 +707,7 @@ int arch_domain_create(struct domain *d, if ( is_idle_domain(d) ) return 0; - ASSERT(config != NULL); + ASSERT(is_shared_domain(d) ? config == NULL : config != NULL); #ifdef CONFIG_IOREQ_SERVER ioreq_domain_init(d); @@ -712,12 +716,16 @@ int arch_domain_create(struct domain *d, d->arch.directmap = flags & CDF_directmap; /* p2m_init relies on some value initialized by the IOMMU subsystem */ - if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 ) + if ( (rc = iommu_domain_init(d, is_shared_domain(d) ? 0 : config->iommu_opts)) != 0 ) goto fail; if ( (rc = p2m_init(d)) != 0 ) goto fail; + /* DOMID_shared is sufficiently constructed after p2m initialization. */ + if ( is_shared_domain(d) ) + return 0; + rc = -ENOMEM; if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL ) goto fail; diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h index c56f6e4398..ea7a7219a3 100644 --- a/xen/arch/arm/include/asm/domain.h +++ b/xen/arch/arm/include/asm/domain.h @@ -31,6 +31,12 @@ enum domain_type { #define is_domain_direct_mapped(d) (d)->arch.directmap +#ifdef CONFIG_STATIC_SHM +extern struct domain *dom_shared; +#else +#define dom_shared NULL +#endif + /* * Is the domain using the host memory layout? * diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index d5d0792ed4..f6a3b04958 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -855,6 +855,20 @@ static bool __init is_dom0less_mode(void) return ( !dom0found && domUfound ); } +#ifdef CONFIG_STATIC_SHM +static void __init setup_shared_domain(void) +{ + /* + * Initialise our DOMID_SHARED domain. + * This domain owns statically shared pages when owner domain is not + * explicitly defined. + */ + dom_shared = domain_create(DOMID_SHARED, NULL, CDF_directmap); + if ( IS_ERR(dom_shared) ) + panic("Failed to create d[SHARED]: %ld\n", PTR_ERR(dom_shared)); +} +#endif + size_t __read_mostly dcache_line_bytes; /* C entry point for boot CPU */ @@ -1022,6 +1036,14 @@ void __init start_xen(unsigned long boot_phys_offset, apply_alternatives_all(); enable_errata_workarounds(); +#ifdef CONFIG_STATIC_SHM + /* + * This needs to be called **after** setup_virt_paging so shared + * domains could successfully do p2m initialization. + */ + setup_shared_domain(); +#endif + /* Create initial domain 0. */ if ( !is_dom0less_mode() ) create_dom0(); diff --git a/xen/common/domain.c b/xen/common/domain.c index 3742322d22..5cdd0b9f5b 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -643,11 +643,14 @@ struct domain *domain_create(domid_t domid, rangeset_domain_initialise(d); - /* DOMID_{XEN,IO,etc} (other than IDLE) are sufficiently constructed. */ - if ( is_system_domain(d) && !is_idle_domain(d) ) + /* + * DOMID_{XEN,IO,etc} (other than IDLE and DOMID_shared) are + * sufficiently constructed. + */ + if ( is_system_domain(d) && !is_idle_domain(d) && !is_shared_domain(d) ) return d; - if ( !is_idle_domain(d) ) + if ( !is_idle_domain(d) && !is_shared_domain(d) ) { if ( !is_hardware_domain(d) ) d->nr_pirqs = nr_static_irqs + extra_domU_irqs; @@ -663,7 +666,7 @@ struct domain *domain_create(domid_t domid, goto fail; init_status |= INIT_arch; - if ( !is_idle_domain(d) ) + if ( !is_idle_domain(d) && !is_shared_domain(d) ) { watchdog_domain_init(d); init_status |= INIT_watchdog; diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index f8749b0787..e5e357969d 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -2616,6 +2616,11 @@ struct domain *get_pg_owner(domid_t domid) switch ( domid ) { +#ifdef CONFIG_STATIC_SHM + case DOMID_SHARED: + pg_owner = rcu_lock_domain(dom_shared); + break; +#endif case DOMID_IO: pg_owner = rcu_lock_domain(dom_io); break; diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c index b278961cc3..a22854001b 100644 --- a/xen/common/vsprintf.c +++ b/xen/common/vsprintf.c @@ -359,10 +359,11 @@ static char *print_domain(char *str, const char *end, const struct domain *d) switch ( d->domain_id ) { - case DOMID_IO: name = "[IO]"; break; - case DOMID_XEN: name = "[XEN]"; break; - case DOMID_COW: name = "[COW]"; break; - case DOMID_IDLE: name = "[IDLE]"; break; + case DOMID_IO: name = "[IO]"; break; + case DOMID_XEN: name = "[XEN]"; break; + case DOMID_COW: name = "[COW]"; break; + case DOMID_IDLE: name = "[IDLE]"; break; + case DOMID_SHARED: name = "[SHARED]"; break; /* * In principle, we could ASSERT_UNREACHABLE() in the default case. * However, this path is used to print out crash information, which diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index e373592c33..2e00741f09 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -612,6 +612,12 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t); /* DOMID_INVALID is used to identify pages with unknown owner. */ #define DOMID_INVALID xen_mk_uint(0x7FF4) +/* + * DOMID_SHARED is used as the owner of statically shared pages, when + * owner is not explicitly defined. + */ +#define DOMID_SHARED xen_mk_uint(0x7FF5) + /* Idle domain. */ #define DOMID_IDLE xen_mk_uint(0x7FFF) diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 24a9a87f83..2fb236f4ea 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -618,6 +618,8 @@ static inline bool is_system_domain(const struct domain *d) return d->domain_id >= DOMID_FIRST_RESERVED; } +#define is_shared_domain(d) ((d)->domain_id == DOMID_SHARED) + #define DOMAIN_DESTROYED (1u << 31) /* assumes atomic_t is >= 32 bits */ #define put_domain(_d) \ if ( atomic_dec_and_test(&(_d)->refcnt) ) domain_destroy(_d)