From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoWpN-0004ep-F2 for qemu-devel@nongnu.org; Tue, 10 Jul 2012 05:33:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SoWpH-00007j-0t for qemu-devel@nongnu.org; Tue, 10 Jul 2012 05:33:53 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:51367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoWpG-00007S-Ng for qemu-devel@nongnu.org; Tue, 10 Jul 2012 05:33:46 -0400 Received: by pbbro12 with SMTP id ro12so12368pbb.4 for ; Tue, 10 Jul 2012 02:33:44 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4FFBF6F3.2090808@redhat.com> Date: Tue, 10 Jul 2012 11:33:39 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1341861429-6297-1-git-send-email-minyard@acm.org> <1341861429-6297-6-git-send-email-minyard@acm.org> In-Reply-To: <1341861429-6297-6-git-send-email-minyard@acm.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/9] IPMI: Add a PC ISA type structure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: minyard@acm.org Cc: Corey Minyard , qemu-devel@nongnu.org Il 09/07/2012 21:17, minyard@acm.org ha scritto: > + > +static Property ipmi_isa_properties[] = { > + DEFINE_PROP_HEX32("type", ISAIPMIState, type, IPMI_KCS), You can add an enum property. There is one example called LostTickPolicy in the tree. Please do not use the generic "type" name; use "interface" for example. Start with an empty enum, and let each of the two patches 6/7 add an item; same for the various switch statements on IPMI_KCS/IPMI_BT/IPMI_SMIC. (Actually if you do this the IPMI_SMIC branches will disappear, right?) > + DEFINE_PROP_HEX32("iobase", ISAIPMIState, iobase, -1), > + DEFINE_PROP_UINT32("irq", ISAIPMIState, isairq, 0), > + DEFINE_PROP_UINT8("slave_addr", ISAIPMIState, slave_addr, 0), > + DEFINE_PROP_PTR("charopts", ISAIPMIState, state.chropts), Here, you should add a "normal" chardev property so that people can use -chardev and -device to create the IPMI interface. The device can be created like this: -chardev ...,id=charipmi -device ipmi,interface=kcs,chardev=charipmi If the chardev is absent, the local interface is used instead. See docs/qdev-device-use.txt for more information. > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static const VMStateDescription vmstate_isa_ipmi = { > + .name = "isa-ipmi", > + .version_id = 3, > + .minimum_version_id = 3, > + .fields = (VMStateField []) { > + VMSTATE_STRUCT(state, ISAIPMIState, 0, vmstate_isa_ipmi, > + IPMIState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static void ipmi_isa_class_initfn(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + ISADeviceClass *ic = ISA_DEVICE_CLASS(klass); > + ic->init = ipmi_isa_initfn; > + dc->vmsd = &vmstate_isa_ipmi; > + dc->props = ipmi_isa_properties; > +} > + > +static TypeInfo ipmi_isa_info = { > + .name = "isa-ipmi", > + .parent = TYPE_ISA_DEVICE, > + .instance_size = sizeof(ISAIPMIState), > + .class_init = ipmi_isa_class_initfn, > +}; > + > +static void ipmi_register_types(void) > +{ > + type_register_static(&ipmi_isa_info); > +} > + > +type_init(ipmi_register_types) > diff --git a/hw/pc.c b/hw/pc.c > index c0acb6a..965e053 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1173,6 +1173,18 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > fd[i] = drive_get(IF_FLOPPY, 0, i); > } > *floppy = fdctrl_init_isa(isa_bus, fd); > + > + i = 0; > + if (do_local_ipmi) { > + ipmi_isa_init(isa_bus, isa_find_free_irq(isa_bus), ipmi_types[i], NULL); > + i++; > + } > + for(; i < MAX_IPMI_DEVICES; i++) { > + if (ipmi_hds[i]) { > + ipmi_isa_init(isa_bus, isa_find_free_irq(isa_bus), > + ipmi_types[i], ipmi_hds[i]); > + } > + } > } > > void pc_pci_device_init(PCIBus *pci_bus) > diff --git a/hw/pc.h b/hw/pc.h > index 33ab689..5b6d947 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -223,6 +223,24 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd) > return true; > } > > +/* IPMI */ > +static inline bool ipmi_isa_init(ISABus *bus, int irq, > + int type, QemuOpts *opts) > +{ > + ISADevice *dev; > + > + dev = isa_try_create(bus, "isa-ipmi"); > + if (!dev) { > + return false; > + } > + qdev_prop_set_uint32(&dev->qdev, "type", type); > + qdev_prop_set_uint32(&dev->qdev, "irq", irq); > + qdev_prop_set_ptr(&dev->qdev, "charopts", opts); > + if (qdev_init(&dev->qdev) < 0) { > + return false; > + }