Re: [PATCH] x86: refine guest_mode()

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

On Mon, Apr 27, 2020 at 04:08:59PM +0200, Jan Beulich wrote:
> On 27.04.2020 11:59, Roger Pau Monné wrote:
> > On Mon, Apr 27, 2020 at 10:03:05AM +0200, Jan Beulich wrote:
> >> --- a/xen/include/asm-x86/regs.h
> >> +++ b/xen/include/asm-x86/regs.h
> >> @@ -10,9 +10,10 @@
> >>      /* Frame pointer must point into current CPU stack. */                
> >>     \
> >>      ASSERT(diff < STACK_SIZE);                                            
> >>     \
> >>      /* If not a guest frame, it must be a hypervisor frame. */            
> >>     \
> >> -    ASSERT((diff == 0) || (r->cs == __HYPERVISOR_CS));                    
> >>     \
> >> +    if ( diff < PRIMARY_STACK_SIZE )                                      
> >>     \
> >> +        ASSERT(!diff || ((r)->cs == __HYPERVISOR_CS));                    
> >>     \
> > 
> > Why not use:
> > 
> > ASSERT(diff >= PRIMARY_STACK_SIZE || !diff || ((r)->cs == __HYPERVISOR_CS));
> 
> Except for the longer (without being helpful imo) string reported if
> the assertion triggers, I see not difference.

Wanted to avoid the empty if on non-debug builds, but I assume the
compiler will already optimize it out.

> > I'm not sure I fully understand this layout, is it possible that you
> > also need to account for the size of cpu_info?
> 
> Depends on how paranoid we want the checking here to be, but in going
> further I wouldn't want this to become sub-page fine-grained if we
> aren't first doing e.g. what I'm mentioning in the post-commit-message
> remark.

Right, leaving it as-is is fine, just wanted to be sure I fully
understand the layout.

Thanks, Roger.