Re: [PATCH] Remove Powerpc OProfile events the kernel will reject

On Mon, 2015-11-16 at 14:46 -0500, William Cohen wrote:
> On 11/13/2015 03:27 PM, Carl E. Love wrote:
> > 
> > 
> > Will:
> > 
> > I have created a patch to remove the Power 8 events that the perf kernel code will reject.  This is a patch
> > to address the issue of the PM_L2_LD_MISS event not working as reported by Chencheng Ye on November 5.  
> > 
> > I don't know why the design had the setup register only writable by the Hypervisor.  I also checked and as far
> > as I could find out there isn't any work being done at the moment to create the needed API to allow the kernel 
> > to request the Hypervisor write the needed bits into the configuration register.  So, we have no option at the 
> > moment but to remove the events.
> > 
> > I actually just commented them out with a detailed explanation in the hope that we will be able to re-enable them
> > in the future.  Please let me know if you have any feedback on the patch.  Thanks for your time.
> > 
> >                          Carl Love
> 
> Hi Carl,
> 
> I took a look at this patch today.  How was the list of events to disabled determined?  Looking at the power8-pmu.c file is the following correct to find the events setting the caching caching bit (look at events that have unit 6,7,8, or 9 and check for non-zero ?
> 
>   egrep "event:0x[1-9a-fA-F][0-9a-fA-F][6789][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F] " events/ppc64/power8/events |more
> 
> Are the following with unit=9 okay? They don't seem to be comments out by the patch:
> 
> $ egrep "event:0x[1-9a-fA-F][0-9a-fA-F][6789][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F] " events/ppc64/power8/events |grep -v "##"
> event:0x819082 counters:0 um:zero minimum:10000 name:PM_L3_CI_USAGE : rotating sample of 16 CI or CO actives
> event:0x83908b counters:2 um:zero minimum:10000 name:PM_L3_CO0_ALLOC : 0.0
> event:0x83908a counters:2 um:zero minimum:10000 name:PM_L3_CO0_BUSY : lifetime, sample of CO machine 0 valid
> event:0x84908d counters:3 um:zero minimum:10000 name:PM_L3_PF0_ALLOC : 0.0
> event:0x84908c counters:3 um:zero minimum:10000 name:PM_L3_PF0_BUSY : lifetime, sample of PF machine 0 valid
> event:0x829084 counters:1 um:zero minimum:10000 name:PM_L3_PF_USAGE : rotating sample of 32 PF actives
> event:0x84908f counters:3 um:zero minimum:10000 name:PM_L3_RD0_ALLOC : 0.0
> event:0x84908e counters:3 um:zero minimum:10000 name:PM_L3_RD0_BUSY : lifetime, sample of RD machine 0 valid
> event:0x829086 counters:1 um:zero minimum:10000 name:PM_L3_RD_USAGE : rotating sample of 16 RD actives
> event:0x839089 counters:2 um:zero minimum:10000 name:PM_L3_SN0_ALLOC : 0.0
> event:0x839088 counters:2 um:zero minimum:10000 name:PM_L3_SN0_BUSY : lifetime, sample of snooper machine 0 valid
> event:0x819080 counters:0 um:zero minimum:10000 name:PM_L3_SN_USAGE : rotating sample of 8 snoop valids
> 
> 
> -Will

Will and Michael:

If you look at a recent kernel has the following code in
linux-4.3/arch/powerpc/perf/power8-pmu.c 



at line 284:

static int power8_get_constraint(u64 event, unsigned long *maskp, unsigned long\
 *valp)
{
        unsigned int unit, pmc, cache, ebb;
        unsigned long mask, value;

        mask = value = 0;

        if (event & ~EVENT_VALID_MASK)
                return -1;

        pmc   = (event >> EVENT_PMC_SHIFT)        & EVENT_PMC_MASK;
        unit  = (event >> EVENT_UNIT_SHIFT)       & EVENT_UNIT_MASK;
        cache = (event >> EVENT_CACHE_SEL_SHIFT)  & EVENT_CACHE_SEL_MASK;
        ebb   = (event >> EVENT_EBB_SHIFT)        & EVENT_EBB_MASK;

        if (pmc) {

The pmc, the unit for the event and the cache setting bits are extracted from the event. The cache
are bits [23:20] of the event, the unit is bits [15:12] of the event as described in the comment at
line 74.

Then at about line 326 we have the code:

        if (unit >= 6 && unit <= 9) {
 	        /*                                                              
                 * L2/L3 events contain a cache selector field, which is        
                 * supposed to be programmed into MMCRC. However MMCRC is only  
                 * HV writable, and there is no API for guest kernels to modify
                 * it. The solution is for the hypervisor to initialise the     
                 * field to zeroes, and for us to only ever allow events that   
                 * have a cache selector of zero. The bank selector (bit 3) is  
                 * irrelevant, as long as the rest of the value is 0.           
                 */
                if (cache & 0x7)
                        return -1;

Which says any event having unit 6, 7, 8 or 9 with the lower three bits of the cache bits that
are not zero will be rejected.  Unfortunately, there is no check for the kernel running as the
hypervisor. So, it doesn't matter what hypervisor is in use or if the system is not virtualized,
the kernel will reject the events.

So, the events you mentioned Will, are not rejected. The comment seems to indicate that the bank
selector bit, the 8, in the events you mentioned is irrelevant.

                     Carl Love


------------------------------------------------------------------------------
Presto, an open source distributed SQL query engine for big data, initially
developed by Facebook, enables you to easily query your data on Hadoop in a 
more interactive manner. Teradata is also now providing full enterprise
support for Presto. Download a free open source copy now.
http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140