Re: [PATCH Xen 4.20] x86/apic: Fix asm() constraints in TMICT calculation

[Jan Beulich <jbeulich@suse.com>]

On 20.04.2026 19:46, Andrew Cooper wrote:
> The encoded MUL is 64 bits, so writes %rdx too.  At a minimum, this needs
> expressing as a clobber.

I'm embarrassed of missing this.

> Also fix a logical disconnect between 'overflow' being the carry flag not the
> overflow flag.  CF and OF are always the same for MUL instructions, so use the
> flag which matches the variable name.

I don't mind this too much, but the use of CF was deliberate: Imo OF is
relevant to signed arithmetic only, whereas CF is the flag to use with
unsigned operations.

> Fixes: d5c70a51bfbe ("x86/APIC: handle overflow in TMICT calculation")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

> I've kept this form because it produces best code generation for GCCs which
> support flag outputs.
> 
> An alternative would be to capture product_hi and check the nonzero-ness, as
> that's how OF/CF are produced in hardware, which would be better code
> generation on very old GCCs.

We could fit both, by further widening the use of ASM_FLAG_OUT().

> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -1317,9 +1317,10 @@ int reprogram_timer(s_time_t timeout)
>  
>          apic_tmict = UINT32_MAX;
>          asm ( "mul %[expire]\n\t"
> -              ASM_FLAG_OUT(, "setc %[cf]")
> -              : "=a" (product), [cf] ASM_FLAG_OUT("=@ccc", "=qm") (overflow)
> -              : "0" ((unsigned long)bus_scale), [expire] "r" (expire) );
> +              ASM_FLAG_OUT(, "seto %[of]")
> +              : "=a" (product), [of] ASM_FLAG_OUT("=@cco", "=qm") (overflow)

Noticing only now - for the non-flag-output case this should be "=&a". With

> +              : "0" ((unsigned long)bus_scale), [expire] "r" (expire)

... %rax also being an input, there's no risk of the compiler using the
register for the other input, but still. Would you mind making that adjustment
as well, while at it?

Jan