Before adding a new vCPU operation to register the secondary time area
by guest-physical address, add code to actually keep such areas up-to-
date.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: Pages aren't marked dirty when written to (matching the handling of
     space mapped by map_vcpu_info() afaict), on the basis that the
     registrations are lost anyway across migration. Plus the contents
     of the areas in question have to be deemed volatile in the first
     place (so saving a "most recent" value is pretty meaningless even
     for e.g. snapshotting).

--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1462,12 +1462,34 @@ static void __update_vcpu_system_time(st
         v->arch.pv.pending_system_time = _u;
 }
 
+static void write_time_guest_area(struct vcpu_time_info *map,
+                                  const struct vcpu_time_info *src)
+{
+    /* 1. Update userspace version. */
+    write_atomic(&map->version, src->version);
+    smp_wmb();
+
+    /* 2. Update all other userspace fields. */
+    *map = *src;
+
+    /* 3. Update userspace version again. */
+    smp_wmb();
+    write_atomic(&map->version, version_update_end(src->version));
+}
+
 bool update_secondary_system_time(struct vcpu *v,
                                   struct vcpu_time_info *u)
 {
     XEN_GUEST_HANDLE(vcpu_time_info_t) user_u = v->arch.time_info_guest;
+    struct vcpu_time_info *map = v->arch.time_guest_area.map;
     struct guest_memory_policy policy = { .nested_guest_mode = false };
 
+    if ( map )
+    {
+        write_time_guest_area(map, u);
+        return true;
+    }
+
     if ( guest_handle_is_null(user_u) )
         return true;

On 19/10/2022 8:41 am, Jan Beulich wrote:
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1462,12 +1462,34 @@ static void __update_vcpu_system_time(st
>          v->arch.pv.pending_system_time = _u;
>  }
>  
> +static void write_time_guest_area(struct vcpu_time_info *map,
> +                                  const struct vcpu_time_info *src)
> +{
> +    /* 1. Update userspace version. */
> +    write_atomic(&map->version, src->version);

version_update_begin()

~Andrew

> +    smp_wmb();
> +
> +    /* 2. Update all other userspace fields. */
> +    *map = *src;
> +
> +    /* 3. Update userspace version again. */
> +    smp_wmb();
> +    write_atomic(&map->version, version_update_end(src->version));
> +}
> +
>  bool update_secondary_system_time(struct vcpu *v,
>                                    struct vcpu_time_info *u)
>  {
>      XEN_GUEST_HANDLE(vcpu_time_info_t) user_u = v->arch.time_info_guest;
> +    struct vcpu_time_info *map = v->arch.time_guest_area.map;
>      struct guest_memory_policy policy = { .nested_guest_mode = false };
>  
> +    if ( map )
> +    {
> +        write_time_guest_area(map, u);
> +        return true;
> +    }
> +
>      if ( guest_handle_is_null(user_u) )
>          return true;
>  
>

On 17.01.2023 21:31, Andrew Cooper wrote:
> On 19/10/2022 8:41 am, Jan Beulich wrote:
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -1462,12 +1462,34 @@ static void __update_vcpu_system_time(st
>>          v->arch.pv.pending_system_time = _u;
>>  }
>>  
>> +static void write_time_guest_area(struct vcpu_time_info *map,
>> +                                  const struct vcpu_time_info *src)
>> +{
>> +    /* 1. Update userspace version. */
>> +    write_atomic(&map->version, src->version);
> 
> version_update_begin()

Not really, no. src->version was already bumped, and the above is
the equivalent of

    /* 2. Update all other userspace fields. */
    __copy_to_guest(user_u, u, 1);

in pre-existing code (which also doesn't bump).

However, you point out a bug in patch 9: There I need to set the
version to ~0 between collect_time_info() and write_time_guest_area(),
to cover for the subsequent version_update_end(). (Using
version_update_begin() there wouldn't be correct, as
force_update_secondary_system_time() is used to first populate the
area, and we also shouldn't leave version at 2 once done, as that
might get in conflict with subsequent updates mirroring the version
from the "main" area.)

Jan