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.
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 transport do not support IOMMU to translate the
addresses hence RingBaseShift will always be set to zero.
- modern virtio transport supports 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 | 19 +++++++++++++++++--
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 | 6 +++++-
OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +-
OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++-
OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +-
OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +-
11 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h
index 9a01932958a2..2e3a6d6edf04 100644
--- a/OvmfPkg/Include/Protocol/VirtioDevice.h
+++ b/OvmfPkg/Include/Protocol/VirtioDevice.h
@@ -156,7 +156,21 @@ EFI_STATUS
@param[in] This This instance of VIRTIO_DEVICE_PROTOCOL
@param[in] Ring The initialized VRING object to take the
- addresses from.
+ 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() for CommonBuffer bus
+ master operation..
+
+ @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
@@ -166,7 +180,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 e5881d537f09..e6279159f8ba 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
@@ -115,7 +115,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 644ec65e1788..67458e56231b 100644
--- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
+++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
@@ -181,11 +181,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 89ccac8c1c04..ef9a00710668 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..5cb003161207 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -132,7 +132,11 @@ 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 bd912cca9b29..b52060d13d97 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
On 08/23/17 14:22, Brijesh Singh wrote:
> 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.
>
> 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.
(1) The paragraph that you now have as first paragraph above was my
suggestion, so thank you for picking it up. However, the second
paragraph should have been deleted; I suggested the now-first paragraph
as a replacement for the now-second one.
I wrote, "to keep our references within the virtio device protocol".
VirtioRingMap() is a VirtioLib function, which is a utility layer on top
of the virtio device protocol. So, as I said, VirtioLib patches may
refer to both VirtioLib and the protocol, but protocol patches should
preferably only refer to the protocol, and not VirtioLib.
VirtioLib --+
| ^ |
| | |
| +-------+
|
v
VirtioDeviceProtocol --+
^ |
| |
+--------+
This is also consistent with the reordering of the patches that I asked
for (and that you implemented well in v3, thank you for it).
So, apologies if I wasn't clear enough of this -- it's not a big deal at
all, I can remove the second paragraph when I push this.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
>
> - MMIO and legacy virtio transport do not support IOMMU to translate the
> addresses hence RingBaseShift will always be set to zero.
>
> - modern virtio transport supports 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 | 19 +++++++++++++++++--
> 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 | 6 +++++-
> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +-
> OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++-
> OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +-
> OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +-
> 11 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h
> index 9a01932958a2..2e3a6d6edf04 100644
> --- a/OvmfPkg/Include/Protocol/VirtioDevice.h
> +++ b/OvmfPkg/Include/Protocol/VirtioDevice.h
> @@ -156,7 +156,21 @@ EFI_STATUS
> @param[in] This This instance of VIRTIO_DEVICE_PROTOCOL
>
> @param[in] Ring The initialized VRING object to take the
> - addresses from.
> + 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() for CommonBuffer bus
> + master operation..
> +
> + @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
> @@ -166,7 +180,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 e5881d537f09..e6279159f8ba 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDevice.h
> @@ -115,7 +115,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 644ec65e1788..67458e56231b 100644
> --- a/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> +++ b/OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceFunctions.c
> @@ -181,11 +181,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 89ccac8c1c04..ef9a00710668 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..5cb003161207 100644
> --- a/OvmfPkg/VirtioGpuDxe/Commands.c
> +++ b/OvmfPkg/VirtioGpuDxe/Commands.c
> @@ -132,7 +132,11 @@ 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 bd912cca9b29..b52060d13d97 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/23/17 22:41, Laszlo Ersek wrote: > On 08/23/17 14:22, Brijesh Singh wrote: >> 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. >> >> 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. > > (1) The paragraph that you now have as first paragraph above was my > suggestion, so thank you for picking it up. However, the second > paragraph should have been deleted; I suggested the now-first paragraph > as a replacement for the now-second one. > > I wrote, "to keep our references within the virtio device protocol". > VirtioRingMap() is a VirtioLib function, which is a utility layer on top > of the virtio device protocol. So, as I said, VirtioLib patches may > refer to both VirtioLib and the protocol, but protocol patches should > preferably only refer to the protocol, and not VirtioLib. > > VirtioLib --+ > | ^ | > | | | > | +-------+ > | > v > VirtioDeviceProtocol --+ > ^ | > | | > +--------+ > > This is also consistent with the reordering of the patches that I asked > for (and that you implemented well in v3, thank you for it). > > So, apologies if I wasn't clear enough of this -- it's not a big deal at > all, I can remove the second paragraph when I push this. > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Thanks! > Laszlo > >> >> - MMIO and legacy virtio transport do not support IOMMU to translate the >> addresses hence RingBaseShift will always be set to zero. >> >> - modern virtio transport supports 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 | 19 +++++++++++++++++-- >> 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 | 6 +++++- >> OvmfPkg/VirtioNetDxe/SnpInitialize.c | 2 +- >> OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 5 ++++- >> OvmfPkg/VirtioRngDxe/VirtioRng.c | 2 +- >> OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +- >> 11 files changed, 42 insertions(+), 12 deletions(-) >> >> diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h >> index 9a01932958a2..2e3a6d6edf04 100644 >> --- a/OvmfPkg/Include/Protocol/VirtioDevice.h >> +++ b/OvmfPkg/Include/Protocol/VirtioDevice.h >> @@ -156,7 +156,21 @@ EFI_STATUS >> @param[in] This This instance of VIRTIO_DEVICE_PROTOCOL >> >> @param[in] Ring The initialized VRING object to take the >> - addresses from. >> + 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() for CommonBuffer bus >> + master operation.. (2) I'll also remove one of the periods. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 08/23/2017 03:43 PM, Laszlo Ersek wrote: > On 08/23/17 22:41, Laszlo Ersek wrote: >> On 08/23/17 14:22, Brijesh Singh wrote: >>> 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. >>> >>> 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. >> >> (1) The paragraph that you now have as first paragraph above was my >> suggestion, so thank you for picking it up. However, the second >> paragraph should have been deleted; I suggested the now-first paragraph >> as a replacement for the now-second one. >> >> I wrote, "to keep our references within the virtio device protocol". >> VirtioRingMap() is a VirtioLib function, which is a utility layer on top >> of the virtio device protocol. So, as I said, VirtioLib patches may >> refer to both VirtioLib and the protocol, but protocol patches should >> preferably only refer to the protocol, and not VirtioLib. >> >> VirtioLib --+ >> | ^ | >> | | | >> | +-------+ >> | >> v >> VirtioDeviceProtocol --+ >> ^ | >> | | >> +--------+ >> >> This is also consistent with the reordering of the patches that I asked >> for (and that you implemented well in v3, thank you for it). >> >> So, apologies if I wasn't clear enough of this -- it's not a big deal at >> all, I can remove the second paragraph when I push this. >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> >> Thanks Laszlo, Sorry I did not realize that second paragraph should have been deleted. Thanks for fixing. -Brijesh _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.