From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web12.48974.1598862152322113495 for ; Mon, 31 Aug 2020 01:22:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=R6ebDhsF; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1598862151; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=l0nJ6KK/M7pGdzKBTXJpUVIADH57b2nC6Yir3oMiP8E=; b=R6ebDhsFP2xvko0c1HJbyU4lfqvDC3k+rXYF4yS15JsdbIC0lEt1ybnL04D/WPAFGOacjS TrPadeEPF0SnhYHivfFHqzeHnF2xFjWnoWEJk82gvkOtBCxnjXgm/bVaHoYKCG36nNz4QK i+VoS7koYl6Y4Z1XjR2Bz5GC8S/W/dQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-241-PT_cxkRzO8-qxTEwKB7Dtw-1; Mon, 31 Aug 2020 04:22:21 -0400 X-MC-Unique: PT_cxkRzO8-qxTEwKB7Dtw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 971AF18A2249; Mon, 31 Aug 2020 08:22:20 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-125.ams2.redhat.com [10.36.113.125]) by smtp.corp.redhat.com (Postfix) with ESMTP id 039642616B; Mon, 31 Aug 2020 08:22:18 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition To: "Grimes, Paul" , "devel@edk2.groups.io" Cc: Michael D Kinney , Liming Gao , Zhiguang Liu References: <20200827204051.777-1-Paul.Grimes@amd.com> <20200827204051.777-2-Paul.Grimes@amd.com> <12ee54f0-976f-239c-81c7-3c6e71cc8892@redhat.com> From: "Laszlo Ersek" Message-ID: <768f5ed7-2bf6-e862-93bf-d8f1789ed29e@redhat.com> Date: Mon, 31 Aug 2020 10:22:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0.002 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US On 08/28/20 20:39, Grimes, Paul wrote: > [AMD Public Use] > > Thanks for the feedback, Lazlo. I agree with your point on the optimal format for these #defines. I think it would be best to submit the current patch as is given that the same feedback could (should?) be applied to various other #defines in the file, > eg: > EFI_ACPI_DMA_BUS_MASTER_MASK 0x04, which only applies to Bit 2 and > EFI_ACPI_IRQ_POLARITY_MASK 0x08, ... Bit 3 and > EFI_ACPI_IRQ_MODE 0x01, ... bit 0 > > IMO if these defines were to be updated for clarity, it should probably be done for the whole file in a separate commit. Sure, I'm OK with the patch as posted. Laszlo > > Thanks, > Paul > > -----Original Message----- > From: Laszlo Ersek > Sent: Friday, August 28, 2020 10:06 AM > To: devel@edk2.groups.io; Grimes, Paul > Cc: Michael D Kinney ; Liming Gao ; Zhiguang Liu > Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Correcting EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT definition > > [CAUTION: External Email] > > On 08/27/20 22:40, Paul wrote: >> In Acpi10.h, EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT is defined as 0x10, but >> should be 0x02 per the ACPI Specification. >> >> REF:https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F >> bugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2937&data=02%7C01%7Cp >> aul.grimes%40amd.com%7C82b28bb6544a4612fc1108d84b749dc6%7C3dd8961fe488 >> 4e608e11a82d994e183d%7C0%7C0%7C637342311528396385&sdata=7vHYIHHaJU >> 4yrXzAWtv5xTf%2BQfclAUBusz278%2F6I%2BRY%3D&reserved=0 >> >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> Signed-off-by: Paul G >> --- >> MdePkg/Include/IndustryStandard/Acpi10.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/MdePkg/Include/IndustryStandard/Acpi10.h >> b/MdePkg/Include/IndustryStandard/Acpi10.h >> index fa06eefbb6e6..adeb5ae8c219 100644 >> --- a/MdePkg/Include/IndustryStandard/Acpi10.h >> +++ b/MdePkg/Include/IndustryStandard/Acpi10.h >> @@ -358,7 +358,7 @@ typedef struct { >> #define EFI_ACPI_DMA_TRANSFER_TYPE_MASK 0x03 >> >> #define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT 0x00 >> >> #define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT_AND_16_BIT 0x01 >> >> -#define EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT 0x10 >> >> +#define EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT 0x02 >> >> >> >> // >> >> // IO Information >> > > Good catch. The ACPI spec text was likely cut n' pasted into the edk2 source, and then prefixed with "0x". The spec says, > > """ > Bits [1:0] DMA transfer type preference, _SIZ > 00 8-bit only > 01 8- and 16-bit > 10 16-bit only > 11 Reserved > """ > > but that's in binary, not in hexadecimal. > > In fact, the leading zero on *all four* macros (including > EFI_ACPI_DMA_TRANSFER_TYPE_MASK) is misleading. In hex, the leading zero in the current macros stands for bits [7:4], which are completely irrelevant for the _SIZ bit-field in the DMA Descriptor. So optimally we'd have > > #define EFI_ACPI_DMA_TRANSFER_TYPE_MASK 0x3 > #define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT 0x0 > #define EFI_ACPI_DMA_TRANSFER_TYPE_8_BIT_AND_16_BIT 0x1 > #define EFI_ACPI_DMA_TRANSFER_TYPE_16_BIT 0x2 > > But I agree the current patch is OK too: > > Reviewed-by: Laszlo Ersek > > I also agree it's a bugfix and should be merged now. > > Thanks > Laszlo >