Hi Laszlo,

Thanks for all the information. Sorry, I missed your reply so i asked Ard a question which you already answered. I have a pretty clear picture regarding PCI/non-discoverable devices from yours and Ard's replies. Thanks a lot.


For the initial MMIO Virtio support implementation I have generally followed the steps you described below. The VirtioMmioDeviceLib and device path protocol are initialized as part of the platform code.

The idea behind the patches were to abstract all these steps from a platform code to EDK2. These steps are common for Virtio MMIO devices so each platform that needs it may not need to implement it. I did not know all details regarding PCI and non-discoverable devices so this approach was looking like a good fit, as it allowed EDK2 code to handle almost all parts of the process. I see all the issues now.

Thanks,
Daniil


On 03/07/2018 04:56 AM, Laszlo Ersek wrote:
Hi Daniil,

On 03/07/18 02:36, Daniil Egranov wrote:
This is an attempt to add MMIO Virtio devices into the
non-discoverable device registration procedure and allow
Virtio PCI drivers to recognize and program such devices
correctly.
The main issue is that the set of MMIO registers is different
from PCI, plus the width of similar registers are not
always the same. The code implements the translation of
the PCI IO registers to MMIO registers.
Another difference between PCI and MMIO Virtio devices found
during the testing is that MMIO devices may require more
registers to be programmed compared to PCI. The VirtioPciDeviceDxe
was patched to detect non-discoverable MMIO devices and allow
calling a PCI MemIo protocol function.

This set of patches was tested with MMIO Virtio Block and
Virtio Net devices.

I'm commenting on this series because of patch 4/4, which is for OvmfPkg.

OvmfPkg supports the following virtio transports:

- virtio-pci, spec version 0.9.5 ("legacy"):

   OvmfPkg/VirtioPciDeviceDxe consumes EFI_PCI_IO_PROTOCOL,
   and produces VIRTIO_DEVICE_PROTOCOL;

- virtio-pci, spec version 1.0 ("modern"):

   OvmfPkg/Virtio10Dxe consumes EFI_PCI_IO_PROTOCOL and produces
   VIRTIO_DEVICE_PROTOCOL;

- virtio-mmio, spec version 0.9.5 ("legacy"):

   OvmfPkg/Library/VirtioMmioDeviceLib takes (a) an EFI_HANDLE that
   carries a vendor-specific device path protocol instance, (b) the base
   address of the virtio-mmio transport (register block), and produces
   VIRTIO_DEVICE_PROTOCOL.

The individual virtio device drivers under OvmfPkg -- such as
VirtioBlkDxe, VirtioGpuDxe, VirtioNetDxe, VirtioRngDxe and VirtioScsiDxe
-- consume VIRTIO_DEVICE_PROTOCOL, and produce the corresponding (device
type specific) UEFI protocols on top. They perform a *union* of the
steps that are required for generic device configuration over either
virtio transport (going through VIRTIO_DEVICE_PROTOCOL), and the
transport drivers take care of mapping the actions to the actual
transports. In some cases, this means simply ignoring an action (because
it has no mapping defined for the given transport type).

Now, this patch set:

(1) extends NonDiscoverableDeviceRegistrationLib, so that a platform
DXE_DRIVER can register a new kind of non-discoverable device,

(2) extends the NonDiscoverablePciDeviceDxe PCI emulation driver in
MdeModulePkg to recognize the new kind of device.

However: the PciIo protocol instances that are consequently produced
represent virtio devices that do *not* conform to the PCI binding of
either virtio specification (0.9.5 or 1.0). Namely,

- The QueueAlign register (at offset 0x3C in the MMIO register block)
   and the GuestPageSize register (at offset 0x28) are only defined for
   the virtio-mmio binding, spec version 0.9.5 ("legacy").

- The QueueNum register (at offset 0x38) is:

   - defined in the virtio-mmio binding, spec version 0.9.5 ("legacy"),

   - defined in the virtio-mmio binding, spec version 1.0 ("modern"),

   - "sort of defined" in the virtio-pci binding, spec version 1.0
     only ("modern" only). (By "sort of defined", I mean the fact that
     the "queue_size" register of the PCI binding is read-write in spec
     version 1.0.)

Therefore adding any actual handling for these registers to
VirtioPciDeviceDxe, which implements the 0.9.5 PCI binding, is wrong.

If your platform provides a virtio device over an MMIO transport, then
the right way to expose the device to the firmware is *not* to

(a) simulate a PciIo interface that does not conform to the PCI binding
     of the virtio-0.9.5 spec,

(b) then patch that shortcoming up in VirtioPciDeviceDxe, which already
     conforms to the PCI binding of the virtio-0.9.5 spec.

Instead, the right way is to use "OvmfPkg/Library/VirtioMmioDeviceLib"
in a platform driver, for producing VIRTIO_DEVICE_PROTOCOL instances on
top of the virtio-mmio transports (MMIO register blocks) directly.

You must already have a platform DXE_DRIVER that produces the
non-discoverable device protocol instances described in (1). I suggest
that you modify that driver to use VirtioMmioDeviceLib instead:
enumerate the same set of virtio-mmio transports that you must already
be doing, and call VirtioMmioInstallDevice() on each. (You can see an
example in "ArmVirtPkg/VirtioFdtDxe".)

This will give you VIRTIO_DEVICE_PROTOCOL instances right off the bat
that can be driven by VirtioBlkDxe, VirtioGpuDxe, etc.

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

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

Reply via email to