On Wed, Sep 29, 2021 at 11:43:32AM +0200, Jan Beulich wrote: > It's not really needed and has been misleading me more than once to try > and spot its "actual" use(s). It should really have been dropped when > the 32-bit specific logic was purged from here. > > Signed-off-by: Jan Beulich <[email protected]>
Reviewed-by: Roger Pau Monné <[email protected]> As it makes the current code clearer. I have one question/concern below. > > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -327,16 +327,13 @@ static void show_guest_stack(struct vcpu > > if ( v != current ) > { > - struct vcpu *vcpu; > - > if ( !guest_kernel_mode(v, regs) ) > { > printk("User mode stack\n"); > return; > } > > - vcpu = maddr_get_owner(read_cr3()) == v->domain ? v : NULL; > - if ( !vcpu ) > + if ( maddr_get_owner(read_cr3()) != v->domain ) Wouldn't it be more accurate to check that the current loaded cr3 matches the one used by v? AFAICT we don't load the cr3 from v, so it's still possible to have diverging per-vcpu page tables and thus end up dumping wrong data? Thanks, Roger.
