[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACH
From: |
Cédric Le Goater |
Subject: |
Re: [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS() |
Date: |
Fri, 24 Aug 2018 18:43:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 08/24/2018 05:38 PM, Greg Kurz wrote:
> On Fri, 24 Aug 2018 17:30:12 +0200
> Cédric Le Goater <address@hidden> wrote:
>
>> On 08/24/2018 05:09 PM, Peter Maydell wrote:
>>> On 21 August 2018 at 05:33, David Gibson <address@hidden> wrote:
>>>> From: Cédric Le Goater <address@hidden>
>>>>
>>>> It should save us some CPU cycles as these routines perform a lot of
>>>> checks.
>>>>
>>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>>> Signed-off-by: David Gibson <address@hidden>
>>>> ---
>>>> hw/ppc/spapr_pci.c | 11 ++++++-----
>>>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>>
>>> Hi; Coverity points out in CID 1395183 that there's a bug in
>>> this part of this patch:
>>>
>>>> @@ -1558,6 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev,
>>>> Error **errp)
>>>> sPAPRMachineState *spapr =
>>>> (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(),
>>>> TYPE_SPAPR_MACHINE);
>>>> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>>>
>>> This has moved the call to SPAPR_MACHINE_GET_CLASS() above
>>> the check for "is spapr NULL", which is wrong, because it
>>> will unconditionally dereference the pointer you pass to it.
>>
>
> I've sent a trivial patch to fix this.
yes. Looks good. But,
>
>> I see. This is a simple fix but the root reason for this check is
>> commit f7d6bfcdc0fe ("spapr_pci: fail gracefully with non-pseries
>> machine types").
>>
>
> Yeah, we also have one in spapr_cpu_core_realize() for the very
> same reason.
>
>> Is there a way to specify which device type can or can not be
>> plugged on a machine ?
>>
>> I suppose we cannot use :
>>
>> machine_class_allow_dynamic_sysbus_dev()
>>
>> for cold plugged devices. Or can we ? That would be better.
>>
>
> Hmm... not sure this would help. The root problem is that many
> places in spapr_pci and spapr_cpu_core assume the machine is
> sPAPR.
which is a perfectly legitimate assumption for a sPAPR only device,
same for spapr_cpu_core. I would think. Shouldn't we enforce
the restriction at the machine level instead and not at the device
level ?
I thought that was the purpose of commit 0bd1909da606 ("machine:
Replace has_dynamic_sysbus with list of allowed devices"), to
make sure machines had a predefined list of user-creatable devices.
I might be missing something.
Thanks,
C.
- [Qemu-devel] [PULL 13/26] target/ppc: simplify bcdadd/sub functions, (continued)
- [Qemu-devel] [PULL 13/26] target/ppc: simplify bcdadd/sub functions, David Gibson, 2018/08/21
- [Qemu-devel] [PULL 03/26] target/ppc: Enable fp exceptions for user-only, David Gibson, 2018/08/21
- [Qemu-devel] [PULL 06/26] target/ppc: Tidy helper_fadd, helper_fsub, David Gibson, 2018/08/21
- [Qemu-devel] [PULL 12/26] xics: don't include "target/ppc/cpu-qom.h" in "hw/ppc/xics.h", David Gibson, 2018/08/21
- [Qemu-devel] [PULL 14/26] spapr: Add a pseries-3.1 machine type, David Gibson, 2018/08/21
- [Qemu-devel] [PULL 09/26] target/ppc: Use non-arithmetic conversions for fp load/store, David Gibson, 2018/08/21
- [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), David Gibson, 2018/08/21
- Re: [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), Peter Maydell, 2018/08/24
- Re: [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), Cédric Le Goater, 2018/08/24
- Re: [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), Greg Kurz, 2018/08/24
- Re: [Qemu-devel] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(),
Cédric Le Goater <=
- Re: [Qemu-devel] [Qemu-ppc] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), Thomas Huth, 2018/08/27
- Re: [Qemu-devel] [Qemu-ppc] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), Greg Kurz, 2018/08/27
- Re: [Qemu-devel] [Qemu-ppc] [PULL 25/26] spapr_pci: factorize the use of SPAPR_MACHINE_GET_CLASS(), Greg Kurz, 2018/08/27
[Qemu-devel] [PULL 22/26] 40p: don't use legacy fw_cfg_init_mem() function, David Gibson, 2018/08/21
[Qemu-devel] [PULL 26/26] ppc: add DBCR based debugging, David Gibson, 2018/08/21
[Qemu-devel] [PULL 05/26] target/ppc: Tidy helper_fmul, David Gibson, 2018/08/21
[Qemu-devel] [PULL 18/26] hw/ppc/ppc405_uc: Convert away from old_mmio, David Gibson, 2018/08/21
[Qemu-devel] [PULL 20/26] hw/ppc: deprecate the machine type 'prep', replaced by '40p', David Gibson, 2018/08/21
[Qemu-devel] [PULL 24/26] mac_newworld: don't use legacy fw_cfg_init_mem() function, David Gibson, 2018/08/21
[Qemu-devel] [PULL 16/26] hw/ppc/prep: Remove ifdeffed-out stub of XCSR code, David Gibson, 2018/08/21