Re: [PATCH 0/4] Virtio non-discoverable devices

[Daniil Egranov <daniil.egranov@arm.com>]

Hello Ard,

Thanks for reply. Please see my comments below.

On 03/07/2018 02:03 AM, Ard Biesheuvel wrote:
> (+ Laszlo)
>
> Hello Daniil,
>
> On 7 March 2018 at 01:36, Daniil Egranov<daniil.egranov@arm.com>  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.
> Why? The purpose of the non-discoverable device layer is to make
> non-PCI controllers that can be driven by PCI class drivers appear as
> PCI devices. We have started using the base non-discoverable device
> protocol for other devices as well, but the PCI wrapper is really only
> intended for PCI class drivers.

I am looking for a proper way to handle multiple MMIO Virtio devices on a single platform. As both PCI and MMIO types of Virtio device programmed in about the same way, non-discoverable devices approach was looking valid.

I understand you point. Correct me if I am wrong but all non-discoverable devices are MMIO devices so if there is PCI version of the device exists, PCI wrapper can be used. The Virtio PCI class devices are using VirtioPciDeviceDxe driver. Is this driver not fitting to the category of PCI class drivers?


> For VirtIO MMIO, we already have a driver model driver, and given that
> you need to patch up differences between MMIO and PCI based virtio in
> your code, I am reluctant to incorporate modifications in to the PCI
> driver to support MMIO devices.

There is no problem with using this driver model. I was interested to 
check if using unified access for both types of Virtio devices through 
the PCI driver is a right thing.

Regarding MMIO devices support in PCI driver. It's an additional MemIo 
call and it's is part of PC IO protocol interface. Is there a concern to 
call it from the PCI driver? It's up to the protocol provider to 
implement it or keep it empty.

> Is this related to virtio 1.0 support?

No it's 0.9.5.

>
> Also, could you please ensure next time that you cc all the relevant
> people? (Please check the Maintainers file)

Sorry, missed that part.

Thanks,
Daniil

>> 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.
>>
>> Daniil Egranov (4):
>>    MdeModulePkg: Added new Virtio non-discoverable type and GUID
>>    NonDiscoverableDeviceRegistrationLib: Added Virtio support
>>    NonDiscoverablePciDeviceDxe: Added MMIO Virtio support
>>    VirtioPciDeviceDxe: Added non-discoverable Virtio support
>>
>>   .../NonDiscoverablePciDeviceDxe.c                  |   3 +-
>>   .../NonDiscoverablePciDeviceDxe.inf                |   5 +-
>>   .../NonDiscoverablePciDeviceIo.c                   | 240 ++++++++++++++++++++-
>>   MdeModulePkg/Include/Guid/NonDiscoverableDevice.h  |   3 +
>>   .../Library/NonDiscoverableDeviceRegistrationLib.h |   1 +
>>   .../NonDiscoverableDeviceRegistrationLib.c         |   3 +
>>   .../NonDiscoverableDeviceRegistrationLib.inf       |   1 +
>>   MdeModulePkg/MdeModulePkg.dec                      |   1 +
>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c       | 143 +++++++++++-
>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h       |  21 +-
>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf  |   4 +-
>>   OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c    | 117 +++++++++-
>>   12 files changed, 528 insertions(+), 14 deletions(-)
>>
>> --
>> 2.11.0
>>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel