All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>,
	remy.noel@blade-group.com,
	QEMU Developers <qemu-devel@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] secondary-vga: unregister vram on unplug.
Date: Tue, 7 Aug 2018 16:09:35 +0100	[thread overview]
Message-ID: <20180807150935.GL2556@work-vm> (raw)
In-Reply-To: <CAFEAcA9mgyCzR7-ANw0YvKdzz-0H50vETrr_sw5bH5D_2PUeNg@mail.gmail.com>

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 7 August 2018 at 15:57, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > * Gerd Hoffmann (kraxel@redhat.com) wrote:
> >> On Fri, Jul 20, 2018 at 10:19:48AM +0200, remy.noel@blade-group.com wrote:
> >> > From: "Remy Noel" <remy.noel@blade-group.com>
> >> >
> >> > When removing a secondary-vga device and then adding it back (or adding
> >> > an other one), qemu aborts with:
> >> >     "RAMBlock "0000:00:02.0/vga.vram" already registered, abort!".
> >> >
> >> > It is caused by the vram staying registered, preventing vga replugging.
> >>
> >> David?  Does that look ok?
> >>
> >> This balances the
> >>
> >>      vmstate_register_ram(&s->vram, s->global_vmstate ?  NULL : DEVICE(obj));
> >>
> >> call in vga_common_init().  I'm wondering whenever the manual cleanup is
> >> actually needed in case owner is not NULL?
> >
> > I can't see anyone who is calling unregister_ram or the functions it
> > calls as part of generic device cleanup, so I think it IS needed
> > to manually do it.
> >
> > Which is a bit worrying since we have vastly more register's than
> > unregister's.
> 
> Paolo suggested in an email last month that vmstate_unregister_ram()
> should simply not exist, because it doesn't actually do anything useful:
> https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg01125.html
> 
> (ie it was added in the first place because we'd ended up with
> two identically named ramblocks, but that only happened because
> a reference-counting bug meant we hadn't deleted the first one
> properly before creating the second.)
> 
> So I think that the bug reported in this thread is similar:
> the problem is not that we're not calling vmstate_unregister_ram(),
> but that when the first instance of secondary-vga is removed
> it is not correctly destroying the ramblock.

Ah yes that makes more sense; I remember there was another similar bug
where a device screwed up and didn't delete it's RAM causing similar
problems.

Dave

> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2018-08-07 15:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-20  8:19 [Qemu-devel] [PATCH] secondary-vga: unregister vram on unplug remy.noel
2018-08-07 13:19 ` Gerd Hoffmann
2018-08-07 14:57   ` Dr. David Alan Gilbert
2018-08-07 15:06     ` Peter Maydell
2018-08-07 15:09       ` Dr. David Alan Gilbert [this message]
2018-08-11 19:07         ` Remy NOEL
2018-08-12 10:19           ` Paolo Bonzini
2018-08-30 11:28             ` Gerd Hoffmann
2018-10-02 11:55               ` Remy NOEL

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180807150935.GL2556@work-vm \
    --to=dgilbert@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=remy.noel@blade-group.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.