| Message ID | 60fb1103-aef3-40dd-afd0-44f594753165@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: use alternative_input() in cache_flush() | expand |
On 30/09/2024 4:09 pm, Jan Beulich wrote: > There's no point using alternative_io() when there are no outputs. > > No functional change. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/flushtlb.c > +++ b/xen/arch/x86/flushtlb.c > @@ -286,11 +286,10 @@ void cache_flush(const void *addr, unsig > * + prefix than a clflush + nop, and hence the prefix is added instead > * of letting the alternative framework fill the gap by appending nops. > */ > - alternative_io("ds; clflush %[p]", > - "data16 clflush %[p]", /* clflushopt */ > - X86_FEATURE_CLFLUSHOPT, > - /* no outputs */, > - [p] "m" (*(const char *)(addr))); > + alternative_input("ds; clflush %[p]", Drop the ; as you're altering the line anyway? Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> However, switching to ds ds will avoid a trailing nop, so will be marginally better. > + "data16 clflush %[p]", /* clflushopt */ > + X86_FEATURE_CLFLUSHOPT, > + [p] "m" (*(const char *)(addr))); > } > > alternative_2("",
On 30.09.2024 17:27, Andrew Cooper wrote: > On 30/09/2024 4:09 pm, Jan Beulich wrote: >> There's no point using alternative_io() when there are no outputs. >> >> No functional change. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/flushtlb.c >> +++ b/xen/arch/x86/flushtlb.c >> @@ -286,11 +286,10 @@ void cache_flush(const void *addr, unsig >> * + prefix than a clflush + nop, and hence the prefix is added instead >> * of letting the alternative framework fill the gap by appending nops. >> */ >> - alternative_io("ds; clflush %[p]", >> - "data16 clflush %[p]", /* clflushopt */ >> - X86_FEATURE_CLFLUSHOPT, >> - /* no outputs */, >> - [p] "m" (*(const char *)(addr))); >> + alternative_input("ds; clflush %[p]", > > Drop the ; as you're altering the line anyway? Oh, sure. > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks. > However, switching to ds ds will avoid a trailing nop, so will be > marginally better. I don't think I understand, which may or may not be due to me not being sure whether "ds ds" is a typo. What trailing NOP are you seeing? The "ds" that's there already covers for the sole trailing NOP that would otherwise result due to the "data16" prefix. Jan
On 30/09/2024 4:39 pm, Jan Beulich wrote: > On 30.09.2024 17:27, Andrew Cooper wrote: >> On 30/09/2024 4:09 pm, Jan Beulich wrote: >>> There's no point using alternative_io() when there are no outputs. >>> >>> No functional change. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> >>> --- a/xen/arch/x86/flushtlb.c >>> +++ b/xen/arch/x86/flushtlb.c >>> @@ -286,11 +286,10 @@ void cache_flush(const void *addr, unsig >>> * + prefix than a clflush + nop, and hence the prefix is added instead >>> * of letting the alternative framework fill the gap by appending nops. >>> */ >>> - alternative_io("ds; clflush %[p]", >>> - "data16 clflush %[p]", /* clflushopt */ >>> - X86_FEATURE_CLFLUSHOPT, >>> - /* no outputs */, >>> - [p] "m" (*(const char *)(addr))); >>> + alternative_input("ds; clflush %[p]", >> Drop the ; as you're altering the line anyway? > Oh, sure. > >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > Thanks. > >> However, switching to ds ds will avoid a trailing nop, so will be >> marginally better. > I don't think I understand, which may or may not be due to me not being > sure whether "ds ds" is a typo. What trailing NOP are you seeing? The > "ds" that's there already covers for the sole trailing NOP that would > otherwise result due to the "data16" prefix. Oh of course. I can't count. Sorry for the noise. ~Andrew
--- a/xen/arch/x86/flushtlb.c +++ b/xen/arch/x86/flushtlb.c @@ -286,11 +286,10 @@ void cache_flush(const void *addr, unsig * + prefix than a clflush + nop, and hence the prefix is added instead * of letting the alternative framework fill the gap by appending nops. */ - alternative_io("ds; clflush %[p]", - "data16 clflush %[p]", /* clflushopt */ - X86_FEATURE_CLFLUSHOPT, - /* no outputs */, - [p] "m" (*(const char *)(addr))); + alternative_input("ds; clflush %[p]", + "data16 clflush %[p]", /* clflushopt */ + X86_FEATURE_CLFLUSHOPT, + [p] "m" (*(const char *)(addr))); } alternative_2("",
There's no point using alternative_io() when there are no outputs. No functional change. Signed-off-by: Jan Beulich <jbeulich@suse.com>