Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <49c5e91a-8e02-2a76-db5d-5f15df3c485f@intel.com>
Date:   Tue, 2 Nov 2021 08:24:08 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.2.1
Subject: Re: [PATCH v2] e1000e: Add a delay to let ME unconfigure s0ix when
 DPG_EXIT_DONE is already flagged
Content-Language: en-US
To:     Kai-Heng Feng <kai.heng.feng@canonical.com>
CC:     Jesse Brandeburg <jesse.brandeburg@intel.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
        AceLan Kao <acelan.kao@canonical.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        "Dima Ruinskiy" <dima.ruinskiy@intel.com>,
        "moderated list:INTEL ETHERNET DRIVERS" 
        <intel-wired-lan@lists.osuosl.org>,
        Linux Netdev List <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "Kraus, NechamaX" <nechamax.kraus@linux.intel.com>,
        "Fuxbrumer, Devora" <devora.fuxbrumer@intel.com>,
        "Avivi, Amir" <amir.avivi@intel.com>,
        "Neftin, Sasha" <sasha.neftin@intel.com>
References: <20211026065112.1366205-1-kai.heng.feng@canonical.com>
 <04ed8307-ab1f-59d6-4454-c759ce4a453b@intel.com>
 <CAAd53p69k-2PVw5RpJOAbe=oBh11U_UqzsyMjxHFbo7xqNBDsQ@mail.gmail.com>
 <f6a4d53a-3ec8-b5cd-9b6c-b14c69d20248@intel.com>
 <CAAd53p67dehgizx1h0ro40YRmKNsbv3Ve=2987kyMUKs7=LOpA@mail.gmail.com>
From:   Sasha Neftin <sasha.neftin@intel.com>
In-Reply-To: <CAAd53p67dehgizx1h0ro40YRmKNsbv3Ve=2987kyMUKs7=LOpA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?clk2WUZHS1hmK3dVdEg1T0Ftdi93S21qQjZzeVc0S0x3NkZMeXRzdlo0YUtX?=
 =?utf-8?B?S1kzRVF0dUZKQzFlR2QyWEFKbzRYMUZKUTczSG1zSFgyZlVUNDRhQmp3aFR3?=
 =?utf-8?B?c1JmOEtCMVp2R3M5Q2lwV3JuNmlwa2M5eHVSam1zWEdPRVZxTlZDR3lsdFFZ?=
 =?utf-8?B?MGNkU3R1RkNFaFFIb1hlNmlFbTJkUUxtVytWUXpOVXlRQ3RmZllIWlNzR280?=
 =?utf-8?B?dUtQSVlpcTNSUFFZNytCTnpJTHl6RlM2K0xNaFh2L3dFTkI2TzI0b1Y1RnJM?=
 =?utf-8?B?VmdkNUdPNFlVQnpmSzgwVzJITktjYXJXeEY0ekhGcHZxS2huWW1TdUhjL25H?=
 =?utf-8?B?TmVENURxckhCVVNiN01xSWZjU1lxbU1LQ3RiUjl3QW5sK0ZzK0tIUXBPUWJ2?=
 =?utf-8?B?eEJtT09BY2pTeFBwSXdoZnJpbVVaRUVjYzhVdzZqT2dyMlVwR1pWbTUveGZ4?=
 =?utf-8?B?bmVUbVZzVk5NUTlsMnlJcWZYNzBJVVlSMjA0bHMwQmY3eGhrY2ZMUHlWUE00?=
 =?utf-8?B?RVNQUmJHQ0NXdzY3ckhKMnBET2JIYXVwaDBBbDFodmpmblJkdmVjWnBydGdZ?=
 =?utf-8?B?L1h3WmpxaHduM0RTMTEyQ2Z6T2F1RXd5Ry9kK2UwYzczNml6d2RUMVRVNEVl?=
 =?utf-8?B?ODBGeXhZcDJNSWljeGZwSXFRaklQVkxsZzBWZ2dsMlFXMVZzR1c2cUxiaVd3?=
 =?utf-8?B?Nk5FQWdGRzFGNlNXVGZKamExOXRITnhVcTUyTTdFbFJIMTIrUFNSeWRzN3hk?=
 =?utf-8?B?Q0tkMUszZWkxdVpFK2cvdThNK203WkNPWjJTVTFMSTF1WUNEejFYbVJvSjJq?=
 =?utf-8?B?MGJOQUN2QUZBUHd1UWhVNEdIcWY4M045ZHhrQU5sTDRWMWZMSEMreHcyb1N4?=
 =?utf-8?B?b3RSbU1zdk5KcSthanVyTko0S1BGZEZlZGgzWGw0TXdHT0I3TW51NUpuY1E0?=
 =?utf-8?B?Y0VFbXVVWUFrRHpmNElrcFZYT2t5dEZ0KzRFZ1lOczVTNlZzK3JrUVFIN0o5?=
 =?utf-8?B?N284cTRLaGwyeSt2YkUrQTJBMjNUY0h3Yy9NNmVRZGtTbCs1VVBFbU9iUVlv?=
 =?utf-8?B?SnNmdi85QTR1cXNVb1E5S1lsVFpCZk1RT0FRbVVSRU92Vi8yZ2NBYm4zYy9B?=
 =?utf-8?B?ZVlJTGFacWVkdmJsK005bWRUb01jRC91YzVKUTlZUU1neGZIK2ZiMUZWdzFE?=
 =?utf-8?B?QkZXRzBZTk5xZWhnZXk1VkJWbms0bU4zcXRWTWJjZitPZDV3YjZiWjZWc01J?=
 =?utf-8?B?ZW5vRDNCZjFMelU3RFRMRXoycFhCenNOdUNUZ1lsVVZJZHNzbHA3THgyS2VZ?=
 =?utf-8?B?R2hWQVl3aFIxeWwySEVxUzk1aWVmMm9ycjhXaVBxQ3cwZjU0ZjlzWENGSGRE?=
 =?utf-8?B?QVEvTG8xcm1RQ0VuVmxyYjlEQUU4WTk1SHFOUFBmV3c5M1dHU21US2tmWFNI?=
 =?utf-8?B?TllLazJEbFpDUW1adGZwNTR5V2hBK0trdnpwZHE1dWgxV0xqUEo5UFRMYkl4?=
 =?utf-8?B?bWF2R0FhaEtua0pwM3h5RkdMQVRjQlNNVHk2TFlNaUVXVGl0MFBnUXRtMEFV?=
 =?utf-8?B?U3JySi9QMnJsbEhJYm4yWVR6UkZzWWR0c05hRG9nVnpYQ0FuT0hLb0RHdFRp?=
 =?utf-8?B?dlZvNlBIYmEreFlQR3FSbXN4OTZ5WWNRR1lKczdkbDAra3hncytoVDgzSmV4?=
 =?utf-8?B?NTNZbWI3QkFGcEhHQ0hvVU9EdnBLOC9sTE8waWpCRVhwN3hzV0lZWEZVWFNx?=
 =?utf-8?B?dEZuWUxCTzdRa3F2VUN0SGJUMkZlQ0U1M0d6MHcwRms2V1JMWFhRTFhCUlgv?=
 =?utf-8?B?WEhEdEYxNnBQYk90bTM1d0N3Rm82bVgvUEhIeExBb0xIa2gxdTVwbXNGWXdD?=
 =?utf-8?B?N0xWQ1VBYlI5UjlyelBBS1JWZEFBb3pQSk5HZGZHYnM2ektCNEZNL2oxVzBy?=
 =?utf-8?B?SDMzWWtwY2FiOVRzU3hQbmFIbHVUaUdBOTNOVGxVd2taU3lRYThWYVFla29u?=
 =?utf-8?B?Q3ZXcWFpaGtBaWlBTGJMSmxnTTF2eThWVWN2dVNDd0pWbmN3L1hPbXlDaldR?=
 =?utf-8?B?ZW9KVExBTXpzcFZrb0hXRXpyYkZTYW5WSkxKcCsxNk80OC9CRkd3b3B3VW1Y?=
 =?utf-8?B?S1NkQkVaZHIyT0dLSjUyTzlHMjdlM3MvSXpRT1pOSytsV3V2QnB6YkdmZldQ?=
 =?utf-8?Q?IIRJvXlC337dcbQ1mLViG18=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 504902c6-fab0-4453-f023-08d99dc965c9
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4787.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2021 06:24:17.4462
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dJK+7IL65emrOyhyuHfJr8vEha97usTjwcutLcnfs3ujjpbMPJy2gtkcYoeYmq4IIqWX/1E3abUJW64VDlKCbw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4668
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/2/2021 05:27, Kai-Heng Feng wrote:
> On Fri, Oct 29, 2021 at 5:14 PM Sasha Neftin <sasha.neftin@intel.com> wrote:
>>
>> On 10/27/2021 01:50, Kai-Heng Feng wrote:
>>> On Tue, Oct 26, 2021 at 4:48 PM Sasha Neftin <sasha.neftin@intel.com> wrote:
>>>>
>>>> On 10/26/2021 09:51, Kai-Heng Feng wrote:
>>>>> On some ADL platforms, DPG_EXIT_DONE is always flagged so e1000e resume
>>>>> polling logic doesn't wait until ME really unconfigures s0ix.
>>>>>
>>>>> So check DPG_EXIT_DONE before issuing EXIT_DPG, and if it's already
>>>>> flagged, wait for 1 second to let ME unconfigure s0ix.
>>>>>
>>>>> Fixes: 3e55d231716e ("e1000e: Add handshake with the CSME to support S0ix")
>>>>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=214821
>>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>>> ---
>>>>> v2:
>>>>>     Add missing "Fixes:" tag
>>>>>
>>>>>     drivers/net/ethernet/intel/e1000e/netdev.c | 7 +++++++
>>>>>     1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
>>>>> index 44e2dc8328a22..cd81ba00a6bc9 100644
>>>>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>>>>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>>>>> @@ -6493,14 +6493,21 @@ static void e1000e_s0ix_exit_flow(struct e1000_adapter *adapter)
>>>>>         u32 mac_data;
>>>>>         u16 phy_data;
>>>>>         u32 i = 0;
>>>>> +     bool dpg_exit_done;
>>>>>
>>>>>         if (er32(FWSM) & E1000_ICH_FWSM_FW_VALID) {
>>>>> +             dpg_exit_done = er32(EXFWSM) & E1000_EXFWSM_DPG_EXIT_DONE;
>>>>>                 /* Request ME unconfigure the device from S0ix */
>>>>>                 mac_data = er32(H2ME);
>>>>>                 mac_data &= ~E1000_H2ME_START_DPG;
>>>>>                 mac_data |= E1000_H2ME_EXIT_DPG;
>>>>>                 ew32(H2ME, mac_data);
>>>>>
>>>>> +             if (dpg_exit_done) {
>>>>> +                     e_warn("DPG_EXIT_DONE is already flagged. This is a firmware bug\n");
>>>>> +                     msleep(1000);
>>>>> +             }
>>>> Thanks for working on the enablement.
>>>> The delay approach is fragile. We need to work with CSME folks to
>>>> understand why _DPG_EXIT_DONE indication is wrong on some ADL platforms.
>>>> Could you provide CSME/BIOS version? dmidecode -t 0 and cat
>>>> /sys/class/mei/mei0/fw_ver
>>>
>>> $ sudo dmidecode -t 0
>>> # dmidecode 3.2
>>> Getting SMBIOS data from sysfs.
>>> SMBIOS 3.4 present.
>>> # SMBIOS implementations newer than version 3.2.0 are not
>>> # fully supported by this version of dmidecode.
>>>
>>> Handle 0x0001, DMI type 0, 26 bytes
>>> BIOS Information
>>>           Vendor: Dell Inc.
>>>           Version: 0.12.68
>>>           Release Date: 10/01/2021
>>>           ROM Size: 48 MB
>>>           Characteristics:
>>>                   PCI is supported
>>>                   PNP is supported
>>>                   BIOS is upgradeable
>>>                   BIOS shadowing is allowed
>>>                   Boot from CD is supported
>>>                   Selectable boot is supported
>>>                   EDD is supported
>>>                   Print screen service is supported (int 5h)
>>>                   8042 keyboard services are supported (int 9h)
>>>                   Serial services are supported (int 14h)
>>>                   Printer services are supported (int 17h)
>>>                   ACPI is supported
>>>                   USB legacy is supported
>>>                   BIOS boot specification is supported
>>>                   Function key-initiated network boot is supported
>>>                   Targeted content distribution is supported
>>>                   UEFI is supported
>>>           BIOS Revision: 0.12
>>>
>>> $ cat /sys/class/mei/mei0/fw_ver
>>> 0:16.0.15.1518
>>> 0:16.0.15.1518
>>> 0:16.0.15.1518
>>>
>> Thank you Kai-Heng. The _DPG_EXIT_DONE bit indication comes from the
>> EXFWSM register controlled by the CSME. We have only read access.  I
>> realized that this indication was set to 1 even before our request to
>> unconfigure the s0ix settings from the CSME. I am wondering. Does after
>> a ~ 1s delay (or less, or more) _DPG_EXIT_DONE indication eventually
>> change by CSME to 0? (is it consistently)
> 
> Never. It's consistently being 1.
no. On my TGL platform is cleared by CSME:
[Sun Oct 31 08:54:40 2021] s0ix exit: EXFWSM register: 0x00000000
[Sun Oct 31 08:54:40 2021] s0ix exit (right after sent H2ME): EXFWSM 
register: 0x00000000
[Sun Oct 31 08:54:40 2021] s0ix exit(after polling): EXFWSM register: 
0x00000001
[Sun Oct 31 08:54:40 2021] e1000e 0000:00:1f.6 enp0s31f6: DPG_EXIT_DONE 
cleared after 130 msec
> 
> Right now we are seeing the same issue on TGL, so I wonder if it's
> better to just revert the CSME series?
no. We need to investigate it and understand what is CSME state we hit. 
Meanwhile few options:
1. use privilege flags to disable s0ix flow for problematic system 
(power will increased)
ethtool --set-priv-flags enp0s31f6 s0ix-enabled off
ethtool --show-priv-flags enp0s31f6
Private flags for enp0s31f6:
s0ix-enabled: off
2. delay as you suggested - less preferable I though
3. I would like to suggest (need to check it) in case the DPG_EXIT_DONE 
is 1 (and polling will be exit immediately) - let's perform enforce 
settings to the CSME, before write request to CSME unconfigure the 
device from s0ix :

if (er32(EXFWSM) & E1000_EXFWSM_DPG_EXIT_DONE)
	mac_data |= E1000_H2ME_ENFORCE_SETTINGS;

I will update Bugzilla: 
https://bugzilla.kernel.org/show_bug.cgi?id=214821 with this information.

I also will need some another information regards SMB state in this case.
> 
> Kai-Heng
> 
>>>>>                 /* Poll up to 2.5 seconds for ME to unconfigure DPG.
>>>>>                  * If this takes more than 1 second, show a warning indicating a
>>>>>                  * firmware bug
>>>>>
>>