From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8E7A522135D37 for ; Wed, 7 Mar 2018 02:50:40 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 925F6D1433; Wed, 7 Mar 2018 10:56:54 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-143.rdu2.redhat.com [10.10.120.143]) by smtp.corp.redhat.com (Postfix) with ESMTP id 85C10215CDA7; Wed, 7 Mar 2018 10:56:53 +0000 (UTC) To: Daniil Egranov Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org, ard.biesheuvel@linaro.org References: <20180307013637.16462-1-daniil.egranov@arm.com> From: Laszlo Ersek Message-ID: <82b8b130-c37b-ca72-aa6a-bf4e9e7b005b@redhat.com> Date: Wed, 7 Mar 2018 11:56:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180307013637.16462-1-daniil.egranov@arm.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 07 Mar 2018 10:56:54 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 07 Mar 2018 10:56:54 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH 0/4] Virtio non-discoverable devices X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Mar 2018 10:50:41 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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