Hi Henry, On 16/01/2023 02:58, Henry Wang wrote: > > > With the change in previous patch, the initial 16 pages in the P2M > pool is not necessary anymore. Drop them for code simplification. > > Also the call to p2m_teardown() from arch_domain_destroy() is not > necessary anymore since the movement of the P2M allocation out of > arch_domain_create(). Drop the code and the above in-code comment > mentioning it. > > Signed-off-by: Henry Wang <henry.w...@arm.com> > --- > I am not entirely sure if I should also drop the "TODO" on top of > the p2m_set_entry(). Because although we are sure there is no > p2m pages populated in domain_create() stage now, but we are not > sure if anyone will add more in the future...Any comments? > --- > xen/arch/arm/include/asm/p2m.h | 4 ---- > xen/arch/arm/p2m.c | 20 +------------------- > 2 files changed, 1 insertion(+), 23 deletions(-) > > diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h > index bf5183e53a..cf06d3cc21 100644 > --- a/xen/arch/arm/include/asm/p2m.h > +++ b/xen/arch/arm/include/asm/p2m.h > @@ -200,10 +200,6 @@ int p2m_init(struct domain *d); > * - p2m_final_teardown() will be called when domain struct is been > * freed. This *cannot* be preempted and therefore one small > * resources should be freed here. > - * Note that p2m_final_teardown() will also call p2m_teardown(), to properly > - * free the P2M when failures happen in the domain creation with P2M pages > - * already in use. In this case p2m_teardown() is called non-preemptively > and > - * p2m_teardown() will always return 0. > */ > int p2m_teardown(struct domain *d, bool allow_preemption); > void p2m_final_teardown(struct domain *d); > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 7de7d822e9..d41a316d18 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1744,13 +1744,9 @@ void p2m_final_teardown(struct domain *d) > /* > * No need to call relinquish_p2m_mapping() here because > * p2m_final_teardown() is called either after > domain_relinquish_resources() > - * where relinquish_p2m_mapping() has been called, or from failure path > of > - * domain_create()/arch_domain_create() where mappings that require > - * p2m_put_l3_page() should never be created. For the latter case, also > see > - * comment on top of the p2m_set_entry() for more info. > + * where relinquish_p2m_mapping() has been called. > */ > > - BUG_ON(p2m_teardown(d, false)); Because you remove this, > ASSERT(page_list_empty(&p2m->pages)); you no longer need this assert, right?
Apart from that: Reviewed-by: Michal Orzel <michal.or...@amd.com> ~Michal
