Re: [PATCH 2/2] domain: expose newly introduced hypercalls as XENFEAT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/10/2023 10:13 am, Roger Pau Monne wrote:
> XENFEAT_runstate_phys_area is exposed to all architectures, while
> XENFEAT_vcpu_time_phys_area is currnelty only implemented for x86, and hence
> the feature flag is also only exposed on x86.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks for doing this.

> ---
>  CHANGELOG.md                  | 2 ++
>  xen/common/kernel.c           | 6 +++++-
>  xen/include/public/features.h | 9 +++++++++
>  3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index e33cf4e1b113..41da710426f6 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -31,6 +31,8 @@ The format is based on [Keep a 
> Changelog](https://keepachangelog.com/en/1.0.0/)
>   - Add Intel Hardware P-States (HWP) cpufreq driver.
>   - On Arm, experimental support for dynamic addition/removal of Xen device 
> tree
>     nodes using a device tree overlay binary (.dtbo).
> + - Introduce two new hypercalls to map the vCPU runstate and time areas by
> +   physical rather than linear addresses.

I'd suggest linear/virtual here.  Linear is the correct term in x86, but
virtual is the correct term in pretty much every other architecture.

> diff --git a/xen/include/public/features.h b/xen/include/public/features.h
> index d2a9175aae67..cffb2f14a562 100644
> --- a/xen/include/public/features.h
> +++ b/xen/include/public/features.h
> @@ -111,6 +111,15 @@
>  #define XENFEAT_not_direct_mapped         16
>  #define XENFEAT_direct_mapped             17
>  
> +/*
> + * Signal whether the hypervisor implements the following hypercalls:

This is not what the hypervisor implements.  It's what the domain is
permitted to use.

There also needs to be a matching patch to public/vcpu.h to require
implementations to check for these feature bits before using the new
hypercalls.

Also,

diff --git a/xen/common/domain.c b/xen/common/domain.c
index b8281d7cff9d..df994bd30fd2 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1998,6 +1998,10 @@ long common_vcpu_op(int cmd, struct vcpu *v
     {
         struct vcpu_register_runstate_memory_area area;
 
+        rc = -ENOSYS;
+        if ( 0 /* TODO: Dom's XENFEAT_runstate_phys_area setting */ )
+            break;
+
         rc = -EFAULT;
         if ( copy_from_guest(&area.addr.p, arg, 1) )
             break;

and a matching one for XENFEAT_vcpu_time_phys_area because I'm even more
serious about this becoming a domain controllable setting following what
OSSTest had to say overnight.

~Andrew