VirtioRingMap() maps the ring buffer system physical to a bus address.
When an IOMMU is used for translating the address then bus address can
start at a different offset from the system physical address.
- MMIO and legacy virtio device does not use IOMMU to translate the
addresses hence RingBaseShift will always be set to zero.
- modern virtio device use IOMMU to translate the address, in next patch
we will update the Virtio10Dxe to use RingBaseShift offset.
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/Include/Protocol/VirtioDevice.h | 5 ++++-
OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h | 3 ++-
OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h | 3 ++-
OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c | 5 ++++-
OvmfPkg/Virtio10Dxe/Virtio10.c | 5 ++++-
OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 2 +-
OvmfPkg/VirtioGpuDxe/Commands.c | 3 ++-
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +-
OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++-
OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +-
OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +-
11 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h
index 14f980d7bf0a..25fd73b847a5 100644
--- a/OvmfPkg/Include/Protocol/VirtioDevice.h
+++ b/OvmfPkg/Include/Protocol/VirtioDevice.h
@@ -156,6 +156,8 @@ EFI_STATUS
@param[in] Ring The initialized VRING object to take the
addresses from.
+ @param[in] RingBaseShift The offset for the Ring Base address.
+
@retval EFI_SUCCESS The data was written successfully.
@retval EFI_UNSUPPORTED The underlying IO device doesn't support the
provided address offset and write size.
@@ -164,7 +166,8 @@ typedef
EFI_STATUS
(EFIAPI *VIRTIO_SET_QUEUE_ADDRESS) (
IN VIRTIO_DEVICE_PROTOCOL *This,
- IN VRING *Ring
+ IN VRING *Ring,
+ IN UINT64 RingBaseShift
);
/**
diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
index b69f6d7b7a85..b5cc091fe820 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
@@ -113,7 +113,8 @@ VirtioMmioSetQueueSel (
EFI_STATUS
VirtioMmioSetQueueAddress (
IN VIRTIO_DEVICE_PROTOCOL *This,
- IN VRING *Ring
+ IN VRING *Ring,
+ IN UINT64 RingBaseShift
);
EFI_STATUS
diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h
index 41df5a98e560..1f0dc45d501e 100644
--- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h
+++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h
@@ -126,7 +126,8 @@ EFI_STATUS
EFIAPI
VirtioPciSetQueueAddress (
IN VIRTIO_DEVICE_PROTOCOL *This,
- IN VRING *Ring
+ IN VRING *Ring,
+ IN UINT64 RingBaseShift
);
EFI_STATUS
diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
index f3f69f324c6c..3d14b1035e91 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
@@ -183,11 +183,14 @@ VirtioMmioSetQueueSel (
EFI_STATUS
VirtioMmioSetQueueAddress (
IN VIRTIO_DEVICE_PROTOCOL *This,
- IN VRING *Ring
+ IN VRING *Ring,
+ IN UINT64 RingBaseShift
)
{
VIRTIO_MMIO_DEVICE *Device;
+ ASSERT (RingBaseShift == 0);
+
Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c
index 413ffa06cf35..d102e1fd8551 100644
--- a/OvmfPkg/Virtio10Dxe/Virtio10.c
+++ b/OvmfPkg/Virtio10Dxe/Virtio10.c
@@ -489,7 +489,8 @@ EFI_STATUS
EFIAPI
Virtio10SetQueueAddress (
IN VIRTIO_DEVICE_PROTOCOL *This,
- IN VRING *Ring
+ IN VRING *Ring,
+ IN UINT64 RingBaseShift
)
{
VIRTIO_1_0_DEV *Dev;
@@ -497,6 +498,8 @@ Virtio10SetQueueAddress (
UINT64 Address;
UINT16 Enable;
+ ASSERT (RingBaseShift == 0);
+
Dev = VIRTIO_1_0_FROM_VIRTIO_DEVICE (This);
Address = (UINTN)Ring->Desc;
diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
index 61b9cab4ff02..bff15fe3add1 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
@@ -745,7 +745,7 @@ VirtioBlkInit (
//
// step 4c -- Report GPFN (guest-physical frame number) of queue.
//
- Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring);
+ Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index c2e4d72feb67..df8ded99edff 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -132,7 +132,8 @@ VirtioGpuInit (
if (EFI_ERROR (Status)) {
goto Failed;
}
- Status = VgpuDev->VirtIo->SetQueueAddress (VgpuDev->VirtIo, &VgpuDev->Ring);
+ Status = VgpuDev->VirtIo->SetQueueAddress (VgpuDev->VirtIo,
+ &VgpuDev->Ring, 0);
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
index 6d9b81a9f939..0ecfe044a977 100644
--- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
+++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
@@ -96,7 +96,7 @@ VirtioNetInitRing (
//
// step 4c -- report GPFN (guest-physical frame number) of queue
//
- Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring);
+ Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, 0);
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c
index 4597095deb78..86f752e1651f 100644
--- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c
+++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c
@@ -183,11 +183,14 @@ EFI_STATUS
EFIAPI
VirtioPciSetQueueAddress (
IN VIRTIO_DEVICE_PROTOCOL *This,
- IN VRING *Ring
+ IN VRING *Ring,
+ IN UINT64 RingBaseShift
)
{
VIRTIO_PCI_DEVICE *Dev;
+ ASSERT (RingBaseShift == 0);
+
Dev = VIRTIO_PCI_DEVICE_FROM_VIRTIO_DEVICE (This);
return VirtioPciIoWrite (Dev, VIRTIO_PCI_OFFSET_QUEUE_ADDRESS, sizeof (UINT32),
diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.c b/OvmfPkg/VirtioRngDxe/VirtioRng.c
index e20602ac7225..0abca488e6cd 100644
--- a/OvmfPkg/VirtioRngDxe/VirtioRng.c
+++ b/OvmfPkg/VirtioRngDxe/VirtioRng.c
@@ -298,7 +298,7 @@ VirtioRngInit (
//
// step 4c -- Report GPFN (guest-physical frame number) of queue.
//
- Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring);
+ Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
index c2f6f412ff40..a983b3df7b9c 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
@@ -855,7 +855,7 @@ VirtioScsiInit (
//
// step 4c -- Report GPFN (guest-physical frame number) of queue.
//
- Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring);
+ Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
if (EFI_ERROR (Status)) {
goto ReleaseQueue;
}
--
2.7.4
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
(1) The "VirtioSetQueueAddress" identifier does not exist in the edk2
tree as a function or protocol member name, so I suggest at least
replacing "VirtioSetQueueAddress" with "SetQueueAddress".
On 08/14/17 13:36, Brijesh Singh wrote:
> VirtioRingMap() maps the ring buffer system physical to a bus address.
> When an IOMMU is used for translating the address then bus address can
> start at a different offset from the system physical address.
(2) I suggest the following wording in order to keep our references
within the virtio device protocol:
For the case when an IOMMU is used for translating system physical
addresses to DMA bus master addresses, the transport-independent
virtio device drivers will be required to map their VRING areas to
bus addresses with VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() calls.
> - MMIO and legacy virtio device does not use IOMMU to translate the
> addresses hence RingBaseShift will always be set to zero.
(3) s/device does not use/transports do not support/
>
> - modern virtio device use IOMMU to translate the address, in next patch
(4) s/device use/transport supports/
> we will update the Virtio10Dxe to use RingBaseShift offset.
>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> OvmfPkg/Include/Protocol/VirtioDevice.h | 5 ++++-
> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h | 3 ++-
> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h | 3 ++-
> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c | 5 ++++-
> OvmfPkg/Virtio10Dxe/Virtio10.c | 5 ++++-
> OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 2 +-
> OvmfPkg/VirtioGpuDxe/Commands.c | 3 ++-
> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +-
> OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++-
> OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +-
> OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +-
> 11 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h
> index 14f980d7bf0a..25fd73b847a5 100644
> --- a/OvmfPkg/Include/Protocol/VirtioDevice.h
> +++ b/OvmfPkg/Include/Protocol/VirtioDevice.h
> @@ -156,6 +156,8 @@ EFI_STATUS
> @param[in] Ring The initialized VRING object to take the
> addresses from.
>
> + @param[in] RingBaseShift The offset for the Ring Base address.
> +
OK, so this comment block is critical.
(5) Please update the documentation of the "Ring" parameter like this:
@param[in] Ring The initialized VRING object to take the
addresses from. The caller is responsible for
ensuring that on input, all Ring->NumPages pages,
starting at Ring->Base, have been successfully
mapped with a single call to
This->MapSharedBuffer().
(This documentation basically provides the specification for our
VirtioRingMap() function.)
(6) Then we can simply define RingBaseShift like this:
@param[in] RingBaseShift Adding this value using UINT64 arithmetic to the
addresses found in Ring translates them from
system memory to bus addresses. The caller shall
calculate RingBaseShift as
(DeviceAddress - (UINT64)(UINTN)HostAddress),
where DeviceAddress and HostAddress (i.e.,
Ring->Base) were output and input parameters of
This->MapSharedBuffer(), respectively.
> @retval EFI_SUCCESS The data was written successfully.
> @retval EFI_UNSUPPORTED The underlying IO device doesn't support the
> provided address offset and write size.
> @@ -164,7 +166,8 @@ typedef
> EFI_STATUS
> (EFIAPI *VIRTIO_SET_QUEUE_ADDRESS) (
> IN VIRTIO_DEVICE_PROTOCOL *This,
> - IN VRING *Ring
> + IN VRING *Ring,
> + IN UINT64 RingBaseShift
> );
>
> /**
> diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> index b69f6d7b7a85..b5cc091fe820 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> @@ -113,7 +113,8 @@ VirtioMmioSetQueueSel (
> EFI_STATUS
> VirtioMmioSetQueueAddress (
> IN VIRTIO_DEVICE_PROTOCOL *This,
> - IN VRING *Ring
> + IN VRING *Ring,
> + IN UINT64 RingBaseShift
> );
>
> EFI_STATUS
> diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h
> index 41df5a98e560..1f0dc45d501e 100644
> --- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h
> +++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h
> @@ -126,7 +126,8 @@ EFI_STATUS
> EFIAPI
> VirtioPciSetQueueAddress (
> IN VIRTIO_DEVICE_PROTOCOL *This,
> - IN VRING *Ring
> + IN VRING *Ring,
> + IN UINT64 RingBaseShift
> );
>
> EFI_STATUS
> diff --git a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> index f3f69f324c6c..3d14b1035e91 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> @@ -183,11 +183,14 @@ VirtioMmioSetQueueSel (
> EFI_STATUS
> VirtioMmioSetQueueAddress (
> IN VIRTIO_DEVICE_PROTOCOL *This,
> - IN VRING *Ring
> + IN VRING *Ring,
> + IN UINT64 RingBaseShift
> )
> {
> VIRTIO_MMIO_DEVICE *Device;
>
> + ASSERT (RingBaseShift == 0);
> +
> Device = VIRTIO_MMIO_DEVICE_FROM_VIRTIO_DEVICE (This);
>
> VIRTIO_CFG_WRITE (Device, VIRTIO_MMIO_OFFSET_QUEUE_PFN,
> diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c
> index 413ffa06cf35..d102e1fd8551 100644
> --- a/OvmfPkg/Virtio10Dxe/Virtio10.c
> +++ b/OvmfPkg/Virtio10Dxe/Virtio10.c
> @@ -489,7 +489,8 @@ EFI_STATUS
> EFIAPI
> Virtio10SetQueueAddress (
> IN VIRTIO_DEVICE_PROTOCOL *This,
> - IN VRING *Ring
> + IN VRING *Ring,
> + IN UINT64 RingBaseShift
> )
> {
> VIRTIO_1_0_DEV *Dev;
> @@ -497,6 +498,8 @@ Virtio10SetQueueAddress (
> UINT64 Address;
> UINT16 Enable;
>
> + ASSERT (RingBaseShift == 0);
> +
> Dev = VIRTIO_1_0_FROM_VIRTIO_DEVICE (This);
>
> Address = (UINTN)Ring->Desc;
> diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> index 61b9cab4ff02..bff15fe3add1 100644
> --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
> @@ -745,7 +745,7 @@ VirtioBlkInit (
> //
> // step 4c -- Report GPFN (guest-physical frame number) of queue.
> //
> - Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring);
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
> if (EFI_ERROR (Status)) {
> goto ReleaseQueue;
> }
> diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
> index c2e4d72feb67..df8ded99edff 100644
> --- a/OvmfPkg/VirtioGpuDxe/Commands.c
> +++ b/OvmfPkg/VirtioGpuDxe/Commands.c
> @@ -132,7 +132,8 @@ VirtioGpuInit (
> if (EFI_ERROR (Status)) {
> goto Failed;
> }
> - Status = VgpuDev->VirtIo->SetQueueAddress (VgpuDev->VirtIo, &VgpuDev->Ring);
> + Status = VgpuDev->VirtIo->SetQueueAddress (VgpuDev->VirtIo,
> + &VgpuDev->Ring, 0);
(7) Please break all arguments to separate lines (and the closing paren
to another separate line).
The rest looks fine, thanks!
Laszlo
> if (EFI_ERROR (Status)) {
> goto ReleaseQueue;
> }
> diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> index 6d9b81a9f939..0ecfe044a977 100644
> --- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> +++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c
> @@ -96,7 +96,7 @@ VirtioNetInitRing (
> //
> // step 4c -- report GPFN (guest-physical frame number) of queue
> //
> - Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring);
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, Ring, 0);
> if (EFI_ERROR (Status)) {
> goto ReleaseQueue;
> }
> diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c
> index 4597095deb78..86f752e1651f 100644
> --- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c
> +++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c
> @@ -183,11 +183,14 @@ EFI_STATUS
> EFIAPI
> VirtioPciSetQueueAddress (
> IN VIRTIO_DEVICE_PROTOCOL *This,
> - IN VRING *Ring
> + IN VRING *Ring,
> + IN UINT64 RingBaseShift
> )
> {
> VIRTIO_PCI_DEVICE *Dev;
>
> + ASSERT (RingBaseShift == 0);
> +
> Dev = VIRTIO_PCI_DEVICE_FROM_VIRTIO_DEVICE (This);
>
> return VirtioPciIoWrite (Dev, VIRTIO_PCI_OFFSET_QUEUE_ADDRESS, sizeof (UINT32),
> diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.c b/OvmfPkg/VirtioRngDxe/VirtioRng.c
> index e20602ac7225..0abca488e6cd 100644
> --- a/OvmfPkg/VirtioRngDxe/VirtioRng.c
> +++ b/OvmfPkg/VirtioRngDxe/VirtioRng.c
> @@ -298,7 +298,7 @@ VirtioRngInit (
> //
> // step 4c -- Report GPFN (guest-physical frame number) of queue.
> //
> - Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring);
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
> if (EFI_ERROR (Status)) {
> goto ReleaseQueue;
> }
> diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> index c2f6f412ff40..a983b3df7b9c 100644
> --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> @@ -855,7 +855,7 @@ VirtioScsiInit (
> //
> // step 4c -- Report GPFN (guest-physical frame number) of queue.
> //
> - Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring);
> + Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0);
> if (EFI_ERROR (Status)) {
> goto ReleaseQueue;
> }
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 08/16/17 23:43, Laszlo Ersek wrote: > (1) The "VirtioSetQueueAddress" identifier does not exist in the edk2 > tree as a function or protocol member name, so I suggest at least > replacing "VirtioSetQueueAddress" with "SetQueueAddress". > > On 08/14/17 13:36, Brijesh Singh wrote: >> VirtioRingMap() maps the ring buffer system physical to a bus address. >> When an IOMMU is used for translating the address then bus address can >> start at a different offset from the system physical address. > > (2) I suggest the following wording in order to keep our references > within the virtio device protocol: > > For the case when an IOMMU is used for translating system physical > addresses to DMA bus master addresses, the transport-independent > virtio device drivers will be required to map their VRING areas to > bus addresses with VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() calls. > >> - MMIO and legacy virtio device does not use IOMMU to translate the >> addresses hence RingBaseShift will always be set to zero. > > (3) s/device does not use/transports do not support/ > >> >> - modern virtio device use IOMMU to translate the address, in next patch > > (4) s/device use/transport supports/ > >> we will update the Virtio10Dxe to use RingBaseShift offset. >> >> Suggested-by: Laszlo Ersek <lersek@redhat.com> >> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> OvmfPkg/Include/Protocol/VirtioDevice.h | 5 ++++- >> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h | 3 ++- >> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h | 3 ++- >> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c | 5 ++++- >> OvmfPkg/Virtio10Dxe/Virtio10.c | 5 ++++- >> OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 2 +- >> OvmfPkg/VirtioGpuDxe/Commands.c | 3 ++- >> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +- >> OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++- >> OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +- >> OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +- >> 11 files changed, 26 insertions(+), 11 deletions(-) >> >> diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h >> index 14f980d7bf0a..25fd73b847a5 100644 >> --- a/OvmfPkg/Include/Protocol/VirtioDevice.h >> +++ b/OvmfPkg/Include/Protocol/VirtioDevice.h >> @@ -156,6 +156,8 @@ EFI_STATUS >> @param[in] Ring The initialized VRING object to take the >> addresses from. >> >> + @param[in] RingBaseShift The offset for the Ring Base address. >> + > > OK, so this comment block is critical. > > (5) Please update the documentation of the "Ring" parameter like this: > > @param[in] Ring The initialized VRING object to take the > addresses from. The caller is responsible for > ensuring that on input, all Ring->NumPages pages, > starting at Ring->Base, have been successfully > mapped with a single call to > This->MapSharedBuffer(). (5a) oops, small (but important) omission: please append "for CommonBuffer bus master operation". Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.