From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 05685AC14B7 for ; Tue, 6 Feb 2024 14:32:32 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=34gf2TAgS9iCbkJC15cGdQEauA/XNf0UT9Er00qhhuQ=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1707229951; v=1; b=TNN+D8EDCtnRhvsLJdJZIfMxK3UiFXCu+iuK3n5xKou4eUcMN1lSr55TkcMHM7iDrVD67dGt YnoaIqMv+P1gAIBEjjbVyRN/wzzI+C4+n9nB/fb6XruVZLWppkZQj9ryDMJCwzK3kySseL55rux 6BEJ+qgPDdCuh7EtL3j4+DA0= X-Received: by 127.0.0.2 with SMTP id SWufYY7687511xbcKPcgcw1b; Tue, 06 Feb 2024 06:32:31 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.22857.1707229950412041871 for ; Tue, 06 Feb 2024 06:32:31 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-299-TfS-IdPKNTuG7L53o5aLQw-1; Tue, 06 Feb 2024 09:32:23 -0500 X-MC-Unique: TfS-IdPKNTuG7L53o5aLQw-1 X-Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B1D8328EC11F; Tue, 6 Feb 2024 14:32:22 +0000 (UTC) X-Received: from [10.39.195.129] (unknown [10.39.195.129]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5AAD61121312; Tue, 6 Feb 2024 14:32:20 +0000 (UTC) Message-ID: Date: Tue, 6 Feb 2024 15:32:19 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v8 14/37] UefiCpuPkg: Add CpuMmuLib to UefiCpuPkg To: devel@edk2.groups.io, lichao@loongson.cn, Pedro Falcato Cc: Eric Dong , Ray Ni , Rahul Kumar , Gerd Hoffmann , Baoqi Zhang , Dongyan Qian , Xianglai Li , Bibo Mao , Andrew Fish , "Kinney, Michael D" , Leif Lindholm References: <20240126062715.3099433-1-lichao@loongson.cn> <20240126062919.3101691-1-lichao@loongson.cn> <3fe0fda8-d32e-679e-2f71-6cc35e7772b8@redhat.com> <17B0898B4883051D.13964@groups.io> <6914d79c-582a-4108-b733-7d5b15537ab4@loongson.cn> From: "Laszlo Ersek" In-Reply-To: <6914d79c-582a-4108-b733-7d5b15537ab4@loongson.cn> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 8eHrAXEV229uxgd3O741pdcLx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=TNN+D8ED; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 2/6/24 03:57, Chao Li wrote: > Hi Pedro, Laszlo, > > I have double-check this patch and I'd like to point out the following: > > 1. Some of the code in this patch actually refers to the Linux kernel. > But only used the name of kernel code , such as PUD, PMD, PTE, Swapper > Page Dir, etc. > > 2. Refer to the LoongArch TLB rules, if we use the multi-level page > table to search and filling, the usual logic is as follows: > >     a. Get the first level page table, called PGD, in reigster PGDL or PGDH. > >     b. PGD will index the 2st Dir, and the 2st Dir will index the 3st > Dir, and so on. > >     c. We would like to use the four-level page table, because if we use > two- or three-level page tables, the page tables will larger and must be > continuous, so we think the four-level is better(Linux kernel also uses > the four-level page tables). > >     d. LoongArch paging logic is shown in the image below(URL: > https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#memory-management-of-page-table-mapping): > > Referring to the above, should I use the old logic, rewrite the Linux > kernel-like code to EFI style and clean up some unused code and redesign > the PEI library, it that OK? I don't like the expression "rewrite the Linux kernel-like code to EFI style". You should not start with, or incorporate, anything that comes from Linux. Whatever you do, make sure it is not a derivative of Linux. Don't *derive* your edk2 contribution from Linux code. I think you are fine to implement any paging logic you prefer, as long as you start with an empty source file, and consult only the public LoongArch architecture specs, plus the rest of edk2 while writing the code. Consider the current source files from this patch "tainted". I don't encourage a piecemeal approach on this, "replace this", "rewrite that". The files look like they are a derivative of Linux code, so any further modifications in these files will just result in more derived code. Let's not play "where does this line come from" games. Scrap it all; implement it again from the ground up, using only the public arch specs. Just my opinion. Laszlo > > > Thanks, > Chao > On 2024/2/4 10:58, Chao Li wrote: >> >> Hi Leif and Pedro, >> >> On 2024/2/2 23:14, Leif Lindholm wrote: >>> On 2024-02-01 19:36, Pedro Falcato wrote: >>>> On Thu, Feb 1, 2024 at 3:05 AM Chao Li wrote: >>>>> >>>>> Hi Pedro and Laszlo, >>>>> >>>>> Part of the code in this patch is indeed quoted from the Linux >>>>> kernel, and do you think it is inapproparate? If so, we need to >>>>> refactor this module, what are you suggests with the refactoring? >>>>> Just remove the unused logic from the Kernel code and keep the >>>>> logic good or refactor from scratch? >>>> >>>> +CC stewards >>>> >>>> Disclaimer: I'm not a lawyer >>>> >>>> It is wildly inappropriate. All of the code was clearly inspired by >>>> GPL and derives from the Linux GPL code, it's not just unused logic. >>>> You should triple check *every other patch* you've sent out for these >>>> kinds of GPL violations. >>> >>> I want to highlight >>> https://github.com/tianocore/edk2/blob/master/ReadMe.rst?plain=1#L177 >>> >>> Chao, by adding your Signed-off-by to any patch and sending it out, >>> you certify that: >>> >>> (a) The contribution was created in whole or in part by me and I >>>     have the right to submit it under the open source license >>>     indicated in the file; or >>> >>> (b) The contribution is based upon previous work that, to the best >>>     of my knowledge, is covered under an appropriate open source >>>     license and I have the right under that license to submit that >>>     work with modifications, whether created in whole or in part >>>     by me, under the same open source license (unless I am >>>     permitted to submit under a different license), as indicated >>>     in the file; or >>> >>> (c) The contribution was provided directly to me by some other >>>     person who certified (a), (b) or (c) and I have not modified >>>     it. >>> >>> (d) I understand and agree that this project and the contribution >>>     are public and that a record of the contribution (including all >>>     personal information I submit with it, including my sign-off) is >>>     maintained indefinitely and may be redistributed consistent with >>>     this project or the open source license(s) involved. >>> >>> Now, that's a bunch of legalese, but it matters. >>> Mistakes happen, but it would have been a massive headache if this >>> had been merged and *then* we found out about this. The *best case* >>> scenario would have been that we would have been forced to revert the >>> whole set. >>> >>> I wouldn't say you need to "triple check every patch", but I would >>> say you need to re-evaluate the existing patches based on this new >>> information you have learned. So that once you resubmit a version as >>> per Laszlos comments in separate email, you are comfortable that the >>> whole submission conforms with the DCO. >>> And if you are unsure - ask. That's never wrong. >>> >>> Pedro - many thanks for this. Owe you one. >> Of couse, I will study the entries you are referring to and I'm sure >> that the Part 1 series is full compliance with terms, and I will >> double/triple check them when I sumbit the Part 2, thank you for >> spotting this and preventing me form making a mistake. >>> >>> / >>>     Leif >>> >>>> There's another way of writing this sort of code (that doesn't involve >>>> all the Linux mm craziness) but I don't know if changing strategies >>>> would be considered getting rid of any shadow of GPL/IP violation. >>>> >>>> (As a side note, I don't really understand IP in the software world. >>>> If you work, say, on GPL software for a moment in time, are you always >>>> going to be "GPL-tainted"? Surely not? Most people in the industry >>>> I've talked to about this say that, yeah, no, corps don't expect that. >>>> But no one really seems to have drawn a line between OK and not-OK, >>>> but rather "please please don't sue us". And in this case I don't know >>>> (but I suspect it'd be uncomfortable) for someone to redesign a >>>> solution right away, after being "tainted". Anyway, tough problem, and >>>> IANAL :/) >>>> >> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115164): https://edk2.groups.io/g/devel/message/115164 Mute This Topic: https://groups.io/mt/103971653/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-