[edk2] [PATCH v3 0/4] OvmfPkg/VirtioScsiDxe: map host address to device address

Brijesh Singh posted 4 patches 6 years, 6 months ago
Failed in applying to current master (apply log)
OvmfPkg/VirtioScsiDxe/VirtioScsi.h |   1 +
OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 306 +++++++++++++++++---
2 files changed, 273 insertions(+), 34 deletions(-)
[edk2] [PATCH v3 0/4] OvmfPkg/VirtioScsiDxe: map host address to device address
Posted by Brijesh Singh 6 years, 6 months ago
The patch updates VirtioScsiDxe to use IOMMU-like member functions to map
the system physical address to device address for buffers (including vring,
device specific request and response pointed by vring descriptor, and any
furter memory reference by those request and response).

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>

Repo: https://github.com/codomania/edk2
Branch: virtio-scsi-3

Changes since v2:
 * changes to address v2 feedback

Brijesh Singh (4):
  OvmfPkg/VirtioScsiDxe: map VRING using VirtioRingMap()
  OvmfPkg/VirtioScsiDxe: add helper to create a fake host adapter error
  OvmfPkg/VirtioScsiDxe: map virtio-scsi request and response buffers
  OvmfPkg/VirtioScsiDxe: negotiate VIRTIO_F_IOMMU_PLATFORM

 OvmfPkg/VirtioScsiDxe/VirtioScsi.h |   1 +
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 306 +++++++++++++++++---
 2 files changed, 273 insertions(+), 34 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v3 0/4] OvmfPkg/VirtioScsiDxe: map host address to device address
Posted by Laszlo Ersek 6 years, 6 months ago
On 08/31/17 17:01, Brijesh Singh wrote:
> The patch updates VirtioScsiDxe to use IOMMU-like member functions to map
> the system physical address to device address for buffers (including vring,
> device specific request and response pointed by vring descriptor, and any
> furter memory reference by those request and response).
>
> 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>
>
> Repo: https://github.com/codomania/edk2
> Branch: virtio-scsi-3
>
> Changes since v2:
>  * changes to address v2 feedback
>
> Brijesh Singh (4):
>   OvmfPkg/VirtioScsiDxe: map VRING using VirtioRingMap()
>   OvmfPkg/VirtioScsiDxe: add helper to create a fake host adapter error
>   OvmfPkg/VirtioScsiDxe: map virtio-scsi request and response buffers
>   OvmfPkg/VirtioScsiDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
>
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.h |   1 +
>  OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 306 +++++++++++++++++---
>  2 files changed, 273 insertions(+), 34 deletions(-)
>

Okay, so these are the updates (cumulatively) that I implemented here:

> diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> index 313b4f66c7f8..7b8c3d22c8de 100644
> --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
> @@ -387,19 +387,23 @@ ParseResponse (
>    return EFI_DEVICE_ERROR;
>  }
>
> +
>  /**
> - * The function can be used to create a fake host adapter error.
> - *
> - * When VirtioScsiPassThru is failed due to some reasons then this function
> - * can be called to contstruct a host adapter error.
> - *
> - * @param[out] Packet         The Extended SCSI Pass Thru Protocol packet that
> - *                            the host adapter error shall be placed in.
> - *
> - * @retval EFI_DEVICE_ERROR  The function returns this status code
> - *                           unconditionally, to be propagated by
> - *                           VirtioScsiPassThru().
> - */
> +
> +  The function can be used to create a fake host adapter error.
> +
> +  When VirtioScsiPassThru() is failed due to some reasons then this function
> +  can be called to construct a host adapter error.
> +
> +  @param[out] Packet  The Extended SCSI Pass Thru Protocol packet that the host
> +                      adapter error shall be placed in.
> +
> +
> +  @retval EFI_DEVICE_ERROR  The function returns this status code
> +                            unconditionally, to be propagated by
> +                            VirtioScsiPassThru().
> +
> +**/
>  STATIC
>  EFI_STATUS
>  ReportHostAdapterError (
> @@ -490,14 +494,15 @@ VirtioScsiPassThru (
>      //  * we perform the request fine
>      //  * but we fail to unmap the "InDataMapping"
>      //
> -    //  In that case simply returing the EFI_DEVICE_ERROR is not sufficient.
> -    //  In addition to the error code we also need to update Packet fields
> -    //  accordingly so that we report the full loss of the incoming transfer.
> -    //  We allocate a temporary buffer and map it with BusMasterCommonBuffer.
> -    //  If the Virtio request is successful then we copy the data from
> -    //  temporary buffer into Packet->InDataBuffer.
> +    // In that case simply returing the EFI_DEVICE_ERROR is not sufficient. In
> +    // addition to the error code we also need to update Packet fields
> +    // accordingly so that we report the full loss of the incoming transfer.
>      //
> -    InDataNumPages = (UINTN)EFI_SIZE_TO_PAGES (Packet->InTransferLength);
> +    // We allocate a temporary buffer and map it with BusMasterCommonBuffer. If
> +    // the Virtio request is successful then we copy the data from temporary
> +    // buffer into Packet->InDataBuffer.
> +    //
> +    InDataNumPages = EFI_SIZE_TO_PAGES ((UINTN)Packet->InTransferLength);
>      Status = Dev->VirtIo->AllocateSharedPages (
>                              Dev->VirtIo,
>                              InDataNumPages,

To patch #2, I added the following editing remark:

[lersek@redhat.com: fix style & typo in ReportHostAdapterError() comment]

To patch #3, I added the following editing remarks:

[lersek@redhat.com: restore lost sentence/paragraph in commit message]
[lersek@redhat.com: reindent/reflow "InDataBuffer" comment block]
[lersek@redhat.com: cast arg, not result, of EFI_SIZE_TO_PAGES() to UINTN]

With the above, the series is now fully Reviewed-by me.

I tested this series as follows:

                    virtio-mmio  legacy PCI        modern PCI
                    -----------  ----------------- --------------------------
  test scenario     AARCH64      IA32     X64      IA32     X64/SEV- X64/SEV+
  ----------------  -----------  -------- -------- -------- -------- --------
  R/W text editor   PASS         PASS     PASS     PASS     PASS     PASS

  shell RECONNECT   PASS         PASS     PASS     PASS     PASS     PASS

  OS boot from it   PASS         PASS     PASS     PASS     PASS     PASS

Therefore, for the series:

Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

Pushed as commit range fefeb416e63b..17cbf7359f04.

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel