On 10/16/13 19:29, Olivier Martin wrote:
> This protocol introduces an abstraction to access the VirtIo
> Configuration and Device spaces.
> The registers in these spaces are located at a different offset and have
> a different width whether the transport layer is either PCI or MMIO. This
> protocol would also allow to support VirtIo PCI devices with MSI-X
> capability in a transparent way (Device space is at a different offset
> when a PCIe device has MSI-X capability).
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Olivier Martin <[email protected]>
> ---
>  OvmfPkg/Include/Protocol/VirtioDevice.h |  376 
> +++++++++++++++++++++++++++++++
>  OvmfPkg/OvmfPkg.dec                     |    1 +
>  2 files changed, 377 insertions(+), 0 deletions(-)
>  create mode 100644 OvmfPkg/Include/Protocol/VirtioDevice.h

I compared this patch agains v3 03/10. I think it's good. One note:

> +// VirtIo Specification Revision: Major[31:24].Minor[23:16].Revision[15:0
> +#define VIRTIO_SPEC_REVISION(major,minor,revision) \
> +  ((((major) & 0xFF) << 24) | (((minor) & 0xFF) << 16) | ((revision) & 
> 0xFFFF))

and

> +struct _VIRTIO_DEVICE_PROTOCOL {
> +  /// VirtIo Specification Revision encoded with VIRTIO_SPEC_REVISION()
> +  UINT32                      Revision;


First, if (major >= 0x80), then VIRTIO_SPEC_REVISION() invokes undefined
behavior (the shifted value cannot be represented in a signed int).

Second, I'm uncertain if tying the protocol revision number to the
virtio spec version number is going to be useful. I have no use cases in
mind right now, so I can't decide either way.

Third, the Revision field should have been UINT64 I guess -- just
because other protocols' revision fields seem to be UINT64. Anyway, 32
bits should be plenty for compatible extensions.

None of the above warrants more churn right now, so

Reviewed-by: Laszlo Ersek <[email protected]>

Thanks!
Laszlo

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to