From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=daniil.egranov@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 820782257C2B1 for ; Mon, 12 Mar 2018 19:49:02 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BD8F61596; Mon, 12 Mar 2018 19:55:22 -0700 (PDT) Received: from [10.118.34.22] (dbox2.austin.arm.com [10.118.34.22]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8095A3F487; Mon, 12 Mar 2018 19:55:22 -0700 (PDT) To: Laszlo Ersek Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org, ard.biesheuvel@linaro.org References: <20180307013637.16462-1-daniil.egranov@arm.com> <82b8b130-c37b-ca72-aa6a-bf4e9e7b005b@redhat.com> From: Daniil Egranov Message-ID: Date: Mon, 12 Mar 2018 21:55:21 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <82b8b130-c37b-ca72-aa6a-bf4e9e7b005b@redhat.com> 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: Tue, 13 Mar 2018 02:49:02 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 >