From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by mx.groups.io with SMTP id smtpd.web10.55322.1679964743621509279 for ; Mon, 27 Mar 2023 17:52:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ventanamicro.com header.s=google header.b=gG4teoaJ; spf=pass (domain: ventanamicro.com, ip: 209.85.214.174, mailfrom: sunilvl@ventanamicro.com) Received: by mail-pl1-f174.google.com with SMTP id w4so10102501plg.9 for ; Mon, 27 Mar 2023 17:52:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1679964743; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=scYO0Jp+omZWCUNwhpKFjfFcNX4gkhZosWb8EEj0Dac=; b=gG4teoaJSo9WlDojJCBbcMY9VvB7JgZotbMiZPYqgblGM/xJbYkfJNHMLpaAUDgmYl fOsjVSNmP8sZFe6d4N/84U8bUP3sXq/1ubWhepYnc0AZNBQ9Ww4bne2b1r4UBPioqSqp rYljD47csOZuaW4HhApK0o/aGMXX872kZGzSc5R3QIy0xLOqyGq7RpfF68IrfwSA4NsT cea7g2Y00KYSAMGrKb3+wUoH1OpAhbnU0s5kc2BFXRaE9r/SyLbedE9/81EQ6RQ8tXF4 Iw94u0TTZfLm1kZBBsQZ2py8U2KAgvEY9v+LHPwywLkTn9jA3uvfEUKIv87nciCFolsf 0l/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679964743; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=scYO0Jp+omZWCUNwhpKFjfFcNX4gkhZosWb8EEj0Dac=; b=FLkxrGWcMFuSQsdh4XxQfubOJ5wSHfB7udqfozFlTO/m9+6A+yVhyMYav1y0HOqNyB Zw0EHlEenVQCVqHSDxrS5estQ0tWMRzAkcmKWv+c7R/2AKSqSHfTIcS6aA8CQDyaf4Qo COgGQaX8T/UTx890AiAM10bQGIAFv4twd3gq9VKHz/3J0ugKVQOkZi+a5F2qN8ohhvVH teK2tEnmWNDI33Xb8SSSVOW+7dyA2l2Ttule3RelNgLpiPNnuM2PtsqM7aT9zCtBGlTH zxByNUIZiNKeDWs/wQrn6hhknE0VgIl2L7UE/fARyKW609K6TvNcG4pmrXo60AeXG2Rk Y9sg== X-Gm-Message-State: AAQBX9fRN3T9HNKvrfIEL/ym9a35YLlOgFtZ81mZRYPx4ZIO8humJ/yr ZgRRX1b1HYT6y4QimX/iTfGPcQ== X-Google-Smtp-Source: AKy350Zvkk6rjIDfL2hMbjoybmZSIJ/AaoQFtTaAJQ2l6qAW0Y7XLTnXIV7vKkyrJBPUqk1JZQTjKA== X-Received: by 2002:a17:90a:1947:b0:23f:962e:825d with SMTP id 7-20020a17090a194700b0023f962e825dmr12899259pjh.1.1679964743032; Mon, 27 Mar 2023 17:52:23 -0700 (PDT) Return-Path: Received: from sunil-laptop ([106.51.184.50]) by smtp.gmail.com with ESMTPSA id g11-20020a170902934b00b00199203a4fa3sm19685501plp.203.2023.03.27.17.52.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Mar 2023 17:52:22 -0700 (PDT) Date: Tue, 28 Mar 2023 06:22:18 +0530 From: "Sunil V L" To: Dhaval Sharma Cc: devel@edk2.groups.io, Andrei Warkentin , Daniel Schaefer Subject: Re: [PATCH v1 1/2] MdePkg/BaseCacheMaintenanceLib: Enable RISCV CMO Message-ID: References: <20230324154342.180062-1-dhaval@rivosinc.com> <20230324154342.180062-2-dhaval@rivosinc.com> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On Mon, Mar 27, 2023 at 11:29:07PM +0530, Dhaval Sharma wrote: > My comments inline: > > On Mon, Mar 27, 2023 at 9:12 PM Sunil V L wrote: > > > Hi Dhaval, > > > > Thank you for looking at CMO support! > > > > General comments first: > > 1) Please have a cover letter patch and move some part of the commit > > message to cover letter. Please CC all maintainers in the cover letter > > also. > > > > https://edk2.groups.io/g/devel/message/101795?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Ccmo%2C20%2C2%2C0%2C97826395. > Is this the one you are looking for? > Yes, sorry I missed it due to mail filters. > > > 2) Please run BaseTools/Scripts/GetMaintainer.py and CC all maintainers. > > > > Sure. > > > 3) Follow > > > > https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process > > > > Have you run CI tests? > > > I actually did run it but I believe the current edk2 CI is using a GCC5 > based compiler. Hence it failed as it did not recognize cmo instructions as > expected. So I submitted this as WIP patch to sort that out first. > Do let me know if I can follow any other better process here. > > > > On Fri, Mar 24, 2023 at 09:13:41PM +0530, Dhaval Sharma wrote: > > > Adding code to support Cache Management Operations > > > (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs > > > Notes: > > > 1. CMO only supports block based Operations. Meaning complete > > > cache flush/invd/clean Operations are not available > > > 2. Current implementation uses ifence instructions but it > > > maybe platform specific. Many platforms may not support cache > > > Operations based on ifence. > > fence.i? > > > Ack. > > > > > > > IMO, it is better to add a new library such as BaseRiscV64CMOLib and > > included conditionally in the DSC for the platforms which support CMO. > > BaseCacheMaintenanceLib will continue to have default fence.i > > implementation. Is there an issue with this? > > > There are 2 libraries involved here. 1. BaseCacheMaintenanceLib. It is a > generic Lib for multiple archs. So yes it is possible to create another > Lib, but I was thinking if it is possible somehow to create a RV specific > Lib. > 2. BaseLib which contains required .S files. For CBO I have added a > separate .S. Again this is generic Baselib for all Arch. So we need to be > able to differentiate in DSC now for both these libs. I am not sure if this > is the > best way to address this. I could try to do inline assembly within > CMOCachelib to address #2. > I was thinking single independent library of CacheMaintenanceLib class for CMO exclusively. Let BaseLib/BaseCacheMaintenanceLib continue to use the default fence.i implementation. The DSC for the platform can chose between default vs CMO libraries. > > > > 3. For now adding CMO on top of ifence as it is not considered > > > harmful. > > > 4. This requires support for GCC12.2 onwards. > > > > > Yeah, this is another challenge like zifencei_zicsr which we could > > workaround and support both older and newer tool chain. But for CMO, > > I don't see any option but to support only GCC12.2+. > > > > How do we support this in CI? > Oliver has a patch to update CI image to GCC12. I think it is not yet merged. But I have not checked whether it is 12.2. You can run CI including that patch with yours and try. https://edk2.groups.io/g/devel/message/101164