.../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(-)
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. 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
(+ 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. 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. Is this related to virtio 1.0 support? Also, could you please ensure next time that you cc all the relevant people? (Please check the Maintainers file) > 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
On 03/07/18 09:03, 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. > > 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. I'm impressed and thankful that you managed to say it in two paragraphs -- I needed ten or more :) Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hello Daniil, Could you please use a text based email client? Gmail does not consider the indentation as threading, so the context below is unintelligible On 8 March 2018 at 08:21, Daniil Egranov <daniil.egranov@arm.com> wrote: > 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? > That is not the point. The Intel guys have decided that the AHCI, XHCI and other drivers (whose specs do no mandate PCI) are implemented as PCI drivers only, which means that they are essentially combining two layers of the driver stack (the PCI part and the _HCI part) Splitting all of those drivers into PCI drivers that produce _HCI protocols and _HCI drivers that produce the USB host, SATA host, etc protocols is not going to be accepted by upstream EDK2, so instead, we decided to turn the PCI 'emulation' that was duplicated across many platforms into a proper abstraction. In the virtio case, we don't have that problem. We have MMIO and PCI support using drivers that use the proper abstractions, and so presenting MMIO devices as PCI devices is really a step backwards. What are you trying to achieve that the current code won't let you? -- Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hello Ard, On 03/08/2018 02:29 AM, Ard Biesheuvel wrote: > Hello Daniil, > > Could you please use a text based email client? Gmail does not > consider the indentation as threading, so the context below is > unintelligible > I forced Thunderbird to plain text. I assumed it should follow a previous email composition style. At least i did not have problem with it so far. I hope it's formated correctly now. > On 8 March 2018 at 08:21, Daniil Egranov <daniil.egranov@arm.com> wrote: >> 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? >> > > That is not the point. The Intel guys have decided that the AHCI, XHCI > and other drivers (whose specs do no mandate PCI) are implemented as > PCI drivers only, which means that they are essentially combining two > layers of the driver stack (the PCI part and the _HCI part) > > Splitting all of those drivers into PCI drivers that produce _HCI > protocols and _HCI drivers that produce the USB host, SATA host, etc > protocols is not going to be accepted by upstream EDK2, so instead, we > decided to turn the PCI 'emulation' that was duplicated across many > platforms into a proper abstraction. > > In the virtio case, we don't have that problem. We have MMIO and PCI > support using drivers that use the proper abstractions, and so > presenting MMIO devices as PCI devices is really a step backwards. > I see, thanks for details. > What are you trying to achieve that the current code won't let you? > I have a situation when a platform has multiple Virtio MMIO devices. The initial way to handle them was installing a device path protocol and calling VirtioMmioDeviceLib for each device as part of the platform DXE driver. The non-discoverable way was hiding all these operations and was looking like more clean approach. In case of multiple Virtio MMIO devices, what will be a proper way to handle them? Thanks, Daniil _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 12 March 2018 at 06:19, Daniil Egranov <daniil.egranov@arm.com> wrote: > Hello Ard, > > On 03/08/2018 02:29 AM, Ard Biesheuvel wrote: >> >> Hello Daniil, >> >> Could you please use a text based email client? Gmail does not >> consider the indentation as threading, so the context below is >> unintelligible >> > > I forced Thunderbird to plain text. I assumed it should follow a previous > email composition style. At least i did not have problem with it so far. I > hope it's formated correctly now. > Thanks. > >> On 8 March 2018 at 08:21, Daniil Egranov <daniil.egranov@arm.com> wrote: >>> >>> 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? >>> >> >> That is not the point. The Intel guys have decided that the AHCI, XHCI >> and other drivers (whose specs do no mandate PCI) are implemented as >> PCI drivers only, which means that they are essentially combining two >> layers of the driver stack (the PCI part and the _HCI part) >> >> Splitting all of those drivers into PCI drivers that produce _HCI >> protocols and _HCI drivers that produce the USB host, SATA host, etc >> protocols is not going to be accepted by upstream EDK2, so instead, we >> decided to turn the PCI 'emulation' that was duplicated across many >> platforms into a proper abstraction. >> >> In the virtio case, we don't have that problem. We have MMIO and PCI >> support using drivers that use the proper abstractions, and so >> presenting MMIO devices as PCI devices is really a step backwards. >> > > I see, thanks for details. > >> What are you trying to achieve that the current code won't let you? >> > > I have a situation when a platform has multiple Virtio MMIO devices. The > initial way to handle them was installing a device path protocol and calling > VirtioMmioDeviceLib for each device as part of the platform DXE driver. The > non-discoverable way was hiding all these operations and was looking like > more clean approach. Well, that is only true because you are using NonDiscoverableDeviceRegistrationLib, which encapsulates those exact same things. > In case of multiple Virtio MMIO devices, what will be a proper way to handle > them? > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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
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
© 2016 - 2024 Red Hat, Inc.