Re: [Qemu-devel] [PATCH v3 10/14] ich9: APIs for pc guest info

[Andreas Färber]

Am 24.07.2013 18:02, schrieb Michael S. Tsirkin:
> This adds APIs that will be used to fill in
> guest info table, implemented using QOM,
> to various ich9 components.
> 
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  hw/acpi/ich9.c            |  6 ++++++
>  hw/isa/lpc_ich9.c         | 19 +++++++++++++++++++
>  hw/pci-host/q35.c         | 10 ++++++++++
>  include/hw/acpi/ich9.h    |  2 ++
>  include/hw/i386/ich9.h    |  3 +++
>  include/hw/pci-host/q35.h |  2 ++
>  6 files changed, 42 insertions(+)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 3fb443d..7ea55e1 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -228,3 +228,9 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>      pm->powerdown_notifier.notify = pm_powerdown_req;
>      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
>  }
> +
> +void ich9_pm_get_acpi_pm_info(ICH9LPCPMRegs *pm, AcpiPmInfo *info)
> +{
> +    info->gpe0_blk = PC_GUEST_PORT_ACPI_PM_BASE + ICH9_PMIO_GPE0_STS;
> +    info->gpe0_blk_len = ICH9_PMIO_GPE0_LEN;
> +}
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index d1921aa..12d4a23 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -562,6 +562,25 @@ static bool ich9_rst_cnt_needed(void *opaque)
>      return (lpc->rst_cnt != 0);
>  }
>  
> +ICH9LPCState *ich9_lpc_find(void)
> +{
> +    bool ambig;
> +    Object *o = object_resolve_path_type("", TYPE_ICH9_LPC_DEVICE, &ambig);
> +
> +    if (ambig) {
> +        return NULL;
> +    }
> +    return ICH9_LPC_DEVICE(o);
> +}
> +
> +void ich9_lpc_get_acpi_pm_info(ICH9LPCState *lpc, AcpiPmInfo *info)
> +{
> +    info->sci_int = 9;

Magic value.

> +    info->acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
> +    info->acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
> +    ich9_pm_get_acpi_pm_info(&lpc->pm, info);
> +}
> +
>  static const VMStateDescription vmstate_ich9_rst_cnt = {
>      .name = "ICH9LPC/rst_cnt",
>      .version_id = 1,
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 6b1b3b7..ca6f495 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -298,6 +298,16 @@ static int mch_init(PCIDevice *d)
>      return 0;
>  }
>  
> +uint64_t mch_mcfg_base(void)
> +{
> +    bool ambiguous;
> +    Object *o = object_resolve_path_type("", TYPE_MCH_PCI_DEVICE, 
> &ambiguous);

If you take the minute to add in q35 machine init
object_property_add_child(qdev_get_machine(), "q35", foo, NULL);
then not only can you use object_resolve_path("/machine/q35/mch") to
access it but everyone including qtest can.

> +    if (!o) {
> +        return 0;
> +    }
> +    return MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;

Another value constant for the device. Taking a wild guess: Is this an
offset of a memory subregion where long-term Anthony's proposed
MemoryRegion QOM'ification would allow us to get rid of this helper?

Anyway, I would like it much better as:

uint64_t mch_mcfg_base()
{
    return MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
}

with the path resolution stuff, i.e. check for necessity, happening in
ACPI code, if we don't get around these helpers for now.

Andreas

> +}
> +
>  static void mch_class_init(ObjectClass *klass, void *data)
>  {
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index b1fe71f..f5e8a88 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -49,4 +49,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>  void ich9_pm_iospace_update(ICH9LPCPMRegs *pm, uint32_t pm_io_base);
>  extern const VMStateDescription vmstate_ich9_pm;
>  
> +void ich9_pm_get_acpi_pm_info(ICH9LPCPMRegs *, AcpiPmInfo *);
> +
>  #endif /* HW_ACPI_ICH9_H */
> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
> index c5f637b..6528dc0 100644
> --- a/include/hw/i386/ich9.h
> +++ b/include/hw/i386/ich9.h
> @@ -66,6 +66,9 @@ typedef struct ICH9LPCState {
>      qemu_irq *ioapic;
>  } ICH9LPCState;
>  
> +ICH9LPCState *ich9_lpc_find(void);
> +void ich9_lpc_get_acpi_pm_info(ICH9LPCState *, AcpiPmInfo *);
> +
>  #define Q35_MASK(bit, ms_bit, ls_bit) \
>  ((uint##bit##_t)(((1ULL << ((ms_bit) + 1)) - 1) & ~((1ULL << ls_bit) - 1)))
>  
> diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index 3cb631e..6337dcf 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -154,4 +154,6 @@ typedef struct Q35PCIHost {
>  #define MCH_PCIE_DEV                           1
>  #define MCH_PCIE_FUNC                          0
>  
> +uint64_t mch_mcfg_base(void);
> +
>  #endif /* HW_Q35_H */
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg