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

[Julien Grall <julien@xxxxxxx>]

Hi Ian,

On 13/10/2021 13:49, Ian Jackson wrote:
> Michal Orzel writes ("[PATCH v7] xen: Expose the PMU to the guests"):
> > 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.
> ...
> > Changes since v6:
> > -fix missing "HVM" for vPMU entry in SUPPORT.md
> The SUPPORT.md changes LGTM, thanks.
>
> > -Virtual Performance Management Unit for HVM guests
> > +Virtual Performance Management Unit
> >   
> > -    Status, x86: Supported, Not security supported
> > +    Status, x86 HVM: Supported, Not security supported
> > +    Status, ARM: Experimental
> > +
> > +On ARM, support for accessing PMU registers from the guests.
> > +There is no interrupt support and Xen will not save/restore
> > +the register values on context switches.
> FTAOD ISTM that this limitation makes the feature very hard to use
> successfully on ARM.  You would need to pin vcpus to dedicated
> pcpus ?
Yes and this is not really the only restriction for the feature. You 
can't even use the PMU properly with an out-of-box domain... But a few 
stakeholders seem to be happy with this hackyness for now.
> I think this is fine for an experimental feature.
>
> I think this patch needs a ARM/hypervisor acks still.
I am in the signed-off-by list. Even if the patch has changed compare 
the original, I feel it is odd to ack my own patch.
From my understanding, my signed-off-by is sufficient serve as an 
approval for the maintainer part. We also have a review form a person of 
a suitable stature in the community (Bertrand). So I think in term of 
approval we are good.
Additionally, from a discussion yesterday on IRC, Stefano was happy with 
this patch (I was the one requesting the resend for SUPPORT.MD). So I 
think we can commit it now.
Let me know if you prefer to wait for a formal Ack from Stefano.

Cheers,

--
Julien Grall