Re: [Qemu-devel] [Qemu-ppc][PATCH v7 1/3] Add USB option in machine options

[Li Zhang <zhlcindy@gmail.com>]

On Tue, Aug 14, 2012 at 6:39 PM, Alexander Graf <agraf@suse.de> wrote:
> On 08/07/2012 04:41 AM, Li Zhang wrote:
>>
>> When -usb option is used, global varible usb_enabled is set.
>> And all the plafrom will create one USB controller according
>> to this variable. In fact, global varibles make code hard
>> to read.
>>
>> So this patch is to remove global variable usb_enabled and
>> add USB option in machine options. All the plaforms will get
>> USB option value from machine options.
>>
>> USB option of machine options will be set either by:
>>    * -usb
>>    * -machine type=pseries,usb=on
>>
>> Both these ways can work now. They both set USB option in
>> machine options. In the future, the first way will be removed.
>>
>> Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com>
>> ---
>>   hw/nseries.c      |    9 +++++++++
>>   hw/pc_piix.c      |    6 ++++++
>>   hw/ppc_newworld.c |   10 +++++++++-
>>   hw/ppc_oldworld.c |    8 ++++++++
>>   hw/ppc_prep.c     |    7 +++++++
>>   hw/pxa2xx.c       |   15 +++++++++++++++
>>   hw/realview.c     |    8 ++++++++
>>   hw/spapr.c        |   12 ++++++++++++
>>   hw/versatilepb.c  |    8 ++++++++
>>   qemu-config.c     |    4 ++++
>>   sysemu.h          |    1 -
>>   vl.c              |   29 +++++++++++++++++++++++------
>>   12 files changed, 109 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/nseries.c b/hw/nseries.c
>> index 4df2670..8e385b7 100644
>> --- a/hw/nseries.c
>> +++ b/hw/nseries.c
>> @@ -1282,6 +1282,9 @@ static void n8x0_init(ram_addr_t ram_size, const
>> char *boot_device,
>>       int sdram_size = binfo->ram_size;
>>       DisplayState *ds;
>>   +    QemuOpts *mach_opts;
>> +    bool usb_enabled = false;
>> +
>>       s->mpu = omap2420_mpu_init(sysmem, sdram_size, cpu_model);
>>         /* Setup peripherals
>> @@ -1322,6 +1325,12 @@ static void n8x0_init(ram_addr_t ram_size, const
>> char *boot_device,
>>       n8x0_dss_setup(s);
>>       n8x0_cbus_setup(s);
>>       n8x0_uart_setup(s);
>> +
>> +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", false);
>> +    }
>> +
>>       if (usb_enabled)
>>           n8x0_usb_setup(s);
>>   diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index 0c0096f..0bf25d0 100644
>> --- a/hw/pc_piix.c
>> +++ b/hw/pc_piix.c
>> @@ -148,6 +148,8 @@ static void pc_init1(MemoryRegion *system_memory,
>>       MemoryRegion *pci_memory;
>>       MemoryRegion *rom_memory;
>>       void *fw_cfg = NULL;
>> +    QemuOpts *mach_opts;
>> +    bool usb_enabled = false;
>>         pc_cpus_init(cpu_model);
>>   @@ -267,6 +269,10 @@ static void pc_init1(MemoryRegion *system_memory,
>>       pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
>>                    floppy, idebus[0], idebus[1], rtc_state);
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", false);
>> +    }
>>       if (pci_enabled && usb_enabled) {
>>           pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>>       }
>> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
>> index 4e2a6e6..1a324eb 100644
>> --- a/hw/ppc_newworld.c
>> +++ b/hw/ppc_newworld.c
>> @@ -159,6 +159,9 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>         linux_boot = (kernel_filename != NULL);
>>   +    QemuOpts *mach_opts;
>> +    bool usb_enabled = false;
>> +
>>       /* init CPUs */
>>       if (cpu_model == NULL)
>>   #ifdef TARGET_PPC64
>> @@ -350,7 +353,7 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>         /* cuda also initialize ADB */
>>       if (machine_arch == ARCH_MAC99_U3) {
>> -        usb_enabled = 1;
>> +        usb_enabled = true;
>>       }
>>       cuda_init(&cuda_mem, pic[0x19]);
>>   @@ -360,6 +363,11 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>       macio_init(pci_bus, PCI_DEVICE_ID_APPLE_UNI_N_KEYL, 0, pic_mem,
>>                  dbdma_mem, cuda_mem, NULL, 3, ide_mem, escc_bar);
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", true);
>> +    }
>> +
>>       if (usb_enabled) {
>>           pci_create_simple(pci_bus, -1, "pci-ohci");
>>       }
>> diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
>> index f2c6908..da05705 100644
>> --- a/hw/ppc_oldworld.c
>> +++ b/hw/ppc_oldworld.c
>> @@ -101,6 +101,9 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>>         linux_boot = (kernel_filename != NULL);
>>   +    QemuOpts *mach_opts;
>> +    bool usb_enabled = true;
>> +
>>       /* init CPUs */
>>       if (cpu_model == NULL)
>>           cpu_model = "G3";
>> @@ -286,6 +289,11 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>>       macio_init(pci_bus, PCI_DEVICE_ID_APPLE_343S1201, 1, pic_mem,
>>                  dbdma_mem, cuda_mem, nvr, 2, ide_mem, escc_bar);
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", true);
>> +    }
>> +
>>       if (usb_enabled) {
>>           pci_create_simple(pci_bus, -1, "pci-ohci");
>>       }
>> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
>> index be2b268..dc294ae 100644
>> --- a/hw/ppc_prep.c
>> +++ b/hw/ppc_prep.c
>> @@ -484,6 +484,9 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>         linux_boot = (kernel_filename != NULL);
>>   +    QemuOpts *mach_opts;
>> +    bool usb_enabled = true;
>> +
>>       /* init CPUs */
>>       if (cpu_model == NULL)
>>           cpu_model = "602";
>> @@ -661,6 +664,10 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>       memory_region_add_subregion(sysmem, 0xFEFF0000, xcsr);
>>   #endif
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", true);
>> +    }
>>       if (usb_enabled) {
>>           pci_create_simple(pci_bus, -1, "pci-ohci");
>>       }
>> diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
>> index d5f1420..ed87797 100644
>> --- a/hw/pxa2xx.c
>> +++ b/hw/pxa2xx.c
>> @@ -2007,6 +2007,9 @@ PXA2xxState *pxa270_init(MemoryRegion
>> *address_space,
>>       PXA2xxState *s;
>>       int i;
>>       DriveInfo *dinfo;
>> +    QemuOpts *mach_opts;
>> +    bool usb_enabled = false;
>> +
>>       s = (PXA2xxState *) g_malloc0(sizeof(PXA2xxState));
>>         if (revision && strncmp(revision, "pxa27", 5)) {
>> @@ -2108,6 +2111,11 @@ PXA2xxState *pxa270_init(MemoryRegion
>> *address_space,
>>           s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi");
>>       }
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", false);
>> +    }
>> +
>>       if (usb_enabled) {
>>           sysbus_create_simple("sysbus-ohci", 0x4c000000,
>>                           qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1));
>> @@ -2144,6 +2152,8 @@ PXA2xxState *pxa255_init(MemoryRegion
>> *address_space, unsigned int sdram_size)
>>       PXA2xxState *s;
>>       int i;
>>       DriveInfo *dinfo;
>> +    QemuOpts *mach_opts;
>> +    bool usb_enabled = false;
>>         s = (PXA2xxState *) g_malloc0(sizeof(PXA2xxState));
>>   @@ -2239,6 +2249,11 @@ PXA2xxState *pxa255_init(MemoryRegion
>> *address_space, unsigned int sdram_size)
>>           s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi");
>>       }
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", false);
>> +    }
>> +
>>       if (usb_enabled) {
>>           sysbus_create_simple("sysbus-ohci", 0x4c000000,
>>                           qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1));
>> diff --git a/hw/realview.c b/hw/realview.c
>> index 19db4d0..543d5dc 100644
>> --- a/hw/realview.c
>> +++ b/hw/realview.c
>> @@ -73,6 +73,8 @@ static void realview_init(ram_addr_t ram_size,
>>       uint32_t proc_id = 0;
>>       uint32_t sys_id;
>>       ram_addr_t low_ram_size;
>> +    QemuOpts *mach_opts;
>> +    bool usb_enabled = true;
>>         switch (board_type) {
>>       case BOARD_EB:
>> @@ -227,6 +229,12 @@ static void realview_init(ram_addr_t ram_size,
>>           sysbus_connect_irq(busdev, 2, pic[50]);
>>           sysbus_connect_irq(busdev, 3, pic[51]);
>>           pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
>> +
>> +        mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +        if (mach_opts) {
>> +            usb_enabled = qemu_opt_get_bool(mach_opts, "usb", true);
>> +        }
>> +
>>           if (usb_enabled) {
>>               pci_create_simple(pci_bus, -1, "pci-ohci");
>>           }
>> diff --git a/hw/spapr.c b/hw/spapr.c
>> index 81c9343..4dc5e59 100644
>> --- a/hw/spapr.c
>> +++ b/hw/spapr.c
>> @@ -575,6 +575,8 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>       long load_limit, rtas_limit, fw_size;
>>       long pteg_shift = 17;
>>       char *filename;
>> +    QemuOpts *mach_opts;
>> +    bool usb_enabled = true;
>>         spapr = g_malloc0(sizeof(*spapr));
>>       QLIST_INIT(&spapr->phbs);
>> @@ -710,6 +712,16 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>           spapr_vscsi_create(spapr->vio_bus);
>>       }
>>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", true);
>> +    }
>> +
>> +    if (usb_enabled) {
>> +        pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus,
>> +                          -1, "pci-ohci");
>> +    }
>> +
>>       if (rma_size < (MIN_RMA_SLOF << 20)) {
>>           fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
>>                   "%ldM guest RMA (Real Mode Area memory)\n",
>> MIN_RMA_SLOF);
>> diff --git a/hw/versatilepb.c b/hw/versatilepb.c
>> index 4fd5d9b..4b2f70d 100644
>> --- a/hw/versatilepb.c
>> +++ b/hw/versatilepb.c
>> @@ -188,6 +188,8 @@ static void versatile_init(ram_addr_t ram_size,
>>       int n;
>>       int done_smc = 0;
>>       DriveInfo *dinfo;
>> +    QemuOpts *mach_opts;
>> +    bool usb_enabled = false;
>>         if (!cpu_model) {
>>           cpu_model = "arm926";
>> @@ -247,6 +249,12 @@ static void versatile_init(ram_addr_t ram_size,
>>               pci_nic_init_nofail(nd, "rtl8139", NULL);
>>           }
>>       }
>> +
>> +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", false);
>> +    }
>> +
>>       if (usb_enabled) {
>>           pci_create_simple(pci_bus, -1, "pci-ohci");
>>       }
>> diff --git a/qemu-config.c b/qemu-config.c
>> index 5c3296b..b86ee36 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -595,6 +595,10 @@ static QemuOptsList qemu_machine_opts = {
>>               .name = "dt_compatible",
>>               .type = QEMU_OPT_STRING,
>>               .help = "Overrides the \"compatible\" property of the dt
>> root node",
>> +        },{
>> +            .name = "usb",
>> +            .type = QEMU_OPT_BOOL,
>> +            .help = "Set on/off to enable/disable usb",
>>           },
>>           { /* End of list */ }
>>       },
>> diff --git a/sysemu.h b/sysemu.h
>> index 4669348..7fbf8f4 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -118,7 +118,6 @@ extern const char *keyboard_layout;
>>   extern int win2k_install_hack;
>>   extern int alt_grab;
>>   extern int ctrl_grab;
>> -extern int usb_enabled;
>>   extern int smp_cpus;
>>   extern int max_cpus;
>>   extern int cursor_hide;
>> diff --git a/vl.c b/vl.c
>> index e71cb30..77e8962 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -198,7 +198,6 @@ CharDriverState *serial_hds[MAX_SERIAL_PORTS];
>>   CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
>>   CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
>>   int win2k_install_hack = 0;
>> -int usb_enabled = 0;
>>   int singlestep = 0;
>>   int smp_cpus = 1;
>>   int max_cpus = 0;
>> @@ -1043,13 +1042,24 @@ static void smp_parse(const char *optarg)
>>   /***********************************************************/
>>   /* USB devices */
>>   +static inline bool usb_enabled(bool default_usb)
>> +{
>> +    QemuOpts *mach_opts;
>> +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        return qemu_opt_get_bool(mach_opts, "usb", default_usb);
>> +    }
>> +    return default_usb;
>> +}
>
>
> If you move this function to sysemu.h and simply do a global
> s/usb_enabled/usb_enabled(false)/ the patch will be a lot smaller and we
> will have a lot less duplicated code.
>
OK, will do that. :)
>
> Alex
>



-- 

Best Regards
-Li