Re: [PATCH v5 03/13] virtio-gpu: Add udmabuf helpers

From: Gerd Hoffmann <kraxel@redhat.com>
To: "Kasireddy, Vivek" <vivek.kasireddy@intel.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [PATCH v5 03/13] virtio-gpu: Add udmabuf helpers
Date: Thu, 20 May 2021 11:50:12 +0200	[thread overview]
Message-ID: <20210520095012.h3naydbrycpuv5xa@sirius.home.kraxel.org> (raw)
In-Reply-To: <957d144a53ed4549944cbb09fdffe2cb@intel.com>

On Thu, May 20, 2021 at 06:23:58AM +0000, Kasireddy, Vivek wrote:
> Hi Gerd,
>  
> > > +#ifdef CONFIG_LINUX
> > 
> > > +void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
> > 
> > > +#else
> > 
> > > +void *virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
> > > +{
> > > +    /* nothing (stub) */
> > > +    return NULL
> > > +}
> > 
> > Fails to build for !linux ...
> > 
> > You can place the stubs in a file in the stubs/ directory instead.
> > They'll be used via weak symbol references instead of #ifdefs then.
> [Kasireddy, Vivek] Will do; should I send the whole series (v6) again with this and the
> other error in patch #10 fixed or just the fixed versions of these specific patches?

New series please.

> I see that in set_scanout:
> 
> g->parent_obj.enable = 1;

Yep.  When the guest configured a scanout for the first time enable is
set.  device reset clears it btw.  And set_scanout_blob should take care
to set enable too.

> VirtIOGPUBase *g = vvga->vgpu;
>
> if (g->enable) {
>     g->hw_ops->gfx_update(g);
> } else {
>     vvga->vga.hw_ops->gfx_update(&vvga->vga);
> }
> 
> Since the parent_obj is different the above code is always going into the else part. 

No.  VirtIOGPU->parent_obj actually *is* VirtIOGPUBase.

> Is the goal here to show the content from virtio-gpu if there is a set_scanout?

Yes.  The idea is to switch into virtio-gpu mode when the guest driver
did load and successfully initialized the scanout.  Go back into vga
mode when the device get reset due to reboot (or other reasons).

> The only way we are able to get everything to work as expected is to enable our virtio-gpu
> guest driver for the VGA device instead of the virtio-gpu PCI device. But we are not sure
> if this would be OK or not. We don't run into these issues for Linux guests as we only 
> enable virtio-gpu-pci as we have -vga none.

I'd suggest to run qemu with "-vga none" or "-nodefaults" so qemu will
not automatically add a display device.  Then explicitly add the device
you want.

  "-device virtio-gpu-pci" is the pure virtio-gpu.
  "-device virtio-vga" is the virtio-gpu device with vga compatibility.

Both should work just fine with both linux and windows.  The only
difference is that you'll get boot messages with virtio-vga (thanks to
vga compat mode) whereas virtio-gpu-pci doesn't display anything until
the guest display driver is loaded.

OVMF can handle both virtio-gpu-pci and virtio-vga so you should see
the grub boot menu with both devices.  A GOP (used by efifb) is only
available with virtio-vga though.  Not sure how windows behaves here,
probably it wants a GOP too for the early boot screen.

The standard vga is "-device VGA", cirrus is "-device cirrus-vga".

HTH,
  Gerd