RE: [patch] x86: ptrace and core-dump extensions for xstate

[Lu, Hongjiu]

> 
> > +	case PTRACE_GETXSTATEREGS:	/* Get the child extended state. */
> > +		return copy_regset_to_user(child,
> > +					   task_user_regset_view(current),
> > +					   REGSET_XSTATE,
> > +					   0, xstate_size,
> > +					   datap);
> > +
> > +	case PTRACE_SETXSTATEREGS:	/* Set the child extended state. */
> > +		return copy_regset_from_user(child,
> > +					     task_user_regset_view(current),
> > +					     REGSET_XSTATE,
> > +					     0, xstate_size,
> > +					     datap);
> 
> I think this is a poor choice of interface for this.  The other existing
> PTRACE_GET*REGS calls use a fixed-size and fixed-layout block that is a
> known constant in the userland ABI.  Here, userland has no way to know
> xstate_size, so you are accessing a chunk of user memory where userland
> doesn't really know how much you are going to access.
> 
> AFAICT from skimming the Intel manuals, there is no specified upper bound
> on how big the xsave size might grow in future processors.  I would think
> that it might be limited to a page, since there is no way to indicate a
> partial copy to restart after a page fault.  So for unpinned access (such
> as in user mode, or the user memory access in the signal frame setup), in
> full-thrashing situations an xsave spanning multiple pages might thrash
> totally and never make progress.  OTOH, the manual does not say that the
> buffer can't span two pages today, just that it has to be 64-byte aligned.
> So perhaps we already have that issue (for signal frame setup or for direct
> user-space uses of xsave/xrstor) and it's just not really an issue that
> matters (thrashing is thrashing--it's already pathological, so who cares if
> it's literal livelock or not).  The upshot being, I don't think there is an
> obvious upper bound that userland can presume statically here.
> 
> AFAICT, the only way for userland to guess xstate_size is to use cpuid
> itself.  Even if that is actually reliable, or even if the kernel gave
> userland some other means to know the kernel's xstate_size value, IMHO that
> is a pretty dubious and error-prone way to construct the ABI.  Usually when
> userland supplies a buffer whose size is not a permanent constant of the
> ABI, userland says how big a buffer it's passing in.
> 

Gdb calls cpuid during startup time to find out the actual xstate_size
on the target machine. It has to be reliable. There is no need for
another ptrace option. But we should document it clearly in ABI.


H.J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/