Re: [PATCH v6] xen: Expose the PMU to the guests

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 13/10/2021 08:45, Michal Orzel wrote:
> Add parameter vpmu to xl domain configuration syntax
> to enable the access to PMU registers by disabling
> the PMU traps(currently only for ARM).
>
> The current status is that the PMU registers are not
> virtualized and the physical registers are directly
> accessible when this parameter is enabled. There is no
> interrupt support and Xen will not save/restore the
> register values on context switches.
>
> According to Arm Arm, section D7.1:
> "The Performance Monitors Extension is common
> to AArch64 operation and AArch32 operation."
> That means we have an ensurance that if PMU is
> present in one exception state, it must also be
> present in the other.
>
> Please note that this feature is experimental.
>
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> Signed-off-by: Julien Grall <julien@xxxxxxx>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> Acked-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
> Changes since v5:
> -extracted from the series as other patches have been merged
> -fold ARM vPMU feature status in SUPPORT.md into existing section for x86
> -fix coding style in libxl
> Changes since v4:
> -update SUPPORT.md
> -move vpmu_is_available assignment after the secondary
> CPUs have been brought up
> -introduce masks for flags that need to be set and
> for flags that are optional
> Changes since v3:
> -fail if vpmu is set but not supported
> Changes since v2:
> -remove redundant check from x86 code
> -do not define bit position and mask separately
> Changes since v1:
> -modify vpmu parameter to be common rather than arch specific
> ---
>   SUPPORT.md                           | 12 ++++++++++--
>   docs/man/xl.cfg.5.pod.in             | 17 +++++++++++++++++
>   tools/golang/xenlight/helpers.gen.go |  6 ++++++
>   tools/golang/xenlight/types.gen.go   |  1 +
>   tools/include/libxl.h                |  6 ++++++
>   tools/libs/light/libxl_create.c      | 10 ++++++++++
>   tools/libs/light/libxl_types.idl     |  2 ++
>   tools/ocaml/libs/xc/xenctrl.ml       |  1 +
>   tools/ocaml/libs/xc/xenctrl.mli      |  1 +
>   tools/xl/xl_parse.c                  |  2 ++
>   xen/arch/arm/domain.c                | 13 ++++++++++---
>   xen/arch/arm/setup.c                 |  3 +++
>   xen/common/domain.c                  | 10 +++++++++-
>   xen/include/asm-arm/domain.h         |  1 +
>   xen/include/public/domctl.h          |  4 +++-
>   15 files changed, 82 insertions(+), 7 deletions(-)
>
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 317392d8f3..d0305abc0b 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -671,11 +671,19 @@ such as KVM, Hyper-V, Bromium, and so on as guests.
>   
>   ### vPMU
>   
> -Virtual Performance Management Unit for HVM guests
> +Virtual Performance Management Unit
While I agree that "HVM guests" doesn't make any sense on Arm, you are 
not adding back for x86. So this is changing the statement support to 
both HVM and PV guest.
I don't know why only HVM guests was previously mentionned. Jan, Andrew? 
Anyway, for this patch, I think we should add...
>   
>       Status, x86: Supported, Not security supported
... add HVM here. Looking at other section, we seem to use:

Status, x86 HVM: ...

One can send a follow-up patch if we want to mention the support status 
for PV guests.
Cheers,

--
Julien Grall