Re: [PATCH] Correct system time calculation

["Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>]

On 12/11/2025 09:53, Owen Smith wrote:
> Yes, x86 doesnt define UnsignedMultiplyExtract128
> I'll make the following change, so x86 still compiles, even though x86 
> Windows should be phased out, now that Win10 is end-of-life.
> 
> #ifdef _X86_
>      Age = (Age * TscSystemMul) >> 32;
> #else
>      Age = UnsignedMultiplyExtract128(Age, TscSystemMul, 32);
> #endif
> 
> Owen

On x86, it should be possible to perform the calculation at 96-bit 
precision for the sake of time sync accuracy (totally untested):

void __cdecl ConvertTimestamp(
     UINT64          *Output,
     const UINT64    *Multiplicand,
     ULONG           Multiplier
     )
{
     __asm {
         ; assuming Multiplicand = (A<<32)+B, Multiplier = X
         mov eax, dword ptr [Multiplicand]
         mul Multiplier
         mov ecx, edx            ; ecx = C = (B*X)>>32
         mov eax, dword ptr [Multiplicand + 4]
         mul Multiplier
         add ecx, eax            ; ecx = C + ((A*X)&((1<<32)-1))
         adc edx, 0              ; accumulate lower carry into (A*X)>>32
         mov [Output], ecx
         mov [Output + 4], edx
     }
}

Though IMO x64 should stick to UnsignedMultiplyExtract128.

> 
> ________________________________________
> From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> Sent: 11 November 2025 5:19 PM
> To: Owen Smith; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH] Correct system time calculation
> 
> On 11/11/2025 15:10, Owen Smith wrote:
>> UnsignedMultiplyExtract128 is not available in the 22000 EWDK
>>
>> Owen
>>
> 
> I managed to build it with EWDK_fe_release_20348_210507-1500.iso. Are
> you building for 32-bit x86?
> 
>> ________________________________________
>> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> on behalf of 
>> Owen Smith <owen.smith@xxxxxxxxxx>
>> Sent: 11 November 2025 8:53 AM
>> To: Tu Dinh; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH] Correct system time calculation
>>
>> Interestingly, xen.h defines the time calculation in a comment, but does not 
>> account for tsc_shift being a signed int8_t value.
>>
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/include/public/xen.h;h=82b9c05a76b7faedded8778fb8274a0d3d5d31e4;hb=HEAD#l677
>>
>> Reviewed-by: Owen Smith <owen.smith@xxxxxxxxxx>
>>
>> ________________________________________
>> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> on behalf of 
>> Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
>> Sent: 10 November 2025 7:13 PM
>> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
>> Cc: Tu Dinh
>> Subject: [PATCH] Correct system time calculation
>>
>> SharedInfoGetTime uses the naive formula ((Tsc << TscShift) *
>> TscSystemMul) >> 32 to calculate the current system time. However, there
>> are two issues: firstly, TscShift may be negative (to indicate a right
>> shift); and secondly, multiplying the shifted Tsc with TscSystemMul will
>> throw away the result's upper 64 bits.
>>
>> Adjust for negative shift amounts and use UnsignedMultiplyExtract128 to
>> correctly perform the multiplication.
>>
>> Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
>> ---
>>    src/xenbus/shared_info.c | 10 ++++++++--
>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/xenbus/shared_info.c b/src/xenbus/shared_info.c
>> index 06e0aec..fe29470 100644
>> --- a/src/xenbus/shared_info.c
>> +++ b/src/xenbus/shared_info.c
>> @@ -356,6 +356,7 @@ SharedInfoGetTime(
>>        ULONGLONG                       NanoSeconds;
>>        ULONGLONG                       Timestamp;
>>        ULONGLONG                       Tsc;
>> +    ULONGLONG                       Age;
>>        ULONGLONG                       SystemTime;
>>        ULONG                           TscSystemMul;
>>        CHAR                            TscShift;
>> @@ -404,10 +405,15 @@ SharedInfoGetTime(
>>        KeLowerIrql(Irql);
>>
>>        // Number of elapsed ticks since timestamp was captured
>> -    Tsc -= Timestamp;
>> +    Age = Tsc - Timestamp;
>> +    if (TscShift < 0)
>> +        Age >>= -TscShift;
>> +    else
>> +        Age <<= TscShift;
>> +    Age = UnsignedMultiplyExtract128(Age, TscSystemMul, 32);
>>
>>        // Time in nanoseconds since boot
>> -    SystemTime += ((Tsc << TscShift) * TscSystemMul) >> 32;
>> +    SystemTime += Age;
>>
>>        Trace("WALLCLOCK TIME AT BOOT: Seconds = %llu NanoSeconds = %llu\n",
>>              Seconds,
>> --
>> 2.51.0.windows.2
>>
>>
>>
>> --
>> Ngoc Tu Dinh | Vates XCP-ng Developer
>>
>> XCP-ng & Xen Orchestra - Vates solutions
>>
>> web: https://vates.tech
>>
>>
>>
>>
> 
> 
> 
> --
> Ngoc Tu Dinh | Vates XCP-ng Developer
> 
> XCP-ng & Xen Orchestra - Vates solutions
> 
> web: https://vates.tech
> 
> 



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech