| Message ID | 20160801171028.11615-8-proskurin@sec.in.tum.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Sergej, On 01/08/16 18:10, Sergej Proskurin wrote: > The p2m initialization now invokes initialization routines responsible > for the allocation and initialization of altp2m structures. The same > applies to teardown routines. The functionality has been adopted from > the x86 altp2m implementation. > > Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > --- > v2: Shared code between host/altp2m init/teardown functions. > Added conditional init/teardown of altp2m. > Altp2m related functions are moved to altp2m.c > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/altp2m.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/p2m.c | 28 +++++++++++++---- > xen/include/asm-arm/altp2m.h | 6 ++++ > xen/include/asm-arm/domain.h | 4 +++ > xen/include/asm-arm/p2m.h | 5 ++++ > 6 files changed, 110 insertions(+), 5 deletions(-) > create mode 100644 xen/arch/arm/altp2m.c > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 23aaf52..4a7f660 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -5,6 +5,7 @@ subdir-$(CONFIG_ARM_64) += efi > subdir-$(CONFIG_ACPI) += acpi > > obj-$(CONFIG_ALTERNATIVE) += alternative.o > +obj-y += altp2m.o > obj-y += bootfdt.o > obj-y += cpu.o > obj-y += cpuerrata.o > diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c > new file mode 100644 > index 0000000..abbd39a > --- /dev/null > +++ b/xen/arch/arm/altp2m.c > @@ -0,0 +1,71 @@ > +/* > + * arch/arm/altp2m.c > + * > + * Alternate p2m > + * Copyright (c) 2016 Sergej Proskurin <proskurin@sec.in.tum.de> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, version 2, > + * as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT ANY > + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS > + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more > + * details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program; If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <asm/p2m.h> > +#include <asm/altp2m.h> > + > +int altp2m_init(struct domain *d) > +{ > + unsigned int i; > + > + spin_lock_init(&d->arch.altp2m_lock); > + > + for ( i = 0; i < MAX_ALTP2M; i++ ) > + { > + d->arch.altp2m_p2m[i] = NULL; > + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; I don't think altp2m_vttbr is useful. There is no real performance impact to free the whole altp2m if the altp2m is destroyed (see altp2m_destroy_by_id) and re-allocated afterwards. The code will actually much simpler. With this solution you will be able to detect if an altp2m is available by testin altp2m_p2m[i] is NULL. > + } > + > + return 0; > +} > + > +void altp2m_teardown(struct domain *d) > +{ > + unsigned int i; > + struct p2m_domain *p2m; > + > + altp2m_lock(d); The lock is not necessary here. > + > + for ( i = 0; i < MAX_ALTP2M; i++ ) > + { > + if ( !d->arch.altp2m_p2m[i] ) > + continue; > + > + p2m = d->arch.altp2m_p2m[i]; > + p2m_free_one(p2m); > + xfree(p2m); > + > + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; > + d->arch.altp2m_p2m[i] = NULL; The domain will never be used afterward, so there is no point to set altp2m_vttbr and altp2m_p2m. > + } > + > + d->arch.altp2m_active = false; Ditto. > + > + altp2m_unlock(d); > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index ff9c0d1..29ec5e5 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -14,6 +14,8 @@ > #include <asm/hardirq.h> > #include <asm/page.h> > > +#include <asm/altp2m.h> > + > #ifdef CONFIG_ARM_64 > static unsigned int __read_mostly p2m_root_order; > static unsigned int __read_mostly p2m_root_level; > @@ -1392,7 +1394,7 @@ void p2m_flush_table(struct p2m_domain *p2m) > free_domheap_page(pg); > } > > -static inline void p2m_free_one(struct p2m_domain *p2m) > +void p2m_free_one(struct p2m_domain *p2m) Please expose p2m_free_one in patch #4. > { > p2m_flush_table(p2m); > > @@ -1415,9 +1417,13 @@ int p2m_init_one(struct domain *d, struct p2m_domain *p2m) > rwlock_init(&p2m->lock); > INIT_PAGE_LIST_HEAD(&p2m->pages); > > - rc = p2m_alloc_vmid(p2m); > - if ( rc != 0 ) > - return rc; > + /* Reused altp2m views keep their VMID. */ > + if ( p2m->vmid == INVALID_VMID ) > + { > + rc = p2m_alloc_vmid(p2m); > + if ( rc != 0 ) > + return rc; > + } My suggestion above will avoid this kind of hack. > > p2m->domain = d; > p2m->access_required = false; > @@ -1441,6 +1447,9 @@ static void p2m_teardown_hostp2m(struct domain *d) > > void p2m_teardown(struct domain *d) > { > + if ( altp2m_enabled(d) ) > + altp2m_teardown(d); > + > p2m_teardown_hostp2m(d); > } > > @@ -1460,7 +1469,16 @@ static int p2m_init_hostp2m(struct domain *d) > > int p2m_init(struct domain *d) > { > - return p2m_init_hostp2m(d); > + int rc; > + > + rc = p2m_init_hostp2m(d); > + if ( rc ) > + return rc; > + > + if ( altp2m_enabled(d) ) I am a bit skeptical that you can fully use altp2m with this check. p2m_init is created at the very beginning when the domain is allocated. So the HVM_PARAM_ALTP2M will not be set. > + rc = altp2m_init(d); > + > + return rc; > } > > int relinquish_p2m_mapping(struct domain *d) > diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h > index d47b249..79ea66b 100644 > --- a/xen/include/asm-arm/altp2m.h > +++ b/xen/include/asm-arm/altp2m.h > @@ -22,6 +22,9 @@ > > #include <xen/sched.h> > > +#define altp2m_lock(d) spin_lock(&(d)->arch.altp2m_lock) > +#define altp2m_unlock(d) spin_unlock(&(d)->arch.altp2m_lock) > + > #define altp2m_enabled(d) ((d)->arch.hvm_domain.params[HVM_PARAM_ALTP2M]) > > /* Alternate p2m on/off per domain */ > @@ -38,4 +41,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v) > return 0; > } > > +int altp2m_init(struct domain *d); > +void altp2m_teardown(struct domain *d); > + > #endif /* __ASM_ARM_ALTP2M_H */ > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index cc4bda0..3c25ea5 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -129,6 +129,10 @@ struct arch_domain > > /* altp2m: allow multiple copies of host p2m */ > bool_t altp2m_active; > + struct p2m_domain *altp2m_p2m[MAX_ALTP2M]; > + uint64_t altp2m_vttbr[MAX_ALTP2M]; > + /* Covers access to members of the struct altp2m. */ I cannot find any "struct altp2m" in the code. > + spinlock_t altp2m_lock; > } __cacheline_aligned; > > struct arch_vcpu > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index 1f9c370..24a1f61 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -9,6 +9,8 @@ > #include <xen/p2m-common.h> > #include <public/memory.h> > > +#define MAX_ALTP2M 10 /* ARM might contain an arbitrary number of > + altp2m views. */ > #define paddr_bits PADDR_BITS > > /* Holds the bit size of IPAs in p2m tables. */ > @@ -212,6 +214,9 @@ void guest_physmap_remove_page(struct domain *d, > > mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn); > > +/* Release resources held by the p2m structure. */ > +void p2m_free_one(struct p2m_domain *p2m); > + > /* > * Populate-on-demand > */ > Regards,
Hi Julien, On 08/03/2016 08:12 PM, Julien Grall wrote: > Hello Sergej, > > On 01/08/16 18:10, Sergej Proskurin wrote: >> The p2m initialization now invokes initialization routines responsible >> for the allocation and initialization of altp2m structures. The same >> applies to teardown routines. The functionality has been adopted from >> the x86 altp2m implementation. >> >> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> >> --- >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: Julien Grall <julien.grall@arm.com> >> --- >> v2: Shared code between host/altp2m init/teardown functions. >> Added conditional init/teardown of altp2m. >> Altp2m related functions are moved to altp2m.c >> --- >> xen/arch/arm/Makefile | 1 + >> xen/arch/arm/altp2m.c | 71 >> ++++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/p2m.c | 28 +++++++++++++---- >> xen/include/asm-arm/altp2m.h | 6 ++++ >> xen/include/asm-arm/domain.h | 4 +++ >> xen/include/asm-arm/p2m.h | 5 ++++ >> 6 files changed, 110 insertions(+), 5 deletions(-) >> create mode 100644 xen/arch/arm/altp2m.c >> >> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile >> index 23aaf52..4a7f660 100644 >> --- a/xen/arch/arm/Makefile >> +++ b/xen/arch/arm/Makefile >> @@ -5,6 +5,7 @@ subdir-$(CONFIG_ARM_64) += efi >> subdir-$(CONFIG_ACPI) += acpi >> >> obj-$(CONFIG_ALTERNATIVE) += alternative.o >> +obj-y += altp2m.o >> obj-y += bootfdt.o >> obj-y += cpu.o >> obj-y += cpuerrata.o >> diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c >> new file mode 100644 >> index 0000000..abbd39a >> --- /dev/null >> +++ b/xen/arch/arm/altp2m.c >> @@ -0,0 +1,71 @@ >> +/* >> + * arch/arm/altp2m.c >> + * >> + * Alternate p2m >> + * Copyright (c) 2016 Sergej Proskurin <proskurin@sec.in.tum.de> >> + * >> + * This program is free software; you can redistribute it and/or >> modify it >> + * under the terms and conditions of the GNU General Public License, >> version 2, >> + * as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope it will be useful, but >> WITHOUT ANY >> + * WARRANTY; without even the implied warranty of MERCHANTABILITY or >> FITNESS >> + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> more >> + * details. >> + * >> + * You should have received a copy of the GNU General Public License >> along with >> + * this program; If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <asm/p2m.h> >> +#include <asm/altp2m.h> >> + >> +int altp2m_init(struct domain *d) >> +{ >> + unsigned int i; >> + >> + spin_lock_init(&d->arch.altp2m_lock); >> + >> + for ( i = 0; i < MAX_ALTP2M; i++ ) >> + { >> + d->arch.altp2m_p2m[i] = NULL; >> + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; > > I don't think altp2m_vttbr is useful. There is no real performance > impact to free the whole altp2m if the altp2m is destroyed (see > altp2m_destroy_by_id) and re-allocated afterwards. > > The code will actually much simpler. With this solution you will be > able to detect if an altp2m is available by testin altp2m_p2m[i] is NULL. > This is true. I did not want to free the entire domain got every time the hostp2m got changed, while altp2m was active (see altp2m_propagate_change). But it would not introduce much more overhead when it does. Thank you. >> + } >> + >> + return 0; >> +} >> + >> +void altp2m_teardown(struct domain *d) >> +{ >> + unsigned int i; >> + struct p2m_domain *p2m; >> + >> + altp2m_lock(d); > > The lock is not necessary here. > Fair, the domain gets destroyed anyway. Thank you. >> + >> + for ( i = 0; i < MAX_ALTP2M; i++ ) >> + { >> + if ( !d->arch.altp2m_p2m[i] ) >> + continue; >> + >> + p2m = d->arch.altp2m_p2m[i]; >> + p2m_free_one(p2m); >> + xfree(p2m); >> + >> + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; >> + d->arch.altp2m_p2m[i] = NULL; > > The domain will never be used afterward, so there is no point to set > altp2m_vttbr and altp2m_p2m. > Makes sense. >> + } >> + >> + d->arch.altp2m_active = false; > > Ditto. > Thanks. >> + >> + altp2m_unlock(d); >> +} >> + >> +/* >> + * Local variables: >> + * mode: C >> + * c-file-style: "BSD" >> + * c-basic-offset: 4 >> + * tab-width: 4 >> + * indent-tabs-mode: nil >> + * End: >> + */ >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index ff9c0d1..29ec5e5 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -14,6 +14,8 @@ >> #include <asm/hardirq.h> >> #include <asm/page.h> >> >> +#include <asm/altp2m.h> >> + >> #ifdef CONFIG_ARM_64 >> static unsigned int __read_mostly p2m_root_order; >> static unsigned int __read_mostly p2m_root_level; >> @@ -1392,7 +1394,7 @@ void p2m_flush_table(struct p2m_domain *p2m) >> free_domheap_page(pg); >> } >> >> -static inline void p2m_free_one(struct p2m_domain *p2m) >> +void p2m_free_one(struct p2m_domain *p2m) > > Please expose p2m_free_one in patch #4. > Ok. >> { >> p2m_flush_table(p2m); >> >> @@ -1415,9 +1417,13 @@ int p2m_init_one(struct domain *d, struct >> p2m_domain *p2m) >> rwlock_init(&p2m->lock); >> INIT_PAGE_LIST_HEAD(&p2m->pages); >> >> - rc = p2m_alloc_vmid(p2m); >> - if ( rc != 0 ) >> - return rc; >> + /* Reused altp2m views keep their VMID. */ >> + if ( p2m->vmid == INVALID_VMID ) >> + { >> + rc = p2m_alloc_vmid(p2m); >> + if ( rc != 0 ) >> + return rc; >> + } > > My suggestion above will avoid this kind of hack. > >> >> p2m->domain = d; >> p2m->access_required = false; >> @@ -1441,6 +1447,9 @@ static void p2m_teardown_hostp2m(struct domain *d) >> >> void p2m_teardown(struct domain *d) >> { >> + if ( altp2m_enabled(d) ) >> + altp2m_teardown(d); >> + >> p2m_teardown_hostp2m(d); >> } >> >> @@ -1460,7 +1469,16 @@ static int p2m_init_hostp2m(struct domain *d) >> >> int p2m_init(struct domain *d) >> { >> - return p2m_init_hostp2m(d); >> + int rc; >> + >> + rc = p2m_init_hostp2m(d); >> + if ( rc ) >> + return rc; >> + >> + if ( altp2m_enabled(d) ) > > I am a bit skeptical that you can fully use altp2m with this check. > p2m_init is created at the very beginning when the domain is > allocated. So the HVM_PARAM_ALTP2M will not be set. > I will respond to this answer, after I have made sure this is the case. You are right, the need for this call depends on which point in the domain initialization process libxl sets the parameter HVM_PARAM_ALTP2M. Thank you. >> + rc = altp2m_init(d); >> + >> + return rc; >> } >> >> int relinquish_p2m_mapping(struct domain *d) >> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h >> index d47b249..79ea66b 100644 >> --- a/xen/include/asm-arm/altp2m.h >> +++ b/xen/include/asm-arm/altp2m.h >> @@ -22,6 +22,9 @@ >> >> #include <xen/sched.h> >> >> +#define altp2m_lock(d) spin_lock(&(d)->arch.altp2m_lock) >> +#define altp2m_unlock(d) spin_unlock(&(d)->arch.altp2m_lock) >> + >> #define altp2m_enabled(d) >> ((d)->arch.hvm_domain.params[HVM_PARAM_ALTP2M]) >> >> /* Alternate p2m on/off per domain */ >> @@ -38,4 +41,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct >> vcpu *v) >> return 0; >> } >> >> +int altp2m_init(struct domain *d); >> +void altp2m_teardown(struct domain *d); >> + >> #endif /* __ASM_ARM_ALTP2M_H */ >> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h >> index cc4bda0..3c25ea5 100644 >> --- a/xen/include/asm-arm/domain.h >> +++ b/xen/include/asm-arm/domain.h >> @@ -129,6 +129,10 @@ struct arch_domain >> >> /* altp2m: allow multiple copies of host p2m */ >> bool_t altp2m_active; >> + struct p2m_domain *altp2m_p2m[MAX_ALTP2M]; >> + uint64_t altp2m_vttbr[MAX_ALTP2M]; >> + /* Covers access to members of the struct altp2m. */ > > I cannot find any "struct altp2m" in the code. > Stalled comment, thank you. >> + spinlock_t altp2m_lock; >> } __cacheline_aligned; >> >> struct arch_vcpu >> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h >> index 1f9c370..24a1f61 100644 >> --- a/xen/include/asm-arm/p2m.h >> +++ b/xen/include/asm-arm/p2m.h >> @@ -9,6 +9,8 @@ >> #include <xen/p2m-common.h> >> #include <public/memory.h> >> >> +#define MAX_ALTP2M 10 /* ARM might contain an arbitrary >> number of >> + altp2m views. */ >> #define paddr_bits PADDR_BITS >> >> /* Holds the bit size of IPAs in p2m tables. */ >> @@ -212,6 +214,9 @@ void guest_physmap_remove_page(struct domain *d, >> >> mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn); >> >> +/* Release resources held by the p2m structure. */ >> +void p2m_free_one(struct p2m_domain *p2m); >> + >> /* >> * Populate-on-demand >> */ >> > Best regards, ~Sergej
On 05/08/16 07:53, Sergej Proskurin wrote: > Hi Julien, Hello Sergej, > On 08/03/2016 08:12 PM, Julien Grall wrote: >> On 01/08/16 18:10, Sergej Proskurin wrote: >>> +int altp2m_init(struct domain *d) >>> +{ >>> + unsigned int i; >>> + >>> + spin_lock_init(&d->arch.altp2m_lock); >>> + >>> + for ( i = 0; i < MAX_ALTP2M; i++ ) >>> + { >>> + d->arch.altp2m_p2m[i] = NULL; >>> + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; >> >> I don't think altp2m_vttbr is useful. There is no real performance >> impact to free the whole altp2m if the altp2m is destroyed (see >> altp2m_destroy_by_id) and re-allocated afterwards. >> >> The code will actually much simpler. With this solution you will be >> able to detect if an altp2m is available by testin altp2m_p2m[i] is NULL. >> > > This is true. I did not want to free the entire domain got every time > the hostp2m got changed, while altp2m was active (see > altp2m_propagate_change). But it would not introduce much more overhead > when it does. Thank you. I think you misunderstood my point. When you flush the altp2m you only need to reset lowest_mapped_gfn and max_mapped_gfn aside freeing intermediate page table. The altp2m should be fully freed when it get destroyed, nobody will use it anyway. This will also simplify a lot the logic. Regards,
Hi Julien, On 08/05/2016 11:20 AM, Julien Grall wrote: > On 05/08/16 07:53, Sergej Proskurin wrote: >> Hi Julien, > > Hello Sergej, > >> On 08/03/2016 08:12 PM, Julien Grall wrote: >>> On 01/08/16 18:10, Sergej Proskurin wrote: >>>> +int altp2m_init(struct domain *d) >>>> +{ >>>> + unsigned int i; >>>> + >>>> + spin_lock_init(&d->arch.altp2m_lock); >>>> + >>>> + for ( i = 0; i < MAX_ALTP2M; i++ ) >>>> + { >>>> + d->arch.altp2m_p2m[i] = NULL; >>>> + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; >>> >>> I don't think altp2m_vttbr is useful. There is no real performance >>> impact to free the whole altp2m if the altp2m is destroyed (see >>> altp2m_destroy_by_id) and re-allocated afterwards. >>> >>> The code will actually much simpler. With this solution you will be >>> able to detect if an altp2m is available by testin altp2m_p2m[i] is >>> NULL. >>> >> >> This is true. I did not want to free the entire domain got every time >> the hostp2m got changed, while altp2m was active (see >> altp2m_propagate_change). But it would not introduce much more overhead >> when it does. Thank you. > > I think you misunderstood my point. When you flush the altp2m you only > need to reset lowest_mapped_gfn and max_mapped_gfn aside freeing > intermediate page table. > I see your point. I will consider your suggestion in the next patch. Thank you. > The altp2m should be fully freed when it get destroyed, nobody will > use it anyway. This will also simplify a lot the logic. Best regards, ~Sergej
Hi Julien, >>> @@ -1460,7 +1469,16 @@ static int p2m_init_hostp2m(struct domain *d) >>> >>> int p2m_init(struct domain *d) >>> { >>> - return p2m_init_hostp2m(d); >>> + int rc; >>> + >>> + rc = p2m_init_hostp2m(d); >>> + if ( rc ) >>> + return rc; >>> + >>> + if ( altp2m_enabled(d) ) >> I am a bit skeptical that you can fully use altp2m with this check. >> p2m_init is created at the very beginning when the domain is >> allocated. So the HVM_PARAM_ALTP2M will not be set. >> > I will respond to this answer, after I have made sure this is the case. > You are right, the need for this call depends on which point in the > domain initialization process libxl sets the parameter HVM_PARAM_ALTP2M. > Thank you. > You were right. I have removed this check from the next patch series. Thank you. Best regards, ~Sergej
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 23aaf52..4a7f660 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -5,6 +5,7 @@ subdir-$(CONFIG_ARM_64) += efi subdir-$(CONFIG_ACPI) += acpi obj-$(CONFIG_ALTERNATIVE) += alternative.o +obj-y += altp2m.o obj-y += bootfdt.o obj-y += cpu.o obj-y += cpuerrata.o diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c new file mode 100644 index 0000000..abbd39a --- /dev/null +++ b/xen/arch/arm/altp2m.c @@ -0,0 +1,71 @@ +/* + * arch/arm/altp2m.c + * + * Alternate p2m + * Copyright (c) 2016 Sergej Proskurin <proskurin@sec.in.tum.de> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program; If not, see <http://www.gnu.org/licenses/>. + */ + +#include <asm/p2m.h> +#include <asm/altp2m.h> + +int altp2m_init(struct domain *d) +{ + unsigned int i; + + spin_lock_init(&d->arch.altp2m_lock); + + for ( i = 0; i < MAX_ALTP2M; i++ ) + { + d->arch.altp2m_p2m[i] = NULL; + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; + } + + return 0; +} + +void altp2m_teardown(struct domain *d) +{ + unsigned int i; + struct p2m_domain *p2m; + + altp2m_lock(d); + + for ( i = 0; i < MAX_ALTP2M; i++ ) + { + if ( !d->arch.altp2m_p2m[i] ) + continue; + + p2m = d->arch.altp2m_p2m[i]; + p2m_free_one(p2m); + xfree(p2m); + + d->arch.altp2m_vttbr[i] = INVALID_VTTBR; + d->arch.altp2m_p2m[i] = NULL; + } + + d->arch.altp2m_active = false; + + altp2m_unlock(d); +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index ff9c0d1..29ec5e5 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -14,6 +14,8 @@ #include <asm/hardirq.h> #include <asm/page.h> +#include <asm/altp2m.h> + #ifdef CONFIG_ARM_64 static unsigned int __read_mostly p2m_root_order; static unsigned int __read_mostly p2m_root_level; @@ -1392,7 +1394,7 @@ void p2m_flush_table(struct p2m_domain *p2m) free_domheap_page(pg); } -static inline void p2m_free_one(struct p2m_domain *p2m) +void p2m_free_one(struct p2m_domain *p2m) { p2m_flush_table(p2m); @@ -1415,9 +1417,13 @@ int p2m_init_one(struct domain *d, struct p2m_domain *p2m) rwlock_init(&p2m->lock); INIT_PAGE_LIST_HEAD(&p2m->pages); - rc = p2m_alloc_vmid(p2m); - if ( rc != 0 ) - return rc; + /* Reused altp2m views keep their VMID. */ + if ( p2m->vmid == INVALID_VMID ) + { + rc = p2m_alloc_vmid(p2m); + if ( rc != 0 ) + return rc; + } p2m->domain = d; p2m->access_required = false; @@ -1441,6 +1447,9 @@ static void p2m_teardown_hostp2m(struct domain *d) void p2m_teardown(struct domain *d) { + if ( altp2m_enabled(d) ) + altp2m_teardown(d); + p2m_teardown_hostp2m(d); } @@ -1460,7 +1469,16 @@ static int p2m_init_hostp2m(struct domain *d) int p2m_init(struct domain *d) { - return p2m_init_hostp2m(d); + int rc; + + rc = p2m_init_hostp2m(d); + if ( rc ) + return rc; + + if ( altp2m_enabled(d) ) + rc = altp2m_init(d); + + return rc; } int relinquish_p2m_mapping(struct domain *d) diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h index d47b249..79ea66b 100644 --- a/xen/include/asm-arm/altp2m.h +++ b/xen/include/asm-arm/altp2m.h @@ -22,6 +22,9 @@ #include <xen/sched.h> +#define altp2m_lock(d) spin_lock(&(d)->arch.altp2m_lock) +#define altp2m_unlock(d) spin_unlock(&(d)->arch.altp2m_lock) + #define altp2m_enabled(d) ((d)->arch.hvm_domain.params[HVM_PARAM_ALTP2M]) /* Alternate p2m on/off per domain */ @@ -38,4 +41,7 @@ static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v) return 0; } +int altp2m_init(struct domain *d); +void altp2m_teardown(struct domain *d); + #endif /* __ASM_ARM_ALTP2M_H */ diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index cc4bda0..3c25ea5 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -129,6 +129,10 @@ struct arch_domain /* altp2m: allow multiple copies of host p2m */ bool_t altp2m_active; + struct p2m_domain *altp2m_p2m[MAX_ALTP2M]; + uint64_t altp2m_vttbr[MAX_ALTP2M]; + /* Covers access to members of the struct altp2m. */ + spinlock_t altp2m_lock; } __cacheline_aligned; struct arch_vcpu diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index 1f9c370..24a1f61 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -9,6 +9,8 @@ #include <xen/p2m-common.h> #include <public/memory.h> +#define MAX_ALTP2M 10 /* ARM might contain an arbitrary number of + altp2m views. */ #define paddr_bits PADDR_BITS /* Holds the bit size of IPAs in p2m tables. */ @@ -212,6 +214,9 @@ void guest_physmap_remove_page(struct domain *d, mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn); +/* Release resources held by the p2m structure. */ +void p2m_free_one(struct p2m_domain *p2m); + /* * Populate-on-demand */
The p2m initialization now invokes initialization routines responsible for the allocation and initialization of altp2m structures. The same applies to teardown routines. The functionality has been adopted from the x86 altp2m implementation. Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Julien Grall <julien.grall@arm.com> --- v2: Shared code between host/altp2m init/teardown functions. Added conditional init/teardown of altp2m. Altp2m related functions are moved to altp2m.c --- xen/arch/arm/Makefile | 1 + xen/arch/arm/altp2m.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ xen/arch/arm/p2m.c | 28 +++++++++++++---- xen/include/asm-arm/altp2m.h | 6 ++++ xen/include/asm-arm/domain.h | 4 +++ xen/include/asm-arm/p2m.h | 5 ++++ 6 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 xen/arch/arm/altp2m.c