Re: [edk2-devel] Does EDK2 ArmVirtPkg has support for a virtio-mmio-blk device

["Laszlo Ersek" <lersek@redhat.com>]

On 02/09/21 16:28, Ard Biesheuvel wrote:
> On Tue, 9 Feb 2021 at 14:41, Laszlo Ersek <lersek@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