Robert,

Some comments on this patch.


>  -/*
>  - * AMD64 specific parameters for the library
>  - */
>   typedef struct {
>  -       pfmlib_amd64_counter_t  pfp_amd64_counters[PMU_AMD64_NUM_COUNTERS];  
>    /* extended counter features */
>  -       uint64_t                        reserved[4];            /* for 
> future use */
>  +       pfmlib_amd64_counter_t pfp_amd64_counters[PMU_AMD64_MAX_COUNTERS]; 
> /* extended counter features */
>  +       uint32_t        flags;          /* use flags */
>  +       uint32_t        reserved1;      /* for future use */
>  +       ibsfetchctl_t   ibsfetchctl;    /* IBS fetch control */
>  +       ibsopctl_t      ibsopctl;       /* IBS execution control */
>  +       uint64_t        reserved2;      /* for future use */
>   } pfmlib_amd64_input_param_t;
>

I don't understand why you need to pass the full ibsfetchctl and
ibsopctl structures.
There is actually only one field that is configurable: sampling period
(cnt). I do not
account for the enable bit which has to be set anyway. So why not just
pass 2 uint32_t
values for the periods of each component of IBS and let the library
internals deal with
the actual layout. This was done this way on Itanium, for instance.

>  +/* A bit mask, meaning multiple usage types may be defined */
>  +#define PFMLIB_AMD64_USE_IBSFETCH      1
>  +#define PFMLIB_AMD64_USE_IBSOP         2
>  +
>   typedef struct {
>  -       uint64_t        reserved[8];            /* for future use */
>  +       uint32_t        ibsfetch_base;  /* Perfmon2 base register index */
>  +       uint32_t        ibsop_base;     /* Perfmon2 base register index */
>  +       uint64_t        reserved[7];    /* for future use */
>   } pfmlib_amd64_output_param_t;

Could you explain the goal of the two bases? I was envisoning a scheme
where in dispatch_ibs()
you would simply add the list of PMDs that are used. It is okay to
have more PMDs that PMCs.
The key is that the PMDs corresponding to the events must be first AND
in the order the events
where given. After that PMDS can be passed in any order.

Thanks.

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
perfmon2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel

Reply via email to