[PATCH] hw/nvme: Remove references to PCI IRQ "pulsing" when asserting

[PATCH] hw/nvme: Remove references to PCI IRQ "pulsing" when asserting

[julia]

The NVMe subsystem logs "pulsing IRQ pin" when it is asserting the PCI(e)
IRQ. This is confusing as it implies a short pulse, not the level-triggered
interrupts PCI(e) uses.

Also remove the pci_irq_pulse() function marked with FIXME as it is no
longer used by any calls.

Signed-off-by: julia <midnight@trainwit.ch>
---
 hw/nvme/trace-events |  2 +-
 include/hw/pci/pci.h | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index 3a67680c6a..5d96d622ff 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -1,6 +1,6 @@
 # successful events
 pci_nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
-pci_nvme_irq_pin(void) "pulsing IRQ pin"
+pci_nvme_irq_pin(void) "asserting IRQ pin"
 pci_nvme_irq_masked(void) "IRQ is masked"
 pci_nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64""
 pci_nvme_dbbuf_config(uint64_t dbs_addr, uint64_t eis_addr) "dbs_addr=0x%"PRIx64" eis_addr=0x%"PRIx64""
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index eb26cac810..863aab0b80 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -667,16 +667,6 @@ static inline void pci_irq_deassert(PCIDevice *pci_dev)
     pci_set_irq(pci_dev, 0);
 }
 
-/*
- * FIXME: PCI does not work this way.
- * All the callers to this method should be fixed.
- */
-static inline void pci_irq_pulse(PCIDevice *pci_dev)
-{
-    pci_irq_assert(pci_dev);
-    pci_irq_deassert(pci_dev);
-}
-
 MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
 void pci_set_power(PCIDevice *pci_dev, bool state);
 
-- 
2.44.1

Re: [PATCH] hw/nvme: Remove references to PCI IRQ "pulsing" when asserting

[Julia]

Actually, it seems that trace_pci_nvme_irq_pin is emitted even if the IRQ is not asserted due to a setting of the interrupt masks.  Which is weird because there's no corresponding one for deasserting. Possibly this should be reworded for 'interrupt is high (but might be masked?)', or just leave it to the standard PCI IRQ traces.

Julia

On Tue, 15 Oct 2024, at 21:33, julia wrote:
> The NVMe subsystem logs "pulsing IRQ pin" when it is asserting the PCI(e)
> IRQ. This is confusing as it implies a short pulse, not the level-triggered
> interrupts PCI(e) uses.
>
> Also remove the pci_irq_pulse() function marked with FIXME as it is no
> longer used by any calls.
>
> Signed-off-by: julia <midnight@trainwit.ch>
> ---
>  hw/nvme/trace-events |  2 +-
>  include/hw/pci/pci.h | 10 ----------
>  2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
> index 3a67680c6a..5d96d622ff 100644
> --- a/hw/nvme/trace-events
> +++ b/hw/nvme/trace-events
> @@ -1,6 +1,6 @@
>  # successful events
>  pci_nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
> -pci_nvme_irq_pin(void) "pulsing IRQ pin"
> +pci_nvme_irq_pin(void) "asserting IRQ pin"
>  pci_nvme_irq_masked(void) "IRQ is masked"
>  pci_nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, 
> prp1=0x%"PRIx64" prp2=0x%"PRIx64""
>  pci_nvme_dbbuf_config(uint64_t dbs_addr, uint64_t eis_addr) 
> "dbs_addr=0x%"PRIx64" eis_addr=0x%"PRIx64""
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index eb26cac810..863aab0b80 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -667,16 +667,6 @@ static inline void pci_irq_deassert(PCIDevice 
> *pci_dev)
>      pci_set_irq(pci_dev, 0);
>  }
> 
> -/*
> - * FIXME: PCI does not work this way.
> - * All the callers to this method should be fixed.
> - */
> -static inline void pci_irq_pulse(PCIDevice *pci_dev)
> -{
> -    pci_irq_assert(pci_dev);
> -    pci_irq_deassert(pci_dev);
> -}
> -
>  MSIMessage pci_get_msi_message(PCIDevice *dev, int vector);
>  void pci_set_power(PCIDevice *pci_dev, bool state);
> 
> -- 
> 2.44.1

Re: [PATCH] hw/nvme: Remove references to PCI IRQ "pulsing" when asserting

[Klaus Jensen]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Oct 18 15:01, Julia wrote:
> Actually, it seems that trace_pci_nvme_irq_pin is emitted even if the
> IRQ is not asserted due to a setting of the interrupt masks.  Which is
> weird because there's no corresponding one for deasserting. Possibly
> this should be reworded for 'interrupt is high (but might be
> masked?)', or just leave it to the standard PCI IRQ traces.
> 

Yeah, I have a patch in queue that does just that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]