| Message ID | 20230605144331.1819452-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Arch Teardown | expand |
On Mon, Jun 05, 2023 at 03:43:30PM +0100, Andrew Cooper wrote: > Plumb it into domain_teardown(). Provide arch_val in the teardown > continuation information for use by arch_domain_teardown(). > > No practical change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien@xen.org> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > CC: Bertrand Marquis <bertrand.marquis@arm.com> > CC: Jens Wiklander <jens.wiklander@linaro.org> > --- > xen/arch/arm/domain.c | 5 +++++ > xen/arch/x86/domain.c | 5 +++++ > xen/common/domain.c | 6 ++++++ > xen/include/xen/domain.h | 1 + > xen/include/xen/sched.h | 1 + > 5 files changed, 18 insertions(+) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index d8ef6501ff8e..b3981d70a442 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -750,6 +750,11 @@ int arch_domain_create(struct domain *d, > return rc; > } > > +int arch_domain_teardown(struct domain *d) > +{ > + return 0; > +} > + > void arch_domain_destroy(struct domain *d) > { > /* IOMMU page table is shared with P2M, always call > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index 39c215316546..5f66c2ae33d7 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -888,6 +888,11 @@ int arch_domain_create(struct domain *d, > return rc; > } > > +int arch_domain_teardown(struct domain *d) > +{ > + return 0; > +} > + > void arch_domain_destroy(struct domain *d) > { > if ( is_hvm_domain(d) ) > diff --git a/xen/common/domain.c b/xen/common/domain.c > index 6a440590fe2a..b0d850e3595b 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -416,6 +416,7 @@ static int domain_teardown(struct domain *d) > PROG_none, > PROG_gnttab_mappings, > PROG_vcpu_teardown, > + PROG_arch_teardown, > PROG_done, > }; > > @@ -436,6 +437,11 @@ static int domain_teardown(struct domain *d) > return rc; > } > > + PROGRESS(arch_teardown): > + rc = arch_domain_teardown(d); > + if ( rc ) > + return rc; > + > PROGRESS(done): > break; > > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h > index 26f9c4f6dd5b..c3348c90748f 100644 > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -80,6 +80,7 @@ int arch_domain_create(struct domain *d, > struct xen_domctl_createdomain *config, > unsigned int flags); > > +int arch_domain_teardown(struct domain *d); > void arch_domain_destroy(struct domain *d); > > void arch_domain_shutdown(struct domain *d); > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index 85242a73d374..854f3e32c00e 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -589,6 +589,7 @@ struct domain > */ > struct { > unsigned int val; > + unsigned int arch_val; While I haven't looked at patch 2, wouldn't such continuation information better be encoded in arch_domain in whatever way is more suitable for each architecture? Thanks, Roger.
On 05/06/2023 4:19 pm, Roger Pau Monné wrote: > On Mon, Jun 05, 2023 at 03:43:30PM +0100, Andrew Cooper wrote: >> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >> index 85242a73d374..854f3e32c00e 100644 >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -589,6 +589,7 @@ struct domain >> */ >> struct { >> unsigned int val; >> + unsigned int arch_val; > While I haven't looked at patch 2, wouldn't such continuation > information better be encoded in arch_domain in whatever way is more > suitable for each architecture? Well, it's filling a hole here on 64bit builds, which it couldn't do in arch_domain. And it's contained inside teardown{} which already signals it as fairly magic. ~Andrew
On 05.06.2023 17:23, Andrew Cooper wrote: > On 05/06/2023 4:19 pm, Roger Pau Monné wrote: >> On Mon, Jun 05, 2023 at 03:43:30PM +0100, Andrew Cooper wrote: >>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >>> index 85242a73d374..854f3e32c00e 100644 >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -589,6 +589,7 @@ struct domain >>> */ >>> struct { >>> unsigned int val; >>> + unsigned int arch_val; >> While I haven't looked at patch 2, wouldn't such continuation >> information better be encoded in arch_domain in whatever way is more >> suitable for each architecture? > > Well, it's filling a hole here on 64bit builds, which it couldn't do in > arch_domain. > > And it's contained inside teardown{} which already signals it as fairly > magic. Plus why have each architecture duplicate the field? I expect none of the arch_domain_teardown() instances will remain without an actual use of the new field, mid to long term. I don't want to override Roger's concern, but from my pov Acked-by: Jan Beulich <jbeulich@suse.com> Jan
On Wed, Jun 07, 2023 at 10:43:24AM +0200, Jan Beulich wrote: > On 05.06.2023 17:23, Andrew Cooper wrote: > > On 05/06/2023 4:19 pm, Roger Pau Monné wrote: > >> On Mon, Jun 05, 2023 at 03:43:30PM +0100, Andrew Cooper wrote: > >>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > >>> index 85242a73d374..854f3e32c00e 100644 > >>> --- a/xen/include/xen/sched.h > >>> +++ b/xen/include/xen/sched.h > >>> @@ -589,6 +589,7 @@ struct domain > >>> */ > >>> struct { > >>> unsigned int val; > >>> + unsigned int arch_val; > >> While I haven't looked at patch 2, wouldn't such continuation > >> information better be encoded in arch_domain in whatever way is more > >> suitable for each architecture? > > > > Well, it's filling a hole here on 64bit builds, which it couldn't do in > > arch_domain. > > > > And it's contained inside teardown{} which already signals it as fairly > > magic. > > Plus why have each architecture duplicate the field? I expect none of > the arch_domain_teardown() instances will remain without an actual > use of the new field, mid to long term. > > I don't want to override Roger's concern, but from my pov > Acked-by: Jan Beulich <jbeulich@suse.com> Oh, no worries, I was meaning to reply I was fine with Andrews justification, but forgot. Thanks, Roger
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index d8ef6501ff8e..b3981d70a442 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -750,6 +750,11 @@ int arch_domain_create(struct domain *d, return rc; } +int arch_domain_teardown(struct domain *d) +{ + return 0; +} + void arch_domain_destroy(struct domain *d) { /* IOMMU page table is shared with P2M, always call diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 39c215316546..5f66c2ae33d7 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -888,6 +888,11 @@ int arch_domain_create(struct domain *d, return rc; } +int arch_domain_teardown(struct domain *d) +{ + return 0; +} + void arch_domain_destroy(struct domain *d) { if ( is_hvm_domain(d) ) diff --git a/xen/common/domain.c b/xen/common/domain.c index 6a440590fe2a..b0d850e3595b 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -416,6 +416,7 @@ static int domain_teardown(struct domain *d) PROG_none, PROG_gnttab_mappings, PROG_vcpu_teardown, + PROG_arch_teardown, PROG_done, }; @@ -436,6 +437,11 @@ static int domain_teardown(struct domain *d) return rc; } + PROGRESS(arch_teardown): + rc = arch_domain_teardown(d); + if ( rc ) + return rc; + PROGRESS(done): break; diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index 26f9c4f6dd5b..c3348c90748f 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -80,6 +80,7 @@ int arch_domain_create(struct domain *d, struct xen_domctl_createdomain *config, unsigned int flags); +int arch_domain_teardown(struct domain *d); void arch_domain_destroy(struct domain *d); void arch_domain_shutdown(struct domain *d); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 85242a73d374..854f3e32c00e 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -589,6 +589,7 @@ struct domain */ struct { unsigned int val; + unsigned int arch_val; struct vcpu *vcpu; } teardown;
Plumb it into domain_teardown(). Provide arch_val in the teardown continuation information for use by arch_domain_teardown(). No practical change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> CC: Bertrand Marquis <bertrand.marquis@arm.com> CC: Jens Wiklander <jens.wiklander@linaro.org> --- xen/arch/arm/domain.c | 5 +++++ xen/arch/x86/domain.c | 5 +++++ xen/common/domain.c | 6 ++++++ xen/include/xen/domain.h | 1 + xen/include/xen/sched.h | 1 + 5 files changed, 18 insertions(+)