<div dir="ltr"><div>My comments inline:</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 27, 2023 at 9:12 PM Sunil V L <<a href="mailto:sunilvl@ventanamicro.com">sunilvl@ventanamicro.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Dhaval,<br>
<br>
Thank you for looking at CMO support!<br>
<br>
General comments first:<br>
1) Please have a cover letter patch and move some part of the commit </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
message to cover letter. Please CC all maintainers in the cover letter<br>
also.<br></blockquote><div><br></div><div><a href="https://edk2.groups.io/g/devel/message/101795?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Ccmo%2C20%2C2%2C0%2C97826395">https://edk2.groups.io/g/devel/message/101795?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Ccmo%2C20%2C2%2C0%2C97826395</a>. Is this the one you are looking for?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
2) Please run BaseTools/Scripts/GetMaintainer.py and CC all maintainers.<br></blockquote><div><br></div><div>Sure.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
3) Follow<br>
<a href="https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process" rel="noreferrer" target="_blank">https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process</a><br>
<br>
Have you run CI tests?<br></blockquote><div>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.</div><div>Do let me know if I can follow any other better process here.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
On Fri, Mar 24, 2023 at 09:13:41PM +0530, Dhaval Sharma wrote:<br>
> Adding code to support Cache Management Operations<br>
> (CMO) defined by RV spec <a href="https://github.com/riscv/riscv-CMOs" rel="noreferrer" target="_blank">https://github.com/riscv/riscv-CMOs</a><br>
> Notes:<br>
> 1. CMO only supports block based Operations. Meaning complete<br>
> cache flush/invd/clean Operations are not available<br>
> 2. Current implementation uses ifence instructions but it<br>
> maybe platform specific. Many platforms may not support cache<br>
> Operations based on ifence.<br>
fence.i?</blockquote><div><br></div><div>Ack. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
<br>
IMO, it is better to add a new library such as BaseRiscV64CMOLib and<br>
included conditionally in the DSC for the platforms which support CMO.<br>
BaseCacheMaintenanceLib will continue to have default fence.i<br>
implementation. Is there an issue with this?<br></blockquote><div>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.</div><div>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</div><div>best way to address this. I could try to do inline assembly within CMOCachelib to address #2.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> 3. For now adding CMO on top of ifence as it is not considered<br>
> harmful.<br>
> 4. This requires support for GCC12.2 onwards.<br>
><br>
Yeah, this is another challenge like zifencei_zicsr which we could<br>
workaround and support both older and newer tool chain. But for CMO, <br>
I don't see any option but to support only GCC12.2+.<br></blockquote><div><br></div><div>How do we support this in CI?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks,<br>
Sunil<br>
</blockquote></div><br clear="all"><div><br></div><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr">Thanks!<div>=D</div></div></div></div>
<div width="1" style="color:white;clear:both">_._,_._,_</div>
<hr>
Groups.io Links:<p>
You receive all messages sent to this group.
<p>
<a target="_blank" href="https://edk2.groups.io/g/devel/message/101978">View/Reply Online (#101978)</a> |
|
<a target="_blank" href="https://groups.io/mt/97826396/1813853">Mute This Topic</a>
| <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br>
<a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> |
<a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |
<a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>
[edk2-devel-archive@redhat.com]<br>
<div width="1" style="color:white;clear:both">_._,_._,_</div>