Re: git: c1e813d12309 - main - hwpmc: Correct selection of Intel fixed counters.

On Mon, May 30, 2022 at 7:05 PM Alexander Motin <mav@freebsd.org> wrote:
>
> The branch main has been updated by mav:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=c1e813d1230915e19a236ec687cadc1051841e56
>
> commit c1e813d1230915e19a236ec687cadc1051841e56
> Author:     Alexander Motin <mav@FreeBSD.org>
> AuthorDate: 2022-05-30 23:46:48 +0000
> Commit:     Alexander Motin <mav@FreeBSD.org>
> CommitDate: 2022-05-31 00:05:15 +0000
>
>     hwpmc: Correct selection of Intel fixed counters.
>
>     Intel json's use event=0 to specify fixed counter number via umask.
>     Alternatively fixed counters have equivalent programmable event/umask.
>
>     MFC after:      1 month
> ---
>  sys/dev/hwpmc/hwpmc_core.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/sys/dev/hwpmc/hwpmc_core.c b/sys/dev/hwpmc/hwpmc_core.c
> index fce97cbd5b8e..73cfee0b3975 100644
> --- a/sys/dev/hwpmc/hwpmc_core.c
> +++ b/sys/dev/hwpmc/hwpmc_core.c
> @@ -245,15 +245,31 @@ iaf_allocate_pmc(int cpu, int ri, struct pmc *pm,
>         ev = IAP_EVSEL_GET(config);
>         umask = IAP_UMASK_GET(config);
>
> -       /* INST_RETIRED.ANY */
> -       if (ev == 0xC0 && ri != 0)
> -               return (EINVAL);
> -       /* CPU_CLK_UNHALTED.THREAD */
> -       if (ev == 0x3C && ri != 1)
> -               return (EINVAL);
> -       /* CPU_CLK_UNHALTED.REF */
> -       if (ev == 0x0 && umask == 0x3 && ri != 2)
> -               return (EINVAL);
> +       if (ev == 0x0) {
> +               if (umask != ri + 1)
> +                       return (EINVAL);

Hey Alexander,

This seems to break system-mode sampling of fixed-mode counters; e.g.,
from the manpage, `pmcstat -S instructions`, and I'm not at all
familiar with these parts of pmc. We come in once with umask=1, ri=0;
then again with umask=1, ri=1. The latter fails and we try umask=1,
ri=2, which again fails, and the PMCALLOCATE op ultimately fails.

Is `umask != ri + 1` correct, or should this be reverted back to
`umask == 0x3 && ri != 2` or something else? I've no idea what the
`umask` values represent in this context.

Thanks,

Kyle Evans