Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, Jun 18, 2014 at 02:35:21PM +0200, Greg Kurz wrote: On Wed, 18 Jun 2014 18:38:14 +0800 Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote: On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). What bothers me is that real hardware can't do this. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Stefan Hi Stefan, Previous versions of this patch set had such comments: virtio data structures are defined as target endian, which assumes that's a fixed value. In fact, that actually means it's platform-specific. The OASIS virtio 1.0 spec will fix this, by making all little endian. We need to support both implementations and we want to share as much code as possible. but these lines got lost between v6 and v7... my bad... :-\ Thanks! I just wasn't mentally prepared when looking through the patch series, the comment puts it into context. Stefan pgp1uVsXg1ZwZ.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote: On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). What bothers me is that real hardware can't do this. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Stefan pgpkT4yxbIJ3Z.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, 18 Jun 2014 18:38:14 +0800 Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote: On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). What bothers me is that real hardware can't do this. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Stefan Hi Stefan, Previous versions of this patch set had such comments: virtio data structures are defined as target endian, which assumes that's a fixed value. In fact, that actually means it's platform-specific. The OASIS virtio 1.0 spec will fix this, by making all little endian. We need to support both implementations and we want to share as much code as possible. but these lines got lost between v6 and v7... my bad... :-\ I agree all of this is a hack but: - it has been on the table for nearly a year - ppc LE distros are already available - the memory accessors part makes sense for 1.0 and, speaking of 1.0, I couldn't find any clue about when QEMU would support this (Cc'ing Rusty for his input), but we (IBM and distro partners) need ppc LE support now. Cheers. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore. signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 18 June 2014 11:38, Stefan Hajnoczi stefa...@gmail.com wrote: What bothers me is that real hardware can't do this. Real hardware doesn't have endianness matches guest CPU endianness semantics, which is what the virtio spec mandates... Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Also for ARM big-endian, and indeed for any CPU with runtime configurable endianness that wants to use the kernel virtio drivers that exist in the real world rather than the theoretical future ones that might some day be written for the 1.0 virtio spec... thanks -- PMM
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, Jun 18, 2014 at 01:53:15PM +0100, Peter Maydell wrote: On 18 June 2014 11:38, Stefan Hajnoczi stefa...@gmail.com wrote: What bothers me is that real hardware can't do this. Real hardware doesn't have endianness matches guest CPU endianness semantics, which is what the virtio spec mandates... So it was buggy. We never thought anyone would do a cross endian CPU :(. We are fixing it in 1.0. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Also for ARM big-endian, and indeed for any CPU with runtime configurable endianness that wants to use the kernel virtio drivers that exist in the real world rather than the theoretical future ones that might some day be written for the 1.0 virtio spec... thanks -- PMM That's not a theoretical future. Spec will almost certainly be frozen two weeks from now. So it is almost certain that drivers will be there in 3.17. Existing distros can then simply backport the drivers - same as they would with any other new hardware. -- MST
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, 18 Jun 2014 16:42:04 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jun 18, 2014 at 01:53:15PM +0100, Peter Maydell wrote: On 18 June 2014 11:38, Stefan Hajnoczi stefa...@gmail.com wrote: What bothers me is that real hardware can't do this. Real hardware doesn't have endianness matches guest CPU endianness semantics, which is what the virtio spec mandates... So it was buggy. We never thought anyone would do a cross endian CPU :(. We are fixing it in 1.0. virtio isn't the only victim... we also have vga. The problem can pop up anywhere you rely on TARGET_WORDS_BIGENDIAN. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Also for ARM big-endian, and indeed for any CPU with runtime configurable endianness that wants to use the kernel virtio drivers that exist in the real world rather than the theoretical future ones that might some day be written for the 1.0 virtio spec... thanks -- PMM That's not a theoretical future. Spec will almost certainly be frozen two weeks from now. So it is almost certain that drivers will be there in 3.17. I don't want argue on the dates but I doubt that all legacy users will switch to 1.0 as soon as it shows up... a transition period may be needed. Existing distros can then simply backport the drivers - same as they would with any other new hardware. Are you saying that upstream QEMU should not to support the transition between legacy and 1.0 at all ? Cheers. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore.
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, Jun 18, 2014 at 04:28:04PM +0200, Greg Kurz wrote: On Wed, 18 Jun 2014 16:42:04 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jun 18, 2014 at 01:53:15PM +0100, Peter Maydell wrote: On 18 June 2014 11:38, Stefan Hajnoczi stefa...@gmail.com wrote: What bothers me is that real hardware can't do this. Real hardware doesn't have endianness matches guest CPU endianness semantics, which is what the virtio spec mandates... So it was buggy. We never thought anyone would do a cross endian CPU :(. We are fixing it in 1.0. virtio isn't the only victim... we also have vga. The problem can pop up anywhere you rely on TARGET_WORDS_BIGENDIAN. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Also for ARM big-endian, and indeed for any CPU with runtime configurable endianness that wants to use the kernel virtio drivers that exist in the real world rather than the theoretical future ones that might some day be written for the 1.0 virtio spec... thanks -- PMM That's not a theoretical future. Spec will almost certainly be frozen two weeks from now. So it is almost certain that drivers will be there in 3.17. I don't want argue on the dates but I doubt that all legacy users will switch to 1.0 as soon as it shows up... a transition period may be needed. Existing distros can then simply backport the drivers - same as they would with any other new hardware. Are you saying that upstream QEMU should not to support the transition between legacy and 1.0 at all ? Cheers. Not at all. I wouldn't invest the time in implementing this feature myself: I would just wait a bit and then work on backporting drivers instead. But you did and the patches are surprisingly clean. So I don't see a reason to keep them out of tree. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore.
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, Jun 18, 2014 at 02:35:21PM +0200, Greg Kurz wrote: On Wed, 18 Jun 2014 18:38:14 +0800 Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote: On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). What bothers me is that real hardware can't do this. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Stefan Hi Stefan, Previous versions of this patch set had such comments: virtio data structures are defined as target endian, which assumes that's a fixed value. In fact, that actually means it's platform-specific. The OASIS virtio 1.0 spec will fix this, by making all little endian. We need to support both implementations and we want to share as much code as possible. but these lines got lost between v6 and v7... my bad... :-\ I agree all of this is a hack but: - it has been on the table for nearly a year - ppc LE distros are already available - the memory accessors part makes sense for 1.0 and, speaking of 1.0, I couldn't find any clue about when QEMU would support this (Cc'ing Rusty for his input), but we (IBM and distro partners) need ppc LE support now. Cheers. QEMU 2.2 I think. One disadvantage of this work is that it removes some of the stimulus for people to work on 1.0, replacing it with hacks. Hmm. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore.
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 18.06.14 17:12, Michael S. Tsirkin wrote: On Wed, Jun 18, 2014 at 02:35:21PM +0200, Greg Kurz wrote: On Wed, 18 Jun 2014 18:38:14 +0800 Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote: On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). What bothers me is that real hardware can't do this. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Stefan Hi Stefan, Previous versions of this patch set had such comments: virtio data structures are defined as target endian, which assumes that's a fixed value. In fact, that actually means it's platform-specific. The OASIS virtio 1.0 spec will fix this, by making all little endian. We need to support both implementations and we want to share as much code as possible. but these lines got lost between v6 and v7... my bad... :-\ I agree all of this is a hack but: - it has been on the table for nearly a year - ppc LE distros are already available - the memory accessors part makes sense for 1.0 and, speaking of 1.0, I couldn't find any clue about when QEMU would support this (Cc'ing Rusty for his input), but we (IBM and distro partners) need ppc LE support now. Cheers. QEMU 2.2 I think. One disadvantage of this work is that it removes some of the stimulus for people to work on 1.0, replacing it with hacks. Hmm. If you prefer I can also apply these patches and send a pull request for them. Alex
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Wed, Jun 18, 2014 at 05:14:17PM +0200, Alexander Graf wrote: On 18.06.14 17:12, Michael S. Tsirkin wrote: On Wed, Jun 18, 2014 at 02:35:21PM +0200, Greg Kurz wrote: On Wed, 18 Jun 2014 18:38:14 +0800 Stefan Hajnoczi stefa...@gmail.com wrote: On Tue, Jun 17, 2014 at 09:40:19AM +0200, Alexander Graf wrote: On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). What bothers me is that real hardware can't do this. Given that VIRTIO 1.0 is always little-endian I guess this is just a temporary hack for ppc little-endian. Would be nice to add a comment so it's clear why this approach is being taken instead of a cleaner solution. Stefan Hi Stefan, Previous versions of this patch set had such comments: virtio data structures are defined as target endian, which assumes that's a fixed value. In fact, that actually means it's platform-specific. The OASIS virtio 1.0 spec will fix this, by making all little endian. We need to support both implementations and we want to share as much code as possible. but these lines got lost between v6 and v7... my bad... :-\ I agree all of this is a hack but: - it has been on the table for nearly a year - ppc LE distros are already available - the memory accessors part makes sense for 1.0 and, speaking of 1.0, I couldn't find any clue about when QEMU would support this (Cc'ing Rusty for his input), but we (IBM and distro partners) need ppc LE support now. Cheers. QEMU 2.2 I think. One disadvantage of this work is that it removes some of the stimulus for people to work on 1.0, replacing it with hacks. Hmm. If you prefer I can also apply these patches and send a pull request for them. Alex Not yet, please. v8 still got some comments, so we need to get v9. Then give people a bit of time to review. If everyone's happy it should be mergeable in about a week from now. Also there are Paolo's patches already in queue which will conflict, not sure it's a good time for you to step up and start merging virtio patches. I'm not nacking, chill :) I know you want these patches very much, but they aren't yet 100% ready anyway and I don't see why, meanwhile, I shouldn't mention the downsides for others to consider. -- MST
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 18 June 2014 15:28, Greg Kurz gk...@linux.vnet.ibm.com wrote: On Wed, 18 Jun 2014 16:42:04 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jun 18, 2014 at 01:53:15PM +0100, Peter Maydell wrote: On 18 June 2014 11:38, Stefan Hajnoczi stefa...@gmail.com wrote: What bothers me is that real hardware can't do this. Real hardware doesn't have endianness matches guest CPU endianness semantics, which is what the virtio spec mandates... So it was buggy. We never thought anyone would do a cross endian CPU :(. We are fixing it in 1.0. virtio isn't the only victim... we also have vga. The problem can pop up anywhere you rely on TARGET_WORDS_BIGENDIAN. No, relying on TARGET_WORDS_BIGENDIAN is fine. It's only a problem if your guest somehow assumes that messing with the CPU state changes the behaviour of devices as a random side effect. That's true for virtio. I'm pushing that it should not be true for VGA (ie that the guest should have to explicitly tell the VGA device be the other endian now). It's also not true for most average devices whose endianness is TARGET_WORDS_BIGENDIAN -- it just means they don't change behaviour, and if the guest wants to be BE on a fundamentally LE hardware device it gets to do the byte swapping... thanks -- PMM
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 18.06.14 17:35, Peter Maydell wrote: On 18 June 2014 15:28, Greg Kurz gk...@linux.vnet.ibm.com wrote: On Wed, 18 Jun 2014 16:42:04 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jun 18, 2014 at 01:53:15PM +0100, Peter Maydell wrote: On 18 June 2014 11:38, Stefan Hajnoczi stefa...@gmail.com wrote: What bothers me is that real hardware can't do this. Real hardware doesn't have endianness matches guest CPU endianness semantics, which is what the virtio spec mandates... So it was buggy. We never thought anyone would do a cross endian CPU :(. We are fixing it in 1.0. virtio isn't the only victim... we also have vga. The problem can pop up anywhere you rely on TARGET_WORDS_BIGENDIAN. No, relying on TARGET_WORDS_BIGENDIAN is fine. It's only a problem if your guest somehow assumes that messing with the CPU state changes the behaviour of devices as a random side effect. That's true for virtio. I'm pushing that it should not be true for VGA (ie that the guest should have to explicitly tell the VGA device be the other endian now). It's also not true for most average devices whose endianness is TARGET_WORDS_BIGENDIAN -- it just means they don't change behaviour, and if the guest wants to be BE on a fundamentally LE hardware device it gets to do the byte swapping... I actually agree with Greg here. We implicitly create different VGA adapters today depending on TARGET_WORDS_BIGENDIAN. The FB endianness should have been separate devices (BE / LE) or runtime configuration from the beginning. Anything that checks TARGET_WORDS_BIGENDIAN in hw/ is very likely to do something pretty wrong ;). Alex
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 18 June 2014 16:37, Alexander Graf ag...@suse.de wrote: I actually agree with Greg here. We implicitly create different VGA adapters today depending on TARGET_WORDS_BIGENDIAN. The FB endianness should have been separate devices (BE / LE) or runtime configuration from the beginning. Anything that checks TARGET_WORDS_BIGENDIAN in hw/ is very likely to do something pretty wrong ;). Directly, yes, it's a bit of a red flag. But a huge % of devices do indirectly, by marking their memory regions as DEVICE_NATIVE_ENDIAN. thanks -- PMM
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 18.06.14 17:40, Peter Maydell wrote: On 18 June 2014 16:37, Alexander Graf ag...@suse.de wrote: I actually agree with Greg here. We implicitly create different VGA adapters today depending on TARGET_WORDS_BIGENDIAN. The FB endianness should have been separate devices (BE / LE) or runtime configuration from the beginning. Anything that checks TARGET_WORDS_BIGENDIAN in hw/ is very likely to do something pretty wrong ;). Directly, yes, it's a bit of a red flag. But a huge % of devices do indirectly, by marking their memory regions as DEVICE_NATIVE_ENDIAN. That's ok and the expected ABI to devices. It's TARGET_WORDS_BIGENDIAN that's a good marker that something's wrong. Alex
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? Stefan pgpsqmJ55CG8r.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 17.06.14 09:36, Stefan Hajnoczi wrote: On Fri, Jun 13, 2014 at 01:18:00PM +0200, Greg Kurz wrote: This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) I'm surprised it's not enough for the virtio device to have an endianness field (big/little). It seems these patches make endianness depend on the CPUState through which the device is being accessed. Can you explain why it's necessary to check the CPUState? They only check CPUState at the point in time of reset, as that's the only case where we can derive the implicit endian configuration from :). After reset, endianness is a simple field in the state struct. Alex
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On Fri, 13 Jun 2014 13:56:13 +0200 Alexander Graf ag...@suse.de wrote: On 13.06.14 13:18, Greg Kurz wrote: Hi, This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) You'll find more detailed changelog in each patch. Please comment and hopefully apply. Thanks ! Reviewed-by: Alexander Graf ag...@suse.de Alex Hi, I've reworked the patch set according to Alex's last remarks. Do any of the other maintainers have pending remarks ? Should I wait or send a v9 now ? Thanks. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore.
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On (Mon) 16 Jun 2014 [17:07:01], Greg Kurz wrote: Hi, I've reworked the patch set according to Alex's last remarks. Do any of the other maintainers have pending remarks ? Should I wait or send a v9 now ? I'm yet to go through the series; I'll get to it in a day or two. Amit
Re: [Qemu-devel] [PATCH v8 00/20] virtio endian-ambivalent target
On 13.06.14 13:18, Greg Kurz wrote: Hi, This version merges the changes requested during the v7 review, remarks from ppc64 dump support review (yes, we talked about virtio there) and the work on virtio subsections migration. Also two new patches have been added: - patch #1 is a preliminary fix for virtio-serial posted by Alexander Graf - patch #9 prepares the work on the virtio_is_big_endian() helper The most significant changes are: - introduction of a new CPU method for virtio - endianness is taken from CPU that resets the device - fastpath virtio memory accessors for fixed endian targets - VMState based virtio subsections (compatibility friendly) You'll find more detailed changelog in each patch. Please comment and hopefully apply. Thanks ! Reviewed-by: Alexander Graf ag...@suse.de Alex
