Hi Sunil,

I agree with your most of comments, and need some time to do the change.

There are some opens below.

3) In the latest series, I see you have 2 commits where first commit
adds .inc file and in the next commit itself it gets removed. I see no
reason to do so.

[Evan] I marked this change in commit log 'Unified .dsc files into a single'

4) Why would .inc files cause fragmentation? Rather, I think a common
.inc (qemu and real servers) will reduce the effort and copying same for
several platforms.

[Evan] I acknowledge that the .inc files have extracted many common parts and reduced some porting effort. However, this approach has not been thoroughly implemented: 1. As I remember, some libraries are defined in multiple files; 2. Several libraries that should belong to the common part are still in the .dsc files for unknown reasons, such as CapsuleLib, BootLogoLib, FrameBufferBltLib, TPM, and so on. Given the unclear separation, it might be better not to implement it at all. The SBSA dsc file follows the traditional approach, and it doesn't seem to cause much inconvenience in usage. Additionally, what worries me is that many firmware engineers are not clear about which parts are common (platform-independent) and which are not. This separation approach might confuse them and make the codebase more chaotic.

Anyway, I will approach this issue with an open mind and welcome any additional suggestions at any time.

BR,

Evan

------------------------------------------------------------------
发件人：Sunil V L <sunilvl@ventanamicro.com>
发送时间：2024年7月10日(星期三) 22:33
收件人：devel<devel@edk2.groups.io>; "evan.chai"<evan.chai@linux.alibaba.com>
主　题：Re: [edk2-devel] 转发：[edk2-devel] [PATCH V2 0/2] Initial commit for RISC-V Qemu-based Server

Hi Evan,

On Wed, Jul 10, 2024 at 02:14:21AM -0700, EvanChai via groups.io wrote:
> Hi Sunil,
> I just saw two comments below.
> 'These images are generic. I don't think we need to have a copy here. Ifyou need to refer to this in any documentation, please add the link tooriginal image/document.'
> ‘It is better to split these documentation changes to a separate commit.’
>
Oh OK.

At high level, here is my feedback.

1) It is wonderful that you have added lot of details including design
of qemu, todo etc. However, I think it is better to keep Readme simple
and provide a reference to RISE wiki page where you can add all these
details. That way, we can avoid having stale data in Readme or need to
do several patches for updates. Better to keep readme with just build
and test instructions and a link to more details. For example, adding
qemu design (ex: memory map) in edk2 readme. Every time qemu design
changes, the edk2 readme becomes stale and keeping them in sync may
not be productive.

2) Please split documentation changes (readme) and platform changes (dsc
etc) to separate commits.

3) In the latest series, I see you have 2 commits where first commit
adds .inc file and in the next commit itself it gets removed. I see no
reason to do so.

4) Why would .inc files cause fragmentation? Rather, I think a common
.inc (qemu and real servers) will reduce the effort and copying same for
several platforms.

5) I have few more comments on the contents of readme as well. But if it
can be moved to wiki, we can discuss and correct there.

> BTW, the only uploaded picture comes from my presentation on the RV server platform proposal. It doesn't have an online source because I drew it myself. Do you have any recommended places where I can upload it? Then we can just reference its link in the README.
>
My recommendation is to keep them in RISE wiki for now.

Thanks,
Sunil