linux-kernel - Re: [PATCH 2/4] perf, x86: Report TSX transaction abort cost as weight

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130813143517.GT19750@two.firstfloor.org>
Date:	Tue, 13 Aug 2013 16:35:17 +0200
From:	Andi Kleen <andi@...stfloor.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Andi Kleen <andi@...stfloor.org>, mingo@...nel.org,
	linux-kernel@...r.kernel.org, acme@...radead.org, jolsa@...hat.com,
	eranian@...gle.com, Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 2/4] perf, x86: Report TSX transaction abort cost as
 weight

> How about something like the below instead? I didn't copy the !fll test
> because I couldn't find why that was. Section 18.10.5.1 (Aug 2012)

!fll is so that if a memory weight is requested we don't overwrite it.

>  	u64 status, dla, dse, lat;
>  };
>  
> -/*
> - * Same as pebs_record_nhm, with two additional fields.
> - */
>  struct pebs_record_hsw {
> -	struct pebs_record_nhm nhm;
> -	/*
> -	 * Real IP of the event. In the Intel documentation this
> -	 * is called eventingrip.
> -	 */
> -	u64 real_ip;
> -	/*
> -	 * TSX tuning information field: abort cycles and abort flags.
> -	 */
> -	u64 tsx_tuning;
> +	u64 flags, ip;
> +	u64 ax, bx, cx, dx;
> +	u64 si, di, bp, sp;
> +	u64 r8,  r9,  r10, r11;
> +	u64 r12, r13, r14, r15;
> +	u64 status, dla, dse, lat;

Seems like an unrelated change.

> +	u64 real_ip; /* the actual eventing ip */
> +	u64 tsx_tuning; /* TSX abort cycles and flags */
>  };
>  
>  void init_debug_store_on_cpu(int cpu)
> @@ -759,16 +754,41 @@ static int intel_pmu_pebs_fixup_ip(struct pt_regs *regs)
>  	return 0;
>  }
>  
> +union hsw_tsx_tuning {
> +	struct {
> +		u64 cycles_last_block : 32,
> +		    hle_abort         : 1,
> +		    rtm_abort         : 1,
> +		    ins_abort         : 1,
> +		    non_ins_abort     : 1,
> +		    retry             : 1,
> +		    mem_data_conflict : 1,
> +		    capacity          : 1;

I think you used an old SDM for this, there were some changes in the
latest.

This would break my next patch which copies the abort bits into 
a new field (well it would need an union at least) 

https://git.kernel.org/cgit/linux/kernel/git/ak/linux-misc.git/commit/?h=hsw/pmu7&id=a88a029a6b3cb95148452584c93cbb4004f77f28

Other than that it seems ok and would likely generate the same
code as mine. I prefer mine as it's simpler (I don't think there
is anything in the kernel that needs to look at the individual bits,
they should be just reported together)

-Andi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/