On 14/02/13 10:54, Pawel Moll wrote:
> To solve the never-ending confusions between hosts and guests
> of different endianess, define all virtio-mmio registers as LE.
>
> This change should be safe at this stage, because no known
> working mixed-endian system exists so there is virtually no
> risk of breaking compatibility.
>
> Signed-off-by: Pawel Moll <[email protected]>
> ---
> virtio-spec.lyx | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index 1ba9992..a00b675 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -56,6 +56,7 @@
> \html_math_output 0
> \html_css_as_file 0
> \html_be_strict false
> +\author -875410574 "Pawel Moll"
> \author -608949062 "Rusty Russell,,,"
> \author -385801441 "Cornelia Huck" [email protected]
> \author 1112500848 "Rusty Russell" [email protected]
> @@ -1850,6 +1851,17 @@ struct vring {
>
> \begin_layout Subsection
> A Note on Virtqueue Endianness
> +\change_inserted -875410574 1360838374
> +
> +\begin_inset CommandInset label
> +LatexCommand label
> +name "sub:Virtqueue-Endianness"
> +
> +\end_inset
> +
> +
> +\change_unchanged
> +
> \end_layout
>
> \begin_layout Standard
> @@ -9850,8 +9862,24 @@ a
> \end_layout
>
> \begin_layout Standard
> +
> +\change_deleted -875410574 1360838214
> The endianness of the registers follows the native endianness of the Guest.
> - Writing to registers described as
> +\change_inserted -875410574 1360838930
> +All register values are organized as Little Endian, similarly to the PCI
> + variant, see also
> +\begin_inset CommandInset ref
> +LatexCommand ref
> +reference "sub:Virtqueue-Endianness"
Shouldn't we exclude the config space? PCI defines it as guest-endian,
and the above tends to indicate that it should be LE with MMIO.
M.
--
Jazz is not dead. It just smells funny...
On Fri, 2013-03-01 at 10:41 +0000, Marc Zyngier wrote:
> On 14/02/13 10:54, Pawel Moll wrote:
> > To solve the never-ending confusions between hosts and guests
> > of different endianess, define all virtio-mmio registers as LE.
> >
> > This change should be safe at this stage, because no known
> > working mixed-endian system exists so there is virtually no
> > risk of breaking compatibility.
> >
> > Signed-off-by: Pawel Moll <[email protected]>
>
> Shouldn't we exclude the config space? PCI defines it as guest-endian,
> and the above tends to indicate that it should be LE with MMIO.
The spec says: "Device-specific configuration space starts at an offset
0x100 and is accessed with byte alignment. Its meaning and size depends
on the device and the driver."
I would hope that "accessed with byte alignment" is enough of a clue in
this subject, but if you think otherwise I can add a sentence stating
this explicitly.
Having said that, Rusty was contemplating enforcing LE config space in
the new PCI layout...
Paweł
On 01/03/13 10:50, Pawel Moll wrote:
> On Fri, 2013-03-01 at 10:41 +0000, Marc Zyngier wrote:
>> On 14/02/13 10:54, Pawel Moll wrote:
>>> To solve the never-ending confusions between hosts and guests
>>> of different endianess, define all virtio-mmio registers as LE.
>>>
>>> This change should be safe at this stage, because no known
>>> working mixed-endian system exists so there is virtually no
>>> risk of breaking compatibility.
>>>
>>> Signed-off-by: Pawel Moll <[email protected]>
>>
>> Shouldn't we exclude the config space? PCI defines it as guest-endian,
>> and the above tends to indicate that it should be LE with MMIO.
>
> The spec says: "Device-specific configuration space starts at an offset
> 0x100 and is accessed with byte alignment. Its meaning and size depends
> on the device and the driver."
>
> I would hope that "accessed with byte alignment" is enough of a clue in
> this subject, but if you think otherwise I can add a sentence stating
> this explicitly.
Well, it was unclear enough for me to get confused... ;-) It would make
sense to have a wording similar to the one in the PCI section.
> Having said that, Rusty was contemplating enforcing LE config space in
> the new PCI layout...
I wouldn't complain about that, and would like to see a similar thing on
MMIO.
M.
--
Jazz is not dead. It just smells funny...
On Fri, 2013-03-01 at 11:21 +0000, Marc Zyngier wrote:
> > Having said that, Rusty was contemplating enforcing LE config space in
> > the new PCI layout...
>
> I wouldn't complain about that, and would like to see a similar thing on
> MMIO.
Wherever PCI goes, MMIO follows :-)
> Well, it was unclear enough for me to get confused... ;-) It would make
> sense to have a wording similar to the one in the PCI section.
How about this?
8<---------
>From a80f01f15397395a8fc49ef424a2d47c8be0937a Mon Sep 17 00:00:00 2001
From: Pawel Moll <[email protected]>
Date: Fri, 1 Mar 2013 12:35:06 +0000
Subject: [PATCH] virtio-spec: Clarify virtio-mmio configuration space organisation
To clarify potential confusion regarding the configuration
space organisation (endianness in particular) the spec
should clearly state that it follows the PCI device behaviour.
Signed-off-by: Pawel Moll <[email protected]>
---
virtio-spec.lyx | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index a00b675..2fba0b0 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -696,6 +696,17 @@ rproc serial
\begin_layout Section
Device Configuration
+\change_inserted -875410574 1362141102
+
+\begin_inset CommandInset label
+LatexCommand label
+name "sec:Device-Configuration"
+
+\end_inset
+
+
+\change_unchanged
+
\end_layout
\begin_layout Standard
@@ -9865,7 +9876,7 @@ a
\change_deleted -875410574 1360838214
The endianness of the registers follows the native endianness of the Guest.
-\change_inserted -875410574 1360838930
+\change_inserted -875410574 1362141146
All register values are organized as Little Endian, similarly to the PCI
variant, see also
\begin_inset CommandInset ref
@@ -9875,6 +9886,15 @@ reference "sub:Virtqueue-Endianness"
\end_inset
.
+ The device-specific configuration space organisation follows the PCI device
+ specification, see
+\begin_inset CommandInset ref
+LatexCommand ref
+reference "sec:Device-Configuration"
+
+\end_inset
+
+.
\change_deleted -875410574 1360838262
--
1.7.10.4
On 01/03/13 12:37, Pawel Moll wrote:
> On Fri, 2013-03-01 at 11:21 +0000, Marc Zyngier wrote:
>>> Having said that, Rusty was contemplating enforcing LE config space in
>>> the new PCI layout...
>>
>> I wouldn't complain about that, and would like to see a similar thing on
>> MMIO.
>
> Wherever PCI goes, MMIO follows :-)
Sweet. Makes my life easier ;-).
>> Well, it was unclear enough for me to get confused... ;-) It would make
>> sense to have a wording similar to the one in the PCI section.
>
> How about this?
>
> 8<---------
> From a80f01f15397395a8fc49ef424a2d47c8be0937a Mon Sep 17 00:00:00 2001
> From: Pawel Moll <[email protected]>
> Date: Fri, 1 Mar 2013 12:35:06 +0000
> Subject: [PATCH] virtio-spec: Clarify virtio-mmio configuration space organisation
>
> To clarify potential confusion regarding the configuration
> space organisation (endianness in particular) the spec
> should clearly state that it follows the PCI device behaviour.
>
> Signed-off-by: Pawel Moll <[email protected]>
Looks good to me!
M.
--
Jazz is not dead. It just smells funny...
Pawel Moll <[email protected]> writes:
> On Fri, 2013-03-01 at 11:21 +0000, Marc Zyngier wrote:
>> > Having said that, Rusty was contemplating enforcing LE config space in
>> > the new PCI layout...
>>
>> I wouldn't complain about that, and would like to see a similar thing on
>> MMIO.
>
> Wherever PCI goes, MMIO follows :-)
Yes, but if you switch from 'guest-endian' to 'little-endian' how will
you tell? For PCI, we'd detect it by using the new layout.
I'd rather you specify MMIO as little endian, and we fix the kernel
config accessors to be endian aware (ie. 8, 16, 32, 64-bit accessors).
Since noone BE is using MMIO right now, it's safe...
Thanks,
Rusty.
On Tue, 2013-03-05 at 00:11 +0000, Rusty Russell wrote:
> Pawel Moll <[email protected]> writes:
> > On Fri, 2013-03-01 at 11:21 +0000, Marc Zyngier wrote:
> >> > Having said that, Rusty was contemplating enforcing LE config space in
> >> > the new PCI layout...
> >>
> >> I wouldn't complain about that, and would like to see a similar thing on
> >> MMIO.
> >
> > Wherever PCI goes, MMIO follows :-)
>
> Yes, but if you switch from 'guest-endian' to 'little-endian' how will
> you tell? For PCI, we'd detect it by using the new layout.
The version register/value. At some point of time there will be a
new(ish) MMIO layout anyway to deal with 64-bit addresses, replacing the
ring page number with two 32-bit hi/lo physical address registers. This
was discussed not long after the driver got merged...
> I'd rather you specify MMIO as little endian, and we fix the kernel
> config accessors to be endian aware (ie. 8, 16, 32, 64-bit accessors).
> Since noone BE is using MMIO right now, it's safe...
That's absolutely fine with me, however I don't see anything I could do
in the virtio_mmio driver and spec - the virtio_config_ops specifies
get/set as void * operations and I simply do byte-by-byte copy. Have I
missed some config/endianess/PCI related discussion?
Paweł
Pawel Moll <[email protected]> writes:
> On Tue, 2013-03-05 at 00:11 +0000, Rusty Russell wrote:
>> Pawel Moll <[email protected]> writes:
>> > On Fri, 2013-03-01 at 11:21 +0000, Marc Zyngier wrote:
>> >> > Having said that, Rusty was contemplating enforcing LE config space in
>> >> > the new PCI layout...
>> >>
>> >> I wouldn't complain about that, and would like to see a similar thing on
>> >> MMIO.
>> >
>> > Wherever PCI goes, MMIO follows :-)
>>
>> Yes, but if you switch from 'guest-endian' to 'little-endian' how will
>> you tell? For PCI, we'd detect it by using the new layout.
>
> The version register/value. At some point of time there will be a
> new(ish) MMIO layout anyway to deal with 64-bit addresses, replacing the
> ring page number with two 32-bit hi/lo physical address registers. This
> was discussed not long after the driver got merged...
As long as you have a plan for older guests...
>> I'd rather you specify MMIO as little endian, and we fix the kernel
>> config accessors to be endian aware (ie. 8, 16, 32, 64-bit accessors).
>> Since noone BE is using MMIO right now, it's safe...
>
> That's absolutely fine with me, however I don't see anything I could do
> in the virtio_mmio driver and spec - the virtio_config_ops specifies
> get/set as void * operations and I simply do byte-by-byte copy. Have I
> missed some config/endianess/PCI related discussion?
Yes, that's exactly what I mean, they'd have to be split into
8/16/32/64 accessors. Which would do byte-by-byte for PCI.
The spec can simply be updated.
Cheers,
Rusty.