| Message ID | 0be04c2530ab90a35e26d34cd2fdea49fa392716.1341204647.git.zhlcindy@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 02.07.2012, at 07:25, zhlcindy@gmail.com wrote: > From: Li Zhang <zhlcindy@linux.vnet.ibm.com> > > Also instanciate the USB keyboard and mouse when that option is used > (you can still use -device to create individual devices without all > the defaults) > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> > --- > hw/spapr.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/hw/spapr.c b/hw/spapr.c > index 973de1b..3648cad 100644 > --- a/hw/spapr.c > +++ b/hw/spapr.c > @@ -45,6 +45,8 @@ > #include "kvm.h" > #include "kvm_ppc.h" > #include "pci.h" > +#include "vga-pci.h" > +#include "usb.h" > > #include "exec-memory.h" > > @@ -82,6 +84,7 @@ > #define PHANDLE_XICP 0x00001111 > > sPAPREnvironment *spapr; > +bool spapr_has_graphics; Globals are a big no-no :). Please move this into sPAPREnvironment. > > qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, > enum xics_irq_type type) > @@ -257,6 +260,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model, > _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop)))); > } > _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device))); > + _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width))); > + _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height))); > + _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth))); Are you sure we want to set these in -nographic or -vga none mode as well? > > _FDT((fdt_end_node(fdt))); > > @@ -503,7 +509,9 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, > } > } > > - spapr_populate_chosen_stdout(fdt, spapr->vio_bus); > + if (!spapr_has_graphics) { > + spapr_populate_chosen_stdout(fdt, spapr->vio_bus); > + } > > _FDT((fdt_pack(fdt))); > > @@ -556,6 +564,16 @@ static void spapr_cpu_reset(void *opaque) > cpu_reset(CPU(cpu)); > } > > +static int spapr_vga_init(PCIBus *pci_bus) > +{ > + if (std_vga_enabled) { > + pci_vga_init(pci_bus); > + } else { > + return 0; > + } > + return 1; This still silently ignores unsupported -vga options, right? Please error out properly if the user passes in -vga cirrus or xql. We need to let him know that what he selected is not available. > +} > + > /* pSeries LPAR / sPAPR hardware init */ > static void ppc_spapr_init(ram_addr_t ram_size, > const char *boot_device, > @@ -712,6 +730,11 @@ static void ppc_spapr_init(ram_addr_t ram_size, > spapr_vscsi_create(spapr->vio_bus); > } > > + /* Graphics */ > + if (spapr_vga_init(QLIST_FIRST(&spapr->phbs)->host_state.bus)) { > + spapr_has_graphics = true; > + } How about spapr_has_graphics = spapr_vga_init(...); If that gets you above 80 characters, just shove the parameter to spapr_vga_init into a variable. Alex
Am 06.07.2012 15:50, schrieb Alexander Graf: > > On 02.07.2012, at 07:25, zhlcindy@gmail.com wrote: > >> @@ -712,6 +730,11 @@ static void ppc_spapr_init(ram_addr_t ram_size, >> spapr_vscsi_create(spapr->vio_bus); >> } >> >> + /* Graphics */ >> + if (spapr_vga_init(QLIST_FIRST(&spapr->phbs)->host_state.bus)) { >> + spapr_has_graphics = true; >> + } > > How about > > spapr_has_graphics = spapr_vga_init(...); > > If that gets you above 80 characters, just shove the parameter to spapr_vga_init into a variable. Further, that expression could use PCIHostState *phb = PCI_HOST_BRIDGE(QLIST_FIRST(&spapr->phbs)); spapr_vga_init(phb->bus) once introduced through the pci_host series. :) Andreas
On Fri, Jul 6, 2012 at 9:50 PM, Alexander Graf <agraf@suse.de> wrote: > > On 02.07.2012, at 07:25, zhlcindy@gmail.com wrote: > >> From: Li Zhang <zhlcindy@linux.vnet.ibm.com> >> >> Also instanciate the USB keyboard and mouse when that option is used >> (you can still use -device to create individual devices without all >> the defaults) >> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Signed-off-by: Li Zhang <zhlcindy@linux.vnet.ibm.com> >> --- >> hw/spapr.c | 29 ++++++++++++++++++++++++++++- >> 1 file changed, 28 insertions(+), 1 deletion(-) >> >> diff --git a/hw/spapr.c b/hw/spapr.c >> index 973de1b..3648cad 100644 >> --- a/hw/spapr.c >> +++ b/hw/spapr.c >> @@ -45,6 +45,8 @@ >> #include "kvm.h" >> #include "kvm_ppc.h" >> #include "pci.h" >> +#include "vga-pci.h" >> +#include "usb.h" >> >> #include "exec-memory.h" >> >> @@ -82,6 +84,7 @@ >> #define PHANDLE_XICP 0x00001111 >> >> sPAPREnvironment *spapr; >> +bool spapr_has_graphics; > > Globals are a big no-no :). Please move this into sPAPREnvironment. > >> >> qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, >> enum xics_irq_type type) >> @@ -257,6 +260,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model, >> _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop)))); >> } >> _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device))); >> + _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width))); >> + _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height))); >> + _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth))); > > Are you sure we want to set these in -nographic or -vga none mode as well? I am not sure about this. Ben, would you please give more information about this? Thanks. > >> >> _FDT((fdt_end_node(fdt))); >> >> @@ -503,7 +509,9 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, >> } >> } >> >> - spapr_populate_chosen_stdout(fdt, spapr->vio_bus); >> + if (!spapr_has_graphics) { >> + spapr_populate_chosen_stdout(fdt, spapr->vio_bus); >> + } >> >> _FDT((fdt_pack(fdt))); >> >> @@ -556,6 +564,16 @@ static void spapr_cpu_reset(void *opaque) >> cpu_reset(CPU(cpu)); >> } >> >> +static int spapr_vga_init(PCIBus *pci_bus) >> +{ >> + if (std_vga_enabled) { >> + pci_vga_init(pci_bus); >> + } else { >> + return 0; >> + } >> + return 1; > > This still silently ignores unsupported -vga options, right? Please error out properly if the user passes in -vga cirrus or xql. We need to let him know that what he selected is not available. Yes, right. It seems that it's not good. OK, I will do that. > >> +} >> + >> /* pSeries LPAR / sPAPR hardware init */ >> static void ppc_spapr_init(ram_addr_t ram_size, >> const char *boot_device, >> @@ -712,6 +730,11 @@ static void ppc_spapr_init(ram_addr_t ram_size, >> spapr_vscsi_create(spapr->vio_bus); >> } >> >> + /* Graphics */ >> + if (spapr_vga_init(QLIST_FIRST(&spapr->phbs)->host_state.bus)) { >> + spapr_has_graphics = true; >> + } > > How about > > spapr_has_graphics = spapr_vga_init(...); > > If that gets you above 80 characters, just shove the parameter to spapr_vga_init into a variable. OK. I see. Thanks. > > > Alex >
On Sun, 2012-07-08 at 23:08 +0800, Li Zhang wrote: > > Are you sure we want to set these in -nographic or -vga none mode as > well? > > I am not sure about this. > > Ben, would you please give more information about this? Doesn't matter much. They are only useful when there is a vga adapter, they don't hurt if there isn't and it shouldn't depend on the presence of -vga since once can add an adapter with -device, so it becomes really tricky to figure out whether to expose them or not, so I made it unconditional. Cheers, Ben.
diff --git a/hw/spapr.c b/hw/spapr.c index 973de1b..3648cad 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -45,6 +45,8 @@ #include "kvm.h" #include "kvm_ppc.h" #include "pci.h" +#include "vga-pci.h" +#include "usb.h" #include "exec-memory.h" @@ -82,6 +84,7 @@ #define PHANDLE_XICP 0x00001111 sPAPREnvironment *spapr; +bool spapr_has_graphics; qemu_irq spapr_allocate_irq(uint32_t hint, uint32_t *irq_num, enum xics_irq_type type) @@ -257,6 +260,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model, _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop)))); } _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device))); + _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width))); + _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height))); + _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth))); _FDT((fdt_end_node(fdt))); @@ -503,7 +509,9 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr, } } - spapr_populate_chosen_stdout(fdt, spapr->vio_bus); + if (!spapr_has_graphics) { + spapr_populate_chosen_stdout(fdt, spapr->vio_bus); + } _FDT((fdt_pack(fdt))); @@ -556,6 +564,16 @@ static void spapr_cpu_reset(void *opaque) cpu_reset(CPU(cpu)); } +static int spapr_vga_init(PCIBus *pci_bus) +{ + if (std_vga_enabled) { + pci_vga_init(pci_bus); + } else { + return 0; + } + return 1; +} + /* pSeries LPAR / sPAPR hardware init */ static void ppc_spapr_init(ram_addr_t ram_size, const char *boot_device, @@ -712,6 +730,11 @@ static void ppc_spapr_init(ram_addr_t ram_size, spapr_vscsi_create(spapr->vio_bus); } + /* Graphics */ + if (spapr_vga_init(QLIST_FIRST(&spapr->phbs)->host_state.bus)) { + spapr_has_graphics = true; + } + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); if (machine_opts) { add_usb = qemu_opt_get_bool(machine_opts, "usb", true); @@ -720,6 +743,10 @@ static void ppc_spapr_init(ram_addr_t ram_size, if (add_usb) { pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus, -1, "pci-ohci"); + if (spapr_has_graphics) { + usbdevice_create("keyboard"); + usbdevice_create("mouse"); + } } if (rma_size < (MIN_RMA_SLOF << 20)) { fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "