On 02/09/21 16:28, Ard Biesheuvel wrote:
> 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.
>
Sigh, thanks.
In the original message, only "VenHw(xxx, 00)" was included, and I
couldn't tell at once whether the vendor GUID was indeed
gVirtioMmioTransportGuid (837DCA9E-E874-4D82-B29A-23FE0E23D1E2). So I
assumed. :/
Ying Fang: in the future, please only paste actual logs. If you edit the
log, you may render it useless. In this instance, you managed to edit
out a relatively important log detail.
Thanks, Ard!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71533): https://edk2.groups.io/g/devel/message/71533
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]
-=-=-=-=-=-=-=-=-=-=-=-
