Re: [PATCH xen 2/2] xen: update system time immediately when VCPUOP_register_vcpu_info

[Dongli Zhang <dongli.zhang@xxxxxxxxxx>]

Hi Jan,

On 10/12/21 1:40 AM, Jan Beulich wrote:
> On 12.10.2021 09:24, Dongli Zhang wrote:
>> The guest may access the pv vcpu_time_info immediately after
>> VCPUOP_register_vcpu_info. This is to borrow the idea of
>> VCPUOP_register_vcpu_time_memory_area, where the
>> force_update_vcpu_system_time() is called immediately when the new memory
>> area is registered.
>>
>> Otherwise, we may observe clock drift at the VM side if the VM accesses
>> the clocksource immediately after VCPUOP_register_vcpu_info().
>>
>> Cc: Joe Jin <joe.jin@xxxxxxxxxx>
>> Signed-off-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>
> 
> While I agree with the change in principle, ...
> 
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1695,6 +1695,8 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, 
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>          rc = map_vcpu_info(v, info.mfn, info.offset);
>>          domain_unlock(d);
>>  
>> +        force_update_vcpu_system_time(v);
> 
> ... I'm afraid you're breaking the Arm build here. Arm will first need
> to gain this function.
> 

Since I am not familiar with the Xen ARM, would you please let me your
suggestion if I just leave ARM as TODO to the ARM developers to verify
and implement force_update_vcpu_system_time()?

I have tested that the below can build with arm64/aarch64.

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 40d67ec342..644c65ecd3 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1695,6 +1695,13 @@ long do_vcpu_op(int cmd, unsigned int vcpuid,
XEN_GUEST_HANDLE_PARAM(void) arg)
         rc = map_vcpu_info(v, info.mfn, info.offset);
         domain_unlock(d);

+#ifdef CONFIG_X86
+        /*
+         * TODO: ARM does not have force_update_vcpu_system_time().
+         */
+        force_update_vcpu_system_time(v);
+#endif
+
         break;
     }



Thank you very much!

Dongli Zhang