On 09/18/13 11:22, Olivier Martin wrote: > > >> -----Original Message----- >> From: Jordan Justen [mailto:jljus...@gmail.com] >> Sent: 18 September 2013 05:53 >> To: Olivier Martin; Laszlo Ersek >> Cc: edk2-devel@lists.sourceforge.net >> Subject: Re: [edk2] [PATCH 3/8] OvmfPkg/VirtioPciDeviceDxe: Implement >> VIRTIO_DEVICE_PROTOCOL for VirtIo Devices over PCI >> >> On Tue, Sep 17, 2013 at 9:54 AM, Olivier Martin >> <olivier.mar...@arm.com> wrote: >> >> Seems like at least a small commit message body might be appropriate. > > You are right. I will send it again with a more appropriate commit message > after having more feedback on the patchset. > >> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Olivier Martin <olivier.mar...@arm.com> >>> --- >>> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c | 637 >> +++++++++++++++++++++ >>> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h | 246 ++++++++ >>> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf | 42 ++ >>> OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 188 ++++++ >>> 4 files changed, 1113 insertions(+), 0 deletions(-) >>> create mode 100644 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c >>> create mode 100644 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h >>> create mode 100644 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf >>> create mode 100644 OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c >> >> <snip> >> >>> +EFI_STATUS >>> +VirtioPciDeviceRead ( >>> + IN VIRTIO_DEVICE_PROTOCOL *This, >>> + IN UINTN FieldOffset, >>> + IN UINTN FieldSize, >>> + IN UINTN BufferSize, >>> + OUT VOID *Buffer >>> + ) >>> +{ >>> + UINTN Count; >>> + EFI_PCI_IO_PROTOCOL_WIDTH Width; >>> + EFI_PCI_IO_PROTOCOL *PciIo; >>> + VIRTIO_PCI_DEVICE *Dev; >>> + >>> + ASSERT (FieldSize == BufferSize); >>> + >>> + Dev = VIRTIO_PCI_DEVICE_FROM_VIRTIO_DEVICE (This); >>> + PciIo = Dev->PciIo; >>> + >>> + Count = 1; >>> + switch (FieldSize) { >>> + case 1: >>> + Width = EfiPciIoWidthUint8; >>> + break; >>> + >>> + case 2: >>> + Width = EfiPciIoWidthUint16; >>> + break; >>> + >>> + case 8: >>> + Count = 2; >>> + // fall through >> >> Laszlo, >> >> This seems to come from OvmfPkg/Library/VirtioLib/VirtioLib.c. Why use >> 2 32-bit ops here rather than 1 64-bit? >> >> -Jordan
The UEFI spec says under EFI_PCI_IO_PROTOCOL.Io.Write(): The I/O operations are carried out exactly as requested. The caller is responsible for any alignment and I/O width issues which the bus, device, platform, or type of I/O might require. For example on some platforms, width requests of EfiPciIoWidthUint64 do not work. I wanted to be careful here, just in case. There has been at least one thread on the list about hardware that didn't support 64-bit writes, but worked with two 32-bit writes. http://thread.gmane.org/gmane.comp.bios.tianocore.devel/2226 > You are also right. From the VirtIo sepc v0.9.5, a 64-bit might be more > appropriate: > > "2.2 Device Configuration > > There may be different widths of accesses to the I/O region; the natural > access > method for each field in the virtio header must be used (i.e. 32-bit > accesses for > 32-bit fields, etc), but the device-specific region can be accessed using > any width > accesses, and should obtain the same results. > Note that this is possible because while the virtio header is PCI (i.e. > little) > endian, the device-specific region is encoded in the native endian of the > guest > (where such distinction is applicable)." The common virtio header has no 64-bit fields. The device-specific fields, following the common header, allow breaking up writes. Thanks, Laszlo ------------------------------------------------------------------------------ LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99! 1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13. http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel