[Qemu-devel] [PATCH 0/4] cuda/mos6522 migration fixes

Mark Cave-Ayland posted 4 patches 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180607171751.11510-1-mark.cave-ayland@ilande.co.uk
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
hw/misc/macio/cuda.c         | 50 +++++++++++++++++++-------------------------
hw/misc/mos6522.c            | 26 ++++++-----------------
include/hw/misc/macio/cuda.h | 27 +++++++++++-------------
include/hw/misc/mos6522.h    |  4 +++-
4 files changed, 42 insertions(+), 65 deletions(-)
[Qemu-devel] [PATCH 0/4] cuda/mos6522 migration fixes
Posted by Mark Cave-Ayland 5 years, 10 months ago
Whilst performing a random migration test for the Mac machines I noticed
a regression (patch 1) which prevented the loadvm from completing
successfully. A big thank you to Peter and David on IRC who pointed me
in the right direction in order to fix the bug.

Once that was working I spent a bit more time analysing the migration
stream and realised that the mos6522 device state wasn't being embedded
within the CUDA device, but instead being maintained separately which is
solved by patch 2.

Patch 3 is something I noticed whilst rearranging the existing code based
upon my better understanding of QOM/qdev and ensures that the timer frequency
is always set correctly post-migration for the device and its parent class.
This leaves no remaining functionality in the mos6522 realize function and so
allows it to be removed.

Finally patch 4 was suggested by Peter on IRC whilst helping me investigate
the original migration issue, and removes the last remaining user of
VMSTATE_TIMER_PTR_TEST from the codebase.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


Mark Cave-Ayland (4):
  mos6522: fix vmstate_mos6522_timer version in vmstate_mos6522
  cuda: embed mos6522_cuda device directly rather than using QOM object
    link
  mos6522: move timer frequency initialisation to mos6522_reset
  mos6522: convert VMSTATE_TIMER_PTR_TEST to VMSTATE_TIMER_PTR

 hw/misc/macio/cuda.c         | 50 +++++++++++++++++++-------------------------
 hw/misc/mos6522.c            | 26 ++++++-----------------
 include/hw/misc/macio/cuda.h | 27 +++++++++++-------------
 include/hw/misc/mos6522.h    |  4 +++-
 4 files changed, 42 insertions(+), 65 deletions(-)

-- 
2.11.0


Re: [Qemu-devel] [PATCH 0/4] cuda/mos6522 migration fixes
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago
On 06/07/2018 02:17 PM, Mark Cave-Ayland wrote:
> Whilst performing a random migration test for the Mac machines I noticed
> a regression (patch 1) which prevented the loadvm from completing
> successfully. A big thank you to Peter and David on IRC who pointed me
> in the right direction in order to fix the bug.
> 
> Once that was working I spent a bit more time analysing the migration
> stream and realised that the mos6522 device state wasn't being embedded
> within the CUDA device, but instead being maintained separately which is
> solved by patch 2.
> 
> Patch 3 is something I noticed whilst rearranging the existing code based
> upon my better understanding of QOM/qdev and ensures that the timer frequency
> is always set correctly post-migration for the device and its parent class.
> This leaves no remaining functionality in the mos6522 realize function and so
> allows it to be removed.
> 
> Finally patch 4 was suggested by Peter on IRC whilst helping me investigate
> the original migration issue, and removes the last remaining user of
> VMSTATE_TIMER_PTR_TEST from the codebase.

You forgot patch 5 "vmstate: Remove VMSTATE_TIMER_PTR_TEST" :)

> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> 
> Mark Cave-Ayland (4):
>   mos6522: fix vmstate_mos6522_timer version in vmstate_mos6522
>   cuda: embed mos6522_cuda device directly rather than using QOM object
>     link
>   mos6522: move timer frequency initialisation to mos6522_reset
>   mos6522: convert VMSTATE_TIMER_PTR_TEST to VMSTATE_TIMER_PTR
> 
>  hw/misc/macio/cuda.c         | 50 +++++++++++++++++++-------------------------
>  hw/misc/mos6522.c            | 26 ++++++-----------------
>  include/hw/misc/macio/cuda.h | 27 +++++++++++-------------
>  include/hw/misc/mos6522.h    |  4 +++-
>  4 files changed, 42 insertions(+), 65 deletions(-)
> 

Re: [Qemu-devel] [PATCH 0/4] cuda/mos6522 migration fixes
Posted by Peter Maydell 5 years, 10 months ago
On 7 June 2018 at 20:18, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> You forgot patch 5 "vmstate: Remove VMSTATE_TIMER_PTR_TEST" :)

It's a generic macro that might have utility in future,
and it fits into a slot in the matrix of possible VMSTATE
macros, so I wouldn't bother. (It's kind of irritating that we
have an almost but not quite orthogonal set of VMSTATE
macros.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] cuda/mos6522 migration fixes
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago
On 06/07/2018 05:17 PM, Peter Maydell wrote:
> On 7 June 2018 at 20:18, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> You forgot patch 5 "vmstate: Remove VMSTATE_TIMER_PTR_TEST" :)
> 
> It's a generic macro that might have utility in future,
> and it fits into a slot in the matrix of possible VMSTATE
> macros, so I wouldn't bother. (It's kind of irritating that we
> have an almost but not quite orthogonal set of VMSTATE
> macros.)

OK!

Re: [Qemu-devel] [PATCH 0/4] cuda/mos6522 migration fixes
Posted by David Gibson 5 years, 10 months ago
On Thu, Jun 07, 2018 at 06:17:47PM +0100, Mark Cave-Ayland wrote:
> Whilst performing a random migration test for the Mac machines I noticed
> a regression (patch 1) which prevented the loadvm from completing
> successfully. A big thank you to Peter and David on IRC who pointed me
> in the right direction in order to fix the bug.
> 
> Once that was working I spent a bit more time analysing the migration
> stream and realised that the mos6522 device state wasn't being embedded
> within the CUDA device, but instead being maintained separately which is
> solved by patch 2.
> 
> Patch 3 is something I noticed whilst rearranging the existing code based
> upon my better understanding of QOM/qdev and ensures that the timer frequency
> is always set correctly post-migration for the device and its parent class.
> This leaves no remaining functionality in the mos6522 realize function and so
> allows it to be removed.
> 
> Finally patch 4 was suggested by Peter on IRC whilst helping me investigate
> the original migration issue, and removes the last remaining user of
> VMSTATE_TIMER_PTR_TEST from the codebase.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Applied to ppc-for-3.0.

> 
> 
> Mark Cave-Ayland (4):
>   mos6522: fix vmstate_mos6522_timer version in vmstate_mos6522
>   cuda: embed mos6522_cuda device directly rather than using QOM object
>     link
>   mos6522: move timer frequency initialisation to mos6522_reset
>   mos6522: convert VMSTATE_TIMER_PTR_TEST to VMSTATE_TIMER_PTR
> 
>  hw/misc/macio/cuda.c         | 50 +++++++++++++++++++-------------------------
>  hw/misc/mos6522.c            | 26 ++++++-----------------
>  include/hw/misc/macio/cuda.h | 27 +++++++++++-------------
>  include/hw/misc/mos6522.h    |  4 +++-
>  4 files changed, 42 insertions(+), 65 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson