From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-002e3701.pphosted.com (mx0b-002e3701.pphosted.com [148.163.143.35]) by mx.groups.io with SMTP id smtpd.web12.2966.1571106731106274602 for ; Mon, 14 Oct 2019 19:32:11 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: hpe.com, ip: 148.163.143.35, mailfrom: prvs=019184da0c=abner.chang@hpe.com) Received: from pps.filterd (m0134423.ppops.net [127.0.0.1]) by mx0b-002e3701.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id x9F2Qloi009291; Tue, 15 Oct 2019 02:32:10 GMT Received: from g2t2354.austin.hpe.com (g2t2354.austin.hpe.com [15.233.44.27]) by mx0b-002e3701.pphosted.com with ESMTP id 2vmmtvwxtd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 15 Oct 2019 02:32:09 +0000 Received: from G2W6311.americas.hpqcorp.net (g2w6311.austin.hp.com [16.197.64.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by g2t2354.austin.hpe.com (Postfix) with ESMTPS id 7449B81; Tue, 15 Oct 2019 02:32:09 +0000 (UTC) Received: from G4W9335.americas.hpqcorp.net (16.208.33.85) by G2W6311.americas.hpqcorp.net (16.197.64.53) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Tue, 15 Oct 2019 02:32:09 +0000 Received: from G4W10204.americas.hpqcorp.net (2002:10cf:5210::10cf:5210) by G4W9335.americas.hpqcorp.net (2002:10d0:2155::10d0:2155) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Tue, 15 Oct 2019 02:32:08 +0000 Received: from NAM01-SN1-obe.outbound.protection.outlook.com (15.241.52.11) by G4W10204.americas.hpqcorp.net (16.207.82.16) with Microsoft SMTP Server (TLS) id 15.0.1367.3 via Frontend Transport; Tue, 15 Oct 2019 02:32:08 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=U5VkAe03N0L0TgLJFTJu2Dvkr806q/RPrBX6Lsx4i3tfm+GMsGF5//5sQnlATyjFfp1IiJKf7wqq+aGQ0AjILOQq/pxE/832j4HsaM/YGTEIdqLGobYc0h1DpyLBQkHbBtgidZqQtKOGmuZx77/tFcNjjLx775LYwDYYCveb2GW64WgL5+3DPmteAKg1FjuR1WTFtxvrsP2U4MDB/puqlEpjXa7vMiGCOOI8dbtgW8qBA81yokIPS3Lx1ced4I/lYxGwW/R7LuvpNgp4TQ+K8JNdfLfexiPnHBfH5HHkR5DNCLaTlVWa6ETSa1HNahDthUuAecTU+j1J0xd0On5XBA== 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-SenderADCheck; bh=fktXwAdLaKVaucwHFRhV6/UHnp2AjBf8StUm4rs0WYE=; b=HSIwIE+tQFL3P4JCf2nb35Q9SAQqz/2Wrmrxa390K5RB9+ZM3Owdz9lgW+NggLogHASJxOinuaJ4odRITTcrC8nNSRcYg5GZVpVYnhmLY2qQw6xQIRLf/33Lo5/oFJz70qyO6fwyTb30p0WUqzYuTsxdHandyynVmS+hZFzM0LOKFT6rgAwdlT0ho/IcGpNsx7eNQYubx5a3cLyCZhiGUtJ/MI9E+YXITQMKJBx/3oOARc3ZjQVML6Sy5gQzj3k15Ju/C69XNz+bHW4GelzmwMb/y6q602xdZWNBQErXuYuZOx1Krcov5Vdo9meB+N1pguqAkHkGdJuz3AH2t0eruw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=hpe.com; dmarc=pass action=none header.from=hpe.com; dkim=pass header.d=hpe.com; arc=none Received: from CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM (10.169.12.151) by CS1PR8401MB0919.NAMPRD84.PROD.OUTLOOK.COM (10.169.14.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2347.16; Tue, 15 Oct 2019 02:32:07 +0000 Received: from CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM ([fe80::4fb:84b9:76e6:1cde]) by CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM ([fe80::4fb:84b9:76e6:1cde%8]) with mapi id 15.20.2347.023; Tue, 15 Oct 2019 02:32:07 +0000 From: "Abner Chang" To: Leif Lindholm , "devel@edk2.groups.io" Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 15/29] RiscVPkg/Library: RISC-V CPU library Thread-Topic: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 15/29] RiscVPkg/Library: RISC-V CPU library Thread-Index: AQHVcaqiAoygLHSn9USitz/gLDPPcadElyMAgBaGBvA= Date: Tue, 15 Oct 2019 02:32:07 +0000 Message-ID: References: <1569198715-31552-1-git-send-email-abner.chang@hpe.com> <1569198715-31552-17-git-send-email-abner.chang@hpe.com> <20190930183112.GZ25504@bivouac.eciton.net> In-Reply-To: <20190930183112.GZ25504@bivouac.eciton.net> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [16.242.247.131] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 91462729-4aff-4304-e638-08d75117dfdb x-ms-office365-filtering-ht: Tenant x-ms-traffictypediagnostic: CS1PR8401MB0919: x-ms-exchange-purlcount: 2 x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:497; x-forefront-prvs: 01917B1794 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(396003)(346002)(136003)(366004)(376002)(39860400002)(13464003)(189003)(199004)(305945005)(7736002)(11346002)(6436002)(71200400001)(26005)(33656002)(6506007)(71190400001)(53546011)(9686003)(446003)(55016002)(102836004)(6246003)(6306002)(8936002)(2906002)(966005)(14454004)(86362001)(186003)(66946007)(76116006)(66446008)(64756008)(66556008)(66476007)(478600001)(66066001)(74316002)(7696005)(52536014)(76176011)(486006)(19627235002)(256004)(6116002)(14444005)(476003)(229853002)(3846002)(99286004)(2501003)(81166006)(81156014)(8676002)(316002)(110136005)(5660300002)(66574012)(25786009);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR8401MB0919;H:CS1PR8401MB1192.NAMPRD84.PROD.OUTLOOK.COM;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: a0blNhAdSf1ed036z3Gug8B6Y4+g3Fb6TkNnafwU6FeOvp/ipvqbsk9YRKZk8z9qvN7oJHqs20/PeImxg7XtaL1QZvhAShuoLCKMh4eX04vg4YJaYP0mMGipfnokzYyT45TOznM76fAdQiE6WWWHZcAyApmbiMXAbc0yB2Rd29mRzgrGbRsRuRikvKW+p/C+MZyWrSBzaGKMpxfDtqLzfBNgjjoBAJuEcJkvDAyE3RX1nODx3gyNs0o6ihk2Do8giUrWnoCVMqRx/RZP681140bAijLcwBufH0R6TgJWsiHpdSVQYXzpqOr9xmck6J9K/cgCdeci6Fp+Ge7RZ5hy6jWNsKCzQUCfm5hONGdsRLnFz23a9hLiqw/gUaZwgiZgnldvj6y2a6nQTbxdHcyIKH/+F/lwS1DNYQ/Ilh1BxB1vb19AtVb003pUXvW3MyvftFmkSnHvAxn5cBtv4Mti7w== x-ms-exchange-transport-forked: True X-MS-Exchange-CrossTenant-Network-Message-Id: 91462729-4aff-4304-e638-08d75117dfdb X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Oct 2019 02:32:07.6783 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: mcmcwleiOjgjky+l4etQoKyD4/LgiR6zONScFbkYZaDffnRssMp3XG2+LuSvbmVgrMdW0UAm4vE+1cDYWr0qKg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR8401MB0919 X-OriginatorOrg: hpe.com X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-HPE-SCL: -1 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8 definitions=2019-10-14_12:2019-10-11,2019-10-14 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 impostorscore=0 adultscore=0 mlxlogscore=999 lowpriorityscore=0 priorityscore=1501 mlxscore=0 clxscore=1015 suspectscore=0 spamscore=0 phishscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-1908290000 definitions=main-1910150020 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: Tuesday, October 1, 2019 2:31 AM > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) > > Subject: Re: [edk2-devel] [edk2-staging/RISC-V-V2 PATCH v2 15/29] > RiscVPkg/Library: RISC-V CPU library >=20 > On Mon, Sep 23, 2019 at 08:31:41AM +0800, Abner Chang wrote: > > This library provides CSR assembly functions to read/write RISC-V > > specific Control and Status registers. > > > > Signed-off-by: Abner Chang > > --- > > RiscVPkg/Include/Library/RiscVCpuLib.h | 68 ++++++++++++++++ > > RiscVPkg/Library/RiscVCpuLib/Cpu.S | 115 > +++++++++++++++++++++++++++ > > RiscVPkg/Library/RiscVCpuLib/RiscVCpuLib.inf | 34 ++++++++ >=20 > Please ensure you have set up an orderfile, as described on > https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s- > unkempt-git-guide-for-edk2-contributors-and-maintainers > or by executing BaseTools/Scripts/SetupGit.py inside each repository. >=20 > > 3 files changed, 217 insertions(+) > > create mode 100644 RiscVPkg/Include/Library/RiscVCpuLib.h > > create mode 100644 RiscVPkg/Library/RiscVCpuLib/Cpu.S > > create mode 100644 RiscVPkg/Library/RiscVCpuLib/RiscVCpuLib.inf > > > > diff --git a/RiscVPkg/Include/Library/RiscVCpuLib.h > > b/RiscVPkg/Include/Library/RiscVCpuLib.h > > new file mode 100644 > > index 0000000..c84d599 > > --- /dev/null > > +++ b/RiscVPkg/Include/Library/RiscVCpuLib.h > > @@ -0,0 +1,68 @@ > > +/** @file > > + RISC-V CPU library definitions. > > + > > + Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development > > + LP. All rights reserved.
> > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > + > > +#ifndef _RISCV_CPU_LIB_H_ > > +#define _RISCV_CPU_LIB_H_ >=20 > Please drop leading _. >=20 > > + > > +#include "RiscV.h" >=20 > Hmm. This raises two concerns. > First - style-wise, "" should not be used for anything but local (same > directory) include files. > Secondly, there are two separate files called RiscV.h introduced by this= patch > series: > RiscVPkg/Include/IndustryStandard/RiscV.h > RiscVPkg/Include/RiscV.h > Have these been split solely in order to have one directly includable in > assembler? >=20 > If so, please merge them (in the IndustryStandard one), and put the C- > specific bits inside an #ifndef __ASSEMBLY__ statement. > If not, please provide a description of their logical split, and rename = one of > them. I have same concerns of the same naming as well.=20 Actually one is for RISC-V industrial standard, another one is the definit= ions of EDK2 RISC-V implementation. I will name the last one to RiscVImpl.h= . >=20 > > + > > +/** > > + RISCV_TRAP_HANDLER > > +**/ > > +typedef > > +VOID > > +(EFIAPI *RISCV_TRAP_HANDLER)( > > + VOID > > + ); > > + > > +VOID > > +RiscVSetScratch (RISCV_MACHINE_MODE_CONTEXT *RiscvContext); >=20 > Please keep all names referring to architectural registers identifiable.= A quick > Internet search suggests this may mean Machine Scratch Register? >=20 > > + > > +UINT32 > > +RiscVGetScratch (VOID); > > + > > +UINT32 > > +RiscVGetTrapCause (VOID); > > + > > +UINT64 > > +RiscVReadMachineTimer (VOID); > > + > > +VOID > > +RiscVSetMachineTimerCmp (UINT64); >=20 > Cmp neds expanding to its full name. >=20 > > + > > +UINT64 > > +RiscVReadMachineTimerCmp(VOID); > > + > > +UINT64 > > +RiscVReadMachineIE(VOID); > > + > > +UINT64 > > +RiscVReadMachineIP(VOID); >=20 > IE/IP needs expanding, unless these are the names used in the architectu= re > reference. If it is, this file needs those terms introduced in a glossar= y section > at the start of this file. >=20 > > + > > +UINT64 > > +RiscVReadMachineStatus(VOID); > > + > > +VOID > > +RiscVWriteMachineStatus(UINT64); > > + > > +UINT64 > > +RiscVReadMachineTvec(VOID); > > + > > +UINT64 > > +RiscVReadMisa (VOID); >=20 > Tvec/Misa need the same treatment as IE/IP. > If the M stands for Machine, it should be written out fully, and likely = isa > should be Isa. >=20 > > + > > +UINT64 > > +RiscVReadMVendorId (VOID); > > + > > +UINT64 > > +RiscVReadMArchId (VOID); > > + > > +UINT64 > > +RiscVReadMImplId (VOID); >=20 > Impl needs expanding - I can't tell whether that means Implementation or > Implmenter. > For all 3 above, is that M only an abbreviated Machine? If so, please wr= ite it > out fully. >=20 > > + > > +#endif > > diff --git a/RiscVPkg/Library/RiscVCpuLib/Cpu.S > > b/RiscVPkg/Library/RiscVCpuLib/Cpu.S > > new file mode 100644 > > index 0000000..f372397 > > --- /dev/null > > +++ b/RiscVPkg/Library/RiscVCpuLib/Cpu.S > > @@ -0,0 +1,115 @@ > > +//------------------------------------------------------------------- > > +----------- > > +// > > +// RISC-V CPU functions. > > +// > > +// Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development > > +LP. All rights reserved.
// // SPDX-License-Identifier: > > +BSD-2-Clause-Patent // > > +//------------------------------------------------------------------- > > +----------- > > +#include > > +#include > > + > > +.data > > + > > +.text > > +.align 3 > > + > > +.global ASM_PFX(RiscVSetScratch) > > +.global ASM_PFX(RiscVGetScratch) > > +.global ASM_PFX(RiscVGetMachineTrapCause) .global > > +ASM_PFX(RiscVReadMachineIE) .global ASM_PFX(RiscVReadMachineIP) > > +.global ASM_PFX(RiscVReadMachineStatus) .global > > +ASM_PFX(RiscVWriteMachineStatus) .global > > +ASM_PFX(RiscVReadMachineTvec) .global > ASM_PFX(RiscVReadMisa) .global > > +ASM_PFX(RiscVReadMVendorId) .global > ASM_PFX(RiscVReadMArchId) .global > > +ASM_PFX(RiscVReadMImplId) >=20 > This could get a lot neater if you replicate what we have for the > ARM/AARCH64 ports and implement an ASM_FUNC macro that does both > the global, and the prefix (and some more stuff that is less imortant no= w we > use lto anyway). >=20 > Have a look in ArmPkg/Include/AsmMacroIoLib*.h >=20 > > +// > > +// Set machine mode scratch. > > +// @param a0 : Pointer to RISCV_MACHINE_MODE_CONTEXT. > > +// > > +ASM_PFX (RiscVSetScratch): > > + csrrw a1, RISCV_CSR_MACHINE_MSCRATCH, a0 > > + ret > > + > > +// > > +// Get machine mode scratch. > > +// @retval a0 : Pointer to RISCV_MACHINE_MODE_CONTEXT. > > +// > > +ASM_PFX (RiscVGetScratch): > > + csrrs a0, RISCV_CSR_MACHINE_MSCRATCH, 0 > > + ret > > + > > +// > > +// Get machine trap cause CSR. > > +// > > +ASM_PFX (RiscVGetMachineTrapCause): > > + csrrs a0, RISCV_CSR_MACHINE_MCAUSE, 0 > > + ret > > + > > +// > > +// Get machine interrupt enable > > +// > > +ASM_PFX (RiscVReadMachineIE): > > + csrr a0, RISCV_CSR_MACHINE_MIE > > + ret > > + > > +// > > +// Get machine interrupt pending > > +// > > +ASM_PFX (RiscVReadMachineIP): > > + csrr a0, RISCV_CSR_MACHINE_MIP > > + ret > > + > > +// > > +// Get machine status > > +// > > +ASM_PFX(RiscVReadMachineStatus): > > + csrr a0, RISCV_CSR_MACHINE_MSTATUS > > + ret > > + > > +// > > +// Set machine status > > +// > > +ASM_PFX(RiscVWriteMachineStatus): > > + csrw RISCV_CSR_MACHINE_MSTATUS, a0 > > + ret > > + > > +// > > +// Get machine trap vector > > +// > > +ASM_PFX(RiscVReadMachineTvec): > > + csrr a0, RISCV_CSR_MACHINE_MTVEC > > + ret > > + > > +// > > +// Read machine ISA > > +// > > +ASM_PFX(RiscVReadMisa): > > + csrr a0, RISCV_CSR_MACHINE_MISA > > + ret > > + > > +// > > +// Read machine vendor ID > > +// > > +ASM_PFX(RiscVReadMVendorId): > > + csrr a0, RISCV_CSR_MACHINE_MVENDORID > > + ret > > + > > +// > > +// Read machine architecture ID > > +// > > +ASM_PFX(RiscVReadMArchId): > > + csrr a0, RISCV_CSR_MACHINE_MARCHID > > + ret > > + > > +// > > +// Read machine implementation ID > > +// > > +ASM_PFX(RiscVReadMImplId): > > + csrr a0, RISCV_CSR_MACHINE_MIMPID > > + ret > > + > > diff --git a/RiscVPkg/Library/RiscVCpuLib/RiscVCpuLib.inf > > b/RiscVPkg/Library/RiscVCpuLib/RiscVCpuLib.inf > > new file mode 100644 > > index 0000000..fc9131b > > --- /dev/null > > +++ b/RiscVPkg/Library/RiscVCpuLib/RiscVCpuLib.inf > > @@ -0,0 +1,34 @@ > > +## @file > > +# RISC-V RV64 CPU library > > +# > > +# Copyright (c) 2016 - 2019, Hewlett Packard Enterprise Development > > +LP. All rights reserved.
# # SPDX-License-Identifier: > > +BSD-2-Clause-Patent # ## > > + > > +[Defines] > > + INF_VERSION =3D 0x0001001b > > + BASE_NAME =3D RiscVCpuLib > > + FILE_GUID =3D 8C6CFB0D-A0EE-40D5-90DA-2E51EAE0= 583F > > + MODULE_TYPE =3D BASE > > + VERSION_STRING =3D 1.0 > > + LIBRARY_CLASS =3D RiscVCpuLib > > + > > +# > > +# The following information is for reference only and not required by= the > build tools. > > +# > > +# VALID_ARCHITECTURES =3D RISCV64 > > +# > > + > > +[Sources] > > + > > +[Sources.RISCV64] > > + Cpu.S > > + > > +[Packages] > > + MdePkg/MdePkg.dec > > + MdeModulePkg/MdeModulePkg.dec >=20 > Please sort the above two alphabetically. >=20 > / > Leif >=20 > > + RiscVPkg/RiscVPkg.dec > > + > > + > > -- > > 2.7.4 > > > > > >=20 > >