On Tue, Apr 01, 2025 at 09:44:09PM +1000, Nicholas Piggin wrote:
> This requires some adjustments to callers to avoid possible behaviour
> changes for PCI devices.
> 
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> ---
>  include/hw/ipmi/ipmi.h     |  5 +++++
>  hw/acpi/ipmi.c             |  2 +-
>  hw/ipmi/isa_ipmi_bt.c      |  1 +
>  hw/ipmi/isa_ipmi_kcs.c     |  1 +
>  hw/ipmi/pci_ipmi_bt.c      | 12 ++++++++++++
>  hw/ipmi/pci_ipmi_kcs.c     | 11 +++++++++++
>  hw/smbios/smbios_type_38.c |  6 +++++-
>  7 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
> index 77a7213ed93..71c4efac8cd 100644
> --- a/include/hw/ipmi/ipmi.h
> +++ b/include/hw/ipmi/ipmi.h
> @@ -90,6 +90,11 @@ typedef struct IPMIFwInfo {
>      } memspace;
>  
>      int interrupt_number;
> +    enum {
> +        IPMI_NO_IRQ = 0,
> +        IPMI_ISA_IRQ,
> +        IPMI_PCI_IRQ,
> +    } irq;

In addition to Phillippe's comment, can you name this "irq_source" to
make it clear what it is?

Overall this patch set looks good.

-corey

>      enum {
>          IPMI_LEVEL_IRQ,
>          IPMI_EDGE_IRQ
> diff --git a/hw/acpi/ipmi.c b/hw/acpi/ipmi.c
> index a20e57d465c..c81cbd2f158 100644
> --- a/hw/acpi/ipmi.c
> +++ b/hw/acpi/ipmi.c
> @@ -55,7 +55,7 @@ static Aml *aml_ipmi_crs(IPMIFwInfo *info)
>          abort();
>      }
>  
> -    if (info->interrupt_number) {
> +    if (info->irq == IPMI_ISA_IRQ && info->interrupt_number) {
>          aml_append(crs, aml_irq_no_flags(info->interrupt_number));
>      }
>  
> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
> index a1b66d5ee82..b5556436b82 100644
> --- a/hw/ipmi/isa_ipmi_bt.c
> +++ b/hw/ipmi/isa_ipmi_bt.c
> @@ -49,6 +49,7 @@ static void isa_ipmi_bt_get_fwinfo(struct IPMIInterface 
> *ii, IPMIFwInfo *info)
>      ISAIPMIBTDevice *iib = ISA_IPMI_BT(ii);
>  
>      ipmi_bt_get_fwinfo(&iib->bt, info);
> +    info->irq = IPMI_ISA_IRQ;
>      info->interrupt_number = iib->isairq;
>      info->i2c_slave_address = iib->bt.bmc->slave_addr;
>      info->uuid = iib->uuid;
> diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
> index d9ebdd5371f..326115f51bb 100644
> --- a/hw/ipmi/isa_ipmi_kcs.c
> +++ b/hw/ipmi/isa_ipmi_kcs.c
> @@ -49,6 +49,7 @@ static void isa_ipmi_kcs_get_fwinfo(IPMIInterface *ii, 
> IPMIFwInfo *info)
>      ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(ii);
>  
>      ipmi_kcs_get_fwinfo(&iik->kcs, info);
> +    info->irq = IPMI_ISA_IRQ;
>      info->interrupt_number = iik->isairq;
>      info->uuid = iik->uuid;
>  }
> diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
> index a3b742d22c9..33ff7190ee8 100644
> --- a/hw/ipmi/pci_ipmi_bt.c
> +++ b/hw/ipmi/pci_ipmi_bt.c
> @@ -38,6 +38,17 @@ struct PCIIPMIBTDevice {
>      uint32_t uuid;
>  };
>  
> +static void pci_ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo 
> *info)
> +{
> +    PCIIPMIBTDevice *pib = PCI_IPMI_BT(ii);
> +
> +    ipmi_bt_get_fwinfo(&pib->bt, info);
> +    info->irq = IPMI_PCI_IRQ;
> +    info->interrupt_number = pci_intx(&pib->dev);
> +    info->i2c_slave_address = pib->bt.bmc->slave_addr;
> +    info->uuid = pib->uuid;
> +}
> +
>  static void pci_ipmi_raise_irq(IPMIBT *ib)
>  {
>      PCIIPMIBTDevice *pib = ib->opaque;
> @@ -125,6 +136,7 @@ static void pci_ipmi_bt_class_init(ObjectClass *oc, void 
> *data)
>  
>      iic->get_backend_data = pci_ipmi_bt_get_backend_data;
>      ipmi_bt_class_init(iic);
> +    iic->get_fwinfo = pci_ipmi_bt_get_fwinfo;
>  }
>  
>  static const TypeInfo pci_ipmi_bt_info = {
> diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
> index 05ba97ec58f..6673b2088ef 100644
> --- a/hw/ipmi/pci_ipmi_kcs.c
> +++ b/hw/ipmi/pci_ipmi_kcs.c
> @@ -38,6 +38,16 @@ struct PCIIPMIKCSDevice {
>      uint32_t uuid;
>  };
>  
> +static void pci_ipmi_bt_get_fwinfo(struct IPMIInterface *ii, IPMIFwInfo 
> *info)
> +{
> +    PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(ii);
> +
> +    ipmi_kcs_get_fwinfo(&pik->kcs, info);
> +    info->irq = IPMI_PCI_IRQ;
> +    info->interrupt_number = pci_intx(&pik->dev);
> +    info->uuid = pik->uuid;
> +}
> +
>  static void pci_ipmi_raise_irq(IPMIKCS *ik)
>  {
>      PCIIPMIKCSDevice *pik = ik->opaque;
> @@ -125,6 +135,7 @@ static void pci_ipmi_kcs_class_init(ObjectClass *oc, void 
> *data)
>  
>      iic->get_backend_data = pci_ipmi_kcs_get_backend_data;
>      ipmi_kcs_class_init(iic);
> +    iic->get_fwinfo = pci_ipmi_kcs_get_fwinfo;
>  }
>  
>  static const TypeInfo pci_ipmi_kcs_info = {
> diff --git a/hw/smbios/smbios_type_38.c b/hw/smbios/smbios_type_38.c
> index 168b886647d..2823929c258 100644
> --- a/hw/smbios/smbios_type_38.c
> +++ b/hw/smbios/smbios_type_38.c
> @@ -72,7 +72,11 @@ static void smbios_build_one_type_38(IPMIFwInfo *info)
>                       " SMBIOS, ignoring this entry.", 
> info->register_spacing);
>          return;
>      }
> -    t->interrupt_number = info->interrupt_number;
> +    if (info->irq == IPMI_ISA_IRQ) {
> +        t->interrupt_number = info->interrupt_number;
> +    } else {
> +        t->interrupt_number = 0;
> +    }
>  
>      SMBIOS_BUILD_TABLE_POST;
>  }
> -- 
> 2.47.1
> 

Reply via email to