On Tue, 9 Feb 2021 at 14:41, Laszlo Ersek <ler...@redhat.com> wrote:
>
> On 02/09/21 03:54, Ying Fang wrote:
>
> > I now realize that we emulate the virtio-blk-device over mmio, and we
> > only emulate virtio-1.0 spec.
> > As mentioned in (1c) , EDK2 only supports virtio-0.95 spec for now, so
> > this maybe a big problem.
> > Since it may not handshake ok if we only emulate virtio-1.0.
>
> Yes.
>
> First, the MMIO transport (as I remember from checking it last time,
> which was quite some time ago) is very different between 0.9.5 and 1.0.
>
> Second, device initialization steps differ:
> - between 0.9.5 MMIO and 0.9.5 PCI,
> - between 0.9.5 and 1.0, regardless of transport.
>
> This means that the device driver code has *some* specifics (=
> abstraction leaks) that relate to the underlying transport (MMIO vs.
> PCI, and 0.9.5 vs. 1.0). See:
>
> OvmfPkg/VirtioBlkDxe/VirtioBlk.c
>
> 752 //
> 753 // Set Page Size - MMIO VirtIo Specific
> 754 //
> 755 Status = Dev->VirtIo->SetPageSize (Dev->VirtIo, EFI_PAGE_SIZE);
>
> 822 //
> 823 // In virtio-1.0, feature negotiation is expected to complete
> before queue
> 824 // discovery, and the device can also reject the selected set of
> features.
> 825 //
> 826 if (Dev->VirtIo->Revision >= VIRTIO_SPEC_REVISION (1, 0, 0)) {
> 827 Status = Virtio10WriteFeatures (Dev->VirtIo, Features,
> &NextDevStat);
>
> 867 //
> 868 // Additional steps for MMIO: align the queue appropriately, and
> set the
> 869 // size. If anything fails from here on, we must unmap the ring
> resources.
> 870 //
> 871 Status = Dev->VirtIo->SetQueueNum (Dev->VirtIo, QueueSize);
>
> 894 //
> 895 // step 5 -- Report understood features.
> 896 //
> 897 if (Dev->VirtIo->Revision < VIRTIO_SPEC_REVISION (1, 0, 0)) {
> 898 Features &= ~(UINT64)(VIRTIO_F_VERSION_1 |
> VIRTIO_F_IOMMU_PLATFORM);
> 899 Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features);
>
> We tried to make these "abstraction leaks" as small as possible; for
> example the MMIO specific operations (SetPageSize, SetQueueNum) are
> performed unconditionally, and it's only the PCI transport backends that
> simply ignore those actions (after performing some sanity checks).
> However, the different order of initialization steps couldn't really be
> hidden (I wasn't comfortable with simply regression-testing the new 1.0
> order against 0.9.5 transports of QEMU, so we kept both init orders).
>
> Virtio MMIO was always classified as "temporary" and "legacy", needed
> only until PCI support would be brought about on ARM. So given the
> increased complexity of Virtio MMIO in the 1.0 spec, I always believed
> that designing and implementing the latter in OVMF would be a waste of
> effort.
>
>
> > I will try to emulate the virtio-0.95 later to see if it is the root
> > cause.
>
> Yes, please either do that, or please add a PCI host.
>
> Given that you do get a BLK0: alias in the UEFI shell, the
> initialization of the device might even *appear* to complete, to OVMF;
> however, the actual virtio transfers likely fail.
>
That BLK0: alias in the UEFI shell is the NOR flash not a virtio block device.
--
Ard.
>
> > BTW, I found it really hard to read and understand the EDK2 code for
> > me, there is a long way to go.
>
> Yes. Edk2 uses PPIs and protocols [*] and library classes / library
> instances and sometimes callback registrations for composability, and so
> edk2 is really difficult to read in comparison to other projects, where
> you can just follow function calls.
>
> In edk2, you have to grep the source code for GUIDs, to understand what
> calls what. It was one of the hardest things for me as well, when
> starting with edk2.
>
> [*] Basically a GUID-identified structure of function pointers, and some
> data fields.
>
> Thanks
> Laszlo
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71526): https://edk2.groups.io/g/devel/message/71526
Mute This Topic: https://groups.io/mt/80471199/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
