From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.6921.1605779539378430888 for ; Thu, 19 Nov 2020 01:52:19 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=jFphHPrw; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605779538; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=sukJdaUOatEiBYpSPjoy/zQ/73dLnmVg+Q12PKCnstw=; b=jFphHPrwyDv21xvG5D78jkIWrP6ozQob7ce+23eVsHLqdNwUqf/ou38v+eOJLCmf/THpKi bKJyslUvh01kUSymkHSNXSrxL9cZBedOAPrypaIweFci3VYvsVyuP2lA3neB0l34YQxQxq ZDMwZa5+8Z6jcWPHjdkUUeE8LzP9FVI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-241-ZQ7Oyyx6Pku0JmZ4VMWMAA-1; Thu, 19 Nov 2020 04:52:16 -0500 X-MC-Unique: ZQ7Oyyx6Pku0JmZ4VMWMAA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2A3C2E74F; Thu, 19 Nov 2020 09:52:15 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-236.ams2.redhat.com [10.36.112.236]) by smtp.corp.redhat.com (Postfix) with ESMTP id 02BA460BE2; Thu, 19 Nov 2020 09:52:13 +0000 (UTC) Subject: Re: [PATCH v1 2/2] UefiCpuPkg/CpuCacheInfoLib: Add new CpuCacheInfoLib. To: jasonlouyun , devel@edk2.groups.io Cc: Ray Ni , Eric Dong , Rahul Kumar References: <20201119023658.926-1-yun.lou@intel.com> <20201119023658.926-2-yun.lou@intel.com> From: "Laszlo Ersek" Message-ID: <6d4f24d4-5433-184d-7c24-7413af22dcdb@redhat.com> Date: Thu, 19 Nov 2020 10:52:12 +0100 MIME-Version: 1.0 In-Reply-To: <20201119023658.926-2-yun.lou@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/19/20 03:36, jasonlouyun wrote: > This library uses a platform agnostic algorithm to get CPU cache > information. It provides user with an API(GetCpuCacheInfo) to get > detailed CPU cache information by each package, each core type > included in this package, and each cache level & type. > > Signed-off-by: Jason Lou > Cc: Ray Ni > Cc: Eric Dong > Cc: Laszlo Ersek > Cc: Rahul Kumar > --- > UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.c | 602 ++++++++++++++++++++ > UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.c | 131 +++++ > UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.c | 130 +++++ > UefiCpuPkg/Include/Library/CpuCacheInfoLib.h | 68 +++ > UefiCpuPkg/Library/CpuCacheInfoLib/CpuCacheInfoLib.uni | 15 + > UefiCpuPkg/Library/CpuCacheInfoLib/DxeCpuCacheInfoLib.inf | 43 ++ > UefiCpuPkg/Library/CpuCacheInfoLib/InternalCpuCacheInfoLib.h | 64 +++ > UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf | 43 ++ > UefiCpuPkg/UefiCpuPkg.dec | 3 + > UefiCpuPkg/UefiCpuPkg.dsc | 4 + > 10 files changed, 1103 insertions(+) I defer this review to the other UefiCpuPkg reviewers / maintainers. Two superficial suggestions: - please file a TianoCore feature request BZ, and reference it in the commit messages in this series - *at least one* of the commit message and the bugzilla ticket, but preferably both, should describe the use case. In other words, what platforms intend to use the new library (especially if they are open source in edk2-platforms or otherwise), and what use cases will benefit from having cache information. Otherwise, this library is just dead code in edk2. I don't insist that the consumers of this library be open source, but I do insist that *something* be publicly explained about the use case. Thanks Laszlo