On Sat, Dec 21, 2019 at 01:12:14PM +0000, Marc Zyngier wrote:
> On Fri, 20 Dec 2019 14:30:09 +0000
> Andrew Murray <andrew.murray@xxxxxxx> wrote:
>
> > From: Sudeep Holla <sudeep.holla@xxxxxxx>
> >
> > On VHE systems, the reset value for MDCR_EL2.E2PB=b00 which defaults
> > to profiling buffer using the EL2 stage 1 translations.
>
> Does the reset value actually matter here? I don't see it being
> specific to VHE systems, and all we're trying to achieve is to restore
> the SPE configuration to a state where it can be used by the host.
>
> > However if the
> > guest are allowed to use profiling buffers changing E2PB settings, we
>
> How can the guest be allowed to change E2PB settings? Or do you mean
> here that allowing the guest to use SPE will mandate changes of the
> E2PB settings, and that we'd better restore the hypervisor state once
> we exit?
>
> > need to ensure we resume back MDCR_EL2.E2PB=b00. Currently we just
> > do bitwise '&' with MDCR_EL2_E2PB_MASK which will retain the value.
> >
> > So fix it by clearing all the bits in E2PB.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> > Signed-off-by: Andrew Murray <andrew.murray@xxxxxxx>
> > ---
> > arch/arm64/kvm/hyp/switch.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> > index 72fbbd86eb5e..250f13910882 100644
> > --- a/arch/arm64/kvm/hyp/switch.c
> > +++ b/arch/arm64/kvm/hyp/switch.c
> > @@ -228,9 +228,7 @@ void deactivate_traps_vhe_put(void)
> > {
> > u64 mdcr_el2 = read_sysreg(mdcr_el2);
> >
> > - mdcr_el2 &= MDCR_EL2_HPMN_MASK |
> > - MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT |
> > - MDCR_EL2_TPMS;
> > + mdcr_el2 &= MDCR_EL2_HPMN_MASK | MDCR_EL2_TPMS;
> >
> > write_sysreg(mdcr_el2, mdcr_el2);
> >
>
> I'm OK with this change, but I believe the commit message could use
> some tidying up.
No problem, I'll update the commit message.
Thanks,
Andrew Murray
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
