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 <[email protected]> > Cc: Ard Biesheuvel <[email protected]> > Cc: Jordan Justen <[email protected]> > Cc: Tom Lendacky <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh <[email protected]> > --- > 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 <[email protected]> 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; > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
