From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45623) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3Ebf-00082H-0Y for qemu-devel@nongnu.org; Sat, 27 Jul 2013 20:13:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V3EbX-0002k4-Jr for qemu-devel@nongnu.org; Sat, 27 Jul 2013 20:13:02 -0400 Received: from cantor2.suse.de ([195.135.220.15]:48200 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V3EbX-0002jx-9t for qemu-devel@nongnu.org; Sat, 27 Jul 2013 20:12:55 -0400 Message-ID: <51F46201.2010106@suse.de> Date: Sun, 28 Jul 2013 02:12:49 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1374681580-17439-1-git-send-email-mst@redhat.com> <1374681580-17439-12-git-send-email-mst@redhat.com> <20130725093248.GA27524@redhat.com> In-Reply-To: <20130725093248.GA27524@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 11/14] piix: APIs for pc guest info List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Anthony Liguori , qemu-devel@nongnu.org, Aurelien Jarno , Gerd Hoffmann Am 25.07.2013 11:32, schrieb Michael S. Tsirkin: > This adds APIs that will be used to fill in guest info table, > implemented using QOM, to various piix components. >=20 > Signed-off-by: Michael S. Tsirkin > --- >=20 > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index c885690..2128f13 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -29,6 +29,7 @@ > #include "exec/ioport.h" > #include "hw/nvram/fw_cfg.h" > #include "exec/address-spaces.h" > +#include "hw/acpi/piix4.h" > =20 > //#define DEBUG > =20 > @@ -63,7 +64,7 @@ typedef struct CPUStatus { > uint8_t sts[PIIX4_PROC_LEN]; > } CPUStatus; > =20 > -typedef struct PIIX4PMState { > +struct PIIX4PMState { > /*< private >*/ > PCIDevice parent_obj; > /*< public >*/ > @@ -96,7 +97,7 @@ typedef struct PIIX4PMState { > =20 > CPUStatus gpe_cpu; > Notifier cpu_added_notifier; > -} PIIX4PMState; > +}; > =20 > #define TYPE_PIIX4_PM "PIIX4_PM" > =20 Here add #define PIIX4_PM(obj) OBJECT_CHECK(PIIX4PMState, obj, TYPE_PIIX4_PM) for the general public ... > @@ -458,6 +459,30 @@ static int piix4_pm_initfn(PCIDevice *dev) > return 0; > } > =20 > +PIIX4PMState *piix4_pm_find(void) > +{ > + bool ambig; > + Object *o =3D object_resolve_path_type("", "PIIX4_PM", &ambig); > + > + if (ambig || !o) { > + return NULL; > + } > + return OBJECT_CHECK(PIIX4PMState, o, "PIIX4_PM"); ... instead of open-coding it where only you use it, please. > +} > + > +void piix4_pm_get_acpi_pm_info(PIIX4PMState *s, AcpiPmInfo *info) > +{ > + info->s3_disabled =3D s->disable_s3; > + info->s4_disabled =3D s->disable_s4; > + info->s4_val =3D s->s4_val; These three are accessible as qdev/QOM properties. > + > + info->acpi_enable_cmd =3D ACPI_ENABLE; > + info->acpi_disable_cmd =3D ACPI_DISABLE; > + info->gpe0_blk =3D GPE_BASE; > + info->gpe0_blk_len =3D GPE_LEN; So here the issue is that values are constant? > + info->sci_int =3D 9; Magic number - use a define to share it with wherever it is used? > +} > + > i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, > qemu_irq sci_irq, qemu_irq smi_irq, > int kvm_enabled, FWCfgState *fw_cfg) > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index de87241..14573ab 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -965,7 +965,7 @@ void mips_malta_init(QEMUMachineInitArgs *args) > pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1); > pci_create_simple(pci_bus, piix4_devfn + 2, "piix4-usb-uhci"); > smbus =3D piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, > - isa_get_irq(NULL, 9), NULL, 0, NULL); > + isa_get_irq(NULL, 9), NULL, 0, NULL, NULL); This looks fishy. Might belong into a different patch? Implementation didn't change above. > /* TODO: Populate SPD eeprom data. */ > smbus_eeprom_init(smbus, 8, NULL, 0); > pit =3D pit_init(isa_bus, 0x40, 0, NULL); > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index 3908860..daefdfb 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -349,6 +349,14 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state= , int *piix3_devfn, > return b; > } > =20 > +PCIBus *find_i440fx(void) > +{ > + PCIHostState *s =3D OBJECT_CHECK(PCIHostState, > + object_resolve_path("/machine/i440f= x", NULL), > + TYPE_PCI_HOST_BRIDGE); Open-coded OBJECT_CHECK() - should use existing PCI_HOST_BRIDGE(...). > + return s ? s->bus : NULL; > +} Is this function really necessary? /machine/i440fx/pci.0 is a trivial addition to the path that's already being used here. You can do: PCIBus *bus =3D PCI_BUS(object_resolve_path("/machine/i440fx/pci.0")); where you actually need to access it. Andreas > + > /* PIIX3 PCI to ISA bridge */ > static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) > { > diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h > new file mode 100644 > index 0000000..2876428 > --- /dev/null > +++ b/include/hw/acpi/piix4.h > @@ -0,0 +1,10 @@ > +#ifndef HW_ACPI_PIIX4_H > +#define HW_ACPI_PIIX4_H > + > +#include "qemu/typedefs.h" > + > +PIIX4PMState *piix4_pm_find(void); > + > +void piix4_pm_get_acpi_pm_info(PIIX4PMState *, AcpiPmInfo *); > + > +#endif > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 7c0bd50..76af5cd 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -186,6 +186,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,= int *piix_devfn, > MemoryRegion *pci_memory, > MemoryRegion *ram_memory); > =20 > +PCIBus *find_i440fx(void); > /* piix4.c */ > extern PCIDevice *piix4_dev; > int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn); > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > index cb66e19..7d42693 100644 > --- a/include/qemu/typedefs.h > +++ b/include/qemu/typedefs.h > @@ -65,6 +65,7 @@ typedef struct QEMUSGList QEMUSGList; > typedef struct SHPCDevice SHPCDevice; > typedef struct FWCfgState FWCfgState; > typedef struct PcGuestInfo PcGuestInfo; > +typedef struct PIIX4PMState PIIX4PMState; > typedef struct AcpiPmInfo AcpiPmInfo; > =20 > #endif /* QEMU_TYPEDEFS_H */ >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg