[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc][PATCH v5 4/4] spapr: Add support for -vga op
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [Qemu-ppc][PATCH v5 4/4] spapr: Add support for -vga option |
Date: |
Fri, 6 Jul 2012 15:50:03 +0200 |
On 02.07.2012, at 07:25, address@hidden wrote:
> From: Li Zhang <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Li Zhang <address@hidden>
> ---
> 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