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
signature.asc
Description: PGP signature