Stephane,
thanks for your comments.
On 27.03.08 22:59:29, stephane eranian wrote:
> > -/*
> > - * 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.
You are right, for IbsOpCtl and IbsFetchCtl there are only the
IbsFetchMaxCnt and IbsOpMaxCnt values to set. Additionally there is an
option flag in IbsFetchCtl that enables randomization (IbsRandEn). For
parameter passing we have to use a struct. Instead of having to
different structures (the input arguments and the PMD registers as in
pfmlib_ita2_ear_t and pfm_ita2_pmc_reg_t), I decided to have one
structure for both and use the IBS register layout to pass
parameters. So, I found it easier to simply pass a control register
value as input parameter. Sure, the user has to know that to setup
here, but this should not be hard to do for whom being familiar with
IBS. Do you think it would be easier to have only a subset of those
parameters needed for configuration (currently this are
ibsfetchmaxcnt, ibsopmaxcnt and ibsranden)?
> > +/* 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.
That exactly is the problem, the dispatch function sets up new PMCs
and PMDs, but it is not clear which register of pfp_pmds[] is used for
IBS fetch and op. There is a reordering required because of the
counter 1:1 mapping between PMCs and PMDs. Thus, the index of the IBS
PMDs may vary and this information can be found in the base values.
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
email: [EMAIL PROTECTED]
-------------------------------------------------------------------------
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
