[PATCH v3 06/15] KVM: arm64: Restore mdcr_el2 from vcpu

[Fuad Tabba]

Hi Will,

On Thu, Aug 12, 2021 at 10:46 AM Will Deacon <will at kernel.org> wrote:
>
> On Wed, Jul 21, 2021 at 08:37:21AM +0100, Fuad Tabba wrote:
> > On Tue, Jul 20, 2021 at 3:53 PM Andrew Jones <drjones at redhat.com> wrote:
> > >
> > > On Mon, Jul 19, 2021 at 05:03:37PM +0100, Fuad Tabba wrote:
> > > > On deactivating traps, restore the value of mdcr_el2 from the
> > > > newly created and preserved host value vcpu context, rather than
> > > > directly reading the hardware register.
> > > >
> > > > Up until and including this patch the two values are the same,
> > > > i.e., the hardware register and the vcpu one. A future patch will
> > > > be changing the value of mdcr_el2 on activating traps, and this
> > > > ensures that its value will be restored.
> > > >
> > > > No functional change intended.
> > >
> > > I'm probably missing something, but I can't convince myself that the host
> > > will end up with the same mdcr_el2 value after deactivating traps after
> > > this patch as before. We clearly now restore whatever we had when
> > > activating traps (presumably whatever we configured at init_el2_state
> > > time), but is that equivalent to what we had before with the masking and
> > > ORing that this patch drops?
> >
> > You're right. I thought that these were actually being initialized to
> > the same values, but having a closer look at the code the mdcr values
> > are not the same as pre-patch. I will fix this.
>
> Can you elaborate on the issue here, please? I was just looking at this
> but aren't you now relying on __init_el2_debug to configure this, which
> should be fine?

I *think* that it should be fine, but as Drew pointed out, the host
does not end up with the same mdcr_el2 value after the deactivation in
this patch as it did after deactivation before this patch. In my v4
(not sent out yet), I have fixed it to ensure that the host does end
up with the same value as the one before this patch. That should make
it easier to check that there's no functional change.

I'll look into it further, and if I can convince myself that there
aren't any issues and that this patch makes the code cleaner, I will
add it as a separate patch instead to make reviewing easier.

Thanks,
/fuad

> Will