On 17.04.2023 15:37, Andrew Cooper wrote:
> 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.
Oh, right - I'm sorry for the noise.
Jan
> 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
