lkml.org 
[lkml]   [2010]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch v2 2/4] x86, ptrace: regset extensions to support xstate
    On 02/09, Suresh Siddha wrote:
    >
    > +int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
    > + unsigned int pos, unsigned int count,
    > + void *kbuf, void __user *ubuf)
    > +{
    > + int ret;
    > + int size = regset->n * regset->size;
    > + struct xsave_hdr_struct *xsave_hdr =
    > + &target->thread.xstate->xsave.xsave_hdr;
    > +
    > + if (!cpu_has_xsave)
    > + return -ENODEV;
    > +
    > + ret = init_fpu(target);
    > + if (ret)
    > + return ret;
    > +
    > + /*
    > + * First copy the fxsave bytes 0..463
    > + */
    > + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
    > + &target->thread.xstate->xsave, 0,
    > + offsetof(struct i387_fxsave_struct,
    > + sw_reserved));
    > + if (!ret)
    > + /*
    > + * Copy the 48bytes defined by software
    > + */
    > + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
    > + xstate_fx_sw_bytes,
    > + offsetof(struct i387_fxsave_struct,
    > + sw_reserved),
    > + offsetof(struct xsave_struct,
    > + xsave_hdr));

    Hmm. Suresh, could you confirm these offsetof's are correct?

    We are copying xstate_fx_sw_bytes array which is u64[6], but
    start_pos == sizeof(i387_fxsave_struct) - padding ?

    While we are here. Perhaps it would be more symmetrical to do

    ret = user_regset_copyout();
    if (ret)
    return ret;

    instead of "if (!ret)" which needs a tab, but this is up to you.

    > + if (!ret)
    > + /*
    > + * Copy the rest of xstate memory layout
    > + */
    > + ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
    > + xsave_hdr,

    Another very minor nit, this is the only user of xsave_hdr, but this
    var was defined at the top. Perhaps it would be a bit more clean to
    have

    struct xsave_struct *xsave = target->thread.xstate->xsave;

    and use it in 1st and 3rd copyout?

    Speaking about the first user_regset_copyout(), perhaps xsave->i387
    would be more clear than xsave?

    Oleg.



    \
     
     \ /
      Last update: 2010-02-10 12:33    [W:0.046 / U:0.808 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site