| Message ID | 1344307320-25094-2-git-send-email-zhlcindy@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
2nd attempt, with mailing lists on cc.... This patch reminded us of another thing that we have been working on. Here is a patch that sits in my patch queue and disables usb for s390 We have not pushed that patch yet, since several libvirt versions will always specify -usb, which will then break. So are there any opinions about handling the usb option for platforms that actually dont support it? Christian - Eugene Dvurechenski (1): USB code fenced for s390 configure | 66 +++++++++++++++++++++++++++++++++++++++++------------ hmp-commands.hx | 6 +++++ hw/Makefile.objs | 5 ++- monitor.c | 4 +++ qemu-options.hx | 4 +++ sysemu.h | 2 + vl.c | 8 +++++- 7 files changed, 76 insertions(+), 19 deletions(-)
On 2012年08月07日 20:19, Christian Borntraeger wrote: > 2nd attempt, with mailing lists on cc.... > This patch reminded us of another thing that we have been working on. > Here is a patch that sits in my patch queue and disables usb for s390 > We have not pushed that patch yet, since several libvirt versions will always > specify -usb, which will then break. > > So are there any opinions about handling the usb option for platforms that > actually dont support it? My patches removed the usb_enabled global variable. So platforms can't use the global variable any more. Platforms created usb controller according to usb_enabled global variable before. Now we add USB option to machine options. For the platfrom, usb_enabled is got from machine option as the following: QemuOpts *mach_opts; bool usb_enabled = false; (If the platform needs to default value as true, it should be set as true ) mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0); if (mach_opts) { usb_enabled = qemu_opt_get_bool(mach_opts, "usb", false); } > > Christian > > - > Eugene Dvurechenski (1): > USB code fenced for s390 > > configure | 66 +++++++++++++++++++++++++++++++++++++++++------------ > hmp-commands.hx | 6 +++++ > hw/Makefile.objs | 5 ++- > monitor.c | 4 +++ > qemu-options.hx | 4 +++ > sysemu.h | 2 + > vl.c | 8 +++++- > 7 files changed, 76 insertions(+), 19 deletions(-) >
On 08/07/2012 06:19 AM, Christian Borntraeger wrote: > 2nd attempt, with mailing lists on cc.... > This patch reminded us of another thing that we have been working on. > Here is a patch that sits in my patch queue and disables usb for s390 > We have not pushed that patch yet, since several libvirt versions will always > specify -usb, which will then break. You are correct that older libvirt always specified -usb; but with the upcoming libvirt 0.10.0, we have fixed things to now allow the user to request starting without -usb. > > So are there any opinions about handling the usb option for platforms that > actually dont support it? Libvirt should probably learn which platforms don't support it and automatically omit the -usb without user intervention on those platforms, but at least we have the fallback to explicitly avoid the -usb now. Knowing which platforms support usb might be easier if there were some QMP query-* command that could be used to learn whether a particular machine type supports it. That is, since this series is adding the new syntax '-machine type=pseries,usb=off', libvirt should be able to query '-machine type=pseries,?' to see what options the given machine has, and preferably through a QMP interface rather than through the command line.
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. Alex
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/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"); > + } > + This needs to go into a separate patch. This patch is about moving the global usb_enabled variable towards a machine opt. It shouldn't modify any code outside of that scope, least of all add usb_enabled support for a new platform! Alex
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 >
On Tue, Aug 14, 2012 at 6:46 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/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"); >> + } >> + > > > This needs to go into a separate patch. This patch is about moving the > global usb_enabled variable towards a machine opt. It shouldn't modify any > code outside of that scope, least of all add usb_enabled support for a new > platform! > I see. I will redo this patch. Send out it later. :) Thanks for your comments. > > Alex >
On Tue, Aug 14, 2012 at 11:01:19PM +0800, Li Zhang wrote: > On Tue, Aug 14, 2012 at 6:46 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/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"); > >> + } > >> + > > > > > > This needs to go into a separate patch. This patch is about moving the > > global usb_enabled variable towards a machine opt. It shouldn't modify any > > code outside of that scope, least of all add usb_enabled support for a new > > platform! > > > I see. I will redo this patch. Send out it later. :) > Thanks for your comments. When you do that, please also change the default to make spapr _not_ have usb.
On 15.08.2012, at 01:10, David Gibson wrote: > On Tue, Aug 14, 2012 at 11:01:19PM +0800, Li Zhang wrote: >> On Tue, Aug 14, 2012 at 6:46 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/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"); >>>> + } >>>> + >>> >>> >>> This needs to go into a separate patch. This patch is about moving the >>> global usb_enabled variable towards a machine opt. It shouldn't modify any >>> code outside of that scope, least of all add usb_enabled support for a new >>> platform! >>> >> I see. I will redo this patch. Send out it later. :) >> Thanks for your comments. > > When you do that, please also change the default to make spapr _not_ > have usb. I thought the idea was to default to usb=on when -vga is used? Alex
On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: > > I see. I will redo this patch. Send out it later. :) > > Thanks for your comments. > > When you do that, please also change the default to make spapr _not_ > have usb. FYI, I originally asked for USB as default ... however it looks like at this stage the price (performance) is too high so either make it default to OFF, or make it default to ON if and only if VGA is also enabled. Cheers, Ben.
On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: >> > I see. I will redo this patch. Send out it later. :) >> > Thanks for your comments. >> >> When you do that, please also change the default to make spapr _not_ >> have usb. > > FYI, I originally asked for USB as default ... however it looks like at > this stage the price (performance) is too high so either make it default > to OFF, or make it default to ON if and only if VGA is also enabled. > Got it, I change the default as false, and when VGA is enabled, set USB option as true. > Cheers, > Ben. > >
On Wed, Aug 15, 2012 at 09:24:34AM +0800, Li Zhang wrote: > On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: > >> > I see. I will redo this patch. Send out it later. :) > >> > Thanks for your comments. > >> > >> When you do that, please also change the default to make spapr _not_ > >> have usb. > > > > FYI, I originally asked for USB as default ... however it looks like at > > this stage the price (performance) is too high so either make it default > > to OFF, or make it default to ON if and only if VGA is also enabled. > > > Got it, I change the default as false, and when VGA is enabled, set > USB option as true. Not quite, actually. The default should depend on VGA, but the explicit usb= option should always override that. Having VGA without USB would be unusual, but it should be possible if you specify it explicitly.
On Wed, Aug 15, 2012 at 9:47 AM, David Gibson <dwg@au1.ibm.com> wrote: > On Wed, Aug 15, 2012 at 09:24:34AM +0800, Li Zhang wrote: >> On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt >> <benh@kernel.crashing.org> wrote: >> > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: >> >> > I see. I will redo this patch. Send out it later. :) >> >> > Thanks for your comments. >> >> >> >> When you do that, please also change the default to make spapr _not_ >> >> have usb. >> > >> > FYI, I originally asked for USB as default ... however it looks like at >> > this stage the price (performance) is too high so either make it default >> > to OFF, or make it default to ON if and only if VGA is also enabled. >> > >> Got it, I change the default as false, and when VGA is enabled, set >> USB option as true. > > Not quite, actually. The default should depend on VGA, but the > explicit usb= option should always override that. Having VGA without > USB would be unusual, but it should be possible if you specify it > explicitly. > Right, explicit usb= option will override the value. I think we can set the usb option value when no usb option is specified. For example, we use -machine type=pseries without usb= option. I am considering as the following: if (vga_enabled) { set_usb_option(true) } if (usb_enabled(false)) { //If vga is enabled and "-machine type=pseries" is specified, it will get true. pci_create_simple(ohci) } > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson >
On Wed, Aug 15, 2012 at 10:50:04AM +0800, Li Zhang wrote: > On Wed, Aug 15, 2012 at 9:47 AM, David Gibson <dwg@au1.ibm.com> wrote: > > On Wed, Aug 15, 2012 at 09:24:34AM +0800, Li Zhang wrote: > >> On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt > >> <benh@kernel.crashing.org> wrote: > >> > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: > >> >> > I see. I will redo this patch. Send out it later. :) > >> >> > Thanks for your comments. > >> >> > >> >> When you do that, please also change the default to make spapr _not_ > >> >> have usb. > >> > > >> > FYI, I originally asked for USB as default ... however it looks like at > >> > this stage the price (performance) is too high so either make it default > >> > to OFF, or make it default to ON if and only if VGA is also enabled. > >> > > >> Got it, I change the default as false, and when VGA is enabled, set > >> USB option as true. > > > > Not quite, actually. The default should depend on VGA, but the > > explicit usb= option should always override that. Having VGA without > > USB would be unusual, but it should be possible if you specify it > > explicitly. > > > Right, explicit usb= option will override the value. > I think we can set the usb option value when no usb option is specified. > For example, we use -machine type=pseries without usb= option. > > I am considering as the following: > > if (vga_enabled) { > set_usb_option(true) > } No, this will override the option given on the command line. > if (usb_enabled(false)) { //If vga is enabled and "-machine > type=pseries" is specified, it will get true. > pci_create_simple(ohci) > } It's much easier than this, you just want if (usb_enabled(vga_enabled))
On Wed, Aug 15, 2012 at 10:57 AM, David Gibson <dwg@au1.ibm.com> wrote: > On Wed, Aug 15, 2012 at 10:50:04AM +0800, Li Zhang wrote: >> On Wed, Aug 15, 2012 at 9:47 AM, David Gibson <dwg@au1.ibm.com> wrote: >> > On Wed, Aug 15, 2012 at 09:24:34AM +0800, Li Zhang wrote: >> >> On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt >> >> <benh@kernel.crashing.org> wrote: >> >> > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: >> >> >> > I see. I will redo this patch. Send out it later. :) >> >> >> > Thanks for your comments. >> >> >> >> >> >> When you do that, please also change the default to make spapr _not_ >> >> >> have usb. >> >> > >> >> > FYI, I originally asked for USB as default ... however it looks like at >> >> > this stage the price (performance) is too high so either make it default >> >> > to OFF, or make it default to ON if and only if VGA is also enabled. >> >> > >> >> Got it, I change the default as false, and when VGA is enabled, set >> >> USB option as true. >> > >> > Not quite, actually. The default should depend on VGA, but the >> > explicit usb= option should always override that. Having VGA without >> > USB would be unusual, but it should be possible if you specify it >> > explicitly. >> > >> Right, explicit usb= option will override the value. >> I think we can set the usb option value when no usb option is specified. >> For example, we use -machine type=pseries without usb= option. >> >> I am considering as the following: >> >> if (vga_enabled) { >> set_usb_option(true) >> } > > No, this will override the option given on the command line. Oh, I didn't realize that. > >> if (usb_enabled(false)) { //If vga is enabled and "-machine >> type=pseries" is specified, it will get true. >> pci_create_simple(ohci) >> } > > It's much easier than this, you just want > if (usb_enabled(vga_enabled)) This is better than mine. :) It seems that this patch needs to work with VGA patch together. :) I need to modify as that. :) > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson >
On Wed, Aug 15, 2012 at 01:44:28PM +0800, Li Zhang wrote: > On Wed, Aug 15, 2012 at 10:57 AM, David Gibson <dwg@au1.ibm.com> wrote: > > On Wed, Aug 15, 2012 at 10:50:04AM +0800, Li Zhang wrote: > >> On Wed, Aug 15, 2012 at 9:47 AM, David Gibson <dwg@au1.ibm.com> wrote: > >> > On Wed, Aug 15, 2012 at 09:24:34AM +0800, Li Zhang wrote: > >> >> On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt > >> >> <benh@kernel.crashing.org> wrote: > >> >> > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: > >> >> >> > I see. I will redo this patch. Send out it later. :) > >> >> >> > Thanks for your comments. > >> >> >> > >> >> >> When you do that, please also change the default to make spapr _not_ > >> >> >> have usb. > >> >> > > >> >> > FYI, I originally asked for USB as default ... however it looks like at > >> >> > this stage the price (performance) is too high so either make it default > >> >> > to OFF, or make it default to ON if and only if VGA is also enabled. > >> >> > > >> >> Got it, I change the default as false, and when VGA is enabled, set > >> >> USB option as true. > >> > > >> > Not quite, actually. The default should depend on VGA, but the > >> > explicit usb= option should always override that. Having VGA without > >> > USB would be unusual, but it should be possible if you specify it > >> > explicitly. > >> > > >> Right, explicit usb= option will override the value. > >> I think we can set the usb option value when no usb option is specified. > >> For example, we use -machine type=pseries without usb= option. > >> > >> I am considering as the following: > >> > >> if (vga_enabled) { > >> set_usb_option(true) > >> } > > > > No, this will override the option given on the command line. > Oh, I didn't realize that. > > > >> if (usb_enabled(false)) { //If vga is enabled and "-machine > >> type=pseries" is specified, it will get true. > >> pci_create_simple(ohci) > >> } > > > > It's much easier than this, you just want > > if (usb_enabled(vga_enabled)) > This is better than mine. :) > It seems that this patch needs to work with VGA patch together. :) > I need to modify as that. :) Not necessarily. In the initial usb pach you can just make the default 'false', then just change that one line in the VGA patch.
On Wed, Aug 15, 2012 at 7:13 PM, David Gibson <dwg@au1.ibm.com> wrote: > On Wed, Aug 15, 2012 at 01:44:28PM +0800, Li Zhang wrote: >> On Wed, Aug 15, 2012 at 10:57 AM, David Gibson <dwg@au1.ibm.com> wrote: >> > On Wed, Aug 15, 2012 at 10:50:04AM +0800, Li Zhang wrote: >> >> On Wed, Aug 15, 2012 at 9:47 AM, David Gibson <dwg@au1.ibm.com> wrote: >> >> > On Wed, Aug 15, 2012 at 09:24:34AM +0800, Li Zhang wrote: >> >> >> On Wed, Aug 15, 2012 at 8:09 AM, Benjamin Herrenschmidt >> >> >> <benh@kernel.crashing.org> wrote: >> >> >> > On Wed, 2012-08-15 at 09:10 +1000, David Gibson wrote: >> >> >> >> > I see. I will redo this patch. Send out it later. :) >> >> >> >> > Thanks for your comments. >> >> >> >> >> >> >> >> When you do that, please also change the default to make spapr _not_ >> >> >> >> have usb. >> >> >> > >> >> >> > FYI, I originally asked for USB as default ... however it looks like at >> >> >> > this stage the price (performance) is too high so either make it default >> >> >> > to OFF, or make it default to ON if and only if VGA is also enabled. >> >> >> > >> >> >> Got it, I change the default as false, and when VGA is enabled, set >> >> >> USB option as true. >> >> > >> >> > Not quite, actually. The default should depend on VGA, but the >> >> > explicit usb= option should always override that. Having VGA without >> >> > USB would be unusual, but it should be possible if you specify it >> >> > explicitly. >> >> > >> >> Right, explicit usb= option will override the value. >> >> I think we can set the usb option value when no usb option is specified. >> >> For example, we use -machine type=pseries without usb= option. >> >> >> >> I am considering as the following: >> >> >> >> if (vga_enabled) { >> >> set_usb_option(true) >> >> } >> > >> > No, this will override the option given on the command line. >> Oh, I didn't realize that. >> > >> >> if (usb_enabled(false)) { //If vga is enabled and "-machine >> >> type=pseries" is specified, it will get true. >> >> pci_create_simple(ohci) >> >> } >> > >> > It's much easier than this, you just want >> > if (usb_enabled(vga_enabled)) >> This is better than mine. :) >> It seems that this patch needs to work with VGA patch together. :) >> I need to modify as that. :) > > Not necessarily. In the initial usb pach you can just make the > default 'false', then just change that one line in the VGA patch. Got it. Thanks David. :) > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson >
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; +} + static int usb_device_add(const char *devname) { const char *p; USBDevice *dev = NULL; - if (!usb_enabled) + if (!usb_enabled(false)) { return -1; + } /* drivers with .usbdevice_name entry in USBDeviceInfo */ dev = usbdevice_create(devname); @@ -1085,8 +1095,9 @@ static int usb_device_del(const char *devname) if (strstart(devname, "host:", &p)) return usb_host_device_close(p); - if (!usb_enabled) + if (!usb_enabled(false)) { return -1; + } p = strchr(devname, '.'); if (!p) @@ -2977,10 +2988,16 @@ int main(int argc, char **argv, char **envp) } break; case QEMU_OPTION_usb: - usb_enabled = 1; + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); + if (machine_opts) { + qemu_opt_set_bool(machine_opts, "usb", true); + } break; case QEMU_OPTION_usbdevice: - usb_enabled = 1; + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); + if (machine_opts) { + qemu_opt_set_bool(machine_opts, "usb", true); + } add_device_config(DEV_USB, optarg); break; case QEMU_OPTION_device: @@ -3526,7 +3543,7 @@ int main(int argc, char **argv, char **envp) current_machine = machine; /* init USB devices */ - if (usb_enabled) { + if (usb_enabled(false)) { if (foreach_device_config(DEV_USB, usb_parse) < 0) exit(1); }
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(-)