On 08.10.2025 18:33, Andrew Cooper wrote: > On 25/09/2025 11:46 am, Jan Beulich wrote: >> --- a/xen/arch/x86/copy_page.S >> +++ b/xen/arch/x86/copy_page.S >> @@ -57,6 +57,6 @@ END(copy_page_cold) >> .endm >> >> FUNC(copy_page_hot) >> - ALTERNATIVE copy_page_movsq, copy_page_movsb, X86_FEATURE_ERMS >> + ALTERNATIVE copy_page_movsq, copy_page_movsb, >> X86_FEATURE_XEN_REP_MOVSB >> RET >> END(copy_page_hot) > > Hmm. > > Overall I think this patch is an improvement. > > But, for any copy_page variants, we know both pointers are 4k aligned, > so will not tickle the problem case.
Then I fear I still haven't understood the bad "may overlap" condition. I thought that with the low 12 bits all identical in a page-copy, this case would _specifically_ trigger the bad behavior. > This does mess with the naming of the synthetic feature. Short of better naming suggestions, I would keep it as is. >> --- a/xen/arch/x86/cpu/amd.c >> +++ b/xen/arch/x86/cpu/amd.c >> @@ -1386,6 +1386,10 @@ static void cf_check init_amd(struct cpu >> >> check_syscfg_dram_mod_en(); >> >> + if (c == &boot_cpu_data && cpu_has(c, X86_FEATURE_ERMS) >> + && c->family != 0x19 /* Zen3/4 */) > > Even if this is Linux style, && on the previous line please. No, precisely because it is Linux style. If and when we change the file to Xen style (which probably we should), such operators would move. Jan
