xen/arch/arm/domain.c | 60 ++++++++++++++++++++++-------------- xen/include/asm-arm/domain.h | 9 +----- 2 files changed, 38 insertions(+), 31 deletions(-)
In commit 0dfffe01d5 "x86: Improve the efficiency of
domain_relinquish_resources()", the x86 version of the function has been
reworked to avoid open-coding the state machine and also add more
documentation.
Bring the Arm version on part with x86 by introducing a documented
PROGRESS() macro to avoid latent bugs and make the new PROG_* states
private to domain_relinquish_resources().
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
---
xen/arch/arm/domain.c | 60 ++++++++++++++++++++++--------------
xen/include/asm-arm/domain.h | 9 +-----
2 files changed, 38 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 6627be2922..31169326b2 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -674,7 +674,6 @@ int arch_domain_create(struct domain *d,
int rc, count = 0;
BUILD_BUG_ON(GUEST_MAX_VCPUS < MAX_VIRT_CPUS);
- d->arch.relmem = RELMEM_not_started;
/* Idle domains do not need this setup */
if ( is_idle_domain(d) )
@@ -950,13 +949,41 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list)
return ret;
}
+/*
+ * Record the current progress. Subsequent hypercall continuations will
+ * logically restart work from this point.
+ *
+ * PROGRESS() markers must not be in the middle of loops. The loop
+ * variable isn't preserved accross a continuation.
+ *
+ * To avoid redundant work, there should be a marker before each
+ * function which may return -ERESTART.
+ */
+enum {
+ PROG_tee = 1,
+ PROG_xen,
+ PROG_page,
+ PROG_mapping,
+ PROG_done,
+};
+
+#define PROGRESS(x) \
+ d->arch.rel_priv = PROG_ ## x; \
+ /* Fallthrough */ \
+ case PROG_ ## x
+
int domain_relinquish_resources(struct domain *d)
{
int ret = 0;
- switch ( d->arch.relmem )
+ /*
+ * This hypercall can take minutes of wallclock time to complete. This
+ * logic implements a co-routine, stashing state in struct domain across
+ * hypercall continuation boundaries.
+ */
+ switch ( d->arch.rel_priv )
{
- case RELMEM_not_started:
+ case 0:
ret = iommu_release_dt_devices(d);
if ( ret )
return ret;
@@ -967,42 +994,27 @@ int domain_relinquish_resources(struct domain *d)
*/
domain_vpl011_deinit(d);
- d->arch.relmem = RELMEM_tee;
- /* Fallthrough */
-
- case RELMEM_tee:
+ PROGRESS(tee):
ret = tee_relinquish_resources(d);
if (ret )
return ret;
- d->arch.relmem = RELMEM_xen;
- /* Fallthrough */
-
- case RELMEM_xen:
+ PROGRESS(xen):
ret = relinquish_memory(d, &d->xenpage_list);
if ( ret )
return ret;
- d->arch.relmem = RELMEM_page;
- /* Fallthrough */
-
- case RELMEM_page:
+ PROGRESS(page):
ret = relinquish_memory(d, &d->page_list);
if ( ret )
return ret;
- d->arch.relmem = RELMEM_mapping;
- /* Fallthrough */
-
- case RELMEM_mapping:
+ PROGRESS(mapping):
ret = relinquish_p2m_mapping(d);
if ( ret )
return ret;
- d->arch.relmem = RELMEM_done;
- /* Fallthrough */
-
- case RELMEM_done:
+ PROGRESS(done):
break;
default:
@@ -1012,6 +1024,8 @@ int domain_relinquish_resources(struct domain *d)
return 0;
}
+#undef PROGRESS
+
void arch_dump_domain_info(struct domain *d)
{
p2m_dump_info(d);
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index d39477a939..d2142c6707 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -56,14 +56,7 @@ struct arch_domain
struct vmmio vmmio;
/* Continuable domain_relinquish_resources(). */
- enum {
- RELMEM_not_started,
- RELMEM_tee,
- RELMEM_xen,
- RELMEM_page,
- RELMEM_mapping,
- RELMEM_done,
- } relmem;
+ unsigned int rel_priv;
struct {
uint64_t offset;
--
2.17.1
On Sun, 19 Apr 2020, Julien Grall wrote: > In commit 0dfffe01d5 "x86: Improve the efficiency of > domain_relinquish_resources()", the x86 version of the function has been > reworked to avoid open-coding the state machine and also add more > documentation. > > Bring the Arm version on part with x86 by introducing a documented > PROGRESS() macro to avoid latent bugs and make the new PROG_* states > private to domain_relinquish_resources(). > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Julien Grall <jgrall@amazon.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/domain.c | 60 ++++++++++++++++++++++-------------- > xen/include/asm-arm/domain.h | 9 +----- > 2 files changed, 38 insertions(+), 31 deletions(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 6627be2922..31169326b2 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -674,7 +674,6 @@ int arch_domain_create(struct domain *d, > int rc, count = 0; > > BUILD_BUG_ON(GUEST_MAX_VCPUS < MAX_VIRT_CPUS); > - d->arch.relmem = RELMEM_not_started; > > /* Idle domains do not need this setup */ > if ( is_idle_domain(d) ) > @@ -950,13 +949,41 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list) > return ret; > } > > +/* > + * Record the current progress. Subsequent hypercall continuations will > + * logically restart work from this point. > + * > + * PROGRESS() markers must not be in the middle of loops. The loop > + * variable isn't preserved accross a continuation. > + * > + * To avoid redundant work, there should be a marker before each > + * function which may return -ERESTART. > + */ > +enum { > + PROG_tee = 1, > + PROG_xen, > + PROG_page, > + PROG_mapping, > + PROG_done, > +}; > + > +#define PROGRESS(x) \ > + d->arch.rel_priv = PROG_ ## x; \ > + /* Fallthrough */ \ > + case PROG_ ## x > + > int domain_relinquish_resources(struct domain *d) > { > int ret = 0; > > - switch ( d->arch.relmem ) > + /* > + * This hypercall can take minutes of wallclock time to complete. This > + * logic implements a co-routine, stashing state in struct domain across > + * hypercall continuation boundaries. > + */ > + switch ( d->arch.rel_priv ) > { > - case RELMEM_not_started: > + case 0: > ret = iommu_release_dt_devices(d); > if ( ret ) > return ret; > @@ -967,42 +994,27 @@ int domain_relinquish_resources(struct domain *d) > */ > domain_vpl011_deinit(d); > > - d->arch.relmem = RELMEM_tee; > - /* Fallthrough */ > - > - case RELMEM_tee: > + PROGRESS(tee): > ret = tee_relinquish_resources(d); > if (ret ) > return ret; > > - d->arch.relmem = RELMEM_xen; > - /* Fallthrough */ > - > - case RELMEM_xen: > + PROGRESS(xen): > ret = relinquish_memory(d, &d->xenpage_list); > if ( ret ) > return ret; > > - d->arch.relmem = RELMEM_page; > - /* Fallthrough */ > - > - case RELMEM_page: > + PROGRESS(page): > ret = relinquish_memory(d, &d->page_list); > if ( ret ) > return ret; > > - d->arch.relmem = RELMEM_mapping; > - /* Fallthrough */ > - > - case RELMEM_mapping: > + PROGRESS(mapping): > ret = relinquish_p2m_mapping(d); > if ( ret ) > return ret; > > - d->arch.relmem = RELMEM_done; > - /* Fallthrough */ > - > - case RELMEM_done: > + PROGRESS(done): > break; > > default: > @@ -1012,6 +1024,8 @@ int domain_relinquish_resources(struct domain *d) > return 0; > } > > +#undef PROGRESS > + > void arch_dump_domain_info(struct domain *d) > { > p2m_dump_info(d); > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index d39477a939..d2142c6707 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -56,14 +56,7 @@ struct arch_domain > struct vmmio vmmio; > > /* Continuable domain_relinquish_resources(). */ > - enum { > - RELMEM_not_started, > - RELMEM_tee, > - RELMEM_xen, > - RELMEM_page, > - RELMEM_mapping, > - RELMEM_done, > - } relmem; > + unsigned int rel_priv; > > struct { > uint64_t offset; > -- > 2.17.1 >
© 2016 - 2024 Red Hat, Inc.