Re: [PATCH v1 00/14] OvmfPkg/Virtio: Add APIs to map system physical to device address

[Laszlo Ersek <lersek@redhat.com>]

On 08/10/17 00:44, Brijesh Singh wrote:
> 
> 
> On 08/09/2017 05:38 PM, Laszlo Ersek wrote:
>> On 08/07/17 13:58, Brijesh Singh wrote:
>>> Currently, virtio drivers provides the system physical address to the
>>> device.
>>> However, some systems may feature an IOMMU that requires the drivers
>>> to pass
>>> the device addresses to the device - which are then translated by the
>>> IOMMU
>>> into physical addresses in memory. The patch series introduces new
>>> member
>>> functions in VIRTIO_DEVICE_PROTOCOL which can be used for mapping a
>>> system
>>> physical address to device address.
>>>
>>> The approach that this patch series takes is to maps the system physical
>>> address to device address for buffers (including rings, device specifc
>>> request and response  pointed by vring descriptor, and any further
>>> memory
>>> reference by those request and response).
>>>
>>> Patch 1 - 3:
>>>   Defines and implements new member functions to map a system
>>> physical address
>>>   to device address. The patch implements Laszlo's suggestion [1].
>>>
>>> Patch 4 - 7:
>>>   Allocates the vring buffer using newly added member functions and
>>> provides
>>>   some helper functions.
>>>
>>> Patch 8:
>>>   Update the virtio-blk driver to use newly added member functions to
>>> map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>      -drive file=${IMAGE},if=none,id=disk0 \
>>>      -device virtio-blk-pci,drive=disk0
>>>
>>>   # $QEMU \
>>>      -drive file=${IMAGE},if=none,id=disk0 \
>>>      -device virtio-blk-pci,drive=disk0,disable-legacy=on
>>>
>>> Patch 9:
>>>   Update the virtio-scsi driver to use newly added member functions
>>> to map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>      -drive file=${IMAGE},if=none,id=disk0 \
>>>      -device scsi-hd,drive=disk0
>>>      -device virtio-scsi-pci,id=scsi \
>>>
>>>   # $QEMU \
>>>      -drive file=${IMAGE},if=none,id=disk0 \
>>>      -device scsi-hd,drive=disk0 \
>>>      -device virtio-scsi-pci,id=scsi,disable-legacy=on \
>>>
>>> Patch 10 - 13:
>>>   Update the virtio-net driver to use newly added member functions to
>>> map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>      -netdev type=tap,id=net0 \
>>>      -device virtio-net-pci,netdev=net0,romfile=
>>>
>>>   # $QEMU \
>>>      -netdev type=tap,id=net0 \
>>>      -device virtio-net-pci,netdev=net0,disable-legacy=on,romfile=
>>>
>>> Patch 14:
>>>   Update the virtio-rng driver to use newly added member functions to
>>> map the
>>>   addresses.
>>>   Verified using the following qemu cli
>>>
>>>   # $QEMU \
>>>      -device virtio-rng-pci
>>>
>>>   # $QEMU \
>>>      -device virtio-rng-pci,disable-legacy=on
>>>
>>>   And succesfully ran RngTest.efi from SecurityPkg/Application
>>>
>>> Repo: https://github.com/codomania/edk2
>>> Branch: virtio-support
>>>
>>> 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>
>>>
>>> Brijesh Singh (14):
>>>    OvmfPkg/Virtio: Introduce new member functions in
>>>      VIRTIO_DEVICE_PROTOCOL
>>>    OvmfPkg/Virtio10Dxe: Implement new member functions
>>>    OvmfPkg/VirtioPciDeviceDxe: Implement new member functions
>>>    OvmfPkg/VirtioLib: Add SharedBuffer helper functions
>>>    OvmfPkg/VirtioLib: Pass VirtIo instance in VringInit/Uinit()
>>>    OvmfPkg/VirtioLib: Add functions to map/unmap VRING
>>>    OvmfPkg/VirtioLib: Use AllocateShared() to allocate vring buffer
>>>    OvmfPkg/VirtioBlkDxe: Use DeviceAddresses in vring descriptors
>>>    OvmfPkg/VirtioScsiDxe: Use DeviceAddresses in vring descriptors
>>>    OvmfPkg/VirtioNetDxe: Allocate Tx and Rx ring using
>>>      AllocateSharedPage()
>>>    OvmfPkg/VirtioNetDxe: Allocate RxBuf using AllocateSharedPages()
>>>    OvmfPkg/VirtioNetDxe: Dynamically allocate transmit header
>>>    OvmfPkg/VirtioNetDxe: Use DeviceAddress in transmit vring descriptors
>>>    OvmfPkg/VirtioRngDxe: Use DeviceAddresses in vring descriptors
>>>
>>>   OvmfPkg/Include/Library/VirtioLib.h             | 198 ++++++++++++-
>>>   OvmfPkg/Include/Protocol/VirtioDevice.h         | 121 ++++++++
>>>   OvmfPkg/VirtioBlkDxe/VirtioBlk.h                |   1 +
>>>   OvmfPkg/VirtioNetDxe/VirtioNet.h                |  25 +-
>>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h    |  34 +++
>>>   OvmfPkg/VirtioRngDxe/VirtioRng.h                |   1 +
>>>   OvmfPkg/VirtioScsiDxe/VirtioScsi.h              |   1 +
>>>   OvmfPkg/Library/VirtioLib/VirtioLib.c           | 296
>>> +++++++++++++++++++-
>>>   OvmfPkg/Virtio10Dxe/Virtio10.c                  | 114 +++++++-
>>>   OvmfPkg/VirtioBlkDxe/VirtioBlk.c                | 104 ++++++-
>>>   OvmfPkg/VirtioGpuDxe/Commands.c                 |   7 +-
>>>   OvmfPkg/VirtioNetDxe/Events.c                   |  24 ++
>>>   OvmfPkg/VirtioNetDxe/SnpGetStatus.c             |  19 +-
>>>   OvmfPkg/VirtioNetDxe/SnpInitialize.c            | 102 +++++--
>>>   OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c         | 119 +++++++-
>>>   OvmfPkg/VirtioNetDxe/SnpShutdown.c              |  16 +-
>>>   OvmfPkg/VirtioNetDxe/SnpTransmit.c              |  22 +-
>>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c    |   7 +-
>>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c |  66 +++++
>>>   OvmfPkg/VirtioRngDxe/VirtioRng.c                |  54 +++-
>>>   OvmfPkg/VirtioScsiDxe/VirtioScsi.c              | 121 +++++++-
>>>   21 files changed, 1378 insertions(+), 74 deletions(-)
>>>
>>
>> The conversion for VirtioGpuDxe seems to be missing; is it on your todo
>> list? (Not that it's urgent at all; in fact I'll suggest delaying the
>> more complex drivers in a minute.)
>>
> 
> Actually I was thinking to delay it for now,

OK, that makes sense even from the complexity standpoint.

> once we get a basic support
> enabled then we can revisit the VirtioGpuDxe, I still need to figure out
> qemu cli and Uefi app to test it.

You can read about my original test cases and test instructions in the
following message:

[2] http://mid.mail-archive.com/20160819124932.29711-1-lersek@redhat.com

(

Some background:

The virtio-vga device combines the Virtio GPU and the traditional VGA
framebuffer that is also seen on QXL and stdvga ("Bochs"). If you use
"-device virtio-vga", then QemuVideoDxe will bind the device (this is
intended). So, the right option for specifically using the virtio GPU is
"-device virtio-gpu-pci".

You can read more about this distinction in the following libvirt commit:

[3] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=706b5b627719

)

Regarding a UEFI app for testing, the UEFI shell and grub suffice:
- do you see the TianoCore boot logo?
- does the UEFI shell work?
- does the shell scroll fine when you press Enter at the bottom of the
  screen?
- does the MODE command work in the shell?
- typing on the serial port, can you disconnect / reconnect the display?
- does grub work?

See again my message [2].

> I will add in TODO list so that.

Sounds good. Once we lay down the foundation (and I can find some time),
I might be able to take on the conversion of VirtioGpuDxe. (If you're OK
with that.)

Thanks!
Laszlo