Re: [Qemu-ppc] [Qemu-devel] [PATCH 2/2] spapr: Add support for -vga option

[Alexander Graf]

On 06/28/2012 09:34 AM, Li Zhang wrote:
> On Wed, Jun 27, 2012 at 10:37 PM, Alexander Graf<address@hidden>  wrote:
> > On 27.06.2012, at 16:32, Andreas Färber wrote:
> >
> > > Am 27.06.2012 15:55, schrieb Li Zhang:
> > > > On Wed, Jun 27, 2012 at 9:47 PM, Andreas Färber<address@hidden>  wrote:
> > > > > Am 18.06.2012 11:34, schrieb Li Zhang:
> > > > > > 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 |   43 ++++++++++++++++++++++++++++++++++++++++++-
> > > > > > 1 files changed, 42 insertions(+), 1 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/spapr.c b/hw/spapr.c
> > > > > > index 8d158d7..c7b6e9d 100644
> > > > > > --- a/hw/spapr.c
> > > > > > +++ b/hw/spapr.c
> > > > > > @@ -45,6 +45,8 @@
> > > > > > #include "kvm.h"
> > > > > > #include "kvm_ppc.h"
> > > > > > #include "pci.h"
> > > > > > +#include "pc.h"
> > > > > This seems wrong for sPAPR.
> > > > pci_vga_init()  is defined in pc.h which is called in the following.
> > > >
> > > > +    } else if (std_vga_enabled) {
> > > > +       pci_vga_init(pci_bus);
> > > Then we should move the declaration to a better place instead. :)
> > >
> > > We seriously shouldn't expect pc.h to build on random targets.
> > > Not sure what the function does, maybe it can be avoided by QOM? Alex?
> > hw/vga-pci.c:DeviceState *pci_vga_init(PCIBus *bus)
> >
> > So why not just extract the definition out into vga-pci.h and include that 
> > instead?
> Hi Alex,
> There is no file called vga-pci.h. Is it ok to create one new file?

Yes, please :). Just create a new patch that splits the definition out 
into a new header file, includes the new header file from pc.h and then 
include only that new header file in spapr.c.

> I saw that hw/vga-pci.c still includes pc.h.
> And all the files which called pci_vga_init()  include pc.h, such as
> ppc_newworld.c.

If you want to get bonus points, check if we can maybe replace other 
pc.h users with this new include as well :).

> > > > > > @@ -510,6 +518,30 @@ static void spapr_cpu_reset(void *opaque)
> > > > > >      cpu_reset(CPU(cpu));
> > > > > > }
> > > > > >
> > > > > > +static int spapr_vga_init(PCIBus *pci_bus)
> > > > > > +{
> > > > > > +    /* Default is nothing */
> > > > > > +#if 0 /* Enable this once we merge a SLOF which works with Cirrus */
> > > > > > +    if (cirrus_vga_enabled) {
> > > > > > +        pci_cirrus_vga_init(pci_bus);
> > > > > > +    } else
> > > > > > +#endif
> > > > > > +    if (vmsvga_enabled) {
> > > > > > +        fprintf(stderr, "Warning: vmware_vga not available,"
> > > > > > +                " using standard VGA instead\n");
> > > > > > +        pci_vga_init(pci_bus);
> > > > > > +#ifdef CONFIG_SPICE
> > > > > > +    } else if (qxl_enabled) {
> > > > > > +        pci_create_simple(pci_bus, -1, "qxl-vga");
> > > > > > +#endif
> > > > > > +    } else if (std_vga_enabled) {
> > > > > > +        pci_vga_init(pci_bus);
> > > > > > +    } else {
> > > > > > +        return 0;
> > > > > > +    }
> > > > > > +    return 1;
> > > > > > +}
> > > > > > +
> > > > > Did you test whether all those paths actually work with ppc? SPICE
> > > > > didn't support ppc host last time I checked. Does it work on x86 host?
> > > > Currently, I test -vga std, it works well.
> > > > SPICE and curris are not supported on pcc. :)
> > > Please elaborate on this: ppc host or guest? If they don't work with
> > > sPAPR ppc guests there's little point in including the code here...
> > There was a project going to get SPICE host support rolling with -M pseries. I 
> > don't think they were actually looking at QXL yet though.
> Sorry, I am not looking at QXL. :)
> Know little about SPICE.

SPICE consists roughly of 2 parts:

  1) guest interface
  2) protocol

The guest interface is essentially the QXL graphics adapter. The 
protocol is what is called SPICE. You can use the QXL device without 
using the SPICE protocol. I'm not sure if it works the other way around, 
but I'd assume so too.


Alex