[PATCH] x86/alternatives: mark .altinstructions r/w

[PATCH] x86/alternatives: mark .altinstructions r/w

[Jan Beulich]

Forever since the introduction of the .priv member it has been wrong to
record the section as r/o.

.alt_call_sites, otoh, is legitimately r/o, hence
__alt_call_sites_{start,end}[] would better reflect that.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -464,7 +464,7 @@ static unsigned int __initdata alt_todo;
 static unsigned int __initdata alt_done;
 
 extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
-extern struct alt_call __alt_call_sites_start[], __alt_call_sites_end[];
+extern const struct alt_call __alt_call_sites_start[], __alt_call_sites_end[];
 
 /*
  * At boot time, we patch alternatives in NMI context.  This means that the
--- a/xen/arch/x86/include/asm/alternative-asm.h
+++ b/xen/arch/x86/include/asm/alternative-asm.h
@@ -59,7 +59,7 @@
 .macro ALTERNATIVE oldinstr, newinstr, feature
     decl_orig(\oldinstr, repl_len(1) - orig_len)
 
-    .pushsection .altinstructions, "a", @progbits
+    .pushsection .altinstructions, "aw", @progbits
     altinstruction_entry .L\@_orig_s, .L\@_repl_s1, \feature, \
         orig_len, repl_len(1), pad_len
 
@@ -82,7 +82,7 @@
 .macro ALTERNATIVE_2 oldinstr, newinstr1, feature1, newinstr2, feature2
     decl_orig(\oldinstr, as_max(repl_len(1), repl_len(2)) - orig_len)
 
-    .pushsection .altinstructions, "a", @progbits
+    .pushsection .altinstructions, "aw", @progbits
 
     altinstruction_entry .L\@_orig_s, .L\@_repl_s1, \feature1, \
         orig_len, repl_len(1), pad_len
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -90,7 +90,7 @@ extern void alternative_instructions(voi
 /* alternative assembly primitive: */
 #define ALTERNATIVE(oldinstr, newinstr, feature)                        \
         OLDINSTR_1(oldinstr, 1)                                         \
-        ".pushsection .altinstructions, \"a\", @progbits\n"             \
+        ".pushsection .altinstructions, \"aw\", @progbits\n"            \
         ALTINSTR_ENTRY(feature, 1)                                      \
         ".section .discard, \"a\", @progbits\n"                         \
         ".byte " alt_total_len "\n" /* total_len <= 255 */              \
@@ -101,7 +101,7 @@ extern void alternative_instructions(voi
 
 #define ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \
         OLDINSTR_2(oldinstr, 1, 2)                                      \
-        ".pushsection .altinstructions, \"a\", @progbits\n"             \
+        ".pushsection .altinstructions, \"aw\", @progbits\n"            \
         ALTINSTR_ENTRY(feature1, 1)                                     \
         ALTINSTR_ENTRY(feature2, 2)                                     \
         ".section .discard, \"a\", @progbits\n"                         \

Re: [PATCH] x86/alternatives: mark .altinstructions r/w

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 09:15:46AM +0200, Jan Beulich wrote:
> Forever since the introduction of the .priv member it has been wrong to
> record the section as r/o.
> 
> .alt_call_sites, otoh, is legitimately r/o, hence
> __alt_call_sites_{start,end}[] would better reflect that.

Right, we don't however have a .init.rodata section, and hence it gets
placed in the .init.data section which is r/w.  No objection about the
addition of const, it's the right thing.

> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger@xenproject.org>

I think you want:

Fixes: 4008c71d7af2 ("x86/alt: Support for automatic padding calculations")

Thanks, Roger.

Re: [PATCH] x86/alternatives: mark .altinstructions r/w

[Jan Beulich]

On 02.09.2026 09:27, Roger Pau Monné wrote:
> On Wed, Sep 02, 2026 at 09:15:46AM +0200, Jan Beulich wrote:
>> Forever since the introduction of the .priv member it has been wrong to
>> record the section as r/o.
>>
>> .alt_call_sites, otoh, is legitimately r/o, hence
>> __alt_call_sites_{start,end}[] would better reflect that.
> 
> Right, we don't however have a .init.rodata section, and hence it gets
> placed in the .init.data section which is r/w.  No objection about the
> addition of const, it's the right thing.
> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks.

> I think you want:
> 
> Fixes: 4008c71d7af2 ("x86/alt: Support for automatic padding calculations")

I was pondering to add this, but then decided against (it's an inconsistency,
not really a bug). Since you ask for it, let me add it then.

Jan

Re: [PATCH] x86/alternatives: mark .altinstructions r/w

[Roger Pau Monné]

On Wed, Sep 02, 2026 at 10:31:56AM +0200, Jan Beulich wrote:
> On 02.09.2026 09:27, Roger Pau Monné wrote:
> > On Wed, Sep 02, 2026 at 09:15:46AM +0200, Jan Beulich wrote:
> >> Forever since the introduction of the .priv member it has been wrong to
> >> record the section as r/o.
> >>
> >> .alt_call_sites, otoh, is legitimately r/o, hence
> >> __alt_call_sites_{start,end}[] would better reflect that.
> > 
> > Right, we don't however have a .init.rodata section, and hence it gets
> > placed in the .init.data section which is r/w.  No objection about the
> > addition of const, it's the right thing.
> > 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Acked-by: Roger Pau Monné <roger@xenproject.org>
> 
> Thanks.
> 
> > I think you want:
> > 
> > Fixes: 4008c71d7af2 ("x86/alt: Support for automatic padding calculations")
> 
> I was pondering to add this, but then decided against (it's an inconsistency,
> not really a bug). Since you ask for it, let me add it then.

I agree it's not a functional bug, but since the commit message
mentions "it has been wrong", I was assuming we might want to signal
when the inconsistency was introduced.  No strong opinion really.

Thanks, Roger.