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

Reply via email to