Re: [Xen-devel] [PATCH 2/7] x86/alt: Clean up struct alt_instr and its users

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Feb 12, 2018 at 04:52:21PM +0000, Roger Pau Monné wrote:
> On Mon, Feb 12, 2018 at 11:23:02AM +0000, Andrew Cooper wrote:
> >  * Rename some fields for consistency and clarity, and use standard types.
> >  * Don't opencode the use of ALT_{ORIG,REPL}_PTR().
> > 
> > No functional change.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> > ---
> > CC: Jan Beulich <JBeulich@xxxxxxxx>
> > CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/alternative.c        | 24 ++++++++++++------------
> >  xen/include/asm-x86/alternative.h | 12 ++++++------
> >  2 files changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
> > index 5c8b6f6..f8ddab5 100644
> > --- a/xen/arch/x86/alternative.c
> > +++ b/xen/arch/x86/alternative.c
> > @@ -163,8 +163,6 @@ void init_or_livepatch apply_alternatives(const struct 
> > alt_instr *start,
> >                                            const struct alt_instr *end)
> >  {
> >      const struct alt_instr *a;
> > -    u8 *instr, *replacement;
> > -    u8 insnbuf[MAX_PATCH_LEN];
> >  
> >      printk(KERN_INFO "alt table %p -> %p\n", start, end);
> >  
> > @@ -179,23 +177,25 @@ void init_or_livepatch apply_alternatives(const 
> > struct alt_instr *start,
> >       */
> >      for ( a = start; a < end; a++ )
> >      {
> > -        instr = (u8 *)&a->instr_offset + a->instr_offset;
> > -        replacement = (u8 *)&a->repl_offset + a->repl_offset;
> > -        BUG_ON(a->replacementlen > a->instrlen);
> > -        BUG_ON(a->instrlen > sizeof(insnbuf));
> > +        uint8_t *orig = ALT_ORIG_PTR(a);
> > +        uint8_t *repl = ALT_REPL_PTR(a);
> > +        uint8_t buf[MAX_PATCH_LEN];
> > +
> > +        BUG_ON(a->repl_len > a->orig_len);
> > +        BUG_ON(a->orig_len > sizeof(buf));
> 
> Would be nice to have an assembly BUILD_BUG_ON equivalent so that this
> could be turned into a compile time check and added to ALTINSTR_ENTRY.

 This function is also used to livepatch the hypervisor.

 I would actually suggest not use BUG_ON here instead but I didn't want
 to add noise to the problem at hand. :-)

 Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel