Re: PT32_[GS]ETXMMREGS for amd64 (Was: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h)

[Michał Górny <mgorny%NetBSD.org@localhost>]

On Fri, 2019-11-22 at 10:10 +0900, Rin Okuyama wrote:
> Hi, thank you for your review!
> 
> On 2019/11/22 0:48, Kamil Rytarowski wrote:
> > On 21.11.2019 10:49, Rin Okuyama wrote:
> ...
> > > I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support:
> > > 
> > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch
> > > 
> > > With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64.
> > > 
> > > Some remarks:
> > > 
> > > (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to <machine/ptrace.h>.
> > >      These are only used for COMPAT_NETBSD32, and not exposed to userland.
> > > 
> > 
> > This is correct.
> > We don't want to export XMMREGS to amd64 users.
> > 
> > Pleae remove /* */ from this part:
> > 
> > +/*
> > +	"PT_GETXMMREGS", \
> > +	"PT_SETXMMREGS"
> > + */
> 
> Yes, I will.
> 
> > This will allow ktruss and related software to have meaningful form for
> > compat32 ptracing.
> > 
> > > (2) COMPAT_NETBSD32 codes are called from process_machdep.c via
> > >      module_hook framework. This may be too much though...
> > > 
> > 
> > I have no opinion here.
> 
> Now, Paul and I are discussed how to improve the usage of module_hook.
> I will update this part in accordance with his advice.
> 
> > > Comments?
> > > 
> > 
> > I will leave this to be reviewed by mgorny@. Adding him to CC.
> 
> I see. Hi, mgorny@. Please look into it!
> > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch
> > > > 

It seems correct at a first glance.  The hook logic is unknown to me, so
I can't comment on that.  If I was to be picky, I'd prefer if changes
in existing code that are not exactly relevant to adding this were split
into a separate patches (e.g. changing int to bool, renaming valid*
func).

-- 
Best regards,
Michał Górny