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 BBD22AC0DFD for ; Mon, 25 Sep 2023 09:40:17 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=zC46Y3YpT1fCStanOWhxFE4oQMArTgXgj/U4ArK7xHw=; 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=1695634816; v=1; b=kCNYWl+JaoXvjN+npsg/aPVu2YLZPdto4M4LJdTnRqTYn7v4l7X2oxybbglqAKwIEmL37jvA /z5j5BzCNXJV35DcQEcROIcmdfZXdFjhxF4/QfKF8xlNqFnYBPsREua6padF1raJQliEBZ9v7AI 6OJ9mhsf7SA83ek2qBPs9JVQ= X-Received: by 127.0.0.2 with SMTP id T3XmYY7687511xooMohuyaJh; Mon, 25 Sep 2023 02:40:16 -0700 X-Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by mx.groups.io with SMTP id smtpd.web10.57159.1695634815916756980 for ; Mon, 25 Sep 2023 02:40:15 -0700 X-Received: from pps.filterd (m0279865.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 38P8dcRT009693; Mon, 25 Sep 2023 09:40:12 GMT X-Received: from nasanppmta05.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3t9pywbuuk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 25 Sep 2023 09:40:12 +0000 X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA05.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 38P9eBFg011378 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 25 Sep 2023 09:40:11 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; Mon, 25 Sep 2023 02:40:10 -0700 Date: Mon, 25 Sep 2023 10:40:06 +0100 From: "Leif Lindholm" To: , CC: , Subject: Re: [edk2-devel] [PATCH] DynamicTablesPkg: IORT generator updates for Rev E.e spec Message-ID: References: <273efda2552363dd2ed5794b00b1b4e87b290909.1695418444.git.swatisrik@nvidia.com> MIME-Version: 1.0 In-Reply-To: <273efda2552363dd2ed5794b00b1b4e87b290909.1695418444.git.swatisrik@nvidia.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-GUID: -1osfqnpqWUihAr5Q7QyCY57XgRjhls8 X-Proofpoint-ORIG-GUID: -1osfqnpqWUihAr5Q7QyCY57XgRjhls8 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: DhcB6qR8vEkbCaRi1URLx4X3x7686176AA= 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=kCNYWl+J; 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 On Fri, Sep 22, 2023 at 15:40:38 -0600, Swatisri Kantamsetti via groups.io wrote: > The IO Remapping Table, Platform Design Document, Revision E.e, > Sept 2022 (https://developer.arm.com/documentation/den0049/ee) > added flags in SMMUv3 node for validity of ID mappings for MSIs > related to control interrupts. > It makes one small addition to SMMUv3 nodes to > describe MSI support independently of wired GSIV support > > Therefore, update the IORT generator to: > - increment IORT table revision to 6 > - increment SMMUV3 node revision to 5 > - for SMMUV3 node revision >=5 check the DeviceID mapping index > valid flag to populate DeviceIdMappingIndex field > > Signed-off-by: Swatisri Kantamsetti > --- > .../Acpi/Arm/AcpiIortLibArm/IortGenerator.c | 35 +++++++++++++++---- > 1 file changed, 28 insertions(+), 7 deletions(-) > > diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > index f28973c1a8..a6e4b49cb1 100644 > --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c > @@ -5,7 +5,7 @@ > SPDX-License-Identifier: BSD-2-Clause-Patent > > @par Reference(s): > - - IO Remapping Table, Platform Design Document, Revision E.d, Feb 2022 > + - IO Remapping Table, Platform Design Document, Revision E.e, Sept 2022 > (https://developer.arm.com/documentation/den0049/) > > **/ > @@ -1554,9 +1554,14 @@ AddSmmuV3Nodes ( > { > SmmuV3Node->Node.Revision = 2; > SmmuV3Node->Node.Identifier = EFI_ACPI_RESERVED_DWORD; > - } else { > + } else if (AcpiTableInfo->AcpiTableRevision < > + EFI_ACPI_IO_REMAPPING_TABLE_REVISION_06) > + { > SmmuV3Node->Node.Revision = 4; > SmmuV3Node->Node.Identifier = NodeList->Identifier; > + } else { > + SmmuV3Node->Node.Revision = 5; > + SmmuV3Node->Node.Identifier = NodeList->Identifier; This is borderline, but I think it would be worth breaking out into a helper function. It feels like this may have further cases added in future. > } > > // SMMUv3 specific data > @@ -1577,11 +1582,27 @@ AddSmmuV3Nodes ( > SmmuV3Node->ProximityDomain = 0; > } > > - if ((SmmuV3Node->Event != 0) && (SmmuV3Node->Pri != 0) && > - (SmmuV3Node->Gerr != 0) && (SmmuV3Node->Sync != 0)) > + /* For older SMMUV3 nodes rev. < 5. > + If all the SMMU control interrupts are GSIV based, > + the DeviceID mapping index field is ignored. > + DeviceID mapping valid flag was introduced in IORT rev E.e > + for SMMUV3 nodes rev. > 5. > + If the DeviceID mapping index valid flag is set to 0, > + DeviceID mapping index field must be ignored. > + Where the SMMU uses message signaled interrupts for > + its control interrupts, DeviceId Mapping Index contains an > + index into the array of ID mapping. > + */ > + if (((SmmuV3Node->Node.Revision < 5) && > + (SmmuV3Node->Event != 0) && > + (SmmuV3Node->Pri != 0) && > + (SmmuV3Node->Gerr != 0) && > + (SmmuV3Node->Sync != 0) > + ) || > + ((SmmuV3Node->Node.Revision >= 5) && > + ((SmmuV3Node->Flags & EFI_ACPI_IORT_SMMUv3_FLAG_DEVICEID_VALID) == 0)) > + ) This is not borderline. I'm sure all of this is correct, but it is no longer human readable. This type of information overload is better kept out of a top-level function. This also means we can give it an informative name. Best Regards, Leif p.s. Apologies Sami, but since I spotted this before it had been merged by CI, I dropped the "push" tag since I wanted to make a comment. You are the maintainer, and the above are my opinions only. If you disagree, feel free to add it back and push as is. (And feel free to do the same to push requests initiated by me in future.) > { > - // If all the SMMU control interrupts are GSIV based, > - // the DeviceID mapping index field is ignored. > SmmuV3Node->DeviceIdMappingIndex = 0; > } else { > SmmuV3Node->DeviceIdMappingIndex = NodeList->DeviceIdMappingIndex; > @@ -2819,7 +2840,7 @@ ACPI_IORT_GENERATOR IortGenerator = { > // ACPI Table Signature > EFI_ACPI_6_4_IO_REMAPPING_TABLE_SIGNATURE, > // ACPI Table Revision supported by this Generator > - EFI_ACPI_IO_REMAPPING_TABLE_REVISION_05, > + EFI_ACPI_IO_REMAPPING_TABLE_REVISION_06, > // Minimum supported ACPI Table Revision > EFI_ACPI_IO_REMAPPING_TABLE_REVISION_00, > // Creator ID > -- > 2.17.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109039): https://edk2.groups.io/g/devel/message/109039 Mute This Topic: https://groups.io/mt/101535844/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-