* [PATCH] xen/arm: Avoid to open-code the relinquish state machine
@ 2020-04-19 9:50 Julien Grall
2020-04-21 23:47 ` Stefano Stabellini
0 siblings, 1 reply; 2+ messages in thread
From: Julien Grall @ 2020-04-19 9:50 UTC (permalink / raw
To: xen-devel
Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien,
Andrew Cooper
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] xen/arm: Avoid to open-code the relinquish state machine
2020-04-19 9:50 [PATCH] xen/arm: Avoid to open-code the relinquish state machine Julien Grall
@ 2020-04-21 23:47 ` Stefano Stabellini
0 siblings, 0 replies; 2+ messages in thread
From: Stefano Stabellini @ 2020-04-21 23:47 UTC (permalink / raw
To: Julien Grall
Cc: xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk,
Andrew Cooper
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-04-21 23:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-19 9:50 [PATCH] xen/arm: Avoid to open-code the relinquish state machine Julien Grall
2020-04-21 23:47 ` Stefano Stabellini
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.