From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758550AbcBXS0y (ORCPT ); Wed, 24 Feb 2016 13:26:54 -0500 Received: from mail-bn1bon0085.outbound.protection.outlook.com ([157.56.111.85]:33885 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753926AbcBXS0w (ORCPT ); Wed, 24 Feb 2016 13:26:52 -0500 Authentication-Results: spf=none (sender IP is 165.204.84.222) smtp.mailfrom=amd.com; alien8.de; dkim=none (message not signed) header.d=none;alien8.de; dmarc=permerror action=none header.from=amd.com; X-WSS-ID: 0O32DWJ-08-D24-02 X-M-MSG: Subject: Re: [PATCH 4/4] x86/mce/AMD: Add comments for easier understanding To: Borislav Petkov References: <1455659111-32074-1-git-send-email-Aravind.Gopalakrishnan@amd.com> <1455659111-32074-5-git-send-email-Aravind.Gopalakrishnan@amd.com> <20160223123530.GB3673@pd.tnic> CC: , , , , , , , , , , , , , , From: Aravind Gopalakrishnan Message-ID: <56CDF5E4.4000206@amd.com> Date: Wed, 24 Feb 2016 12:26:44 -0600 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160223123530.GB3673@pd.tnic> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.180.168.240] X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:165.204.84.222;CTRY:US;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(428002)(189002)(164054003)(24454002)(479174004)(377454003)(199003)(11100500001)(2906002)(230700001)(4326007)(54356999)(65956001)(23676002)(65806001)(5008740100001)(5004730100002)(1096002)(1220700001)(83506001)(47776003)(3846002)(50466002)(586003)(59896002)(77096005)(6116002)(2950100001)(80316001)(106466001)(50986999)(76176999)(87936001)(92566002)(105586002)(87266999)(65816999)(33656002)(36756003)(86362001)(101416001)(189998001)(110136002);DIR:OUT;SFP:1101;SCL:1;SRVR:DM3PR12MB0860;H:atltwp02.amd.com;FPR:;SPF:None;MLV:sfv;MX:1;A:1;LANG:en; X-MS-Office365-Filtering-Correlation-Id: 819de1b7-9df9-4c18-b976-08d33d480f64 X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0860;2:n91z1KM5IQy8DWfjGHcuv3EWU51E24/rzjQuBrHWx3mEa3vHmeovdr/GHlYjz0O4NNyVv6gNgBUNYnsq8YM2c++2ME5QhpHEb1149gVrah/yNzFJ1XT0qm62PQXlUmxfRHHoDYsTrVCQEGxRdiP0VynnhIJhMHQIqUwG90qLJ/2PIdwVjH76jucAVJ9fp6hW;3:RD83KWMYToycNrbSqCLNZzr/G4eQaJegaf8cswVirhDRwtr+Iwd9GSpRVw/TD07sJjT8L4KDl1IvBtnaSn3t0+kFmzRCliFeR2dLpKRwtrXVy5l7SAJaa4yRcYpC/VqHi6QQPR7ZNsZOwSaAJN0SWwwyjvV9A1nbIrdZMsKwbRIVHCVs2XDcIwpGC7UhORoG9MnLKCH21+cmr5UY6wcGMvQqnTLhR+oLmLZlJ5Z4n7Q=;25:5c3wH+T2+zTD1BOMLNXZAsMaooIsuZ81QZ+yBl7xpc6bf24unPBLxBF4qH+FCPTtjR2eNKOe/+ydco8/a72WVKH+xDT97Hu3jpQfvWs5gnuuQz1ZIJbiAjMU6VrWk5/jbLznNx1VyEvIFJh4NRKPv398wTLGtiJYYpf1dfInn2+C5KeYOpEenvqRGXdbQXK+Xx53dXjDf+UVcC9PQ51GdtQ+Q67b+H8k9vDmt204jbz08jGSsp0FNvE0qanckPvxGMMLTOuLMmslO1UqgY5Y8W/9hkFprogEu8nUIRcp9FUH8Fcjz8ni0Rlz3ytdz4BF X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM3PR12MB0860; X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0860;20:AwXF6Ad8Vhe0HujLSB3piRUBmNimzLgTG0aTB+4f7nmqcyBpWfQezNt34DKJpOYKRR5vF0Hdnood9gslMvW9DPC54btC10Hqe10tkDhe/Vk8WIi6twp617vuX1jAFMs5iNthRO4UzAK+dJU0jHwqsR7JKDTqqKkjzbLQQd+t6tAX0K/gb8DVG0KGeSY3aqOJa834FuXF74G3hP/88JNJ3n/VKnmN9cmBLKbMjzZeraeqXztaCk3jhZvmhl409dWMQRt4JGp1H54Ku+05Gh241nk+KUFipmTu3g9XD6EPWZpHuYtYWkJKGqA/Y6Xv53OLgosWAhXU+B+1gmkJDPZA8KEp7TMhDc9B9PyMf1vLkhtDJS+r7k4DMIsPA59RBRje85jK2nu9evh7urEPVe2iFjIxjhVm1s3Q/kkjoJPS1eoTTC+bA6z5an9Vu4uZ9L+JaZsdmDH7UN9kWHedR11GCgOQB3WirmAsJ4VBeFoesmPBJpin+lDXG2gFAsexr7xk X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(13024025)(13015025)(13018025)(13017025)(13023025)(3002001)(10201501046);SRVR:DM3PR12MB0860;BCL:0;PCL:0;RULEID:;SRVR:DM3PR12MB0860; X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0860;4:MGfcz7llGuRjWMVPxJF2D+8aMwCbb3LmuRI3P9DsDAVY83Yw1Rjy8VEsCNoZwTB8wcdsuez6eff80r5QK7DVncxkY7Nt+8zvRF4LuFNgRH+fZHJOY9Dep0aQU2NqU7iDLgTOTKhLSMBml4bIDbxUvMSN8BE+29OBS77xawUyz0V4GLbLuyfN8WrNQ0YEXyp4m8K9at6Q+QuqFVJWD7XbAxpbUDn5eBGqRTi/ECrIRN/j32EeCCdI23IQ7hxeClzz6vVj8k/Fso3PwmdrlFzzWNTO3YvUaosXW2PCx4u0HfyvGNgb+ST8W8rl/BcKAxIMEitgdCBHsRmo1hzXXbpIo8pJhRSJUcK4SJIScw9MIOrslPu32KsZDcbXq/kshCplIJIfpvMt6a2/+ptxx2uzvf2Xg8RoW32zCJf4UoEUqlIXuUFMWLYkBZroi7h3ZR1aEnvqYAEmcHjT0dyEEZkjlA== X-Forefront-PRVS: 08626BE3A5 X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtETTNQUjEyTUIwODYwOzIzOjdndXc1azJVeGZWNGxPRFBuRlFKdy9kYTJG?= =?utf-8?B?RnVXdTBwaUxtKzRIVzVnMlZhV3FkekZtdHREOUh2WEFVdlpwWUxlN2twUE9k?= =?utf-8?B?TnAvSVlhYTBBWW1GSGtuODZobGF0d1FQM2hQTUd1QURoZTgyQUdxOUovREFO?= =?utf-8?B?cHV1QUt6b3VVeDdjYzJ2a2lXanl5Rm53eTlPN1VpZHRVQThqVkJXZGd4dSt5?= =?utf-8?B?OWZQK3JOejd3SVBKYXBna28zYjZCOWRqa2hJL3lRNzhhNXZLRXlrMDdxaDR2?= =?utf-8?B?UTJGWE9vamI1OGx5aXFFRUNYNkRLT2Y2cUM1elRxdU9qbEtnUTRIZkh3OWp0?= =?utf-8?B?cTQ2MThaZUcvMWlVb1F1aEloK3J3bDMvYm9VNXRnL3NhVzlhb1pnMjN1ekdh?= =?utf-8?B?RGtidHlHWEtoVnlCRTlOMHZ2cTgzcEVxU0JMVzRpcCtnK1hwNHZBUGtUK2Jr?= =?utf-8?B?UTRhbU1QcXhVd1R5ZGpnMlpSK2tONU45cUptVmE3MUtVdVlJMWgyNGsxUXBO?= =?utf-8?B?ekw2RXFsMEN2WVFCdXk3eTBCUFc0UW5jOC8rd3ZNQW1udlpKS3ZsbjladGdr?= =?utf-8?B?clFraUtNWXNDUUxDTGRIcllZbmpmanJJSmRMMGNiWEJpY2w1ZExEMzl4ZzdT?= =?utf-8?B?QVVEa25vZ1huUDdhRG5JclZZcGFQTzBtV0dnMmxrdGZaTjh2SFdta1duU2Nw?= =?utf-8?B?SzdCVmlqZkJvV0ZTNjY2UmFPWk9UMElyQXJDVGZDNEFoSVlmV2hPc3MrZk1u?= =?utf-8?B?dmo0Q0tEa08yamQ5ZHJnN0JWNjJQVmFuc1JVRkF1TUdVL3pUTXdNSkpmTit4?= =?utf-8?B?a2NZaC9ubHFDc1ZnQWFPVHdsdmY4TzFPUGo0bVlWVzBnbFgvNm5pK296aHg5?= =?utf-8?B?ZFpsSEwxcGpJNTNrWHh2Nkp5NTJSNTFHVVFyc2pRNDYrWUNObWlPb01EZUp2?= =?utf-8?B?NVhYR1ZvRUpxbVJMV2RrVFVWb21ibGpSTmxDcFUwOHB2VFJ0a01tMnVQWlg4?= =?utf-8?B?VWJ3TnFIRjdFemhxZWtvbjZYN2dTZGZyTzdTRjlGQ1pjTnAwZ1U1UmUrLy9T?= =?utf-8?B?QkMvTnJXTlVkb0p1VG0wUm9NbDJrYnlOak5ka3ozRUQ5VHMvZG1IeldhYW1N?= =?utf-8?B?VmlrVmNKWnovdDFkRCs4MVp6U29xMnExR2FLQzFueTBtcnFwYWhtSzFPZU1W?= =?utf-8?B?ajBwcjRWb1JZMFdITW1JWUJ0bHNrV3pGU0xHYU5vbXJ5N1hnSTJoRnUzUUI2?= =?utf-8?B?K2NMRlBNdERjNUo5MkNPNytqTzNyM1k1NGxLQ0ZwTG4ydlk0Y1VDcFcxQ0tJ?= =?utf-8?B?LytoQlJKY0hSSVQ1K2pMblBjVy9OZ2hTQWdjMXNZQnJhNlVreHlJb2drVjEr?= =?utf-8?B?aDlvVkZXK09hQnA1bmFvdElhRFNsL0N4SEdnRGQ5dFhJZEYra3hqZEhLK2xa?= =?utf-8?B?cHhvc2hVaHZ5SzVNT2VpQWpqenZYSnp4VTRFb1BrenVkeFRuTDdncjZzNVFv?= =?utf-8?Q?pNHlipx/RpjeovRWQc2pHMkL34qQTKhW6u/GaYImHk895D?= X-Microsoft-Exchange-Diagnostics: 1;DM3PR12MB0860;5:EaeNMpGbVjJem29f6ydzG0e+qMuRcBF9CLcSDy/TVukIwoX8DcAYhFPaiqNr02cr/xW5iN6n1hTiJRBBQLmZagDmZoQ28Xd+HlY4FjOy3xh/bIbUbEwaE2dS4M30qxv7j7O+WIBtIfIVry6E357+Vw==;24:g0k7un9tVJ46DiTfhffqn0xHbsnNxWqlV5v1ScxoJJWfASNdACC/DrsDG+lI1ZrPT9uBBKp2ym7I8Fc3p8MWHOEjyCMuWhycQ5lIUv1WzYU=;20:R1N7I9kXxgZQJGwzbqeSYMxTETXhfiP35w0ix8jQtbix0nbSvcAOrwzSnlPf5oxcYXtaagXU/iJT+dnnlnbp++f2GQsQB4cB0T4rXF0hAi0wx7NRXaKMWNydvH6JvfidWrx3V4PYTD7Q1f0s2V0s7LI0WHVo+PV8IH2ogHI6pGs5ksLyf2CjiSepa4Ao2XznU4XZBPKQkuJeF6pqXaBv7AtWEfwt0JzTqU3PciicN+zru/0yYpBD8YzTffyFexqe SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Feb 2016 18:26:48.1914 (UTC) X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.222];Helo=[atltwp02.amd.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR12MB0860 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/23/2016 6:35 AM, Borislav Petkov wrote: > On Tue, Feb 16, 2016 at 03:45:11PM -0600, Aravind Gopalakrishnan wrote: >> >> /* >> + * Set the error_count and interrupt_enable sysfs attributes here. >> + * This function gets called during the init phase and when someone >> + * makes changes to either of the sysfs attributes. >> + * During init phase, we also program Interrupt type as 'APIC' and >> + * verify if LVT offset obtained from MCx_MISC is valid. >> * Called via smp_call_function_single(), must be called with correct >> * cpu affinity. >> */ > I don't think that's what threshold_restart_bank() does... Hmm, we call this from mce_threshold_block_init() with set_lvt_off = 1 to write LVT offset value to MCi_MISC. And we call this from store_interrupt_enable() to program APIC INT TYPE- if (tr->b->interrupt_enable) hi |= INT_TYPE_APIC; and from store_threshold_limit() to re-set the "error count"- hi = (hi & ~MASK_ERR_COUNT_HI) | (new_count & THRESHOLD_MAX); So I thought it fit the description as to "what" it does.. > Also, that comment is too much - it shouldn't explain "what" but "why". How about- "This function provides user with capabilities to re-program the 'thresold_limit' and 'interrupt_enable' sysfs attributes" >> @@ -262,6 +267,11 @@ static int setup_APIC_deferred_error(int reserved, int new) >> return reserved; >> } >> >> +/* >> + * Obtain LVT offset from MSR_CU_DEF_ERR and call >> + * setup_APIC_deferred_error() to program relevant APIC register. >> + * Also, register a deferred error interrupt handler >> + */ > No, that's basically spelling what the code does. Ok, I'll remove this. >> static void deferred_error_interrupt_enable(struct cpuinfo_x86 *c) >> { >> u32 low = 0, high = 0; >> @@ -338,6 +348,14 @@ nextaddr_out: >> return addr; >> } >> >> +/* >> + * struct threshold_block descriptor tracks useful info regarding the >> + * banks' MISC register. Among other things, it tracks whether interrupt >> + * is possible for the given bank, the threshold limit and the sysfs object >> + * that outputs these info. > That should be in form of comments explaining what the members of struct > threshold_block are, where that struct is defined. Ok, I'll remove comments here and add it to arch/x86/include/asm/amd_nb.h >> Initializing the struct here, programming >> + * LVT offset for threshold interrupts and registering a interrupt handler >> + * if we haven't already done so > Also spelling the code. Will remove this Thanks, -Aravind.