Re: [Qemu-devel] [PATCH v3 14/15] spapr/irq: initialize the IRQ device only once

[David Gibson]

On Fri, Mar 22, 2019 at 08:07:23AM +0100, Cédric Le Goater wrote:
> On 3/22/19 3:15 AM, David Gibson wrote:
> > On Thu, Mar 21, 2019 at 03:49:13PM +0100, Cédric Le Goater wrote:
> >> Add a check to make sure that the routine initializing the emulated
> >> IRQ device is called once. We don't have much to test on the XICS
> >> side, so we introduce a 'static bool'. Not very elegant but works well
> >> enough.
> > 
> > What's the rationale for this?
> 
> The rationale for initializing the IRQ device only once is that reset
> will do the initialization when KVM support is added for the dual 
> interrupt mode. When using the emulated device, some inits can only be
> done once. I should have probably added this patch at the end of the 
> patchset. 
> 
> The rationale for the 'static bool' is pragmatism. Others solutions
> could be to  :
> 
>   1. add a new 'init' attribute under ICSState
>   2. modify spapr_register_hypercall() and spapr_rtas_register()
>      to allow multiple definitions
>   3. add a spapr_is_registered_hypercall() helper
>   4. add a device fini routine (but it doesn't work for XIVE)
> 
> I picked the most obvious and less intrusive one. solution 1 was
> my next favored one.

Hm, I see.  I _really_ dislike static locals because they're
effectively global state that's very hard to spot.  So, yeah, I'd go
with option (1) above.  It doesn't need to be exposed to the user or
migrated, so it should be very straightforward.

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