qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: "Jason Wang" <jasowang@redhat.com>,
	"Riku Voipio" <riku.voipio@iki.fi>,
	groug@kaod.org, qemu-devel@nongnu.org, qemu-ppc@nongnu.org,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	philmd@redhat.com, "Laurent Vivier" <laurent@vivier.eu>
Subject: Re: [PATCH v2 29/33] spapr, xics, xive: Move SpaprIrq::reset hook logic into activate/deactivate
Date: Tue, 1 Oct 2019 13:07:47 +1000	[thread overview]
Message-ID: <20191001030747.GO11105@umbus.fritz.box> (raw)
In-Reply-To: <9cbf05e4-c404-589f-e03f-9808f964a762@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Mon, Sep 30, 2019 at 09:29:14PM +0200, Cédric Le Goater wrote:
> On 30/09/2019 10:25, David Gibson wrote:
> > On Mon, Sep 30, 2019 at 08:11:56AM +0200, Cédric Le Goater wrote:
> >> On 27/09/2019 07:50, David Gibson wrote:
> >>> It turns out that all the logic in the SpaprIrq::reset hooks (and some in
> >>> the SpaprIrq::post_load hooks) isn't really related to resetting the irq
> >>> backend (that's handled by the backends' own reset routines).  Rather its
> >>> about getting the backend ready to be the active interrupt controller or
> >>> stopping being the active interrupt controller - reset (and post_load) is
> >>> just the only time that changes at present.
> >>
> >> This is a 'critical' part which impacts all the migration cases: 
> >>
> >> ic-mode=xics,xive,dual + kernel_irqchip=on/off + TCG
> > 
> > Yes... and?
> 
> and it's fragile.

How so?  Explicitly having logic for when an intc becomes active or
inactive, and having a single callsite which does that and updates the
active controller seems less fragile to me.  At least compared to
having the update to the active controller (implicit in changing the
CAS vector) and the logic to get the controllers ready for being
active/inactive in totally different places and relying on the fact
they both only happen at reset for them to travel together.

> 
> >>> To make this flow clearer, move the logic into the explicit backend
> >>> activate and deactivate hooks.
> >>
> >> I don't see where the hooks are called ?
> > 
> > spapr_irq_reset()
> >   -> spapr_irq_update_active_intc()
> >     -> set_active_intc()
> >       -> activate/deactivate hooks
> > 
> > Similarly via spapr_irq_post_load().
> > 
> > I'm hoping to add one at CAS time to avoid the CAS reboot, too.
> 
> OK. I think the first 22 patches are ready, just some minor comments
> if I am correct. Could you merge them ?

I might repost first, because one of the changes Greg suggested to
error handling caused a larger than expected number of ripple on
fixes.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2019-10-01  3:44 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-27  5:49 [PATCH v2 00/33] spapr: IRQ subsystem cleanup David Gibson
2019-09-27  5:49 ` [PATCH v2 01/33] xics: Minor fixes for XICSFabric interface David Gibson
2019-09-27  7:17   ` Greg Kurz
2019-09-27  5:49 ` [PATCH v2 02/33] xics: Eliminate 'reject', 'resend' and 'eoi' class hooks David Gibson
2019-09-27  5:49 ` [PATCH v2 03/33] xics: Rename misleading ics_simple_*() functions David Gibson
2019-09-27  5:49 ` [PATCH v2 04/33] xics: Eliminate reset hook David Gibson
2019-09-27  7:19   ` Greg Kurz
2019-09-27  5:50 ` [PATCH v2 05/33] xics: Merge TYPE_ICS_BASE and TYPE_ICS_SIMPLE classes David Gibson
2019-09-27  5:50 ` [PATCH v2 06/33] xics: Create sPAPR specific ICS subtype David Gibson
2019-09-27  7:22   ` Greg Kurz
2019-09-27  5:50 ` [PATCH v2 07/33] spapr: Fold spapr_phb_lsi_qirq() into its single caller David Gibson
2019-09-27  5:50 ` [PATCH v2 08/33] spapr: Replace spapr_vio_qirq() helper with spapr_vio_irq_pulse() helper David Gibson
2019-09-27  5:50 ` [PATCH v2 09/33] spapr: Clarify and fix handling of nr_irqs David Gibson
2019-09-27  7:53   ` Greg Kurz
2019-09-27  7:58     ` David Gibson
2019-09-27  5:50 ` [PATCH v2 10/33] spapr: Eliminate nr_irqs parameter to SpaprIrq::init David Gibson
2019-09-27  7:57   ` Greg Kurz
2019-09-27  5:50 ` [PATCH v2 11/33] spapr: Fix indexing of XICS irqs David Gibson
2019-09-27  5:50 ` [PATCH v2 12/33] spapr: Simplify spapr_qirq() handling David Gibson
2019-09-27  5:50 ` [PATCH v2 13/33] spapr: Eliminate SpaprIrq:get_nodename method David Gibson
2019-09-27  5:50 ` [PATCH v2 14/33] spapr: Remove unhelpful tracepoints from spapr_irq_free_xics() David Gibson
2019-09-27  5:50 ` [PATCH v2 15/33] spapr: Handle freeing of multiple irqs in frontend only David Gibson
2019-09-27  5:50 ` [PATCH v2 16/33] spapr, xics, xive: Better use of assert()s on irq claim/free paths David Gibson
2019-09-27  5:50 ` [PATCH v2 17/33] xive: Improve irq claim/free path David Gibson
2019-09-27  8:40   ` Greg Kurz
2019-09-30  1:39     ` David Gibson
2019-09-27  5:50 ` [PATCH v2 18/33] spapr: Use less cryptic representation of which irq backends are supported David Gibson
2019-09-27  5:50 ` [PATCH v2 19/33] spapr: Eliminate SpaprIrq::init hook David Gibson
2019-09-27  5:50 ` [PATCH v2 20/33] spapr, xics, xive: Introduce SpaprInterruptController QOM interface David Gibson
2019-09-27  9:52   ` Greg Kurz
2019-09-30  5:24   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 21/33] spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController David Gibson
2019-09-27 10:16   ` Greg Kurz
2019-09-30  1:49     ` David Gibson
2019-09-30  5:28       ` Cédric Le Goater
2019-09-30  6:14         ` David Gibson
2019-09-30 10:13           ` Cédric Le Goater
2019-10-01  2:31             ` David Gibson
2019-10-01  5:43               ` Cédric Le Goater
2019-10-01  6:47                 ` David Gibson
2019-10-01  7:41                   ` Cédric Le Goater
2019-10-01  8:11                     ` David Gibson
2019-10-01 11:43                       ` Cédric Le Goater
2019-10-02  1:11                         ` David Gibson
2019-09-30  2:37     ` David Gibson
2019-09-30  5:30   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 22/33] spapr, xics, xive: Move irq claim and free " David Gibson
2019-09-27 12:16   ` Greg Kurz
2019-09-30  2:39     ` David Gibson
2019-09-30  5:36       ` Cédric Le Goater
2019-09-30  5:33   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 23/33] spapr: Formalize notion of active interrupt controller David Gibson
2019-09-27 14:16   ` Greg Kurz
2019-09-30  5:39   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 24/33] spapr, xics, xive: Move set_irq from SpaprIrq to SpaprInterruptController David Gibson
2019-09-27 14:27   ` Greg Kurz
2019-09-30  2:41     ` David Gibson
2019-09-30  7:22       ` Greg Kurz
2019-09-30  8:28         ` David Gibson
2019-09-30  5:48   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 25/33] spapr, xics, xive: Move print_info " David Gibson
2019-09-27 14:31   ` Greg Kurz
2019-09-27  5:50 ` [PATCH v2 26/33] spapr, xics, xive: Move dt_populate " David Gibson
2019-09-27 14:38   ` Greg Kurz
2019-09-30  5:51   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 27/33] spapr, xics, xive: Match signatures for XICS and XIVE KVM connect routines David Gibson
2019-09-27 14:49   ` Greg Kurz
2019-09-30  5:52   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 28/33] spapr: Remove SpaprIrq::init_kvm hook David Gibson
2019-09-27 15:04   ` Greg Kurz
2019-09-30  5:55   ` Cédric Le Goater
2019-09-27  5:50 ` [PATCH v2 29/33] spapr, xics, xive: Move SpaprIrq::reset hook logic into activate/deactivate David Gibson
2019-09-30  6:11   ` Cédric Le Goater
2019-09-30  8:25     ` David Gibson
2019-09-30 19:29       ` Cédric Le Goater
2019-10-01  3:07         ` David Gibson [this message]
2019-09-27  5:50 ` [PATCH v2 30/33] spapr, xics, xive: Move SpaprIrq::post_load hook to backends David Gibson
2019-09-27  5:50 ` [PATCH v2 31/33] spapr: Remove SpaprIrq::nr_msis David Gibson
2019-09-27 15:17   ` Greg Kurz
2019-09-27  5:50 ` [PATCH v2 32/33] spapr: Move SpaprIrq::nr_xirqs to SpaprMachineClass David Gibson
2019-09-27 15:22   ` Greg Kurz
2019-09-30  2:44     ` David Gibson
2019-09-27  5:50 ` [PATCH v2 33/33] spapr: Remove last pieces of SpaprIrq David Gibson

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=20191001030747.GO11105@umbus.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=clg@kaod.org \
    --cc=groug@kaod.org \
    --cc=jasowang@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=riku.voipio@iki.fi \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).