| Message ID | 20240718215744.3892072-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/domain: Idle domain creation improvements | expand |
On Thu, 18 Jul 2024, Andrew Cooper wrote: > The idle domain needs d->arch.ctxt_switch initialised on x86. Implement the > new arch_init_idle_domain() in order to do this. > > Right now this double-initalises the ctxt_switch pointer, but it's safe and > will stop happening when domain_create() is cleaned up as a consequence. > > 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: 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: Michal Orzel <michal.orzel@amd.com> > CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > xen/arch/x86/domain.c | 19 ++++++++++++------- > xen/arch/x86/include/asm/domain.h | 3 +++ > 2 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index ccadfe0c9e70..eff905c6c6e5 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -768,6 +768,17 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) > return true; > } > > +void __init arch_init_idle_domain(struct domain *d) > +{ > + static const struct arch_csw idle_csw = { > + .from = paravirt_ctxt_switch_from, > + .to = paravirt_ctxt_switch_to, > + .tail = idle_loop, > + }; > + > + d->arch.ctxt_switch = &idle_csw; > +} > + > int arch_domain_create(struct domain *d, > struct xen_domctl_createdomain *config, > unsigned int flags) > @@ -783,13 +794,7 @@ int arch_domain_create(struct domain *d, > /* Minimal initialisation for the idle domain. */ > if ( unlikely(is_idle_domain(d)) ) > { > - static const struct arch_csw idle_csw = { > - .from = paravirt_ctxt_switch_from, > - .to = paravirt_ctxt_switch_to, > - .tail = idle_loop, > - }; > - > - d->arch.ctxt_switch = &idle_csw; > + arch_init_idle_domain(d); I don't understand why you need this call to arch_init_idle_domain(d) here given the other call to arch_init_idle_domain added by patch #1. Also I am not sure why you didn't move the line below (cpu_policy = ZERO_BLOCK_PTR) to arch_init_idle_domain as well but I admit I don't know this code I realize you are removing all this code in patch #4... but still why the line below is not needed anymore? And why do we need to add an extra call to arch_init_idle_domain temporarily in this patch? > d->arch.cpu_policy = ZERO_BLOCK_PTR; /* Catch stray misuses. */ > > diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h > index f5daeb182baa..bca3258d69ac 100644 > --- a/xen/arch/x86/include/asm/domain.h > +++ b/xen/arch/x86/include/asm/domain.h > @@ -779,6 +779,9 @@ struct arch_vcpu_io { > /* Maxphysaddr supportable by the paging infrastructure. */ > unsigned int domain_max_paddr_bits(const struct domain *d); > > +#define arch_init_idle_domain arch_init_idle_domain > +void arch_init_idle_domain(struct domain *d); > + > #endif /* __ASM_DOMAIN_H__ */ > > /* > -- > 2.39.2 >
On 31/07/2024 12:44 am, Stefano Stabellini wrote: > On Thu, 18 Jul 2024, Andrew Cooper wrote: >> The idle domain needs d->arch.ctxt_switch initialised on x86. Implement the >> new arch_init_idle_domain() in order to do this. >> >> Right now this double-initalises the ctxt_switch pointer, but it's safe and >> will stop happening when domain_create() is cleaned up as a consequence. >> >> 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: 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: Michal Orzel <michal.orzel@amd.com> >> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> >> --- >> xen/arch/x86/domain.c | 19 ++++++++++++------- >> xen/arch/x86/include/asm/domain.h | 3 +++ >> 2 files changed, 15 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c >> index ccadfe0c9e70..eff905c6c6e5 100644 >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -768,6 +768,17 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) >> return true; >> } >> >> +void __init arch_init_idle_domain(struct domain *d) >> +{ >> + static const struct arch_csw idle_csw = { >> + .from = paravirt_ctxt_switch_from, >> + .to = paravirt_ctxt_switch_to, >> + .tail = idle_loop, >> + }; >> + >> + d->arch.ctxt_switch = &idle_csw; >> +} >> + >> int arch_domain_create(struct domain *d, >> struct xen_domctl_createdomain *config, >> unsigned int flags) >> @@ -783,13 +794,7 @@ int arch_domain_create(struct domain *d, >> /* Minimal initialisation for the idle domain. */ >> if ( unlikely(is_idle_domain(d)) ) >> { >> - static const struct arch_csw idle_csw = { >> - .from = paravirt_ctxt_switch_from, >> - .to = paravirt_ctxt_switch_to, >> - .tail = idle_loop, >> - }; >> - >> - d->arch.ctxt_switch = &idle_csw; >> + arch_init_idle_domain(d); > I don't understand why you need this call to arch_init_idle_domain(d) > here given the other call to arch_init_idle_domain added by patch #1. It's stale from an older version of the series. I'll drop it, which will reduce the churn in this as well as patch 4. > Also I am not sure why you didn't move the line below (cpu_policy = > ZERO_BLOCK_PTR) to arch_init_idle_domain as well but I admit I don't > know this code. See the thread on patch 4 with Jan. It's intentional. This was a safety check which is dubious (it's not special enough to warrant it on its own), and has never tripped. I'll note it's intentional omission in the commit message. ~Andrew
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index ccadfe0c9e70..eff905c6c6e5 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -768,6 +768,17 @@ static bool emulation_flags_ok(const struct domain *d, uint32_t emflags) return true; } +void __init arch_init_idle_domain(struct domain *d) +{ + static const struct arch_csw idle_csw = { + .from = paravirt_ctxt_switch_from, + .to = paravirt_ctxt_switch_to, + .tail = idle_loop, + }; + + d->arch.ctxt_switch = &idle_csw; +} + int arch_domain_create(struct domain *d, struct xen_domctl_createdomain *config, unsigned int flags) @@ -783,13 +794,7 @@ int arch_domain_create(struct domain *d, /* Minimal initialisation for the idle domain. */ if ( unlikely(is_idle_domain(d)) ) { - static const struct arch_csw idle_csw = { - .from = paravirt_ctxt_switch_from, - .to = paravirt_ctxt_switch_to, - .tail = idle_loop, - }; - - d->arch.ctxt_switch = &idle_csw; + arch_init_idle_domain(d); d->arch.cpu_policy = ZERO_BLOCK_PTR; /* Catch stray misuses. */ diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h index f5daeb182baa..bca3258d69ac 100644 --- a/xen/arch/x86/include/asm/domain.h +++ b/xen/arch/x86/include/asm/domain.h @@ -779,6 +779,9 @@ struct arch_vcpu_io { /* Maxphysaddr supportable by the paging infrastructure. */ unsigned int domain_max_paddr_bits(const struct domain *d); +#define arch_init_idle_domain arch_init_idle_domain +void arch_init_idle_domain(struct domain *d); + #endif /* __ASM_DOMAIN_H__ */ /*
The idle domain needs d->arch.ctxt_switch initialised on x86. Implement the new arch_init_idle_domain() in order to do this. Right now this double-initalises the ctxt_switch pointer, but it's safe and will stop happening when domain_create() is cleaned up as a consequence. 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: 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: Michal Orzel <michal.orzel@amd.com> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/x86/domain.c | 19 ++++++++++++------- xen/arch/x86/include/asm/domain.h | 3 +++ 2 files changed, 15 insertions(+), 7 deletions(-)