| Message ID | 1469031064-23344-17-git-send-email-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 22/07/16 08:46, Sergej Proskurin wrote: > Hi Julien, Hello Sergej, > On 07/20/2016 06:10 PM, Julien Grall wrote: >> The field vttbr holds the base address of the translation table for >> guest. Its value will depends on how the p2m has been initialized and >> will only be used by the code code. >> >> So move the field from arch_domain to p2m_domain. This will also ease >> the implementation of altp2m. >> --- >> xen/arch/arm/p2m.c | 11 +++++++---- >> xen/arch/arm/traps.c | 2 +- >> xen/include/asm-arm/domain.h | 1 - >> xen/include/asm-arm/p2m.h | 3 +++ >> 4 files changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index c407e6a..c52081a 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -107,10 +107,14 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr) >> >> static void p2m_load_VTTBR(struct domain *d) >> { >> + struct p2m_domain *p2m = &d->arch.p2m; >> + > > This is ok to me. Further altp2m implementation can easily extend this > code base. > > Also, as your patch (patch #17) as well eliminates the possibility that > the idle domain reaches this function the following check could be > potentially removed (as far as I know, p2m_load_VTTBR is reached only > through p2m_restore_state). I will not remove this check because this is not the goal of this patch and each patch should boot Xen without any dependencies on a follow-up patch: p2m_load_VTTBR is used flush_tlb_domain until patch #18 and p2m_restore_state does not yet have the is_idle_* check (it will be added in patch #17). Regards,
Hi Stefano, On 27/07/16 01:57, Stefano Stabellini wrote: > On Wed, 20 Jul 2016, Julien Grall wrote: >> The field vttbr holds the base address of the translation table for >> guest. Its value will depends on how the p2m has been initialized and >> will only be used by the code code. > ^ code code? I think I wanted to say "P2M code". Regards, > >> So move the field from arch_domain to p2m_domain. This will also ease >> the implementation of altp2m. >> --- >> xen/arch/arm/p2m.c | 11 +++++++---- >> xen/arch/arm/traps.c | 2 +- >> xen/include/asm-arm/domain.h | 1 - >> xen/include/asm-arm/p2m.h | 3 +++ >> 4 files changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index c407e6a..c52081a 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -107,10 +107,14 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr) >> >> static void p2m_load_VTTBR(struct domain *d) >> { >> + struct p2m_domain *p2m = &d->arch.p2m; >> + >> if ( is_idle_domain(d) ) >> return; >> - BUG_ON(!d->arch.vttbr); >> - WRITE_SYSREG64(d->arch.vttbr, VTTBR_EL2); >> + >> + ASSERT(p2m->vttbr); >> + >> + WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2); >> isb(); /* Ensure update is visible */ >> } >> >> @@ -1298,8 +1302,7 @@ static int p2m_alloc_table(struct domain *d) >> >> p2m->root = page; >> >> - d->arch.vttbr = page_to_maddr(p2m->root) >> - | ((uint64_t)p2m->vmid&0xff)<<48; >> + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; >> >> /* >> * Make sure that all TLBs corresponding to the new VMID are flushed >> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c >> index 06a8ee5..65c6fb4 100644 >> --- a/xen/arch/arm/traps.c >> +++ b/xen/arch/arm/traps.c >> @@ -880,7 +880,7 @@ void vcpu_show_registers(const struct vcpu *v) >> ctxt.ifsr32_el2 = v->arch.ifsr; >> #endif >> >> - ctxt.vttbr_el2 = v->domain->arch.vttbr; >> + ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr; >> >> _show_registers(&v->arch.cpu_info->guest_cpu_user_regs, &ctxt, 1, v); >> } >> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h >> index 4e9d8bf..9452fcd 100644 >> --- a/xen/include/asm-arm/domain.h >> +++ b/xen/include/asm-arm/domain.h >> @@ -48,7 +48,6 @@ struct arch_domain >> >> /* Virtual MMU */ >> struct p2m_domain p2m; >> - uint64_t vttbr; >> >> struct hvm_domain hvm_domain; >> gfn_t *grant_table_gfn; >> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h >> index ce28e8a..53c4d78 100644 >> --- a/xen/include/asm-arm/p2m.h >> +++ b/xen/include/asm-arm/p2m.h >> @@ -32,6 +32,9 @@ struct p2m_domain { >> /* Current VMID in use */ >> uint8_t vmid; >> >> + /* Current Translation Table Base Register for the p2m */ >> + uint64_t vttbr; >> + >> /* >> * Highest guest frame that's ever been mapped in the p2m >> * Only takes into account ram and foreign mapping >> -- >> 1.9.1 >> >
Hi, On 20/07/16 17:10, Julien Grall wrote: > The field vttbr holds the base address of the translation table for > guest. Its value will depends on how the p2m has been initialized and > will only be used by the code code. > > So move the field from arch_domain to p2m_domain. This will also ease > the implementation of altp2m. I forgot to add my signed-off-by in this patch as well :/. I will add in the next version. Regards, > --- > xen/arch/arm/p2m.c | 11 +++++++---- > xen/arch/arm/traps.c | 2 +- > xen/include/asm-arm/domain.h | 1 - > xen/include/asm-arm/p2m.h | 3 +++ > 4 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index c407e6a..c52081a 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -107,10 +107,14 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr) > > static void p2m_load_VTTBR(struct domain *d) > { > + struct p2m_domain *p2m = &d->arch.p2m; > + > if ( is_idle_domain(d) ) > return; > - BUG_ON(!d->arch.vttbr); > - WRITE_SYSREG64(d->arch.vttbr, VTTBR_EL2); > + > + ASSERT(p2m->vttbr); > + > + WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2); > isb(); /* Ensure update is visible */ > } > > @@ -1298,8 +1302,7 @@ static int p2m_alloc_table(struct domain *d) > > p2m->root = page; > > - d->arch.vttbr = page_to_maddr(p2m->root) > - | ((uint64_t)p2m->vmid&0xff)<<48; > + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; > > /* > * Make sure that all TLBs corresponding to the new VMID are flushed > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 06a8ee5..65c6fb4 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -880,7 +880,7 @@ void vcpu_show_registers(const struct vcpu *v) > ctxt.ifsr32_el2 = v->arch.ifsr; > #endif > > - ctxt.vttbr_el2 = v->domain->arch.vttbr; > + ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr; > > _show_registers(&v->arch.cpu_info->guest_cpu_user_regs, &ctxt, 1, v); > } > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 4e9d8bf..9452fcd 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -48,7 +48,6 @@ struct arch_domain > > /* Virtual MMU */ > struct p2m_domain p2m; > - uint64_t vttbr; > > struct hvm_domain hvm_domain; > gfn_t *grant_table_gfn; > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h > index ce28e8a..53c4d78 100644 > --- a/xen/include/asm-arm/p2m.h > +++ b/xen/include/asm-arm/p2m.h > @@ -32,6 +32,9 @@ struct p2m_domain { > /* Current VMID in use */ > uint8_t vmid; > > + /* Current Translation Table Base Register for the p2m */ > + uint64_t vttbr; > + > /* > * Highest guest frame that's ever been mapped in the p2m > * Only takes into account ram and foreign mapping >
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index c407e6a..c52081a 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -107,10 +107,14 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr) static void p2m_load_VTTBR(struct domain *d) { + struct p2m_domain *p2m = &d->arch.p2m; + if ( is_idle_domain(d) ) return; - BUG_ON(!d->arch.vttbr); - WRITE_SYSREG64(d->arch.vttbr, VTTBR_EL2); + + ASSERT(p2m->vttbr); + + WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2); isb(); /* Ensure update is visible */ } @@ -1298,8 +1302,7 @@ static int p2m_alloc_table(struct domain *d) p2m->root = page; - d->arch.vttbr = page_to_maddr(p2m->root) - | ((uint64_t)p2m->vmid&0xff)<<48; + p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48; /* * Make sure that all TLBs corresponding to the new VMID are flushed diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 06a8ee5..65c6fb4 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -880,7 +880,7 @@ void vcpu_show_registers(const struct vcpu *v) ctxt.ifsr32_el2 = v->arch.ifsr; #endif - ctxt.vttbr_el2 = v->domain->arch.vttbr; + ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr; _show_registers(&v->arch.cpu_info->guest_cpu_user_regs, &ctxt, 1, v); } diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 4e9d8bf..9452fcd 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -48,7 +48,6 @@ struct arch_domain /* Virtual MMU */ struct p2m_domain p2m; - uint64_t vttbr; struct hvm_domain hvm_domain; gfn_t *grant_table_gfn; diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index ce28e8a..53c4d78 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -32,6 +32,9 @@ struct p2m_domain { /* Current VMID in use */ uint8_t vmid; + /* Current Translation Table Base Register for the p2m */ + uint64_t vttbr; + /* * Highest guest frame that's ever been mapped in the p2m * Only takes into account ram and foreign mapping