Re: [PATCH 2/3] x86/alternatives: Rework get_ideal_nops()

Andrew Cooper <[email protected]> Wed, 5 Aug 2026 07:34:07 +0100
Newsgroups gmane.comp.emulators.xen.devel
Message-ID <[email protected]>
On 05/08/2026 7:04 am, Jan Beulich wrote:
> On 04.08.2026 19:13, Andrew Cooper wrote:
>> On 02/06/2025 10:57 am, Jan Beulich wrote:
>>> On 22.05.2025 17:00, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/alternative.c
>>>> +++ b/xen/arch/x86/alternative.c
>>>> @@ -20,7 +20,7 @@
>>>>  #define MAX_PATCH_LEN (255-1)
>>>>  
>>>>  #ifdef K8_NOP1
>>>> -static const unsigned char k8nops[] init_or_livepatch_const = {
>>>> +static const unsigned char k8_nops[] init_or_livepatch_const = {
>>>>      K8_NOP1,
>>>>      K8_NOP2,
>>>>      K8_NOP3,
>>>> @@ -31,22 +31,10 @@ static const unsigned char k8nops[] init_or_livepatch_const = {
>>>>      K8_NOP8,
>>>>      K8_NOP9,
>>>>  };
>>>> -static const unsigned char * const k8_nops[ASM_NOP_MAX+1] init_or_livepatch_constrel = {
>>> ... the (at least visual) connection to ASM_NOP_MAX. Could I talk you into
>>> adding build time array-size checks for both arrays, to restore the
>>> connection?
>> Sorry, but I have no idea what you're asking for here.
>     BUILD_BUG_ON(ARRAY_SIZE(k8_nops) != ASM_NOP_MAX);
>     BUILD_BUG_ON(ARRAY_SIZE(p6_nops) != ASM_NOP_MAX);

The arrays are 45 bytes (and elements) long.  ASM_NOP_MAX is 9.

>
>> The use of ASM_NOP_MAX was latently buggy before; it was easy to create
>> a NULL deference if the initialiser wasn't filled in when ASM_NOP_MAX
>> changed.
> Partly, yes. But why make it worse when it can be made at least somewhat
> better?

On the contrary, I've removed an incorrect (and ineffective) attempt to
tie to ASM_NOP_MAX, and consider this form better than what was there
before.

~Andrew