On 2016-04-05 09:46:33, Laszlo Ersek wrote:
> (I've snipped liberally below, but I've been careful to follow up on all
> of your comments.)
> 
> On 04/05/16 18:08, Jordan Justen wrote:
> > On 2016-04-05 01:31:27, Laszlo Ersek wrote:
> >> On 04/05/16 09:03, Jordan Justen wrote:
> >>> On 2016-03-14 05:53:23, Laszlo Ersek wrote:
> >>>> These header files are intentionally minimal, and intentionally kept 
> >>>> apart
> >>>> from the VirtIo 0.9.5 headers.
> >>>>
> >>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> >>>> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> >>>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> >>>> ---
> >>>>  OvmfPkg/Include/IndustryStandard/Virtio10.h    | 81 ++++++++++++++++++++
> >>>>  OvmfPkg/Include/IndustryStandard/Virtio10Net.h | 31 ++++++++
> >>>>  2 files changed, 112 insertions(+)
> >>>>
> >>>> diff --git a/OvmfPkg/Include/IndustryStandard/Virtio10.h 
> >>>> b/OvmfPkg/Include/IndustryStandard/Virtio10.h
> >>>> new file mode 100644
> >>>> index 000000000000..722475c4ea55
> >>>> --- /dev/null
> >>>> +++ b/OvmfPkg/Include/IndustryStandard/Virtio10.h
> > 
> > How about including Virtio10.h from Virtio.h?
> 
> Okay... What for? Definitions exposed by either header do not depend on
> definitions from the other header.
> 

Once again, because this follows the example of Acpi.h and Pci.h...

> 
> > What about naming this VirtioNet10.h instead?
> 
> I've been extremely concsious and careful to insert the "10" and "1_0"
> (as appropriate) strings right after the "virtio" prefix. The
> "modernity" (the 1.0 thing) characterizes the entire virtio spec first,
> and it may have consequences for various device types second.
> 

Ok.

> > 
> > For some reason, it just seems out of place in EDK II. I don't
> > personally have a problem with nesting structures, but I'm just trying
> > to think about what this would look like if it lived in MdePkg
> > instead.
> 
> Ard mentions the good example of device path protocols for nesting
> (thank you, Ard).
> 

No, this is not a good example. What seems odd (for EDK II) is
embedding structures to extend the fields for a newer version of the
spec.

> Also, this stuff definitely doesn't target MdePkg; similarly to the
> other files under "OvmfPkg/Include/IndustryStandard". (Not the smallest
> of which is the entire Xen subdirectory, imported whole-sale and very
> lightly touched up from Linux, ignoring the edk2 traditions almost
> completely...)
> 

True. The argument was that we'd be able to 'sync' updates. Not sure
there was much reality to that though... :(

> ... Clearly, I never try to "break" MdePkg requirements as some "sneaky
> goal" in OvmfPkg, but please let us not start applying MdePkg
> requirements to patches that do not target MdePkg. The virtio spec is a
> specification about virtualization. Whatever we need of it belongs under
> OvmfPkg.
> 
> I believe the patch doesn't conflict with the edk2 coding style document.
> 

I don't think it conflicts either.

I wonder if Mike or Andrew have any thoughts about why the ACPI
includes (and arguably EDK II) go out of their way to define flattened
(duplicated) structures for different spec versions.

Once again, I personally don't mind the sub-structure based on
versions if Mike and Andrew don't have anything to add...

-Jordan
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to