| Message ID | 20220531031241.90374-7-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | populate/unpopulate memory when domain on static allocation | expand |
On 31.05.2022 05:12, Penny Zheng wrote: > --- a/xen/arch/arm/include/asm/domain.h > +++ b/xen/arch/arm/include/asm/domain.h > @@ -31,6 +31,10 @@ enum domain_type { > > #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap) > > +#ifdef CONFIG_STATIC_MEMORY > +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem) > +#endif Why is this in the Arm header, rather than ... > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d, > #ifdef CONFIG_ARM > /* Should domain memory be directly mapped? */ > #define CDF_directmap (1U << 1) > +/* Is domain memory on static allocation? */ > +#define CDF_staticmem (1U << 2) > +#endif > + > +#ifndef is_domain_using_staticmem > +#define is_domain_using_staticmem(d) ((void)(d), false) > #endif ... here (with what you have here now simply becoming the #else part)? Once living here, I expect it also can be an inline function rather than a macro, with the #ifdef merely inside its body. Jan
Hi Jan > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: Tuesday, May 31, 2022 4:41 PM > To: Penny Zheng <Penny.Zheng@arm.com> > Cc: Wei Chen <Wei.Chen@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 v5 6/9] xen/arm: introduce CDF_staticmem > > On 31.05.2022 05:12, Penny Zheng wrote: > > --- a/xen/arch/arm/include/asm/domain.h > > +++ b/xen/arch/arm/include/asm/domain.h > > @@ -31,6 +31,10 @@ enum domain_type { > > > > #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap) > > > > +#ifdef CONFIG_STATIC_MEMORY > > +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem) > > +#endif > > Why is this in the Arm header, rather than ... > > > --- a/xen/include/xen/domain.h > > +++ b/xen/include/xen/domain.h > > @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d, > > #ifdef CONFIG_ARM > > /* Should domain memory be directly mapped? */ > > #define CDF_directmap (1U << 1) > > +/* Is domain memory on static allocation? */ > > +#define CDF_staticmem (1U << 2) > > +#endif > > + > > +#ifndef is_domain_using_staticmem > > +#define is_domain_using_staticmem(d) ((void)(d), false) > > #endif > > ... here (with what you have here now simply becoming the #else part)? > Once living here, I expect it also can be an inline function rather than a macro, > with the #ifdef merely inside its body. > In order to avoid bring the chicken and egg problem in xen/include/xen/domain.h, I may need to move the static inline function to xen/include/xen/sched.h(which has already included domain.h header). > Jan
On 02.06.2022 12:07, Penny Zheng wrote: >> From: Jan Beulich <jbeulich@suse.com> >> Sent: Tuesday, May 31, 2022 4:41 PM >> >> On 31.05.2022 05:12, Penny Zheng wrote: >>> --- a/xen/arch/arm/include/asm/domain.h >>> +++ b/xen/arch/arm/include/asm/domain.h >>> @@ -31,6 +31,10 @@ enum domain_type { >>> >>> #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap) >>> >>> +#ifdef CONFIG_STATIC_MEMORY >>> +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem) >>> +#endif >> >> Why is this in the Arm header, rather than ... >> >>> --- a/xen/include/xen/domain.h >>> +++ b/xen/include/xen/domain.h >>> @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d, >>> #ifdef CONFIG_ARM >>> /* Should domain memory be directly mapped? */ >>> #define CDF_directmap (1U << 1) >>> +/* Is domain memory on static allocation? */ >>> +#define CDF_staticmem (1U << 2) >>> +#endif >>> + >>> +#ifndef is_domain_using_staticmem >>> +#define is_domain_using_staticmem(d) ((void)(d), false) >>> #endif >> >> ... here (with what you have here now simply becoming the #else part)? >> Once living here, I expect it also can be an inline function rather than a macro, >> with the #ifdef merely inside its body. >> > > In order to avoid bring the chicken and egg problem in xen/include/xen/domain.h, > I may need to move the static inline function to xen/include/xen/sched.h(which > has already included domain.h header). Hmm, yes, if made an inline function it would need to move to sched.h. But as a macro it could live here (without one half needing to live in the Arm header). Jan
On Tue, 31 May 2022, Penny Zheng wrote: > In order to have an easy and quick way to find out whether this domain memory > is statically configured, this commit introduces a new flag CDF_staticmem and a > new helper is_domain_using_staticmem() to tell. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> I realize Jan asked for improvements but I just wanted to say that on my side it looks good. > --- > v5 changes: > - guard "is_domain_using_staticmem" under CONFIG_STATIC_MEMORY > - #define is_domain_using_staticmem zero if undefined > --- > v4 changes: > - no changes > --- > v3 changes: > - change name from "is_domain_static()" to "is_domain_using_staticmem" > --- > v2 changes: > - change name from "is_domain_on_static_allocation" to "is_domain_static()" > --- > xen/arch/arm/domain_build.c | 5 ++++- > xen/arch/arm/include/asm/domain.h | 4 ++++ > xen/include/xen/domain.h | 6 ++++++ > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 7ddd16c26d..f6e2e44c1e 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -3287,9 +3287,12 @@ void __init create_domUs(void) > if ( !dt_device_is_compatible(node, "xen,domain") ) > continue; > > + if ( dt_find_property(node, "xen,static-mem", NULL) ) > + flags |= CDF_staticmem; > + > if ( dt_property_read_bool(node, "direct-map") ) > { > - if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, "xen,static-mem", NULL) ) > + if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !(flags & CDF_staticmem) ) > panic("direct-map is not valid for domain %s without static allocation.\n", > dt_node_name(node)); > > diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h > index fe7a029ebf..6bb999aff0 100644 > --- a/xen/arch/arm/include/asm/domain.h > +++ b/xen/arch/arm/include/asm/domain.h > @@ -31,6 +31,10 @@ enum domain_type { > > #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap) > > +#ifdef CONFIG_STATIC_MEMORY > +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem) > +#endif > + > /* > * Is the domain using the host memory layout? > * > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h > index 1c3c88a14d..c613afa57e 100644 > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d, > #ifdef CONFIG_ARM > /* Should domain memory be directly mapped? */ > #define CDF_directmap (1U << 1) > +/* Is domain memory on static allocation? */ > +#define CDF_staticmem (1U << 2) > +#endif > + > +#ifndef is_domain_using_staticmem > +#define is_domain_using_staticmem(d) ((void)(d), false) > #endif > > /* > -- > 2.25.1 >
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 7ddd16c26d..f6e2e44c1e 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -3287,9 +3287,12 @@ void __init create_domUs(void) if ( !dt_device_is_compatible(node, "xen,domain") ) continue; + if ( dt_find_property(node, "xen,static-mem", NULL) ) + flags |= CDF_staticmem; + if ( dt_property_read_bool(node, "direct-map") ) { - if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, "xen,static-mem", NULL) ) + if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !(flags & CDF_staticmem) ) panic("direct-map is not valid for domain %s without static allocation.\n", dt_node_name(node)); diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h index fe7a029ebf..6bb999aff0 100644 --- a/xen/arch/arm/include/asm/domain.h +++ b/xen/arch/arm/include/asm/domain.h @@ -31,6 +31,10 @@ enum domain_type { #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap) +#ifdef CONFIG_STATIC_MEMORY +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem) +#endif + /* * Is the domain using the host memory layout? * diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index 1c3c88a14d..c613afa57e 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d, #ifdef CONFIG_ARM /* Should domain memory be directly mapped? */ #define CDF_directmap (1U << 1) +/* Is domain memory on static allocation? */ +#define CDF_staticmem (1U << 2) +#endif + +#ifndef is_domain_using_staticmem +#define is_domain_using_staticmem(d) ((void)(d), false) #endif /*
In order to have an easy and quick way to find out whether this domain memory is statically configured, this commit introduces a new flag CDF_staticmem and a new helper is_domain_using_staticmem() to tell. Signed-off-by: Penny Zheng <penny.zheng@arm.com> --- v5 changes: - guard "is_domain_using_staticmem" under CONFIG_STATIC_MEMORY - #define is_domain_using_staticmem zero if undefined --- v4 changes: - no changes --- v3 changes: - change name from "is_domain_static()" to "is_domain_using_staticmem" --- v2 changes: - change name from "is_domain_on_static_allocation" to "is_domain_static()" --- xen/arch/arm/domain_build.c | 5 ++++- xen/arch/arm/include/asm/domain.h | 4 ++++ xen/include/xen/domain.h | 6 ++++++ 3 files changed, 14 insertions(+), 1 deletion(-)