Message ID | 166759202922.3281208.6231300030834095574.stgit@bmoger-ubuntu |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp609304wru; Fri, 4 Nov 2022 13:01:51 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4YqcqTQhT+dqIwGTNaz/OKZmLe3s5Wiw7U8Zu2b6Vnfe1Oih5KRpasy2FkAkmapBzo5W7N X-Received: by 2002:a17:90b:4b8c:b0:213:6a84:b4b0 with SMTP id lr12-20020a17090b4b8c00b002136a84b4b0mr48880702pjb.207.1667592111234; Fri, 04 Nov 2022 13:01:51 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1667592111; cv=pass; d=google.com; s=arc-20160816; b=bSf8ufRihZdqva93kyN0N4d90r3+60tbXvItg242uXKVlAMsOlwxNyrH66LkZAYlQc wz//vipcujxKmr2K8XasQyVCrEE7zz7i9gvrlhXG4CNKsNgZbQ2ihHiFJVEYB01PZnSl yfI+EzlUCL4wGoiq+DM/XJ79XxkiMm94FaTrcblRNzo44eiVQx7iM8zURRpR5NAE1hFR aPAR6KhjWkNq+vwnwMOP1AT6cEBeQNrVWDjClCZJnZ9bfp5FpChSBNzHYuVzJZctq2uj LxYxLUEHKcnH6OY5OwlN6WEG4LoWWzlP7fDC5AykCT2/RfgqDN2Sn8JrOiI9QprPPY6e +0yw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:cc:to:from :subject:dkim-signature; bh=VCKm/SDQ0q4pCk0+0bzeM5Il0UmQw+4Ee6tUR4g01cs=; b=yCvVLo/zySafvcHBpyD0jqc3C1pIcJPjwYc6MY1jacFMKA5lRd7YR+7dQ02OJQ5+UX G5cVT0cgNMpQVk2toiB4DKRwRXZTZaZ1GGLrn3wQDjk3OIxUsL5QZiDjRYlNCYF6wAHH j4Cx+95fg6qENxC953/AfHFS+j3qXm3Ib/3s4FvhSz0u927C+WJnLGCD4tKxUueIyoCh 5GOJ3cESuROhZ1ZOsr4KGgkcWi0CYidsiM94rxaC+bcIlmHB4+OBv6eJrjrqSmxX/Buz NGNos1VIjtUDBQ3Vj5bX6/cU17UiSs6xKta9V1e6IPUnC4nsThDG9W92UsN4qZ8yHw85 bhXA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=3soBWCHs; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s22-20020a63ff56000000b0046f3dfb9282si501269pgk.52.2022.11.04.13.01.37; Fri, 04 Nov 2022 13:01:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@amd.com header.s=selector1 header.b=3soBWCHs; arc=pass (i=1 spf=pass spfdomain=amd.com dmarc=pass fromdomain=amd.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amd.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229579AbiKDUAy (ORCPT <rfc822;hjfbswb@gmail.com> + 99 others); Fri, 4 Nov 2022 16:00:54 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47770 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229720AbiKDUAl (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 4 Nov 2022 16:00:41 -0400 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2051.outbound.protection.outlook.com [40.107.223.51]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3ABBE49B68; Fri, 4 Nov 2022 13:00:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Xg/KE5vWXwd4nJkMm6CIEe7MTi71MfyAC5YIQRIyMiXdlNORlHDjh5qSWXF04Zh7PEKyPS9lgCSIJuwBuce14IILwMvsf7/ZZwM1txWJEwmixpGBT8XEnIjm/bKnlCJKhroHxlXEPAXpf2gXiqkveOZUu1BLz0vhbVvD4EB372UlzxzOOKbQRxa7c/qFPz3+B71fIim0uvaZzLNodYzd+3Qt2xL6dDQvsm8XT7CyS0ZppPB06b7mWbdroqtwYHddMy8IBj/fvRjExeZas7w5bVRLq5ouRKPths1SPcG4WOpzkGbrmVCHFXXivOgIeYU5oRzqdAd2a7mesCiYycw/2w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=VCKm/SDQ0q4pCk0+0bzeM5Il0UmQw+4Ee6tUR4g01cs=; b=mE0Cl9BTlIOYWhtuRblGV7gg3OMSwodZaROKW8wy0xedkkIta+g8ZgjBR/Zl/xWXl1nk3YoF5lvRH5lpD1fOouD9vxtO+PH7bw7goZL5ztfREUXdfkPprmA3x/fgY3yLabO27Dsi7jff/KqU7RwsxR/yV36bH3imaKOyEhxnyoth7wn/PNTtnRPTckc0EX0GyzoGYM2w+oqXUibaV57qAzJ3v1IN8tixxaLPQ19mnRvy/Ut5/ZeU1zleJQH0q2FE10sFNpmfKf67XwDHnhwC9JA0PQwKTtPJQr5eVLcfq143QLVyijdKPQkk3/nLB02bjEwmBCBEXkRV3OhWO9KljA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=quicinc.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=VCKm/SDQ0q4pCk0+0bzeM5Il0UmQw+4Ee6tUR4g01cs=; b=3soBWCHsMJjtRO2NylqsK41acKQbudf3C3qJAWm5c/9af1vywONy+ZkbrjDxdJZKVL/dEUXi3cC2VSewYwgCugSrSogDBFLmB7xiq5r+oIAKoDweHZbxnIGYLSoyhu3HpvAtUWpvaBQccfIKRcRUgE32KMfw0+haPAnmmhZSWck= Received: from DM6PR11CA0017.namprd11.prod.outlook.com (2603:10b6:5:190::30) by DM4PR12MB6010.namprd12.prod.outlook.com (2603:10b6:8:6a::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5769.19; Fri, 4 Nov 2022 20:00:31 +0000 Received: from DM6NAM11FT046.eop-nam11.prod.protection.outlook.com (2603:10b6:5:190:cafe::7f) by DM6PR11CA0017.outlook.office365.com (2603:10b6:5:190::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5791.22 via Frontend Transport; Fri, 4 Nov 2022 20:00:31 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by DM6NAM11FT046.mail.protection.outlook.com (10.13.172.121) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5791.20 via Frontend Transport; Fri, 4 Nov 2022 20:00:31 +0000 Received: from [127.0.1.1] (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Fri, 4 Nov 2022 15:00:29 -0500 Subject: [PATCH v8 05/13] x86/resctrl: Detect and configure Slow Memory Bandwidth Allocation From: Babu Moger <babu.moger@amd.com> To: <corbet@lwn.net>, <reinette.chatre@intel.com>, <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de> CC: <fenghua.yu@intel.com>, <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>, <paulmck@kernel.org>, <akpm@linux-foundation.org>, <quic_neeraju@quicinc.com>, <rdunlap@infradead.org>, <damien.lemoal@opensource.wdc.com>, <songmuchun@bytedance.com>, <peterz@infradead.org>, <jpoimboe@kernel.org>, <pbonzini@redhat.com>, <babu.moger@amd.com>, <chang.seok.bae@intel.com>, <pawan.kumar.gupta@linux.intel.com>, <jmattson@google.com>, <daniel.sneddon@linux.intel.com>, <sandipan.das@amd.com>, <tony.luck@intel.com>, <james.morse@arm.com>, <linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <bagasdotme@gmail.com>, <eranian@google.com> Date: Fri, 4 Nov 2022 15:00:29 -0500 Message-ID: <166759202922.3281208.6231300030834095574.stgit@bmoger-ubuntu> In-Reply-To: <166759188265.3281208.11769277079826754455.stgit@bmoger-ubuntu> References: <166759188265.3281208.11769277079826754455.stgit@bmoger-ubuntu> User-Agent: StGit/1.1.dev103+g5369f4c MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB04.amd.com (10.181.40.145) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DM6NAM11FT046:EE_|DM4PR12MB6010:EE_ X-MS-Office365-Filtering-Correlation-Id: 9d634fd3-4c49-4906-5333-08dabe9f3a8b X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: QisL0pUCQsE3L1+wSbZ+it6KeZsn8/NLlFSVSkfJfwDL7K3BvaBLz0/qVxzCyGVRM9eCw8nFTLevXln8wpeXf1X5y/MK7cVdBfIbP/uOL3kfptUT8VzEuz3A15tlZOjAI2LdGWiPpckp+is9roX1+7uwecYESK8Bwshp6me2HgzIjs0kDLN3azznAubvDXejbxrkzrvxCnCCWlLcm4001dcTnzGAMx/uubpkztpi1Ty9XzOrMihtxIo+j03NEUfHn4HuQEGjPiusB5CwQnAQEdb+CvtQ+XS1pOrPrc8HufA/SpaxiHy35EpTFtu6FT8LDEI5l5jdx9dMPr7TwFGkQH7oS5zSjKUZtXeiwZGSY5OAvfDoygp+IPqWXH9+HxgL1525lebrZZiz9puHgv8L3nIQ7nlsXVZsrRfV8NzwYPyTOiS9j5b0hYMjdT2fIBHicNFrRpef+YXjY9fI0OIWfcNMNYcWGB3NZdlK9GihzAsvshhhkFstvR1bzUG5QYCwp1G8ePqlz7jbC90yz/WDn6bGWpZvtkddBBFIHVUA9yv376JpV96vRrW5CgIch7VHWS/3SYBaHjhT+H1njmVmv+NSw0xglbFDvl8SLGO3UY1zaGHfJNSgmTQj9YBnnGF3rJfJDkN2VHel7kBw/thIntlVfUmtpTpmZ/24nyuSE9wPRTOYasvDHbyfyjIm9BhED0P7jIlW3ytMEpJefY7qIGIGWSSbibEl1aSxc9h1fyH/EGV8t20UAXoYHkp/RG+IJ7N1Dj5teYN2SLRABcbzfvR1rGuKNPu8WPrl9cim77NpwjcciqEzVJF39YoibKMOaH+0/Oja1NGUVv3G/OjPUA== X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230022)(7916004)(4636009)(396003)(39860400002)(376002)(346002)(136003)(451199015)(40470700004)(36840700001)(46966006)(478600001)(110136005)(426003)(9686003)(33716001)(186003)(26005)(54906003)(16526019)(40480700001)(336012)(40460700003)(36860700001)(83380400001)(44832011)(47076005)(2906002)(7416002)(16576012)(70586007)(8936002)(8676002)(70206006)(4326008)(5660300002)(316002)(41300700001)(82310400005)(86362001)(103116003)(356005)(82740400003)(81166007)(71626007)(36900700001);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Nov 2022 20:00:31.6852 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 9d634fd3-4c49-4906-5333-08dabe9f3a8b 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.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT046.eop-nam11.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6010 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748597065651658900?= X-GMAIL-MSGID: =?utf-8?q?1748597065651658900?= |
Series |
Support for AMD QoS new features
|
|
Commit Message
Moger, Babu
Nov. 4, 2022, 8 p.m. UTC
The QoS slow memory configuration details are available via
CPUID_Fn80000020_EDX_x02. Detect the available details and
initialize the rest to defaults.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
arch/x86/kernel/cpu/resctrl/core.c | 36 +++++++++++++++++++++++++++--
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +-
arch/x86/kernel/cpu/resctrl/internal.h | 1 +
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++--
4 files changed, 41 insertions(+), 6 deletions(-)
Comments
Hi Babu, On 11/4/2022 1:00 PM, Babu Moger wrote: > The QoS slow memory configuration details are available via > CPUID_Fn80000020_EDX_x02. Detect the available details and > initialize the rest to defaults. > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > arch/x86/kernel/cpu/resctrl/core.c | 36 +++++++++++++++++++++++++++-- > arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- > 4 files changed, 41 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c > index e31c98e2fafc..6571d08e2b0d 100644 > --- a/arch/x86/kernel/cpu/resctrl/core.c > +++ b/arch/x86/kernel/cpu/resctrl/core.c > @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) > if (!r) > return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; > > + /* > + * The software controller support is only applicable to MBA resource. > + * Make sure to check for resource type again. > + */ /again/d Not all callers of is_mba_sc() check if it is called for an MBA resource. > + if (r->rid != RDT_RESOURCE_MBA) > + return false; > + > return r->membw.mba_sc; > } > > @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r) > struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); > union cpuid_0x10_3_eax eax; > union cpuid_0x10_x_edx edx; > - u32 ebx, ecx; > + u32 ebx, ecx, subleaf; > > - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); > + /* > + * Query CPUID_Fn80000020_EDX_x01 for MBA and > + * CPUID_Fn80000020_EDX_x02 for SMBA > + */ > + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; > + > + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); > hw_res->num_closid = edx.split.cos_max + 1; > r->default_ctrl = MAX_MBA_BW_AMD; > > @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) > return false; > } > > +static __init bool get_slow_mem_config(void) > +{ > + struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA]; > + > + if (!rdt_cpu_has(X86_FEATURE_SMBA)) > + return false; > + > + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) > + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); > + > + return false; > +} > + > static __init bool get_rdt_alloc_resources(void) > { > struct rdt_resource *r; > @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) > if (get_mem_config()) > ret = true; > > + if (get_slow_mem_config()) > + ret = true; > + > return ret; > } > > @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) > } else if (r->rid == RDT_RESOURCE_MBA) { > hw_res->msr_base = MSR_IA32_MBA_BW_BASE; > hw_res->msr_update = mba_wrmsr_amd; > + } else if (r->rid == RDT_RESOURCE_SMBA) { > + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; > + hw_res->msr_update = mba_wrmsr_amd; > } > } > } I mentioned earlier that this can be moved to init of rdt_resources_all[]. No strong preference, leaving here works also. Reinette
[AMD Official Use Only - General] Hi Reinette, > -----Original Message----- > From: Reinette Chatre <reinette.chatre@intel.com> > Sent: Tuesday, November 22, 2022 6:13 PM > To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net; > tglx@linutronix.de; mingo@redhat.com; bp@alien8.de > Cc: fenghua.yu@intel.com; dave.hansen@linux.intel.com; x86@kernel.org; > hpa@zytor.com; paulmck@kernel.org; akpm@linux-foundation.org; > quic_neeraju@quicinc.com; rdunlap@infradead.org; > damien.lemoal@opensource.wdc.com; songmuchun@bytedance.com; > peterz@infradead.org; jpoimboe@kernel.org; pbonzini@redhat.com; > chang.seok.bae@intel.com; pawan.kumar.gupta@linux.intel.com; > jmattson@google.com; daniel.sneddon@linux.intel.com; Das1, Sandipan > <Sandipan.Das@amd.com>; tony.luck@intel.com; james.morse@arm.com; > linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; > bagasdotme@gmail.com; eranian@google.com > Subject: Re: [PATCH v8 05/13] x86/resctrl: Detect and configure Slow Memory > Bandwidth Allocation > > Hi Babu, > > On 11/4/2022 1:00 PM, Babu Moger wrote: > > The QoS slow memory configuration details are available via > > CPUID_Fn80000020_EDX_x02. Detect the available details and initialize > > the rest to defaults. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > --- > > arch/x86/kernel/cpu/resctrl/core.c | 36 > +++++++++++++++++++++++++++-- > > arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- > > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- > > 4 files changed, 41 insertions(+), 6 deletions(-) > > > > diff --git a/arch/x86/kernel/cpu/resctrl/core.c > > b/arch/x86/kernel/cpu/resctrl/core.c > > index e31c98e2fafc..6571d08e2b0d 100644 > > --- a/arch/x86/kernel/cpu/resctrl/core.c > > +++ b/arch/x86/kernel/cpu/resctrl/core.c > > @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) > > if (!r) > > return > rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; > > > > + /* > > + * The software controller support is only applicable to MBA resource. > > + * Make sure to check for resource type again. > > + */ > > /again/d Sure. > > Not all callers of is_mba_sc() check if it is called for an MBA resource. > > > + if (r->rid != RDT_RESOURCE_MBA) > > + return false; > > + > > return r->membw.mba_sc; > > } > > > > @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct > rdt_resource *r) > > struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); > > union cpuid_0x10_3_eax eax; > > union cpuid_0x10_x_edx edx; > > - u32 ebx, ecx; > > + u32 ebx, ecx, subleaf; > > > > - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); > > + /* > > + * Query CPUID_Fn80000020_EDX_x01 for MBA and > > + * CPUID_Fn80000020_EDX_x02 for SMBA > > + */ > > + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; > > + > > + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); > > hw_res->num_closid = edx.split.cos_max + 1; > > r->default_ctrl = MAX_MBA_BW_AMD; > > > > @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) > > return false; > > } > > > > +static __init bool get_slow_mem_config(void) { > > + struct rdt_hw_resource *hw_res = > > +&rdt_resources_all[RDT_RESOURCE_SMBA]; > > + > > + if (!rdt_cpu_has(X86_FEATURE_SMBA)) > > + return false; > > + > > + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) > > + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); > > + > > + return false; > > +} > > + > > static __init bool get_rdt_alloc_resources(void) { > > struct rdt_resource *r; > > @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) > > if (get_mem_config()) > > ret = true; > > > > + if (get_slow_mem_config()) > > + ret = true; > > + > > return ret; > > } > > > > @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) > > } else if (r->rid == RDT_RESOURCE_MBA) { > > hw_res->msr_base = MSR_IA32_MBA_BW_BASE; > > hw_res->msr_update = mba_wrmsr_amd; > > + } else if (r->rid == RDT_RESOURCE_SMBA) { > > + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; > > + hw_res->msr_update = mba_wrmsr_amd; > > } > > } > > } > > I mentioned earlier that this can be moved to init of rdt_resources_all[]. No > strong preference, leaving here works also. Sure. Will do. Thanks Babu > > Reinette
Hi Reinette, On 11/22/22 18:12, Reinette Chatre wrote: > Hi Babu, > > On 11/4/2022 1:00 PM, Babu Moger wrote: >> The QoS slow memory configuration details are available via >> CPUID_Fn80000020_EDX_x02. Detect the available details and >> initialize the rest to defaults. >> >> Signed-off-by: Babu Moger <babu.moger@amd.com> >> --- >> arch/x86/kernel/cpu/resctrl/core.c | 36 +++++++++++++++++++++++++++-- >> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- >> arch/x86/kernel/cpu/resctrl/internal.h | 1 + >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- >> 4 files changed, 41 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c >> index e31c98e2fafc..6571d08e2b0d 100644 >> --- a/arch/x86/kernel/cpu/resctrl/core.c >> +++ b/arch/x86/kernel/cpu/resctrl/core.c >> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) >> if (!r) >> return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; >> >> + /* >> + * The software controller support is only applicable to MBA resource. >> + * Make sure to check for resource type again. >> + */ > /again/d > > Not all callers of is_mba_sc() check if it is called for an MBA resource. > >> + if (r->rid != RDT_RESOURCE_MBA) >> + return false; >> + >> return r->membw.mba_sc; >> } >> >> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r) >> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); >> union cpuid_0x10_3_eax eax; >> union cpuid_0x10_x_edx edx; >> - u32 ebx, ecx; >> + u32 ebx, ecx, subleaf; >> >> - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); >> + /* >> + * Query CPUID_Fn80000020_EDX_x01 for MBA and >> + * CPUID_Fn80000020_EDX_x02 for SMBA >> + */ >> + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; >> + >> + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); >> hw_res->num_closid = edx.split.cos_max + 1; >> r->default_ctrl = MAX_MBA_BW_AMD; >> >> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) >> return false; >> } >> >> +static __init bool get_slow_mem_config(void) >> +{ >> + struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA]; >> + >> + if (!rdt_cpu_has(X86_FEATURE_SMBA)) >> + return false; >> + >> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) >> + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); >> + >> + return false; >> +} >> + >> static __init bool get_rdt_alloc_resources(void) >> { >> struct rdt_resource *r; >> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) >> if (get_mem_config()) >> ret = true; >> >> + if (get_slow_mem_config()) >> + ret = true; >> + >> return ret; >> } >> >> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) >> } else if (r->rid == RDT_RESOURCE_MBA) { >> hw_res->msr_base = MSR_IA32_MBA_BW_BASE; >> hw_res->msr_update = mba_wrmsr_amd; >> + } else if (r->rid == RDT_RESOURCE_SMBA) { >> + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; >> + hw_res->msr_update = mba_wrmsr_amd; >> } >> } >> } > I mentioned earlier that this can be moved to init of > rdt_resources_all[]. No strong preference, leaving here works > also. I am little confused about this comment. Initialization of rdt_resources_all in core.c is mostly generic initialization. The msr_base and msr_update routines here are vendor specific. I would prefer to keep this in rdt_init_res_defs_amd.Is that ok? Thanks Babu
Hi Babu, On 11/30/2022 10:43 AM, Moger, Babu wrote: > On 11/22/22 18:12, Reinette Chatre wrote: >> On 11/4/2022 1:00 PM, Babu Moger wrote: >>> The QoS slow memory configuration details are available via >>> CPUID_Fn80000020_EDX_x02. Detect the available details and >>> initialize the rest to defaults. >>> >>> Signed-off-by: Babu Moger <babu.moger@amd.com> >>> --- >>> arch/x86/kernel/cpu/resctrl/core.c | 36 +++++++++++++++++++++++++++-- >>> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- >>> arch/x86/kernel/cpu/resctrl/internal.h | 1 + >>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- >>> 4 files changed, 41 insertions(+), 6 deletions(-) >>> >>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c >>> index e31c98e2fafc..6571d08e2b0d 100644 >>> --- a/arch/x86/kernel/cpu/resctrl/core.c >>> +++ b/arch/x86/kernel/cpu/resctrl/core.c >>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) >>> if (!r) >>> return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; >>> >>> + /* >>> + * The software controller support is only applicable to MBA resource. >>> + * Make sure to check for resource type again. >>> + */ >> /again/d >> >> Not all callers of is_mba_sc() check if it is called for an MBA resource. >> >>> + if (r->rid != RDT_RESOURCE_MBA) >>> + return false; >>> + >>> return r->membw.mba_sc; >>> } >>> >>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r) >>> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); >>> union cpuid_0x10_3_eax eax; >>> union cpuid_0x10_x_edx edx; >>> - u32 ebx, ecx; >>> + u32 ebx, ecx, subleaf; >>> >>> - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); >>> + /* >>> + * Query CPUID_Fn80000020_EDX_x01 for MBA and >>> + * CPUID_Fn80000020_EDX_x02 for SMBA >>> + */ >>> + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; >>> + >>> + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); >>> hw_res->num_closid = edx.split.cos_max + 1; >>> r->default_ctrl = MAX_MBA_BW_AMD; >>> >>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) >>> return false; >>> } >>> >>> +static __init bool get_slow_mem_config(void) >>> +{ >>> + struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA]; >>> + >>> + if (!rdt_cpu_has(X86_FEATURE_SMBA)) >>> + return false; >>> + >>> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) >>> + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); >>> + >>> + return false; >>> +} >>> + >>> static __init bool get_rdt_alloc_resources(void) >>> { >>> struct rdt_resource *r; >>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) >>> if (get_mem_config()) >>> ret = true; >>> >>> + if (get_slow_mem_config()) >>> + ret = true; >>> + >>> return ret; >>> } >>> >>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) >>> } else if (r->rid == RDT_RESOURCE_MBA) { >>> hw_res->msr_base = MSR_IA32_MBA_BW_BASE; >>> hw_res->msr_update = mba_wrmsr_amd; >>> + } else if (r->rid == RDT_RESOURCE_SMBA) { >>> + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; >>> + hw_res->msr_update = mba_wrmsr_amd; >>> } >>> } >>> } >> I mentioned earlier that this can be moved to init of >> rdt_resources_all[]. No strong preference, leaving here works >> also. > > I am little confused about this comment. Initialization of > rdt_resources_all in core.c is mostly generic initialization. The msr_base > and msr_update routines here are vendor specific. I would prefer to keep > this in This is a contradiction. Yes, rdt_resources_all[] initialization in core.c is indeed generic initialization, so why is SMBA there? If this was really generic initialization then the entire initialization of SMBA resource should rather move to AMD specific code. SMBA is an AMD only feature yet its resource initialization is fragmented with one portion treated as generic and another portion treated as vendor specific while it all is vendor specific. The current fragmentation is not clear to me. Keeping the initialization as you have in patch #2 is the simplest and that is what prompted me to suggest the move to keep initialization together at that location. > > rdt_init_res_defs_amd.Is that ok? The generic vs non-generic initialization argument is not convincing to me. Could you please elaborate why you prefer it this way? I already mentioned that I do not have a strong preference but I would like to understand what the motivation for this split initialization is. Reinette
On 11/30/22 14:07, Reinette Chatre wrote: > Hi Babu, > > On 11/30/2022 10:43 AM, Moger, Babu wrote: >> On 11/22/22 18:12, Reinette Chatre wrote: >>> On 11/4/2022 1:00 PM, Babu Moger wrote: >>>> The QoS slow memory configuration details are available via >>>> CPUID_Fn80000020_EDX_x02. Detect the available details and >>>> initialize the rest to defaults. >>>> >>>> Signed-off-by: Babu Moger <babu.moger@amd.com> >>>> --- >>>> arch/x86/kernel/cpu/resctrl/core.c | 36 +++++++++++++++++++++++++++-- >>>> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- >>>> arch/x86/kernel/cpu/resctrl/internal.h | 1 + >>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- >>>> 4 files changed, 41 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c >>>> index e31c98e2fafc..6571d08e2b0d 100644 >>>> --- a/arch/x86/kernel/cpu/resctrl/core.c >>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c >>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) >>>> if (!r) >>>> return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; >>>> >>>> + /* >>>> + * The software controller support is only applicable to MBA resource. >>>> + * Make sure to check for resource type again. >>>> + */ >>> /again/d >>> >>> Not all callers of is_mba_sc() check if it is called for an MBA resource. >>> >>>> + if (r->rid != RDT_RESOURCE_MBA) >>>> + return false; >>>> + >>>> return r->membw.mba_sc; >>>> } >>>> >>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r) >>>> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); >>>> union cpuid_0x10_3_eax eax; >>>> union cpuid_0x10_x_edx edx; >>>> - u32 ebx, ecx; >>>> + u32 ebx, ecx, subleaf; >>>> >>>> - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); >>>> + /* >>>> + * Query CPUID_Fn80000020_EDX_x01 for MBA and >>>> + * CPUID_Fn80000020_EDX_x02 for SMBA >>>> + */ >>>> + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; >>>> + >>>> + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); >>>> hw_res->num_closid = edx.split.cos_max + 1; >>>> r->default_ctrl = MAX_MBA_BW_AMD; >>>> >>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) >>>> return false; >>>> } >>>> >>>> +static __init bool get_slow_mem_config(void) >>>> +{ >>>> + struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA]; >>>> + >>>> + if (!rdt_cpu_has(X86_FEATURE_SMBA)) >>>> + return false; >>>> + >>>> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) >>>> + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); >>>> + >>>> + return false; >>>> +} >>>> + >>>> static __init bool get_rdt_alloc_resources(void) >>>> { >>>> struct rdt_resource *r; >>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) >>>> if (get_mem_config()) >>>> ret = true; >>>> >>>> + if (get_slow_mem_config()) >>>> + ret = true; >>>> + >>>> return ret; >>>> } >>>> >>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) >>>> } else if (r->rid == RDT_RESOURCE_MBA) { >>>> hw_res->msr_base = MSR_IA32_MBA_BW_BASE; >>>> hw_res->msr_update = mba_wrmsr_amd; >>>> + } else if (r->rid == RDT_RESOURCE_SMBA) { >>>> + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; >>>> + hw_res->msr_update = mba_wrmsr_amd; >>>> } >>>> } >>>> } >>> I mentioned earlier that this can be moved to init of >>> rdt_resources_all[]. No strong preference, leaving here works >>> also. >> I am little confused about this comment. Initialization of >> rdt_resources_all in core.c is mostly generic initialization. The msr_base >> and msr_update routines here are vendor specific. I would prefer to keep >> this in > This is a contradiction. Yes, rdt_resources_all[] initialization in core.c > is indeed generic initialization, so why is SMBA there? If this was really > generic initialization then the entire initialization of SMBA resource > should rather move to AMD specific code. > > SMBA is an AMD only feature yet its resource initialization is fragmented > with one portion treated as generic and another portion treated as vendor > specific while it all is vendor specific. > > The current fragmentation is not clear to me. Keeping the initialization > as you have in patch #2 is the simplest and that is what prompted me > to suggest the move to keep initialization together at that location. > >> rdt_init_res_defs_amd.Is that ok? > The generic vs non-generic initialization argument is not convincing to me. > Could you please elaborate why you prefer it this way? I already mentioned > that I do not have a strong preference but I would like to understand what > the motivation for this split initialization is. > I dont have any strong argument. I was thinking, in case Intel supports this resource in the future then they only have to change rdt_init_res_defs_intel. Thanks Babu
Hi Babu, On 11/30/2022 12:40 PM, Moger, Babu wrote: > On 11/30/22 14:07, Reinette Chatre wrote: >> On 11/30/2022 10:43 AM, Moger, Babu wrote: >>> On 11/22/22 18:12, Reinette Chatre wrote: >>>> On 11/4/2022 1:00 PM, Babu Moger wrote: >>>>> The QoS slow memory configuration details are available via >>>>> CPUID_Fn80000020_EDX_x02. Detect the available details and >>>>> initialize the rest to defaults. >>>>> >>>>> Signed-off-by: Babu Moger <babu.moger@amd.com> >>>>> --- >>>>> arch/x86/kernel/cpu/resctrl/core.c | 36 +++++++++++++++++++++++++++-- >>>>> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- >>>>> arch/x86/kernel/cpu/resctrl/internal.h | 1 + >>>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- >>>>> 4 files changed, 41 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c >>>>> index e31c98e2fafc..6571d08e2b0d 100644 >>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c >>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c >>>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) >>>>> if (!r) >>>>> return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; >>>>> >>>>> + /* >>>>> + * The software controller support is only applicable to MBA resource. >>>>> + * Make sure to check for resource type again. >>>>> + */ >>>> /again/d >>>> >>>> Not all callers of is_mba_sc() check if it is called for an MBA resource. >>>> >>>>> + if (r->rid != RDT_RESOURCE_MBA) >>>>> + return false; >>>>> + >>>>> return r->membw.mba_sc; >>>>> } >>>>> >>>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r) >>>>> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); >>>>> union cpuid_0x10_3_eax eax; >>>>> union cpuid_0x10_x_edx edx; >>>>> - u32 ebx, ecx; >>>>> + u32 ebx, ecx, subleaf; >>>>> >>>>> - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); >>>>> + /* >>>>> + * Query CPUID_Fn80000020_EDX_x01 for MBA and >>>>> + * CPUID_Fn80000020_EDX_x02 for SMBA >>>>> + */ >>>>> + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; >>>>> + >>>>> + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); >>>>> hw_res->num_closid = edx.split.cos_max + 1; >>>>> r->default_ctrl = MAX_MBA_BW_AMD; >>>>> >>>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) >>>>> return false; >>>>> } >>>>> >>>>> +static __init bool get_slow_mem_config(void) >>>>> +{ >>>>> + struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA]; >>>>> + >>>>> + if (!rdt_cpu_has(X86_FEATURE_SMBA)) >>>>> + return false; >>>>> + >>>>> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) >>>>> + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); >>>>> + >>>>> + return false; >>>>> +} >>>>> + >>>>> static __init bool get_rdt_alloc_resources(void) >>>>> { >>>>> struct rdt_resource *r; >>>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) >>>>> if (get_mem_config()) >>>>> ret = true; >>>>> >>>>> + if (get_slow_mem_config()) >>>>> + ret = true; >>>>> + >>>>> return ret; >>>>> } >>>>> >>>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) >>>>> } else if (r->rid == RDT_RESOURCE_MBA) { >>>>> hw_res->msr_base = MSR_IA32_MBA_BW_BASE; >>>>> hw_res->msr_update = mba_wrmsr_amd; >>>>> + } else if (r->rid == RDT_RESOURCE_SMBA) { >>>>> + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; >>>>> + hw_res->msr_update = mba_wrmsr_amd; >>>>> } >>>>> } >>>>> } >>>> I mentioned earlier that this can be moved to init of >>>> rdt_resources_all[]. No strong preference, leaving here works >>>> also. >>> I am little confused about this comment. Initialization of >>> rdt_resources_all in core.c is mostly generic initialization. The msr_base >>> and msr_update routines here are vendor specific. I would prefer to keep >>> this in >> This is a contradiction. Yes, rdt_resources_all[] initialization in core.c >> is indeed generic initialization, so why is SMBA there? If this was really >> generic initialization then the entire initialization of SMBA resource >> should rather move to AMD specific code. >> >> SMBA is an AMD only feature yet its resource initialization is fragmented >> with one portion treated as generic and another portion treated as vendor >> specific while it all is vendor specific. >> >> The current fragmentation is not clear to me. Keeping the initialization >> as you have in patch #2 is the simplest and that is what prompted me >> to suggest the move to keep initialization together at that location. >> >>> rdt_init_res_defs_amd.Is that ok? >> The generic vs non-generic initialization argument is not convincing to me. >> Could you please elaborate why you prefer it this way? I already mentioned >> that I do not have a strong preference but I would like to understand what >> the motivation for this split initialization is. >> > I dont have any strong argument. I was thinking, in case Intel supports > this resource in the future then they only have to change > rdt_init_res_defs_intel. I agree that this is not a strong argument. If this happens then Intel can split the initialization also. This is also not the only bits that would need changing since only __rdt_get_mem_config_amd() can initialize an SMBA resource. It does not sound like there is a clear winner. To answer your earlier question more succinctly, yes, from my perspective you can keep the change to rdt_init_res_defs_amd(). At least with this change things would be more familiar between MBA and SMBA and it will be obvious that SMBA is not supported by Intel. Reinette
[AMD Official Use Only - General] Hi Reinette, > -----Original Message----- > From: Reinette Chatre <reinette.chatre@intel.com> > Sent: Wednesday, November 30, 2022 6:36 PM > To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net; > tglx@linutronix.de; mingo@redhat.com; bp@alien8.de > Cc: fenghua.yu@intel.com; dave.hansen@linux.intel.com; x86@kernel.org; > hpa@zytor.com; paulmck@kernel.org; akpm@linux-foundation.org; > quic_neeraju@quicinc.com; rdunlap@infradead.org; > damien.lemoal@opensource.wdc.com; songmuchun@bytedance.com; > peterz@infradead.org; jpoimboe@kernel.org; pbonzini@redhat.com; > chang.seok.bae@intel.com; pawan.kumar.gupta@linux.intel.com; > jmattson@google.com; daniel.sneddon@linux.intel.com; Das1, Sandipan > <Sandipan.Das@amd.com>; tony.luck@intel.com; james.morse@arm.com; > linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; > bagasdotme@gmail.com; eranian@google.com > Subject: Re: [PATCH v8 05/13] x86/resctrl: Detect and configure Slow Memory > Bandwidth Allocation > > Hi Babu, > > On 11/30/2022 12:40 PM, Moger, Babu wrote: > > On 11/30/22 14:07, Reinette Chatre wrote: > >> On 11/30/2022 10:43 AM, Moger, Babu wrote: > >>> On 11/22/22 18:12, Reinette Chatre wrote: > >>>> On 11/4/2022 1:00 PM, Babu Moger wrote: > >>>>> The QoS slow memory configuration details are available via > >>>>> CPUID_Fn80000020_EDX_x02. Detect the available details and > >>>>> initialize the rest to defaults. > >>>>> > >>>>> Signed-off-by: Babu Moger <babu.moger@amd.com> > >>>>> --- > >>>>> arch/x86/kernel/cpu/resctrl/core.c | 36 > +++++++++++++++++++++++++++-- > >>>>> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 2 +- > >>>>> arch/x86/kernel/cpu/resctrl/internal.h | 1 + > >>>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 8 ++++-- > >>>>> 4 files changed, 41 insertions(+), 6 deletions(-) > >>>>> > >>>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c > >>>>> b/arch/x86/kernel/cpu/resctrl/core.c > >>>>> index e31c98e2fafc..6571d08e2b0d 100644 > >>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c > >>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c > >>>>> @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) > >>>>> if (!r) > >>>>> return > >>>>> rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; > >>>>> > >>>>> + /* > >>>>> + * The software controller support is only applicable to MBA > resource. > >>>>> + * Make sure to check for resource type again. > >>>>> + */ > >>>> /again/d > >>>> > >>>> Not all callers of is_mba_sc() check if it is called for an MBA resource. > >>>> > >>>>> + if (r->rid != RDT_RESOURCE_MBA) > >>>>> + return false; > >>>>> + > >>>>> return r->membw.mba_sc; > >>>>> } > >>>>> > >>>>> @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct > rdt_resource *r) > >>>>> struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); > >>>>> union cpuid_0x10_3_eax eax; > >>>>> union cpuid_0x10_x_edx edx; > >>>>> - u32 ebx, ecx; > >>>>> + u32 ebx, ecx, subleaf; > >>>>> > >>>>> - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); > >>>>> + /* > >>>>> + * Query CPUID_Fn80000020_EDX_x01 for MBA and > >>>>> + * CPUID_Fn80000020_EDX_x02 for SMBA > >>>>> + */ > >>>>> + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; > >>>>> + > >>>>> + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, > >>>>> +&edx.full); > >>>>> hw_res->num_closid = edx.split.cos_max + 1; > >>>>> r->default_ctrl = MAX_MBA_BW_AMD; > >>>>> > >>>>> @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) > >>>>> return false; > >>>>> } > >>>>> > >>>>> +static __init bool get_slow_mem_config(void) { > >>>>> + struct rdt_hw_resource *hw_res = > >>>>> +&rdt_resources_all[RDT_RESOURCE_SMBA]; > >>>>> + > >>>>> + if (!rdt_cpu_has(X86_FEATURE_SMBA)) > >>>>> + return false; > >>>>> + > >>>>> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) > >>>>> + return __rdt_get_mem_config_amd(&hw_res- > >r_resctrl); > >>>>> + > >>>>> + return false; > >>>>> +} > >>>>> + > >>>>> static __init bool get_rdt_alloc_resources(void) { > >>>>> struct rdt_resource *r; > >>>>> @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) > >>>>> if (get_mem_config()) > >>>>> ret = true; > >>>>> > >>>>> + if (get_slow_mem_config()) > >>>>> + ret = true; > >>>>> + > >>>>> return ret; > >>>>> } > >>>>> > >>>>> @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) > >>>>> } else if (r->rid == RDT_RESOURCE_MBA) { > >>>>> hw_res->msr_base = MSR_IA32_MBA_BW_BASE; > >>>>> hw_res->msr_update = mba_wrmsr_amd; > >>>>> + } else if (r->rid == RDT_RESOURCE_SMBA) { > >>>>> + hw_res->msr_base = > MSR_IA32_SMBA_BW_BASE; > >>>>> + hw_res->msr_update = mba_wrmsr_amd; > >>>>> } > >>>>> } > >>>>> } > >>>> I mentioned earlier that this can be moved to init of > >>>> rdt_resources_all[]. No strong preference, leaving here works also. > >>> I am little confused about this comment. Initialization of > >>> rdt_resources_all in core.c is mostly generic initialization. The > >>> msr_base and msr_update routines here are vendor specific. I would > >>> prefer to keep this in > >> This is a contradiction. Yes, rdt_resources_all[] initialization in > >> core.c is indeed generic initialization, so why is SMBA there? If > >> this was really generic initialization then the entire initialization > >> of SMBA resource should rather move to AMD specific code. > >> > >> SMBA is an AMD only feature yet its resource initialization is > >> fragmented with one portion treated as generic and another portion > >> treated as vendor specific while it all is vendor specific. > >> > >> The current fragmentation is not clear to me. Keeping the > >> initialization as you have in patch #2 is the simplest and that is > >> what prompted me to suggest the move to keep initialization together at > that location. > >> > >>> rdt_init_res_defs_amd.Is that ok? > >> The generic vs non-generic initialization argument is not convincing to me. > >> Could you please elaborate why you prefer it this way? I already > >> mentioned that I do not have a strong preference but I would like to > >> understand what the motivation for this split initialization is. > >> > > I dont have any strong argument. I was thinking, in case Intel > > supports this resource in the future then they only have to change > > rdt_init_res_defs_intel. > > I agree that this is not a strong argument. If this happens then Intel can split the > initialization also. This is also not the only bits that would need changing since > only __rdt_get_mem_config_amd() can initialize an SMBA resource. > > It does not sound like there is a clear winner. To answer your earlier question > more succinctly, yes, from my perspective you can keep the change to > rdt_init_res_defs_amd(). At least with this change things would be more > familiar between MBA and SMBA and it will be obvious that SMBA is not > supported by Intel. Will do. Thanks Babu
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index e31c98e2fafc..6571d08e2b0d 100644 --- a/arch/x86/kernel/cpu/resctrl/core.c +++ b/arch/x86/kernel/cpu/resctrl/core.c @@ -162,6 +162,13 @@ bool is_mba_sc(struct rdt_resource *r) if (!r) return rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl.membw.mba_sc; + /* + * The software controller support is only applicable to MBA resource. + * Make sure to check for resource type again. + */ + if (r->rid != RDT_RESOURCE_MBA) + return false; + return r->membw.mba_sc; } @@ -225,9 +232,15 @@ static bool __rdt_get_mem_config_amd(struct rdt_resource *r) struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); union cpuid_0x10_3_eax eax; union cpuid_0x10_x_edx edx; - u32 ebx, ecx; + u32 ebx, ecx, subleaf; - cpuid_count(0x80000020, 1, &eax.full, &ebx, &ecx, &edx.full); + /* + * Query CPUID_Fn80000020_EDX_x01 for MBA and + * CPUID_Fn80000020_EDX_x02 for SMBA + */ + subleaf = (r->rid == RDT_RESOURCE_SMBA) ? 2 : 1; + + cpuid_count(0x80000020, subleaf, &eax.full, &ebx, &ecx, &edx.full); hw_res->num_closid = edx.split.cos_max + 1; r->default_ctrl = MAX_MBA_BW_AMD; @@ -750,6 +763,19 @@ static __init bool get_mem_config(void) return false; } +static __init bool get_slow_mem_config(void) +{ + struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_SMBA]; + + if (!rdt_cpu_has(X86_FEATURE_SMBA)) + return false; + + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) + return __rdt_get_mem_config_amd(&hw_res->r_resctrl); + + return false; +} + static __init bool get_rdt_alloc_resources(void) { struct rdt_resource *r; @@ -780,6 +806,9 @@ static __init bool get_rdt_alloc_resources(void) if (get_mem_config()) ret = true; + if (get_slow_mem_config()) + ret = true; + return ret; } @@ -869,6 +898,9 @@ static __init void rdt_init_res_defs_amd(void) } else if (r->rid == RDT_RESOURCE_MBA) { hw_res->msr_base = MSR_IA32_MBA_BW_BASE; hw_res->msr_update = mba_wrmsr_amd; + } else if (r->rid == RDT_RESOURCE_SMBA) { + hw_res->msr_base = MSR_IA32_SMBA_BW_BASE; + hw_res->msr_update = mba_wrmsr_amd; } } } diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c index 1df0e3262bca..2dd4b8c47f23 100644 --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c @@ -209,7 +209,7 @@ static int parse_line(char *line, struct resctrl_schema *s, unsigned long dom_id; if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP && - r->rid == RDT_RESOURCE_MBA) { + (r->rid == RDT_RESOURCE_MBA || r->rid == RDT_RESOURCE_SMBA)) { rdt_last_cmd_puts("Cannot pseudo-lock MBA resource\n"); return -EINVAL; } diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h index 43d9f6f5a931..16e3c6e03c79 100644 --- a/arch/x86/kernel/cpu/resctrl/internal.h +++ b/arch/x86/kernel/cpu/resctrl/internal.h @@ -14,6 +14,7 @@ #define MSR_IA32_L2_CBM_BASE 0xd10 #define MSR_IA32_MBA_THRTL_BASE 0xd50 #define MSR_IA32_MBA_BW_BASE 0xc0000200 +#define MSR_IA32_SMBA_BW_BASE 0xc0000280 #define MSR_IA32_QM_CTR 0x0c8e #define MSR_IA32_QM_EVTSEL 0x0c8d diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index e5a48f05e787..8a3dafc0dbf7 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -1213,7 +1213,7 @@ static bool rdtgroup_mode_test_exclusive(struct rdtgroup *rdtgrp) list_for_each_entry(s, &resctrl_schema_all, list) { r = s->res; - if (r->rid == RDT_RESOURCE_MBA) + if (r->rid == RDT_RESOURCE_MBA || r->rid == RDT_RESOURCE_SMBA) continue; has_cache = true; list_for_each_entry(d, &r->domains, list) { @@ -1402,7 +1402,8 @@ static int rdtgroup_size_show(struct kernfs_open_file *of, ctrl = resctrl_arch_get_config(r, d, closid, type); - if (r->rid == RDT_RESOURCE_MBA) + if (r->rid == RDT_RESOURCE_MBA || + r->rid == RDT_RESOURCE_SMBA) size = ctrl; else size = rdtgroup_cbm_to_size(r, d, ctrl); @@ -2845,7 +2846,8 @@ static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp) list_for_each_entry(s, &resctrl_schema_all, list) { r = s->res; - if (r->rid == RDT_RESOURCE_MBA) { + if (r->rid == RDT_RESOURCE_MBA || + r->rid == RDT_RESOURCE_SMBA) { rdtgroup_init_mba(r, rdtgrp->closid); if (is_mba_sc(r)) continue;