Re: [Xen-devel] [PATCH] x86/shadow: Drop incorrect diagnostic when shadowing TSS.RSP0

[Tim Deegan <tim@xxxxxxx>]

At 13:32 +0100 on 08 Apr (1554730320), Andrew Cooper wrote:
> On 08/04/2019 13:11, Jan Beulich wrote:
> >>>> On 08.04.19 at 13:37, <andrew.cooper3@xxxxxxxxxx> wrote:
> >> On 08/04/2019 11:14, Jan Beulich wrote:
> >>>>>> On 05.04.19 at 21:09, <andrew.cooper3@xxxxxxxxxx> wrote:
> >>>> --- a/xen/arch/x86/mm/shadow/multi.c
> >>>> +++ b/xen/arch/x86/mm/shadow/multi.c
> >>>> @@ -3305,8 +3305,9 @@ static int sh_page_fault(struct vcpu *v,
> >>>>      {
> >>>>          /*
> >>>>           * If we are in the middle of injecting an exception or 
> >>>> interrupt then
> >>>> -         * we should not emulate: it is not the instruction at %eip 
> >>>> that caused
> >>>> -         * the fault. Furthermore it is almost certainly the case the 
> >>>> handler
> >>>> +         * we should not emulate: the fault is a side effect of the 
> >>>> processor
> >>>> +         * trying to push an exception frame onto a stack which has yet 
> >>>> to be
> >>>> +         * shadowed.  Furthermore it is almost certainly the case the 
> >>>> handler
> >>>>           * stack is currently considered to be a page table, so we 
> >>>> should
> >>>>           * unshadow the faulting page before exiting.
> >>>>           */
> > I'm (at least) mildly confused: I follow what you write (I think), but
> > you again say "the stack always becomes shadowed". My original
> > question was whether you really mean that, as stacks, if at all,
> > should get shadowed only unintentionally (and hence get un-shadowed
> > immediately when that condition is detected). That is, my (slightly
> > rephrased) question stands: Do you perhaps mean the page tables
> > mapping the stack to become shadowed, rather than the stack itself?
> 
> I guess this is an issue of terminology, to which I defer to Tim to judge.

Hi,

Sorry for the delay; I have been away.

FWIW I prefer the original comment, and I think that referring to the
stack as "not yet shadowed" is confusing when the problem might be
that the stack itself is indeed shadowed.  I'd also be happy with just
saying "the fault is a side effect of the processor trying to push an
exception frame onto the stack."

Happy to see the debug message being removed if it's being triggered
in the real world.  IIRC it has not proved to be useful since that
code was first developed.  So, with the comment adjusted,
Acked-by: Tim Deegan <tim@xxxxxxx>

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel