On 04/05/16 19:26, Jordan Justen wrote:
> 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 <[email protected]>
>>>>>> Cc: Jordan Justen <[email protected]>
>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>> Signed-off-by: Laszlo Ersek <[email protected]>
>>>>>> ---
>>>>>> 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...
Wait a minute, that's not a good analogy.
(a) Looking at Acpi.h and Pci.h, those are "catch-all" headers. They
say, "if you include me, I'll give you everything ACPI (or everything
PCI), across all versions of the relevant standards".
(b) Furthermore, regarding any such header file there that concerns
itself with a specific version of the PCI or ACPI specs, the header with
the higher version number includes the header with the lower version number.
Specifics:
- Acpi.h -> Acpi61.h -> Acpi60.h -> Acpi51.h -> Acpi50.h -> ...
- Pci.h -> Pci30.h -> Pci23.h -> Pci22.h
-> PciExpress21.h
-> PciExpress30.h -> PciExpress21.h (strange...)
As I said, this is not a good analogy. Because: Virtio.h never wanted to
be a catch-all header. To closely imitate the Acpi.h and Pci.h situation
above, I would have to rename Virtio.h to Virtio095.h first, then
include it in Virtio10.h, then introduce a *new* file called Virtio.h,
which should include Virtio10.h.
At the moment, Virtio.h is not the catch-all header, it is actually the
base (= lowest version number) header. So your suggestion would
establish the inverse direction, compared to Acpi.h and Pci.h.
Now, if you indeed proposed
Virtio.h [new] -> Virtio10.h -> Virtio095.h [to be renamed from
the current Virtio.h]
then I would say, "a lot of churn for nothing".
>>> 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.
I named another example, PCI_TYPE00 and PCI_TYPE01, both including
PCI_DEVICE_INDEPENDENT_REGION as first member, all the while being
separated from each other by significantly more than a version change:
they correspond to different specs.
My point is, it doesn't seem consistent to forbid sub-structure
extraction for inter-version sharing, when there is a (quite central)
example for *inter-spec* sharing (i.e., bridging a much larger
conceptual distance).
Anyway, I don't want to obsess about this any longer; as far as I
understand, you agreed to embedding VIRTIO_NET_REQ, conditional on my
renaming of the Legacy field, which I will certainly do. Thanks.
> 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.
Yes, that would be interesting to know. I wouldn't generalize the
question to the entirety of edk2 (see especially the PCI_TYPE00 and
PCI_TYPE01 counter-example above), but the ACPI examples are strange.
... I can make one attempt at speculating: I guess each version of the
ACPI spec defines the then-current ACPI structures as flat lists of
members. In other words, they do not refer to earlier versions of the
spec. In turn, MdePkg developers probably find it easier to trans-code
the spec to C source by following the flat listing. Extracting a common
sub-structure would be *more* work.
This does not apply to the virtio specs however, as far as
VIRTIO_1_0_NET_REQ and VIRTIO_NET_REQ. Both 0.9.5 and 1.0 have language
that calls the "num_buffers" field an *addition* to VIRTIO_NET_REQ.
> Once again, I personally don't mind the sub-structure based on
> versions if Mike and Andrew don't have anything to add...
Alright, let's see.
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
