qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH v6 2/4] spapr: introduce a fixed IRQ number space


From: Cédric Le Goater
Subject: Re: [Qemu-ppc] [PATCH v6 2/4] spapr: introduce a fixed IRQ number space
Date: Wed, 1 Aug 2018 09:14:43 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

[ ... ]

>>> +typedef struct sPAPRMachineState sPAPRMachineState;
>>> +
>>
>> Old compilers (GCC < 4.6) might complain about 'redefinition of typedef' if
>> some file, say hw/ppc/spapr.c, includes both this header and "hw/ppc/xics.h".
>> We had several build breaks detected by 'make address@hidden'...
>> The correct way to address this would be to move the typedef to the
>> "qemu/typedefs.h" header.
>>
>> This being said, address@hidden vanished with commit e7b3af81597,
>> so I guess we don't support such old distros anymore, and we can live with
>> duplicate typedefs.

I have a rhel6 vm for such tests but QEMU now requires python3 and 
glib-2.40 and maybe more stuff. I am not sure one can compile QEMU 3.1
on rhel/centos 6 anymore :/


[ ... ]

>>>      /* Release previous MSIs */
>>>      if (msi) {
>>> +        if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
>>> +            spapr_irq_msi_free(spapr, msi->first_irq, msi->num);
>>> +        }
>>
>> SPAPR_MACHINE_GET_CLASS() does all the recursive type checking, and you
>> call it three times. Even if this isn't a hot path, maybe cache this in
>> an smc variable at the beginning of the function as we do pretty much
>> everywhere else. Also this would give prettier code IMHO.
> 
> I agree with Greg that this would be a nice improvement, but it can
> wait until a followup.

The sPAPR code base is very stable so it's not too much work to respin.
FYI, most of the XIVE v4 patchset still applies without a change.

Tell me if you find any other issues and I will resend.

Thanks,

C.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]