[PATCH-for-5.1 1/3] hw/ide/ahci: Use qdev gpio rather than qemu_allocate_irqs()

Philippe Mathieu-Daudé posted 3 patches 5 years, 10 months ago
Maintainers: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>
There is a newer version of this series
[PATCH-for-5.1 1/3] hw/ide/ahci: Use qdev gpio rather than qemu_allocate_irqs()
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago
Switch to using the qdev gpio API which is preferred over
qemu_allocate_irqs(). One step to eventually deprecate and
remove qemu_allocate_irqs() one day.

Patch created mechanically using spatch with this script
inspired from commit d6ef883d9d7:

  @@
  typedef qemu_irq;
  identifier irqs, handler;
  expression opaque, count, i;
  @@
  -   qemu_irq *irqs;
      ...
  -   irqs = qemu_allocate_irqs(handler, opaque, count);
  +   qdev_init_gpio_in(DEVICE(opaque), handler, count);
      <+...
  -   irqs[i]
  +   qdev_get_gpio_in(DEVICE(opaque), i)
      ...+>
  ?-  g_free(irqs);

Inspired-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ide/ahci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 13d91e109a..ef0a0a22ee 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1534,19 +1534,18 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
 
 void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
 {
-    qemu_irq *irqs;
     int i;
 
     s->as = as;
     s->ports = ports;
     s->dev = g_new0(AHCIDevice, ports);
     ahci_reg_init(s);
-    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
+    qdev_init_gpio_in(DEVICE(s), ahci_irq_set, s->ports);
     for (i = 0; i < s->ports; i++) {
         AHCIDevice *ad = &s->dev[i];
 
         ide_bus_new(&ad->port, sizeof(ad->port), qdev, i, 1);
-        ide_init2(&ad->port, irqs[i]);
+        ide_init2(&ad->port, qdev_get_gpio_in(DEVICE(s), i));
 
         ad->hba = s;
         ad->port_no = i;
@@ -1554,7 +1553,6 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
         ad->port.dma->ops = &ahci_dma_ops;
         ide_register_restart_cb(&ad->port);
     }
-    g_free(irqs);
 }
 
 void ahci_uninit(AHCIState *s)
-- 
2.21.1


Re: [PATCH-for-5.1 1/3] hw/ide/ahci: Use qdev gpio rather than qemu_allocate_irqs()
Posted by Philippe Mathieu-Daudé 5 years, 10 months ago
[sending again as my previous mail was rejected, sorry if you get this
twice]

On 4/12/20 11:29 PM, Philippe Mathieu-Daudé wrote:
> Switch to using the qdev gpio API which is preferred over
> qemu_allocate_irqs(). One step to eventually deprecate and
> remove qemu_allocate_irqs() one day.
> 
> Patch created mechanically using spatch with this script
> inspired from commit d6ef883d9d7:
> 
>   @@
>   typedef qemu_irq;
>   identifier irqs, handler;
>   expression opaque, count, i;
>   @@
>   -   qemu_irq *irqs;
>       ...
>   -   irqs = qemu_allocate_irqs(handler, opaque, count);
>   +   qdev_init_gpio_in(DEVICE(opaque), handler, count);
>       <+...
>   -   irqs[i]
>   +   qdev_get_gpio_in(DEVICE(opaque), i)
>       ...+>
>   ?-  g_free(irqs);
> 
> Inspired-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/ide/ahci.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 13d91e109a..ef0a0a22ee 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1534,19 +1534,18 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
>  
>  void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>  {
> -    qemu_irq *irqs;
>      int i;
>  
>      s->as = as;
>      s->ports = ports;
>      s->dev = g_new0(AHCIDevice, ports);
>      ahci_reg_init(s);
> -    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
> +    qdev_init_gpio_in(DEVICE(s), ahci_irq_set, s->ports);

This is wrong as AHCIState is not a QOM DEVICE... see commit bb639f829f1:

---
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index c055d6ba6b..c9b3805415 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -287,6 +287,8 @@ struct AHCIDevice {
 };

 typedef struct AHCIState {
+    DeviceState *container;
+
     AHCIDevice *dev;
     AHCIControlRegs control_regs;
     MemoryRegion mem;
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 02d85fa0e9..d83efa47a4 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -121,9 +121,9 @@ static uint32_t  ahci_port_read(AHCIState *s, int
port, int offset)

 static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
 {
-    AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
-    PCIDevice *pci_dev =
-        (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
+    DeviceState *dev_state = s->container;
+    PCIDevice *pci_dev = (PCIDevice *)
object_dynamic_cast(OBJECT(dev_state),
+
TYPE_PCI_DEVICE);

     DPRINTF(0, "raise irq\n");

@@ -136,9 +136,9 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice
*dev)

 static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
 {
-    AHCIPCIState *d = container_of(s, AHCIPCIState, ahci);
-    PCIDevice *pci_dev =
-        (PCIDevice *)object_dynamic_cast(OBJECT(d), TYPE_PCI_DEVICE);
+    DeviceState *dev_state = s->container;
+    PCIDevice *pci_dev = (PCIDevice *)
object_dynamic_cast(OBJECT(dev_state),
+
TYPE_PCI_DEVICE);

     DPRINTF(0, "lower irq\n");

@@ -1436,6 +1436,7 @@ void ahci_init(AHCIState *s, DeviceState *qdev,
AddressSpace *as, int ports)
     s->as = as;
     s->ports = ports;
     s->dev = g_new0(AHCIDevice, ports);
+    s->container = qdev;
     ahci_reg_init(s);
     /* XXX BAR size should be 1k, but that breaks, so bump it to 4k for
now */
     memory_region_init_io(&s->mem, OBJECT(qdev), &ahci_mem_ops, s,
---

Using s/DEVICE(s)/qdev/ works although.

>      for (i = 0; i < s->ports; i++) {
>          AHCIDevice *ad = &s->dev[i];
>  
>          ide_bus_new(&ad->port, sizeof(ad->port), qdev, i, 1);
> -        ide_init2(&ad->port, irqs[i]);
> +        ide_init2(&ad->port, qdev_get_gpio_in(DEVICE(s), i));
>  
>          ad->hba = s;
>          ad->port_no = i;
> @@ -1554,7 +1553,6 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>          ad->port.dma->ops = &ahci_dma_ops;
>          ide_register_restart_cb(&ad->port);
>      }
> -    g_free(irqs);
>  }
>  
>  void ahci_uninit(AHCIState *s)
> 

Re: [PATCH-for-5.1 1/3] hw/ide/ahci: Use qdev gpio rather than qemu_allocate_irqs()
Posted by Alistair Francis 5 years, 10 months ago
On Sun, Apr 12, 2020 at 2:29 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Switch to using the qdev gpio API which is preferred over
> qemu_allocate_irqs(). One step to eventually deprecate and
> remove qemu_allocate_irqs() one day.
>
> Patch created mechanically using spatch with this script
> inspired from commit d6ef883d9d7:
>
>   @@
>   typedef qemu_irq;
>   identifier irqs, handler;
>   expression opaque, count, i;
>   @@
>   -   qemu_irq *irqs;
>       ...
>   -   irqs = qemu_allocate_irqs(handler, opaque, count);
>   +   qdev_init_gpio_in(DEVICE(opaque), handler, count);
>       <+...
>   -   irqs[i]
>   +   qdev_get_gpio_in(DEVICE(opaque), i)
>       ...+>
>   ?-  g_free(irqs);
>
> Inspired-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/ide/ahci.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 13d91e109a..ef0a0a22ee 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1534,19 +1534,18 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
>
>  void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>  {
> -    qemu_irq *irqs;
>      int i;
>
>      s->as = as;
>      s->ports = ports;
>      s->dev = g_new0(AHCIDevice, ports);
>      ahci_reg_init(s);
> -    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
> +    qdev_init_gpio_in(DEVICE(s), ahci_irq_set, s->ports);
>      for (i = 0; i < s->ports; i++) {
>          AHCIDevice *ad = &s->dev[i];
>
>          ide_bus_new(&ad->port, sizeof(ad->port), qdev, i, 1);
> -        ide_init2(&ad->port, irqs[i]);
> +        ide_init2(&ad->port, qdev_get_gpio_in(DEVICE(s), i));
>
>          ad->hba = s;
>          ad->port_no = i;
> @@ -1554,7 +1553,6 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>          ad->port.dma->ops = &ahci_dma_ops;
>          ide_register_restart_cb(&ad->port);
>      }
> -    g_free(irqs);
>  }
>
>  void ahci_uninit(AHCIState *s)
> --
> 2.21.1
>
>

Re: [PATCH-for-5.1 1/3] hw/ide/ahci: Use qdev gpio rather than qemu_allocate_irqs()
Posted by BALATON Zoltan 5 years, 10 months ago
On Mon, 13 Apr 2020, Alistair Francis wrote:
> On Sun, Apr 12, 2020 at 2:29 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Switch to using the qdev gpio API which is preferred over
>> qemu_allocate_irqs(). One step to eventually deprecate and
>> remove qemu_allocate_irqs() one day.

Not only that, it also fixes leaking allocated irqs if ahci can be 
plugged. (That was the reason for the original patch to fix the leak.)

Regards,
BALATON Zoltan

>> Patch created mechanically using spatch with this script
>> inspired from commit d6ef883d9d7:
>>
>>   @@
>>   typedef qemu_irq;
>>   identifier irqs, handler;
>>   expression opaque, count, i;
>>   @@
>>   -   qemu_irq *irqs;
>>       ...
>>   -   irqs = qemu_allocate_irqs(handler, opaque, count);
>>   +   qdev_init_gpio_in(DEVICE(opaque), handler, count);
>>       <+...
>>   -   irqs[i]
>>   +   qdev_get_gpio_in(DEVICE(opaque), i)
>>       ...+>
>>   ?-  g_free(irqs);
>>
>> Inspired-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
>> ---
>>  hw/ide/ahci.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 13d91e109a..ef0a0a22ee 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -1534,19 +1534,18 @@ void ahci_init(AHCIState *s, DeviceState *qdev)
>>
>>  void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>>  {
>> -    qemu_irq *irqs;
>>      int i;
>>
>>      s->as = as;
>>      s->ports = ports;
>>      s->dev = g_new0(AHCIDevice, ports);
>>      ahci_reg_init(s);
>> -    irqs = qemu_allocate_irqs(ahci_irq_set, s, s->ports);
>> +    qdev_init_gpio_in(DEVICE(s), ahci_irq_set, s->ports);
>>      for (i = 0; i < s->ports; i++) {
>>          AHCIDevice *ad = &s->dev[i];
>>
>>          ide_bus_new(&ad->port, sizeof(ad->port), qdev, i, 1);
>> -        ide_init2(&ad->port, irqs[i]);
>> +        ide_init2(&ad->port, qdev_get_gpio_in(DEVICE(s), i));
>>
>>          ad->hba = s;
>>          ad->port_no = i;
>> @@ -1554,7 +1553,6 @@ void ahci_realize(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
>>          ad->port.dma->ops = &ahci_dma_ops;
>>          ide_register_restart_cb(&ad->port);
>>      }
>> -    g_free(irqs);
>>  }
>>
>>  void ahci_uninit(AHCIState *s)
>> --
>> 2.21.1
>>
>>
>
>
Re: [PATCH-for-5.1 1/3] hw/ide/ahci: Use qdev gpio rather than qemu_allocate_irqs()
Posted by John Snow 5 years, 9 months ago

On 4/12/20 5:29 PM, Philippe Mathieu-Daudé wrote:
> Switch to using the qdev gpio API which is preferred over
> qemu_allocate_irqs(). One step to eventually deprecate and
> remove qemu_allocate_irqs() one day.
> 
> Patch created mechanically using spatch with this script
> inspired from commit d6ef883d9d7:
> 
>   @@
>   typedef qemu_irq;
>   identifier irqs, handler;
>   expression opaque, count, i;
>   @@
>   -   qemu_irq *irqs;
>       ...
>   -   irqs = qemu_allocate_irqs(handler, opaque, count);
>   +   qdev_init_gpio_in(DEVICE(opaque), handler, count);
>       <+...
>   -   irqs[i]
>   +   qdev_get_gpio_in(DEVICE(opaque), i)
>       ...+>
>   ?-  g_free(irqs);
> 
> Inspired-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: John Snow <jsnow@redhat.com>

Acked-by: John Snow <jsnow@redhat.com>