Re: [Xen-devel] [PATCH 2/3] x86: Port the basic alternative mechanism from Linux to Xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.05.14 at 17:40, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 26/05/2014 08:27, Feng Wu wrote:
>> +#define MAX_PATCH_LEN (255-1)
> 
> Where does this number come from?  An individual instruction can't be
> longer than 15 bytes.

The lengths get encoded as bytes. And of course we want to be able
to patch more than a single instruction if needed.

>> +        /* 0xe8 is a relative jump; fix the offset. */
>> +        if ( *insnbuf == 0xe8 && a->replacementlen == 5 )
>> +            *(s32 *)(insnbuf + 1) += replacement - instr;
> 
> 0xe8 is the call instruction.  Calling it "a relative jump" is slightly
> misleading.  Why does it need special treatment here?

When the comment got added in Linux, it should have either made
match the code, or the code should have got changed to mask the
low bit.

In any event, the special treatment is necessary so that the
replacement instruction can be written as a mnemonic with
(relative displacement) operand - since the displacement changes
when the instruction gets copied, it needs to get adjusted here.

>> +void __init alternative_instructions(void)
> 
> This function would be more descriptive as
> "patch_alternative_instructions" or so.

Otoh staying close to the Linux original also has its benefits. But
that would then probably also imply retaining Linux coding style in
this file.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel