Re: [Qemu-devel] [PATCH 11/15] piix3: introduce register_set_irq and register_map_irq

[Blue Swirl]

On Thu, Aug 12, 2010 at 2:09 PM,  <address@hidden> wrote:
> From: Anthony PERARD <address@hidden>
>
> This patch introduces a generic function registration mechanism for
> set_irq and map_irq in piix3, so that the two calls can be
> overridden with platform specific functions whenever needed.
> The patch also implements and registers the Xen specific version of the
> functions.

I'd avoid the registration, see my comments for the other registration patch.

>
> Signed-off-by: Anthony PERARD <address@hidden>
> Signed-off-by: Stefano Stabellini <address@hidden>
> ---
>  hw/pc.h             |    3 +++
>  hw/piix_pci.c       |   17 ++++++++++++++++-
>  hw/xen_machine_fv.c |   14 ++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/hw/pc.h b/hw/pc.h
> index 63b0249..ee562cd 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -5,6 +5,7 @@
>  #include "ioport.h"
>  #include "isa.h"
>  #include "fdc.h"
> +#include "pci.h"
>
>  /* PC-style peripherals (also used by other machines).  */
>
> @@ -138,6 +139,8 @@ int pcspk_audio_init(qemu_irq *pic);
>  struct PCII440FXState;
>  typedef struct PCII440FXState PCII440FXState;
>
> +void piix3_register_set_irq(pci_set_irq_fn set_irq);
> +void piix3_register_map_irq(pci_map_irq_fn map_irq);
>  PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn, 
> qemu_irq *pic, ram_addr_t ram_size);
>  void i440fx_init_memory_mappings(PCII440FXState *d);
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index f152a0f..56e3f61 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -55,6 +55,21 @@ struct PCII440FXState {
>  #define I440FX_SMRAM    0x72
>
>  static void piix3_set_irq(void *opaque, int irq_num, int level);
> +static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
> +
> +static pci_set_irq_fn piix3_set_irq_handler = piix3_set_irq;
> +static pci_map_irq_fn piix3_map_irq_handler = pci_slot_get_pirq;
> +
> +/* Must be called before call i440fx_init() */
> +void piix3_register_set_irq(pci_set_irq_fn set_irq)
> +{
> +    piix3_set_irq_handler = set_irq;
> +}
> +
> +void piix3_register_map_irq(pci_map_irq_fn map_irq)
> +{
> +    piix3_map_irq_handler = map_irq;
> +}
>
>  /* return the global irq number corresponding to a given device irq
>    pin. We could also use the bus number to have a more precise
> @@ -235,7 +250,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix3_devfn, qemu_irq *
>     piix3 = DO_UPCAST(PIIX3State, dev,
>                       pci_create_simple_multifunction(b, -1, true, "PIIX3"));
>     piix3->pic = pic;
> -    pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4);
> +    pci_bus_irqs(b, piix3_set_irq_handler, piix3_map_irq_handler, piix3, 4);
>     (*pi440fx_state)->piix3 = piix3;
>
>     *piix3_devfn = piix3->dev.devfn;
> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
> index 58237d6..5d553b6 100644
> --- a/hw/xen_machine_fv.c
> +++ b/hw/xen_machine_fv.c
> @@ -50,6 +50,18 @@ static void xen_vm_change_state_handler(void *opaque, int 
> running, int reason)
>         xen_main_loop_prepare();
>  }
>
> +static int xen_piix3_map_irq(PCIDevice *pci_dev, int irq_num)
> +{
> +    return irq_num + ((pci_dev->devfn >> 3) << 2);
> +}
> +
> +static void xen_piix3_set_irq(void *opaque, int irq_num, int level)
> +{
> +    xc_hvm_set_pci_intx_level(xen_xc, xen_domid, 0, 0, irq_num >> 2,
> +            irq_num & 3, level);
> +}
> +
> +
>  static void xen_init_fv(ram_addr_t ram_size,
>                         const char *boot_device,
>                         const char *kernel_filename,
> @@ -126,6 +138,8 @@ static void xen_init_fv(ram_addr_t ram_size,
>
>     isa_irq = qemu_allocate_irqs(isa_irq_handler, isa_irq_state, 24);
>
> +    piix3_register_set_irq(xen_piix3_set_irq);
> +    piix3_register_map_irq(xen_piix3_map_irq);
>     pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, isa_irq, ram_size);
>     isa_bus_irqs(isa_irq);
>
> --
> 1.7.0.4
>
>
>