On 17/04/2023 1:35 pm, Jan Beulich wrote:
> On 17.04.2023 14:13, Andrew Cooper wrote:
>> --- a/xen/common/livepatch.c
>> +++ b/xen/common/livepatch.c
>> @@ -803,28 +803,84 @@ static int prepare_payload(struct payload *payload,
>> if ( sec )
>> {
>> #ifdef CONFIG_HAS_ALTERNATIVE
>> + /*
>> + * (As of April 2023), Alternatives are formed of:
>> + * - An .altinstructions section with an array of struct
>> alt_instr's.
>> + * - An .altinstr_replacement section containing instructions.
>> + *
>> + * An individual alt_instr contains:
>> + * - An orig reference, pointing into .text with a nonzero length
>> + * - A repl reference, pointing into .altinstr_replacement
>> + *
>> + * It is legal to have zero-length replacements, meaning it is legal
>> + * for the .altinstr_replacement section to be empty too. An
>> + * implementation detail means that a zero-length replacement's repl
>> + * reference will still be in the .altinstr_replacement section.
> Didn't you agree that "will" is not really true, and it's at best "may", but
> then also doesn't really matter here in the first place (suggesting that the
> sentence might best be dropped, to avoid drawing attention to something that
> might at best confuse the reader as to its relevance)?
Only that "will be at 0" wasn't actually true.
Right now, the repl reference *will* be somewhere in
altinstr_replacement. It is discussed here because it is what the check
enforces.
As an implementation detail, it is of course free to change in the
future if needs be.
~Andrew
