On 09/18/13 11:22, Olivier Martin wrote:
>
>
>> -----Original Message-----
>> From: Jordan Justen [mailto:[email protected]]
>> Sent: 18 September 2013 05:53
>> To: Olivier Martin; Laszlo Ersek
>> Cc: [email protected]
>> 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
>> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
