[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/4] hw/misc/pvpanic: Add the MMIO interface
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/4] hw/misc/pvpanic: Add the MMIO interface |
Date: |
Thu, 18 Oct 2018 15:19:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
On 18/10/2018 15:08, Peter Maydell wrote:
> On 18 October 2018 at 14:04, Philippe Mathieu-Daudé <address@hidden> wrote:
>> Signed-off-by: Peng Hao <address@hidden>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> [PMD: Use TYPE_PVPANIC definition, split in 2 patches]
>> ---
>> Peng: I hope this is now more obvious how you could reuse the pvpanic device.
>>
>> hw/misc/pvpanic.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
>> index 4f552e1533..b81c5fa633 100644
>> --- a/hw/misc/pvpanic.c
>> +++ b/hw/misc/pvpanic.c
>> @@ -2,10 +2,12 @@
>> * QEMU simulated pvpanic device.
>> *
>> * Copyright Fujitsu, Corp. 2013
>> + * Copyright (c) 2018 ZTE Ltd.
>> *
>> * Authors:
>> * Wen Congyang <address@hidden>
>> * Hu Tao <address@hidden>
>> + * Peng Hao <address@hidden>
>> *
>> * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> * See the COPYING file in the top-level directory.
>> @@ -47,7 +49,10 @@ static void handle_event(int event)
>>
>> typedef struct PVPanicState {
>> /*< private >*/
>> - ISADevice isadev;
>> + union {
>> + ISADevice isadev;
>> + SysBusDevice busdev;
>> + };
>
> This field is the parent-type for the QOM object, so I don't
> think it makes sense for it to be a union. Any one QOM object
> should have a single distinct parent type. (There are other
> examples of "some more or less similar device has ISA and
> SysBus or PCI and SysBus variations" which should provide
> some models to copy from.
OK, I understand as I can use "Device parent_obj" instead of that union?
But then we want:
pvpanic_isa_info.instance_size =
sizeof(PVPanic_Common_State) + sizeof(ISADevice)
pvpanic_mmio_info.instance_size =
sizeof(PVPanic_Common_State) + sizeof(SysBusDevice)
So to avoid union, I have to use:
typedef struct PVPanicCommonState {
MemoryRegion mr;
uint16_t ioport;
} PVPanicCommonState;
typedef struct PVPanicISAState {
/*< private >*/
ISADevice isadev;
/*< public >*/
PVPanicCommonState common;
} PVPanicISAState;
#define PVPANIC_ISA(obj) \
OBJECT_CHECK(PVPanicISAState, (obj), TYPE_PVPANIC)
typedef struct PVPanicMMIOState {
/*< private >*/
SysBusDevice busdev;
/*< public >*/
PVPanicCommonState common;
} PVPanicMMIOState;
#define PVPANIC_MMIO(obj) \
OBJECT_CHECK(PVPanicMMIOState, (obj), TYPE_PVPANIC)
And later:
-->8--
@@ -92,3 +104,3 @@ static void pvpanic_isa_realizefn(DeviceState *dev,
Error **errp)
ISADevice *d = ISA_DEVICE(dev);
- PVPanicState *s = PVPANIC(dev);
+ PVPanicState *s = PVPANIC_ISA(dev);
FWCfgState *fw_cfg = fw_cfg_find();
@@ -125,3 +137,3 @@ static TypeInfo pvpanic_isa_info = {
.parent = TYPE_ISA_DEVICE,
- .instance_size = sizeof(PVPanicState),
+ .instance_size = sizeof(PVPanicISAState),
.instance_init = pvpanic_isa_initfn,
@@ -151,3 +163,3 @@ static void pvpanic_mmio_initfn(Object *obj)
{
- PVPanicState *s = PVPANIC(obj);
+ PVPanicState *s = PVPANIC_MMIO(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
@@ -169,3 +181,3 @@ static TypeInfo pvpanic_mmio_info = {
.parent = TYPE_SYS_BUS_DEVICE,
- .instance_size = sizeof(PVPanicState),
+ .instance_size = sizeof(PVPanicMMIOState),
.instance_init = pvpanic_mmio_initfn,
---
Am I correct?
Thanks,
Phil.