From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1D38F21E47D65 for ; Wed, 23 Aug 2017 14:35:55 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E2FA64A6F1; Wed, 23 Aug 2017 21:38:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E2FA64A6F1 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-43.phx2.redhat.com [10.3.116.43]) by smtp.corp.redhat.com (Postfix) with ESMTP id 732817AA46; Wed, 23 Aug 2017 21:38:27 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel References: <1503490967-5559-1-git-send-email-brijesh.singh@amd.com> <1503490967-5559-12-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: <1a6804f9-6d58-33dc-2c15-407aa7834e91@redhat.com> Date: Wed, 23 Aug 2017 23:38:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1503490967-5559-12-git-send-email-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 23 Aug 2017 21:38:29 +0000 (UTC) Subject: Re: [PATCH v3 11/23] OvmfPkg/VirtioLib: change the parameter of VirtioAppendDesc() to UINT64 X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Aug 2017 21:35:55 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/23/17 14:22, Brijesh Singh wrote: > The patch change the "BufferPhysAddr" parameter of VirtioAppendDesc() > from type UINTN to UINT64. > > UINTN is appropriate as long as we pass system memory references. After > the introduction of this feature, that's no longer the case in general. (1) s/this feature/bus master device addresses/ > Should we implement "real" IOMMU support at some point, UINTN could > break in 32-bit builds of OVMF. > > Suggested-by: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Tom Lendacky > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh > --- > OvmfPkg/Include/Library/VirtioLib.h | 35 +++++++++--------- > OvmfPkg/Library/VirtioLib/VirtioLib.c | 38 ++++++++++---------- > 2 files changed, 37 insertions(+), 36 deletions(-) > > diff --git a/OvmfPkg/Include/Library/VirtioLib.h b/OvmfPkg/Include/Library/VirtioLib.h > index c3e56ea23b89..a966311ac941 100644 > --- a/OvmfPkg/Include/Library/VirtioLib.h > +++ b/OvmfPkg/Include/Library/VirtioLib.h > @@ -151,33 +151,34 @@ VirtioPrepare ( > The caller is responsible for initializing *Indices with VirtioPrepare() > first. > > - @param[in,out] Ring The virtio ring to append the buffer to, as a > - descriptor. > + @param[in,out] Ring The virtio ring to append the buffer to, > + as a descriptor. > > - @param[in] BufferPhysAddr (Guest pseudo-physical) start address of the > - transmit / receive buffer. > + @param[in] BufferDeviceAddress (Bus master device)) start address of the (2) Unbalanced parentheses > + transmit / receive buffer. > > - @param[in] BufferSize Number of bytes to transmit or receive. > + @param[in] BufferSize Number of bytes to transmit or receive. > > - @param[in] Flags A bitmask of VRING_DESC_F_* flags. The caller > - computes this mask dependent on further buffers to > - append and transfer direction. > - VRING_DESC_F_INDIRECT is unsupported. The > - VRING_DESC.Next field is always set, but the host > - only interprets it dependent on VRING_DESC_F_NEXT. > + @param[in] Flags A bitmask of VRING_DESC_F_* flags. The > + caller computes this mask dependent on > + further buffers to append and transfer > + direction. VRING_DESC_F_INDIRECT is > + unsupported. The VRING_DESC.Next field is > + always set, but the host only interprets > + it dependent on VRING_DESC_F_NEXT. > > - @param[in,out] Indices Indices->HeadDescIdx is not accessed. > - On input, Indices->NextDescIdx identifies the next > - descriptor to carry the buffer. On output, > - Indices->NextDescIdx is incremented by one, modulo > - 2^16. > + @param[in,out] Indices Indices->HeadDescIdx is not accessed. > + On input, Indices->NextDescIdx identifies > + the next descriptor to carry the buffer. > + On output, Indices->NextDescIdx is > + incremented by one, modulo 2^16. > > **/ > VOID > EFIAPI > VirtioAppendDesc ( > IN OUT VRING *Ring, > - IN UINTN BufferPhysAddr, > + IN UINT64 BufferDeviceAddress, > IN UINT32 BufferSize, > IN UINT16 Flags, > IN OUT DESC_INDICES *Indices > diff --git a/OvmfPkg/Library/VirtioLib/VirtioLib.c b/OvmfPkg/Library/VirtioLib/VirtioLib.c > index e5366e385f5d..fcd484fffada 100644 > --- a/OvmfPkg/Library/VirtioLib/VirtioLib.c > +++ b/OvmfPkg/Library/VirtioLib/VirtioLib.c > @@ -189,7 +189,6 @@ VirtioPrepare ( > Indices->NextDescIdx = Indices->HeadDescIdx; > } > > - > /** > > Append a contiguous buffer for transmission / reception via the virtio ring. > @@ -205,33 +204,34 @@ VirtioPrepare ( > The caller is responsible for initializing *Indices with VirtioPrepare() > first. > > - @param[in,out] Ring The virtio ring to append the buffer to, as a > - descriptor. > + @param[in,out] Ring The virtio ring to append the buffer to, > + as a descriptor. > > - @param[in] BufferPhysAddr (Guest pseudo-physical) start address of the > - transmit / receive buffer. > + @param[in] BufferDeviceAddress (Bus master device)) start address of the (3) Same as (2), unbalanced parens. With these fixed up (which I plan to do before pushing the patches): Reviewed-by: Laszlo Ersek Thanks Laszlo > + transmit / receive buffer. > > - @param[in] BufferSize Number of bytes to transmit or receive. > + @param[in] BufferSize Number of bytes to transmit or receive. > > - @param[in] Flags A bitmask of VRING_DESC_F_* flags. The caller > - computes this mask dependent on further buffers to > - append and transfer direction. > - VRING_DESC_F_INDIRECT is unsupported. The > - VRING_DESC.Next field is always set, but the host > - only interprets it dependent on VRING_DESC_F_NEXT. > + @param[in] Flags A bitmask of VRING_DESC_F_* flags. The > + caller computes this mask dependent on > + further buffers to append and transfer > + direction. VRING_DESC_F_INDIRECT is > + unsupported. The VRING_DESC.Next field is > + always set, but the host only interprets > + it dependent on VRING_DESC_F_NEXT. > > - @param[in,out] Indices Indices->HeadDescIdx is not accessed. > - On input, Indices->NextDescIdx identifies the next > - descriptor to carry the buffer. On output, > - Indices->NextDescIdx is incremented by one, modulo > - 2^16. > + @param[in,out] Indices Indices->HeadDescIdx is not accessed. > + On input, Indices->NextDescIdx identifies > + the next descriptor to carry the buffer. > + On output, Indices->NextDescIdx is > + incremented by one, modulo 2^16. > > **/ > VOID > EFIAPI > VirtioAppendDesc ( > IN OUT VRING *Ring, > - IN UINTN BufferPhysAddr, > + IN UINT64 BufferDeviceAddress, > IN UINT32 BufferSize, > IN UINT16 Flags, > IN OUT DESC_INDICES *Indices > @@ -240,7 +240,7 @@ VirtioAppendDesc ( > volatile VRING_DESC *Desc; > > Desc = &Ring->Desc[Indices->NextDescIdx++ % Ring->QueueSize]; > - Desc->Addr = BufferPhysAddr; > + Desc->Addr = BufferDeviceAddress; > Desc->Len = BufferSize; > Desc->Flags = Flags; > Desc->Next = Indices->NextDescIdx % Ring->QueueSize; >