On Fri, Jul 6, 2012 at 9:50 PM, Alexander Graf <ag...@suse.de> wrote: > > On 02.07.2012, at 07:25, zhlci...@gmail.com wrote: > >> From: Li Zhang <zhlci...@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 <b...@kernel.crashing.org> >> Signed-off-by: Li Zhang <zhlci...@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 > -- Best Regards -Li