Hi, Boris,thanks for your review
> -----Original Message-----
> From: Borislav Petkov [mailto:b...@alien8.de]
> Sent: Monday, August 24, 2015 4:50 PM
> To: Chen, Yu C
> Cc: r...@rjwysocki.net; pa...@ucw.cz; t...@linutronix.de;
> mi...@redhat.com; h...@zytor.com; Zhang, Rui; l...@kernel.org;
> x...@kernel.org; linux...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] [v2] x86, suspend: Save/restore extra MSR registers for
> suspend
>
> On Fri, Aug 21, 2015 at 07:53:34PM +0800, Chen Yu wrote:
> > +struct msr_type {
> > + unsigned int msr_id;
> > + bool msr_saved;
> > + u64 msr_value;
> > +};
>
> These definitions look awfully close to struct msr_info in include/asm/msr.h
>
> Maybe reuse them instead of growing yet another type...
>
OK, I'll use msr_info instead of msr_id and msr_value:
struct msr_type {
bool msr_saved;
struct msr_info rv;
};
> > +static void msr_save_context(struct saved_context *ctxt) {
> > + int i = 0;
> > +
> > + for (i = 0; i < ctxt->msr_for_save.num; i++) {
> > + struct msr_type *msr =
> > + &ctxt->msr_for_save.msr_array[i];
>
> No need for the line breaks here, let them stick out for better readability.
>
OK, will do.
> > + msr->msr_saved = !rdmsrl_safe(msr->msr_id,
> > + &msr->msr_value);
> > + }
> > + struct msr_type *msr =
> > + &ctxt->msr_for_save.msr_array[i];
> > + if (msr->msr_saved)
> > + wrmsrl(msr->msr_id, msr->msr_value);
>
> Ditto.
>
OK, will do.
Best Regards,
Yu
