Re: [PATCH 2/3] x86/PV: replace assertions in '0' debug key stack dumping

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Oct 18, 2021 at 04:37:27PM +0200, Jan Beulich wrote:
> On 18.10.2021 16:26, Roger Pau Monné wrote:
> > On Wed, Sep 29, 2021 at 11:42:54AM +0200, Jan Beulich wrote:
> >> While it was me to add them, I'm afraid I don't see justification for
> >> the assertions: A vCPU may very well have got preempted while in user
> >> mode. Limit compat guest user mode stack dumps to the containing page
> >> (like is done when using do_page_walk()), and suppress their dumping
> >> altogether for 64-bit Dom0.
> > 
> > I'm slightly lost by this last sentence...
> > 
> >> @@ -328,7 +329,12 @@ static void show_guest_stack(struct vcpu
> >>      {
> >>          struct vcpu *vcpu;
> >>  
> >> -        ASSERT(guest_kernel_mode(v, regs));
> >> +        if ( !guest_kernel_mode(v, regs) )
> >> +        {
> >> +            printk("User mode stack\n");
> >> +            return;
> >> +        }
> > 
> > ...as you seem to unconditionally prevent the dump regardless of
> > whether it's dom0 or domU as long as it's not a kernel stack?
> 
> Well, Dom0 comes into play by way of me talking about debug key '0'.
> I've replaced "Dom0" by "domains" in the sentence.

Thanks, with that:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> > I assume when running in PV 64bit mode user-space could be executing a
> > 32bit program and hence Xen could then misprint the stack as a 64bit
> > one?
> 
> That's not a primary concern, I would think. The real problem is
> do_page_walk() doing
> 
>     unsigned long mfn = pagetable_get_pfn(v->arch.guest_table);
> 
> first thing: No consideration of guest user mode here at all. And
> I didn't want to teach it without real need.

Oh, indeed. It would need to use guest_table_user at least.

Roger.