| Message ID | 20160801171028.11615-6-proskurin@sec.in.tum.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Sergej, Title: s/altp2m/p2m/ On 01/08/16 18:10, Sergej Proskurin wrote: > The initially named function "p2m_alloc_table" allocated pages solely > required for the host's p2m. The new implementation leaves p2m > allocation related parts inside of this function to generally initialize > p2m/altp2m tables. This is done generically, as the host's p2m and > altp2m tables are allocated similarly. Since this function will be used > by the altp2m initialization routines, it is not made static. In > addition, this commit provides the overlay function "p2m_table_init" > that is used for the host's p2m allocation/initialization. > > Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > --- > v2: Removed altp2m table initialization from "p2m_table_init". > --- > xen/arch/arm/p2m.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 17f3299..63fe3d9 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1277,11 +1277,11 @@ void guest_physmap_remove_page(struct domain *d, > p2m_remove_mapping(d, gfn, (1 << page_order), mfn); > } > > -static int p2m_alloc_table(struct domain *d) > +int p2m_alloc_table(struct p2m_domain *p2m) The function p2m_alloc_table should not be exposed outside of the p2m. I explained it why in the previous patch. > { > - struct p2m_domain *p2m = &d->arch.p2m; > - struct page_info *page; > unsigned int i; > + struct page_info *page; > + struct vttbr *vttbr = &p2m->vttbr; > > page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0); > if ( page == NULL ) > @@ -1293,11 +1293,28 @@ static int p2m_alloc_table(struct domain *d) > > p2m->root = page; > > - p2m->vttbr.vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; > + /* Initialize the VTTBR associated with the allocated p2m table. */ > + vttbr->vttbr = 0; > + vttbr->vmid = p2m->vmid & 0xff; > + vttbr->baddr = page_to_maddr(p2m->root); This change does not belong to this patch. If we want to use VTTBR, it should be in patch #3. > + > + return 0; > +} > + > +static int p2m_table_init(struct domain *d) > +{ > + int rc; > + struct p2m_domain *p2m = p2m_get_hostp2m(d); > + > + rc = p2m_alloc_table(p2m); > + if ( rc != 0 ) > + return -ENOMEM; > + > + d->arch.altp2m_active = false; Please avoid to spread the change for altp2m everywhere. The addition of altp2m in the p2m code should really be a single patch. > > /* > * Make sure that all TLBs corresponding to the new VMID are flushed > - * before using it > + * before using it. > */ > p2m_flush_tlb(p2m); > > @@ -1440,7 +1457,7 @@ static int p2m_init_hostp2m(struct domain *d) > if ( rc ) > return rc; > > - return p2m_alloc_table(d); > + return p2m_table_init(d); > } > > int p2m_init(struct domain *d) > Regards,
Hi Julien, On 08/03/2016 07:57 PM, Julien Grall wrote: > Hello Sergej, > > Title: s/altp2m/p2m/ > > On 01/08/16 18:10, Sergej Proskurin wrote: >> The initially named function "p2m_alloc_table" allocated pages solely >> required for the host's p2m. The new implementation leaves p2m >> allocation related parts inside of this function to generally initialize >> p2m/altp2m tables. This is done generically, as the host's p2m and >> altp2m tables are allocated similarly. Since this function will be used >> by the altp2m initialization routines, it is not made static. In >> addition, this commit provides the overlay function "p2m_table_init" >> that is used for the host's p2m allocation/initialization. >> >> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> >> --- >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: Julien Grall <julien.grall@arm.com> >> --- >> v2: Removed altp2m table initialization from "p2m_table_init". >> --- >> xen/arch/arm/p2m.c | 29 +++++++++++++++++++++++------ >> 1 file changed, 23 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index 17f3299..63fe3d9 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -1277,11 +1277,11 @@ void guest_physmap_remove_page(struct domain *d, >> p2m_remove_mapping(d, gfn, (1 << page_order), mfn); >> } >> >> -static int p2m_alloc_table(struct domain *d) >> +int p2m_alloc_table(struct p2m_domain *p2m) > > The function p2m_alloc_table should not be exposed outside of the p2m. > I explained it why in the previous patch. > It will be considered in the next patch. >> { >> - struct p2m_domain *p2m = &d->arch.p2m; >> - struct page_info *page; >> unsigned int i; >> + struct page_info *page; >> + struct vttbr *vttbr = &p2m->vttbr; >> >> page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0); >> if ( page == NULL ) >> @@ -1293,11 +1293,28 @@ static int p2m_alloc_table(struct domain *d) >> >> p2m->root = page; >> >> - p2m->vttbr.vttbr = page_to_maddr(p2m->root) | >> ((uint64_t)p2m->vmid & 0xff) << 48; >> + /* Initialize the VTTBR associated with the allocated p2m table. */ >> + vttbr->vttbr = 0; >> + vttbr->vmid = p2m->vmid & 0xff; >> + vttbr->baddr = page_to_maddr(p2m->root); > > This change does not belong to this patch. If we want to use VTTBR, it > should be in patch #3. > Will not be considered if we decide to drop the patch #03. >> + >> + return 0; >> +} >> + >> +static int p2m_table_init(struct domain *d) >> +{ >> + int rc; >> + struct p2m_domain *p2m = p2m_get_hostp2m(d); >> + >> + rc = p2m_alloc_table(p2m); >> + if ( rc != 0 ) >> + return -ENOMEM; >> + >> + d->arch.altp2m_active = false; > > Please avoid to spread the change for altp2m everywhere. The addition > of altp2m in the p2m code should really be a single patch. > Ok, thank you. I will move this part to altp2m_init. >> >> /* >> * Make sure that all TLBs corresponding to the new VMID are >> flushed >> - * before using it >> + * before using it. >> */ >> p2m_flush_tlb(p2m); >> >> @@ -1440,7 +1457,7 @@ static int p2m_init_hostp2m(struct domain *d) >> if ( rc ) >> return rc; >> >> - return p2m_alloc_table(d); >> + return p2m_table_init(d); >> } >> >> int p2m_init(struct domain *d) >> > > Best regards, ~Sergej
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 17f3299..63fe3d9 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1277,11 +1277,11 @@ void guest_physmap_remove_page(struct domain *d, p2m_remove_mapping(d, gfn, (1 << page_order), mfn); } -static int p2m_alloc_table(struct domain *d) +int p2m_alloc_table(struct p2m_domain *p2m) { - struct p2m_domain *p2m = &d->arch.p2m; - struct page_info *page; unsigned int i; + struct page_info *page; + struct vttbr *vttbr = &p2m->vttbr; page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0); if ( page == NULL ) @@ -1293,11 +1293,28 @@ static int p2m_alloc_table(struct domain *d) p2m->root = page; - p2m->vttbr.vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; + /* Initialize the VTTBR associated with the allocated p2m table. */ + vttbr->vttbr = 0; + vttbr->vmid = p2m->vmid & 0xff; + vttbr->baddr = page_to_maddr(p2m->root); + + return 0; +} + +static int p2m_table_init(struct domain *d) +{ + int rc; + struct p2m_domain *p2m = p2m_get_hostp2m(d); + + rc = p2m_alloc_table(p2m); + if ( rc != 0 ) + return -ENOMEM; + + d->arch.altp2m_active = false; /* * Make sure that all TLBs corresponding to the new VMID are flushed - * before using it + * before using it. */ p2m_flush_tlb(p2m); @@ -1440,7 +1457,7 @@ static int p2m_init_hostp2m(struct domain *d) if ( rc ) return rc; - return p2m_alloc_table(d); + return p2m_table_init(d); } int p2m_init(struct domain *d)
The initially named function "p2m_alloc_table" allocated pages solely required for the host's p2m. The new implementation leaves p2m allocation related parts inside of this function to generally initialize p2m/altp2m tables. This is done generically, as the host's p2m and altp2m tables are allocated similarly. Since this function will be used by the altp2m initialization routines, it is not made static. In addition, this commit provides the overlay function "p2m_table_init" that is used for the host's p2m allocation/initialization. Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Julien Grall <julien.grall@arm.com> --- v2: Removed altp2m table initialization from "p2m_table_init". --- xen/arch/arm/p2m.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)