Re: [Xen-devel] [PATCH 02/18] xen/arm: Restore HCR_EL2 register

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 22 Mar 2017, Marc Zyngier wrote:
> On 22/03/17 12:45, Mark Rutland wrote:
> > On Wed, Mar 22, 2017 at 12:16:20PM +0000, Julien Grall wrote:
> >> (CC Mark for the TLB question)
> > 
> > [Adding Marc since he should understand this better than I do]
> > 
> > I've trimmed a lot of context here, since it wasn't clear if it was
> > relevant to the question. If there's something I've missed, please point
> > that out explicitly.
> > 
> >> I am not entirely sure if we can only restore HCR_RW. My concern is
> >> some of the bits are cached in the TLBs. Looking at the ARM ARM
> >> (both v7 and v8 see D4.7 in DDI0487A.k_iss10775): "In these
> >> descriptions, TLB entries for a translation regime for a particular
> >> Exception level are out of context when executing at a higher
> >> Exception level.". Which I interpret as TLB result cannot be cached
> >> when translating a guest VA to guest PA at EL2. So I guess restoring
> >> only HCR_RW might be fine.
> > 
> > My understanding is that when a translation regime is out-of-context,
> > the only requirement is that the core does not begin speculative walks
> > for that translation regime. See ARM DDI 0487A.k_iss10775, page D4-1735,
> > "Use of out-of-context translation regimes".
> 
> +1. An AT instruction is certainly allowed to hit in the TLB, and the
> only thing the core is not allowed to do is to speculate (because you
> cannot restore a guest context atomically).
> 
> > I believe that an explicit address translation involving a translation
> > regime can validly make use of (any part of) that regime's state and/or
> > allocate new TLB entries for that regime.
> > 
> > It looks like you're asking if you can avoid installing all of the
> > registers related to the EL1&0 regime when performing an EL1&0
> > translation at EL2, right?
> > 
> > I do not believe that is valid; my understanding is that all of the
> > relevant registers need to be installed first.
> 
> Indeed. I can't see how the you can perform an AT without first
> restoring all of the context that define how the page tables are parsed
> and including all the element that may be used to hit in the TLBs. Even
> worse, you could create TLB entries (as you said, AT doesn't preclude
> TLBs from being populated) that would conflict with an existing one on
> the next lookup, generating a Conflict Abort.
> 
> In short, not restoring the full VM context is doomed.
> 
> Now, what is the actual question? ;-)

Hi Marc, thanks for jumping in.

When we receive an SError in Xen, we determine if it should be injected
into the guest or "handled" in Xen (by "handle" I mean crash the
system). In case it should be injected into the guest, we set the
relevant bit in vcpu->arch.hcr_el2 (the saved version of HCR_EL2). This
patch moves the WRITE(vcpu->arch.hcr_el2, HCR_EL2) from context switch
related functions (p2m_restore_state) to leave_hypervisor_tail, which is
the last point we can move it to, before exiting Xen. That way, we are
sure to inject an abort into the guest, no matter exactly when we
receive the SError. So far so good, right?

Now that leaves the question: what are we going to do at context switch
time? From the SErrors and HCR_VA point of view, there is no need to
restore the vcpu->arch.hcr_el2 of the next vcpu immediately, we can
still do it later in leave_hypervisor_tail. But we do need the right
HCR_RW immediately, because otherwise AT could behave erratically after
the context switch and before leave_hypervisor_tail.

Given that restoring vcpu->arch.hcr_el2 twice (once in
leave_hypervisor_tail and once at context switch) looks weird, I
suggested to only restore the HCR_RW bit at context switch time and do a
full restore of vcpu->arch.hcr_el2 in leave_hypervisor_tail: the benefit
would be a clear separation of responsibility of the two operations.
Hence the question above. But it looks like it's not safe?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel