On 27.09.2023 17:24, Roger Pau Monné wrote:
> On Wed, May 03, 2023 at 05:57:40PM +0200, Jan Beulich wrote:
>> The registration by virtual/linear address has downsides: At least on
>> x86 the access is expensive for HVM/PVH domains. Furthermore for 64-bit
>> PV domains the area is inaccessible (and hence cannot be updated by Xen)
>> when in guest-user mode.
>>
>> Introduce a new vCPU operation allowing to register the runstate area by
>> guest-physical address.
>>
>> An at least theoretical downside to using physically registered areas is
>> that PV then won't see dirty (and perhaps also accessed) bits set in its
>> respective page table entries.
>>
>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> Reviewed-by: Roger Pau Monné <roger....@citrix.com>
Thanks.
> One comment below.
>
>> --- a/xen/include/public/vcpu.h
>> +++ b/xen/include/public/vcpu.h
>> @@ -221,6 +221,19 @@ struct vcpu_register_time_memory_area {
>> typedef struct vcpu_register_time_memory_area
>> vcpu_register_time_memory_area_t;
>> DEFINE_XEN_GUEST_HANDLE(vcpu_register_time_memory_area_t);
>>
>> +/*
>> + * Like the respective VCPUOP_register_*_memory_area, just using the
>> "addr.p"
>> + * field of the supplied struct as a guest physical address (i.e. in GFN
>> space).
>> + * The respective area may not cross a page boundary. Pass ~0 to
>> unregister an
>> + * area. Note that as long as an area is registered by physical address,
>> the
>> + * linear address based area will not be serviced (updated) by the
>> hypervisor.
>> + *
>> + * Note that the area registered via VCPUOP_register_runstate_memory_area
>> will
>> + * be updated in the same manner as the one registered via virtual address
>> PLUS
>> + * VMASST_TYPE_runstate_update_flag engaged by the domain.
>> + */
>> +#define VCPUOP_register_runstate_phys_area 14
>
> Just to make it more obvious, it might be nice to add a note in the
> comment on VCPUOP_register_runstate_memory_area that `p` can also be
> used with the `VCPUOP_register_runstate_phys_area` hypercall.
I can add a reference there (and then in the later patch also for the
time area), but I don't think it wants or needs to emphasize p. It
merely needs to point to the alternative sub-function imo.
Jan
