| Message ID | 20160801171028.11615-13-proskurin@sec.in.tum.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Sergej, On 01/08/16 18:10, Sergej Proskurin wrote: > Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> > --- > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com> > --- > xen/arch/arm/altp2m.c | 32 ++++++++++++++++++++++++++++++++ > xen/arch/arm/hvm.c | 2 +- > xen/include/asm-arm/altp2m.h | 4 ++++ > 3 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c > index 80ed553..7404f42 100644 > --- a/xen/arch/arm/altp2m.c > +++ b/xen/arch/arm/altp2m.c > @@ -33,6 +33,38 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v) > return v->domain->arch.altp2m_p2m[index]; > } > > +int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx) > +{ > + struct vcpu *v; > + int rc = -EINVAL; > + > + if ( idx >= MAX_ALTP2M ) > + return rc; > + > + domain_pause_except_self(d); > + > + altp2m_lock(d); > + > + if ( d->arch.altp2m_vttbr[idx] != INVALID_VTTBR ) > + { > + for_each_vcpu( d, v ) > + if ( idx != vcpu_altp2m(v).p2midx ) > + { > + atomic_dec(&altp2m_get_altp2m(v)->active_vcpus); > + vcpu_altp2m(v).p2midx = idx; > + atomic_inc(&altp2m_get_altp2m(v)->active_vcpus); > + } > + > + rc = 0; > + } If a domain is calling the function on itself, the current vCPU will not switch to the new altp2m because the VTTBR is only restore during context switch. However, I am not sure why you store a p2midx rather than directly a pointer to the p2m. > + > + altp2m_unlock(d); > + > + domain_unpause_except_self(d); > + > + return rc; > +} > + > static void altp2m_vcpu_reset(struct vcpu *v) > { > struct altp2mvcpu *av = &vcpu_altp2m(v); > diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c > index df29cdc..3b508df 100644 > --- a/xen/arch/arm/hvm.c > +++ b/xen/arch/arm/hvm.c > @@ -129,7 +129,7 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg) > break; > > case HVMOP_altp2m_switch_p2m: > - rc = -EOPNOTSUPP; > + rc = altp2m_switch_domain_altp2m_by_id(d, a.u.view.view); > break; > > case HVMOP_altp2m_set_mem_access: > diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h > index afa1580..790bb33 100644 > --- a/xen/include/asm-arm/altp2m.h > +++ b/xen/include/asm-arm/altp2m.h > @@ -49,6 +49,10 @@ void altp2m_teardown(struct domain *d); > void altp2m_vcpu_initialise(struct vcpu *v); > void altp2m_vcpu_destroy(struct vcpu *v); > > +/* Switch alternate p2m for entire domain */ > +int altp2m_switch_domain_altp2m_by_id(struct domain *d, > + unsigned int idx); > + > /* Make a specific alternate p2m valid. */ > int altp2m_init_by_id(struct domain *d, > unsigned int idx); > Regards,
Hi Julien, On 08/04/2016 01:51 PM, Julien Grall wrote: > Hello Sergej, > > On 01/08/16 18:10, Sergej Proskurin wrote: >> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> >> --- >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: Julien Grall <julien.grall@arm.com> >> --- >> xen/arch/arm/altp2m.c | 32 ++++++++++++++++++++++++++++++++ >> xen/arch/arm/hvm.c | 2 +- >> xen/include/asm-arm/altp2m.h | 4 ++++ >> 3 files changed, 37 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c >> index 80ed553..7404f42 100644 >> --- a/xen/arch/arm/altp2m.c >> +++ b/xen/arch/arm/altp2m.c >> @@ -33,6 +33,38 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v) >> return v->domain->arch.altp2m_p2m[index]; >> } >> >> +int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int >> idx) >> +{ >> + struct vcpu *v; >> + int rc = -EINVAL; >> + >> + if ( idx >= MAX_ALTP2M ) >> + return rc; >> + >> + domain_pause_except_self(d); >> + >> + altp2m_lock(d); >> + >> + if ( d->arch.altp2m_vttbr[idx] != INVALID_VTTBR ) >> + { >> + for_each_vcpu( d, v ) >> + if ( idx != vcpu_altp2m(v).p2midx ) >> + { >> + atomic_dec(&altp2m_get_altp2m(v)->active_vcpus); >> + vcpu_altp2m(v).p2midx = idx; >> + atomic_inc(&altp2m_get_altp2m(v)->active_vcpus); >> + } >> + >> + rc = 0; >> + } > > If a domain is calling the function on itself, the current vCPU will > not switch to the new altp2m because the VTTBR is only restore during > context switch. Is this really the case that a switch from a guest to Xen could result in no context switch at all? I just saw the asserts in context_switch: [...] ASSERT(prev != next); [...] However, I thought that every trap/hypercall to Xen would eventually restore the VCPU's state. In this case, I will need to directly write to VTTBR_EL2. Thank you. > > However, I am not sure why you store a p2midx rather than directly a > pointer to the p2m. > Again, this idea has been also adopted from the x86 implementation. And I really do not see any reason to change that. Do you? >> + >> + altp2m_unlock(d); >> + >> + domain_unpause_except_self(d); >> + >> + return rc; >> +} >> + >> static void altp2m_vcpu_reset(struct vcpu *v) >> { >> struct altp2mvcpu *av = &vcpu_altp2m(v); >> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c >> index df29cdc..3b508df 100644 >> --- a/xen/arch/arm/hvm.c >> +++ b/xen/arch/arm/hvm.c >> @@ -129,7 +129,7 @@ static int >> do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg) >> break; >> >> case HVMOP_altp2m_switch_p2m: >> - rc = -EOPNOTSUPP; >> + rc = altp2m_switch_domain_altp2m_by_id(d, a.u.view.view); >> break; >> >> case HVMOP_altp2m_set_mem_access: >> diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h >> index afa1580..790bb33 100644 >> --- a/xen/include/asm-arm/altp2m.h >> +++ b/xen/include/asm-arm/altp2m.h >> @@ -49,6 +49,10 @@ void altp2m_teardown(struct domain *d); >> void altp2m_vcpu_initialise(struct vcpu *v); >> void altp2m_vcpu_destroy(struct vcpu *v); >> >> +/* Switch alternate p2m for entire domain */ >> +int altp2m_switch_domain_altp2m_by_id(struct domain *d, >> + unsigned int idx); >> + >> /* Make a specific alternate p2m valid. */ >> int altp2m_init_by_id(struct domain *d, >> unsigned int idx); >> > Best regards, ~Sergej
diff --git a/xen/arch/arm/altp2m.c b/xen/arch/arm/altp2m.c index 80ed553..7404f42 100644 --- a/xen/arch/arm/altp2m.c +++ b/xen/arch/arm/altp2m.c @@ -33,6 +33,38 @@ struct p2m_domain *altp2m_get_altp2m(struct vcpu *v) return v->domain->arch.altp2m_p2m[index]; } +int altp2m_switch_domain_altp2m_by_id(struct domain *d, unsigned int idx) +{ + struct vcpu *v; + int rc = -EINVAL; + + if ( idx >= MAX_ALTP2M ) + return rc; + + domain_pause_except_self(d); + + altp2m_lock(d); + + if ( d->arch.altp2m_vttbr[idx] != INVALID_VTTBR ) + { + for_each_vcpu( d, v ) + if ( idx != vcpu_altp2m(v).p2midx ) + { + atomic_dec(&altp2m_get_altp2m(v)->active_vcpus); + vcpu_altp2m(v).p2midx = idx; + atomic_inc(&altp2m_get_altp2m(v)->active_vcpus); + } + + rc = 0; + } + + altp2m_unlock(d); + + domain_unpause_except_self(d); + + return rc; +} + static void altp2m_vcpu_reset(struct vcpu *v) { struct altp2mvcpu *av = &vcpu_altp2m(v); diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c index df29cdc..3b508df 100644 --- a/xen/arch/arm/hvm.c +++ b/xen/arch/arm/hvm.c @@ -129,7 +129,7 @@ static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg) break; case HVMOP_altp2m_switch_p2m: - rc = -EOPNOTSUPP; + rc = altp2m_switch_domain_altp2m_by_id(d, a.u.view.view); break; case HVMOP_altp2m_set_mem_access: diff --git a/xen/include/asm-arm/altp2m.h b/xen/include/asm-arm/altp2m.h index afa1580..790bb33 100644 --- a/xen/include/asm-arm/altp2m.h +++ b/xen/include/asm-arm/altp2m.h @@ -49,6 +49,10 @@ void altp2m_teardown(struct domain *d); void altp2m_vcpu_initialise(struct vcpu *v); void altp2m_vcpu_destroy(struct vcpu *v); +/* Switch alternate p2m for entire domain */ +int altp2m_switch_domain_altp2m_by_id(struct domain *d, + unsigned int idx); + /* Make a specific alternate p2m valid. */ int altp2m_init_by_id(struct domain *d, unsigned int idx);
Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de> --- Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/altp2m.c | 32 ++++++++++++++++++++++++++++++++ xen/arch/arm/hvm.c | 2 +- xen/include/asm-arm/altp2m.h | 4 ++++ 3 files changed, 37 insertions(+), 1 deletion(-)