Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, 2013-02-13 at 15:28 +, Marc Zyngier wrote: > Well, the spec clearly says that the registers reflect the endianess of > the guest, and it makes sense: when performing the MMIO access, KVM > needs to convert between host and guest endianess. It's actually a horrible idea :-) What does "guest endianness" means from a qemu perspective if your emulated CPU can operate in either mode ? It's actually been causing endless problems, besides linux doesn't have "Sane" MMIO accessors that say "current endianness". ioreadN/writeN are LE, realN/writeN are LE, ioreadNbe/iowriteNbe are BE always, the only "whatever my endianness is" are the __raw ones which also don't have barriers etc... > > Having said that, does the change make everything else work with a BE > > guest? (I assume we're talking about the guest being BE, right? ;-) If > > so it means that the host is not following the current spec and it > > treats all the registers as LE. > > Yes, I only care about a BE guest. And no, not much is actually working > (kvmtool is not happy about the guest addresses it finds in the > virtio-ring). Need to dive into it and understand what needs to be fixed... > > >> - Reading the MAGIC register byte by byte. Is that allowed? The spec > >> only says it is 32bit wide. > > > > And the spirit of the spec was: _exactly 32bit wide_. It's just simpler > > to implement one access width on the host side. > > I guessed as much... Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, 2013-02-13 at 15:28 +, Marc Zyngier wrote: Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. It's actually a horrible idea :-) What does guest endianness means from a qemu perspective if your emulated CPU can operate in either mode ? It's actually been causing endless problems, besides linux doesn't have Sane MMIO accessors that say current endianness. ioreadN/writeN are LE, realN/writeN are LE, ioreadNbe/iowriteNbe are BE always, the only whatever my endianness is are the __raw ones which also don't have barriers etc... Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... - Reading the MAGIC register byte by byte. Is that allowed? The spec only says it is 32bit wide. And the spirit of the spec was: _exactly 32bit wide_. It's just simpler to implement one access width on the host side. I guessed as much... Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
"Michael S. Tsirkin" writes: > On Wed, Feb 13, 2013 at 03:28:52PM +, Marc Zyngier wrote: >> On 13/02/13 15:08, Pawel Moll wrote: >> > On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: >> >> Using readl() to read the magic value and then memcmp() to check it >> >> fails on BE, as bytes will be the other way around (by virtue of >> >> the registers to follow the endianess of the guest). >> > >> > Hm. Interesting. I missed the fact that readl() as a "PCI operation" >> > will always assume LE values... >> > >> >> Fix it by encoding the magic as an integer instead of a string. >> >> So I'm not completely sure this is the right fix, >> > >> > It seems right, however... >> > >> >> - Using __raw_readl() instead. Is that a generic enough API? >> >> >> > ... this implies that either the spec is wrong (as it should say: the >> > device registers are always LE, in the PCI spirit) or all readl()s & co. >> > should be replaced with __raw equivalents. >> >> Well, the spec clearly says that the registers reflect the endianess of >> the guest, and it makes sense: when performing the MMIO access, KVM >> needs to convert between host and guest endianess. >> >> > Having said that, does the change make everything else work with a BE >> > guest? (I assume we're talking about the guest being BE, right? ;-) If >> > so it means that the host is not following the current spec and it >> > treats all the registers as LE. >> >> Yes, I only care about a BE guest. And no, not much is actually working >> (kvmtool is not happy about the guest addresses it finds in the >> virtio-ring). Need to dive into it and understand what needs to be fixed... > > Does it work for qemu? I know people were using virtio on BE there. It's had no end of problems: the powerpc folk are quite upset with me. If I were doing virtio again, I'd use LE everywhere: device authors *expect* to worry about endian, and it's confusing for them when they don't. It's tempting to use LE for the PCI config space for the new layout, for example. If you want to specify virtio-mmio as LE, feel free. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On 13/02/13 16:53, Michael S. Tsirkin wrote: > On Wed, Feb 13, 2013 at 03:28:52PM +, Marc Zyngier wrote: >> On 13/02/13 15:08, Pawel Moll wrote: >>> On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: Using readl() to read the magic value and then memcmp() to check it fails on BE, as bytes will be the other way around (by virtue of the registers to follow the endianess of the guest). >>> >>> Hm. Interesting. I missed the fact that readl() as a "PCI operation" >>> will always assume LE values... >>> Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, >>> >>> It seems right, however... >>> - Using __raw_readl() instead. Is that a generic enough API? >>> ... this implies that either the spec is wrong (as it should say: the >>> device registers are always LE, in the PCI spirit) or all readl()s & co. >>> should be replaced with __raw equivalents. >> >> Well, the spec clearly says that the registers reflect the endianess of >> the guest, and it makes sense: when performing the MMIO access, KVM >> needs to convert between host and guest endianess. >> >>> Having said that, does the change make everything else work with a BE >>> guest? (I assume we're talking about the guest being BE, right? ;-) If >>> so it means that the host is not following the current spec and it >>> treats all the registers as LE. >> >> Yes, I only care about a BE guest. And no, not much is actually working >> (kvmtool is not happy about the guest addresses it finds in the >> virtio-ring). Need to dive into it and understand what needs to be fixed... > > Does it work for qemu? I know people were using virtio on BE there. No idea, haven't tried yet, and my guest "platform" isn't supported by QEMU. Is someone actually using BE guests on LE hosts? That would be very interesting to compare things... M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, Feb 13, 2013 at 03:28:52PM +, Marc Zyngier wrote: > On 13/02/13 15:08, Pawel Moll wrote: > > On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: > >> Using readl() to read the magic value and then memcmp() to check it > >> fails on BE, as bytes will be the other way around (by virtue of > >> the registers to follow the endianess of the guest). > > > > Hm. Interesting. I missed the fact that readl() as a "PCI operation" > > will always assume LE values... > > > >> Fix it by encoding the magic as an integer instead of a string. > >> So I'm not completely sure this is the right fix, > > > > It seems right, however... > > > >> - Using __raw_readl() instead. Is that a generic enough API? > >> > > ... this implies that either the spec is wrong (as it should say: the > > device registers are always LE, in the PCI spirit) or all readl()s & co. > > should be replaced with __raw equivalents. > > Well, the spec clearly says that the registers reflect the endianess of > the guest, and it makes sense: when performing the MMIO access, KVM > needs to convert between host and guest endianess. > > > Having said that, does the change make everything else work with a BE > > guest? (I assume we're talking about the guest being BE, right? ;-) If > > so it means that the host is not following the current spec and it > > treats all the registers as LE. > > Yes, I only care about a BE guest. And no, not much is actually working > (kvmtool is not happy about the guest addresses it finds in the > virtio-ring). Need to dive into it and understand what needs to be fixed... Does it work for qemu? I know people were using virtio on BE there. > >> - Reading the MAGIC register byte by byte. Is that allowed? The spec > >> only says it is 32bit wide. > > > > And the spirit of the spec was: _exactly 32bit wide_. It's just simpler > > to implement one access width on the host side. > > I guessed as much... > > M. > -- > Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On 13/02/13 15:46, Pawel Moll wrote: > On Wed, 2013-02-13 at 15:28 +, Marc Zyngier wrote: Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, >>> >>> It seems right, however... >>> - Using __raw_readl() instead. Is that a generic enough API? >>> ... this implies that either the spec is wrong (as it should say: the >>> device registers are always LE, in the PCI spirit) or all readl()s & co. >>> should be replaced with __raw equivalents. >> >> Well, the spec clearly says that the registers reflect the endianess of >> the guest, and it makes sense: when performing the MMIO access, KVM >> needs to convert between host and guest endianess. > > The virtio-mmio spec says so because it seemed like a good idea at the > time ;-) after reading the PCI device spec. But - as I said - I missed > the fact that the readl()-like accessors will always do le32_to_cpu(). > Apparently ioread32() does the same (there's a separate ioread32be()). Maybe. There's so much byte swapping at every possible level that my head spins... ;-) > So I'm not sure that the spec is correct in this aspect any more. Maybe > it should specify the registers as LE always, similarly to PCI? This > problem is already covered by "2.3.1 A Note on Virtqueue Endianness" in > the spec... This section basically covers shared memory, and there is not much we can do about it. When it comes to the registers (that actually trap into the hypervisor), it probably makes sense to declare them as LE indeed. >>> Having said that, does the change make everything else work with a BE >>> guest? (I assume we're talking about the guest being BE, right? ;-) If >>> so it means that the host is not following the current spec and it >>> treats all the registers as LE. >> >> Yes, I only care about a BE guest. And no, not much is actually working >> (kvmtool is not happy about the guest addresses it finds in the >> virtio-ring). Need to dive into it and understand what needs to be fixed... > > Do the other registers like queuenum make sense? Could it be that the > page number of the ring you're getting has wrong endianness? The addresses are definitely wrong. kvmtool is spitting things like: Warning: unable to translate guest address 0xe8fd828f to host which tends to indicate that yes, page numbers are the other way around. Cross-endianness shared memory fun. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, 2013-02-13 at 15:28 +, Marc Zyngier wrote: > >> Fix it by encoding the magic as an integer instead of a string. > >> So I'm not completely sure this is the right fix, > > > > It seems right, however... > > > >> - Using __raw_readl() instead. Is that a generic enough API? > >> > > ... this implies that either the spec is wrong (as it should say: the > > device registers are always LE, in the PCI spirit) or all readl()s & co. > > should be replaced with __raw equivalents. > > Well, the spec clearly says that the registers reflect the endianess of > the guest, and it makes sense: when performing the MMIO access, KVM > needs to convert between host and guest endianess. The virtio-mmio spec says so because it seemed like a good idea at the time ;-) after reading the PCI device spec. But - as I said - I missed the fact that the readl()-like accessors will always do le32_to_cpu(). Apparently ioread32() does the same (there's a separate ioread32be()). So I'm not sure that the spec is correct in this aspect any more. Maybe it should specify the registers as LE always, similarly to PCI? This problem is already covered by "2.3.1 A Note on Virtqueue Endianness" in the spec... > > Having said that, does the change make everything else work with a BE > > guest? (I assume we're talking about the guest being BE, right? ;-) If > > so it means that the host is not following the current spec and it > > treats all the registers as LE. > > Yes, I only care about a BE guest. And no, not much is actually working > (kvmtool is not happy about the guest addresses it finds in the > virtio-ring). Need to dive into it and understand what needs to be fixed... Do the other registers like queuenum make sense? Could it be that the page number of the ring you're getting has wrong endianness? Paweł -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On 13/02/13 15:08, Pawel Moll wrote: > On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: >> Using readl() to read the magic value and then memcmp() to check it >> fails on BE, as bytes will be the other way around (by virtue of >> the registers to follow the endianess of the guest). > > Hm. Interesting. I missed the fact that readl() as a "PCI operation" > will always assume LE values... > >> Fix it by encoding the magic as an integer instead of a string. >> So I'm not completely sure this is the right fix, > > It seems right, however... > >> - Using __raw_readl() instead. Is that a generic enough API? >> > ... this implies that either the spec is wrong (as it should say: the > device registers are always LE, in the PCI spirit) or all readl()s & co. > should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. > Having said that, does the change make everything else work with a BE > guest? (I assume we're talking about the guest being BE, right? ;-) If > so it means that the host is not following the current spec and it > treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... >> - Reading the MAGIC register byte by byte. Is that allowed? The spec >> only says it is 32bit wide. > > And the spirit of the spec was: _exactly 32bit wide_. It's just simpler > to implement one access width on the host side. I guessed as much... M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: > Using readl() to read the magic value and then memcmp() to check it > fails on BE, as bytes will be the other way around (by virtue of > the registers to follow the endianess of the guest). Hm. Interesting. I missed the fact that readl() as a "PCI operation" will always assume LE values... > Fix it by encoding the magic as an integer instead of a string. > So I'm not completely sure this is the right fix, It seems right, however... > - Using __raw_readl() instead. Is that a generic enough API? > ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s & co. should be replaced with __raw equivalents. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. > - Reading the MAGIC register byte by byte. Is that allowed? The spec > only says it is 32bit wide. And the spirit of the spec was: _exactly 32bit wide_. It's just simpler to implement one access width on the host side. Paweł -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] virt_mmio: fix signature checking for BE guests
Using readl() to read the magic value and then memcmp() to check it
fails on BE, as bytes will be the other way around (by virtue of
the registers to follow the endianess of the guest).
Fix it by encoding the magic as an integer instead of a string.
Cc: Rusty Russell
Cc: Michael S. Tsirkin
Cc: Pawel Moll
Signed-off-by: Marc Zyngier
---
So I'm not completely sure this is the right fix, and I can imagine
other ways to cure the problem:
- Reading the MAGIC register byte by byte. Is that allowed? The spec
only says it is 32bit wide.
- Using __raw_readl() instead. Is that a generic enough API?
drivers/virtio/virtio_mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 31f966f..3811e94 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
/* Check magic value */
magic = readl(vm_dev->base + VIRTIO_MMIO_MAGIC_VALUE);
- if (memcmp(, "virt", 4) != 0) {
+ if (magic != ('v' | 'i' << 8 | 'r' << 16 | 't' << 24)) {
dev_warn(>dev, "Wrong magic value 0x%08lx!\n", magic);
return -ENODEV;
}
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] virt_mmio: fix signature checking for BE guests
Using readl() to read the magic value and then memcmp() to check it
fails on BE, as bytes will be the other way around (by virtue of
the registers to follow the endianess of the guest).
Fix it by encoding the magic as an integer instead of a string.
Cc: Rusty Russell ru...@rustcorp.com.au
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Pawel Moll pawel.m...@arm.com
Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
So I'm not completely sure this is the right fix, and I can imagine
other ways to cure the problem:
- Reading the MAGIC register byte by byte. Is that allowed? The spec
only says it is 32bit wide.
- Using __raw_readl() instead. Is that a generic enough API?
drivers/virtio/virtio_mmio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 31f966f..3811e94 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -470,7 +470,7 @@ static int virtio_mmio_probe(struct platform_device *pdev)
/* Check magic value */
magic = readl(vm_dev-base + VIRTIO_MMIO_MAGIC_VALUE);
- if (memcmp(magic, virt, 4) != 0) {
+ if (magic != ('v' | 'i' 8 | 'r' 16 | 't' 24)) {
dev_warn(pdev-dev, Wrong magic value 0x%08lx!\n, magic);
return -ENODEV;
}
--
1.8.1.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: Using readl() to read the magic value and then memcmp() to check it fails on BE, as bytes will be the other way around (by virtue of the registers to follow the endianess of the guest). Hm. Interesting. I missed the fact that readl() as a PCI operation will always assume LE values... Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. - Reading the MAGIC register byte by byte. Is that allowed? The spec only says it is 32bit wide. And the spirit of the spec was: _exactly 32bit wide_. It's just simpler to implement one access width on the host side. Paweł -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On 13/02/13 15:08, Pawel Moll wrote: On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: Using readl() to read the magic value and then memcmp() to check it fails on BE, as bytes will be the other way around (by virtue of the registers to follow the endianess of the guest). Hm. Interesting. I missed the fact that readl() as a PCI operation will always assume LE values... Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... - Reading the MAGIC register byte by byte. Is that allowed? The spec only says it is 32bit wide. And the spirit of the spec was: _exactly 32bit wide_. It's just simpler to implement one access width on the host side. I guessed as much... M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, 2013-02-13 at 15:28 +, Marc Zyngier wrote: Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. The virtio-mmio spec says so because it seemed like a good idea at the time ;-) after reading the PCI device spec. But - as I said - I missed the fact that the readl()-like accessors will always do le32_to_cpu(). Apparently ioread32() does the same (there's a separate ioread32be()). So I'm not sure that the spec is correct in this aspect any more. Maybe it should specify the registers as LE always, similarly to PCI? This problem is already covered by 2.3.1 A Note on Virtqueue Endianness in the spec... Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... Do the other registers like queuenum make sense? Could it be that the page number of the ring you're getting has wrong endianness? Paweł -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On 13/02/13 15:46, Pawel Moll wrote: On Wed, 2013-02-13 at 15:28 +, Marc Zyngier wrote: Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. The virtio-mmio spec says so because it seemed like a good idea at the time ;-) after reading the PCI device spec. But - as I said - I missed the fact that the readl()-like accessors will always do le32_to_cpu(). Apparently ioread32() does the same (there's a separate ioread32be()). Maybe. There's so much byte swapping at every possible level that my head spins... ;-) So I'm not sure that the spec is correct in this aspect any more. Maybe it should specify the registers as LE always, similarly to PCI? This problem is already covered by 2.3.1 A Note on Virtqueue Endianness in the spec... This section basically covers shared memory, and there is not much we can do about it. When it comes to the registers (that actually trap into the hypervisor), it probably makes sense to declare them as LE indeed. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... Do the other registers like queuenum make sense? Could it be that the page number of the ring you're getting has wrong endianness? The addresses are definitely wrong. kvmtool is spitting things like: Warning: unable to translate guest address 0xe8fd828f to host which tends to indicate that yes, page numbers are the other way around. Cross-endianness shared memory fun. M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On Wed, Feb 13, 2013 at 03:28:52PM +, Marc Zyngier wrote: On 13/02/13 15:08, Pawel Moll wrote: On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: Using readl() to read the magic value and then memcmp() to check it fails on BE, as bytes will be the other way around (by virtue of the registers to follow the endianess of the guest). Hm. Interesting. I missed the fact that readl() as a PCI operation will always assume LE values... Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... Does it work for qemu? I know people were using virtio on BE there. - Reading the MAGIC register byte by byte. Is that allowed? The spec only says it is 32bit wide. And the spirit of the spec was: _exactly 32bit wide_. It's just simpler to implement one access width on the host side. I guessed as much... M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
On 13/02/13 16:53, Michael S. Tsirkin wrote: On Wed, Feb 13, 2013 at 03:28:52PM +, Marc Zyngier wrote: On 13/02/13 15:08, Pawel Moll wrote: On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: Using readl() to read the magic value and then memcmp() to check it fails on BE, as bytes will be the other way around (by virtue of the registers to follow the endianess of the guest). Hm. Interesting. I missed the fact that readl() as a PCI operation will always assume LE values... Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... Does it work for qemu? I know people were using virtio on BE there. No idea, haven't tried yet, and my guest platform isn't supported by QEMU. Is someone actually using BE guests on LE hosts? That would be very interesting to compare things... M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] virt_mmio: fix signature checking for BE guests
Michael S. Tsirkin m...@redhat.com writes: On Wed, Feb 13, 2013 at 03:28:52PM +, Marc Zyngier wrote: On 13/02/13 15:08, Pawel Moll wrote: On Wed, 2013-02-13 at 14:25 +, Marc Zyngier wrote: Using readl() to read the magic value and then memcmp() to check it fails on BE, as bytes will be the other way around (by virtue of the registers to follow the endianess of the guest). Hm. Interesting. I missed the fact that readl() as a PCI operation will always assume LE values... Fix it by encoding the magic as an integer instead of a string. So I'm not completely sure this is the right fix, It seems right, however... - Using __raw_readl() instead. Is that a generic enough API? ... this implies that either the spec is wrong (as it should say: the device registers are always LE, in the PCI spirit) or all readl()s co. should be replaced with __raw equivalents. Well, the spec clearly says that the registers reflect the endianess of the guest, and it makes sense: when performing the MMIO access, KVM needs to convert between host and guest endianess. Having said that, does the change make everything else work with a BE guest? (I assume we're talking about the guest being BE, right? ;-) If so it means that the host is not following the current spec and it treats all the registers as LE. Yes, I only care about a BE guest. And no, not much is actually working (kvmtool is not happy about the guest addresses it finds in the virtio-ring). Need to dive into it and understand what needs to be fixed... Does it work for qemu? I know people were using virtio on BE there. It's had no end of problems: the powerpc folk are quite upset with me. If I were doing virtio again, I'd use LE everywhere: device authors *expect* to worry about endian, and it's confusing for them when they don't. It's tempting to use LE for the PCI config space for the new layout, for example. If you want to specify virtio-mmio as LE, feel free. Cheers, Rusty. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
