> -----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

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)."





------------------------------------------------------------------------------
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