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 5ED057803ED for ; Tue, 12 Sep 2023 09:36:14 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=OEa9CYF9V0hbnIlwjTYR/5LcfX4kYtd5IPZlaYje1m4=; c=relaxed/simple; d=groups.io; h=Date:From:To:CC:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20140610; t=1694511373; v=1; b=oXMXmGVLkD9tjheqr7usPwYbufDTOEr+VFAf4Tl1AR+1FUbVUl6dVW97UzDMmyvbLin+esbY H4tlKOk3ajwlR2jGMVfrToLocdfjmjQkdr+RcldxawjU1C7u7WcPs90mp2ARwG82173B0fYYChy bUdcii6LjvMD3J4w6mqqDnoA= X-Received: by 127.0.0.2 with SMTP id 35GkYY7687511xXppWXG92zd; Tue, 12 Sep 2023 02:36:13 -0700 X-Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web11.21242.1694511372271117295 for ; Tue, 12 Sep 2023 02:36:12 -0700 X-Received: from pps.filterd (m0279870.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 38C6uq6J024166; Tue, 12 Sep 2023 09:36:08 GMT X-Received: from nasanppmta03.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3t1xkju2av-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 12 Sep 2023 09:36:07 +0000 X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA03.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 38C9Zr2h018830 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 12 Sep 2023 09:35:53 GMT X-Received: from qc-i7.hemma.eciton.net (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.36; Tue, 12 Sep 2023 02:35:50 -0700 Date: Tue, 12 Sep 2023 10:35:47 +0100 From: "Leif Lindholm" To: Jeff Brasen CC: , , , , Vidya Sagar , Shanker Donthineni Subject: Re: [edk2-devel] [PATCH 1/2] DynamicTablesPkg: AML Code generation for I/O ranges Message-ID: References: <64a84c1fedd66114f46ddd5d47ff922d68169d4c.1694475927.git.jbrasen@nvidia.com> MIME-Version: 1.0 In-Reply-To: <64a84c1fedd66114f46ddd5d47ff922d68169d4c.1694475927.git.jbrasen@nvidia.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-GUID: JDQz9-z57lPr01WmvNMqA-loGh9zT5VZ X-Proofpoint-ORIG-GUID: JDQz9-z57lPr01WmvNMqA-loGh9zT5VZ 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,quic_llindhol@quicinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: Wy70nCc0yLhYVBupsgKnPz3vx7686176AA= Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=oXMXmGVL; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.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 Hi Jeff, On Mon, Sep 11, 2023 at 23:48:57 +0000, Jeff Brasen wrote: > From: Vidya Sagar > > Add helper functions to generate AML Resource Data describing I/O > ranges of four words long. API AmlCodeGenRdQWordIo () is exposed. > > Reviewed-by: Shanker Donthineni The above isn't really applicable to upstream. Although I feel less strongly about that than > Signed-off-by: Vidya Sagar The DCO is a statement that you have performed basic legal due diligence on the provenance of the change. I'm uncomfortable with people making such statements on behalf of others. If this is being upstreamed from a downstream repository, such that the review trail is available there, then both of these could be fine. But I think it would be useful to include a link to the patch in that repository in the commit message in that case. One technical, but not necessarily for this set (it just made me spot it), note below. > Signed-off-by: Jeff Brasen > --- > .../Include/Library/AmlLib/AmlLib.h | 67 ++++++++++++++ > .../AmlLib/CodeGen/AmlResourceDataCodeGen.c | 90 +++++++++++++++++++ > 2 files changed, 157 insertions(+) > > diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > index 9210c5091548..8e24cecdd77b 100644 > --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > @@ -683,6 +683,73 @@ AmlCodeGenRdWordBusNumber ( > OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL > ); > > +/** Code generation for the "QWordIO ()" ASL function. > + > + The Resource Data effectively created is a QWord Address Space Resource > + Data. Cf ACPI 6.4: > + - s6.4.3.5.1 "QWord Address Space Descriptor". > + - s19.6.109 "QWordIO". > + > + The created resource data node can be: > + - appended to the list of resource data elements of the NameOpNode. > + In such case NameOpNode must be defined by a the "Name ()" ASL statement > + and initially contain a "ResourceTemplate ()". > + - returned through the NewRdNode parameter. > + > + See ACPI 6.4 spec, s19.6.109 for more. > + > + @param [in] IsResourceConsumer ResourceUsage parameter. > + @param [in] IsMinFixed Minimum address is fixed. > + @param [in] IsMaxFixed Maximum address is fixed. > + @param [in] IsPosDecode Decode parameter > + @param [in] IsaRanges Possible values are: > + 0-Reserved > + 1-NonISAOnly > + 2-ISAOnly > + 3-EntireRange This is an existing antipattern which this patch (rightly) adheres to when adding an additional variant of an existing API. But this also pushes the count to three functions in the same file where we're doing enum-but-in-doxygen and then keep magic values in the code. I think someone should rewrite this as an enum and get rid of the magic values in the callers. An additional antipattern is that because the doxygen stanza becomes exessively bulky, it leaves out actually documenting the parameter at all. But as I said, that's not the fault of this set, and does not need to be fixed by it. / Leif > + @param [in] AddressGranularity Address granularity. > + @param [in] AddressMinimum Minimum address. > + @param [in] AddressMaximum Maximum address. > + @param [in] AddressTranslation Address translation. > + @param [in] RangeLength Range length. > + @param [in] ResourceSourceIndex Resource Source index. > + Unused. Must be 0. > + @param [in] ResourceSource Resource Source. > + Unused. Must be NULL. > + @param [in] IsDenseTranslation TranslationDensity parameter. > + @param [in] IsTypeStatic TranslationType parameter. > + @param [in] NameOpNode NameOp object node defining a named object. > + If provided, append the new resource data > + node to the list of resource data elements > + of this node. > + @param [out] NewRdNode If provided and success, > + contain the created node. > + > + @retval EFI_SUCCESS The function completed successfully. > + @retval EFI_INVALID_PARAMETER Invalid parameter. > + @retval EFI_OUT_OF_RESOURCES Could not allocate memory. > +**/ > +EFI_STATUS > +EFIAPI > +AmlCodeGenRdQWordIo ( > + IN BOOLEAN IsResourceConsumer, > + IN BOOLEAN IsMinFixed, > + IN BOOLEAN IsMaxFixed, > + IN BOOLEAN IsPosDecode, > + IN UINT8 IsaRanges, > + IN UINT64 AddressGranularity, > + IN UINT64 AddressMinimum, > + IN UINT64 AddressMaximum, > + IN UINT64 AddressTranslation, > + IN UINT64 RangeLength, > + IN UINT8 ResourceSourceIndex, > + IN CONST CHAR8 *ResourceSource, > + IN BOOLEAN IsDenseTranslation, > + IN BOOLEAN IsTypeStatic, > + IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL > + OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL > + ); > + > /** Code generation for the "QWordMemory ()" ASL function. > > The Resource Data effectively created is a QWord Address Space Resource > diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c > index 4ca63ccd2396..9c6700b9e08c 100644 > --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c > +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c > @@ -1012,6 +1012,96 @@ AmlCodeGenRdQWordSpace ( > return LinkRdNode (RdNode, NameOpNode, NewRdNode); > } > > +/** Code generation for the "QWordIO ()" ASL function. > + > + The Resource Data effectively created is a QWord Address Space Resource > + Data. Cf ACPI 6.4: > + - s6.4.3.5.1 "QWord Address Space Descriptor". > + - s19.6.109 "QWordIO". > + > + The created resource data node can be: > + - appended to the list of resource data elements of the NameOpNode. > + In such case NameOpNode must be defined by a the "Name ()" ASL statement > + and initially contain a "ResourceTemplate ()". > + - returned through the NewRdNode parameter. > + > + See ACPI 6.4 spec, s19.6.109 for more. > + > + @param [in] IsResourceConsumer ResourceUsage parameter. > + @param [in] IsMinFixed Minimum address is fixed. > + @param [in] IsMaxFixed Maximum address is fixed. > + @param [in] IsPosDecode Decode parameter > + @param [in] IsaRanges Possible values are: > + 0-Reserved > + 1-NonISAOnly > + 2-ISAOnly > + 3-EntireRange > + @param [in] AddressGranularity Address granularity. > + @param [in] AddressMinimum Minimum address. > + @param [in] AddressMaximum Maximum address. > + @param [in] AddressTranslation Address translation. > + @param [in] RangeLength Range length. > + @param [in] ResourceSourceIndex Resource Source index. > + Unused. Must be 0. > + @param [in] ResourceSource Resource Source. > + Unused. Must be NULL. > + @param [in] IsDenseTranslation TranslationDensity parameter. > + @param [in] IsTypeStatic TranslationType parameter. > + @param [in] NameOpNode NameOp object node defining a named object. > + If provided, append the new resource data > + node to the list of resource data elements > + of this node. > + @param [out] NewRdNode If provided and success, > + contain the created node. > + > + @retval EFI_SUCCESS The function completed successfully. > + @retval EFI_INVALID_PARAMETER Invalid parameter. > + @retval EFI_OUT_OF_RESOURCES Could not allocate memory. > +**/ > +EFI_STATUS > +EFIAPI > +AmlCodeGenRdQWordIo ( > + IN BOOLEAN IsResourceConsumer, > + IN BOOLEAN IsMinFixed, > + IN BOOLEAN IsMaxFixed, > + IN BOOLEAN IsPosDecode, > + IN UINT8 IsaRanges, > + IN UINT64 AddressGranularity, > + IN UINT64 AddressMinimum, > + IN UINT64 AddressMaximum, > + IN UINT64 AddressTranslation, > + IN UINT64 RangeLength, > + IN UINT8 ResourceSourceIndex, > + IN CONST CHAR8 *ResourceSource, > + IN BOOLEAN IsDenseTranslation, > + IN BOOLEAN IsTypeStatic, > + IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL > + OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL > + ) > +{ > + return AmlCodeGenRdQWordSpace ( > + ACPI_ADDRESS_SPACE_TYPE_IO, > + IsResourceConsumer, > + IsPosDecode, > + IsMinFixed, > + IsMaxFixed, > + RdIoRangeSpecificFlags ( > + IsaRanges, > + IsDenseTranslation, > + IsTypeStatic > + ), > + AddressGranularity, > + AddressMinimum, > + AddressMaximum, > + AddressTranslation, > + RangeLength, > + ResourceSourceIndex, > + ResourceSource, > + NameOpNode, > + NewRdNode > + ); > +} > + > /** Code generation for the "QWordMemory ()" ASL function. > > The Resource Data effectively created is a QWord Address Space Resource > -- > 2.25.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108523): https://edk2.groups.io/g/devel/message/108523 Mute This Topic: https://groups.io/mt/101305535/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-