[Intel-gfx] Ask for comments of getting guest framebuffer in igvt-g

[Zhiyuan Lv]

Hi Kevin,

On Tue, Mar 08, 2016 at 10:44:39AM +0800, Tian, Kevin wrote:
> > From: Zhiyuan Lv
> > Sent: Thursday, March 03, 2016 5:51 PM
> > 
> > Dear i915 developers,
> > 
> > Here I have one topic hoping to get your comments and suggestions.
> > Basically it is about graphics virtualization(igvt-g), for the purpose
> > of host system to get virtual machine's framebuffer. We would like to
> > hear your opinions about some design opens. Below is the
> > patch and some more detailed description. We appreciate your time
> > on that, and thanks in advance for any comments!
> > 
> > https://patchwork.freedesktop.org/patch/71852/
> > 
> > When people try igvt-g, one common question we heard is how to get
> > guest VM's framebuffer. It is for various purposes:
> > 
> >  - A compositor in host (it can be QEMU itself or other viewer
> >    applications) can use the contents to render a window in host;
> > 
> >  - Remote protocol can easily handle it to support 3D/Media
> >    accelerated VMs;
> > 
> > The specific requirements include:
> > 
> >  - Be able to map the guest framebuffer so that host CPU can read it;
> >  - Be able to export guest framebuffer through dam_buf;
> >  - Be able to direct render with guest framebuffers;
> > 
> > In order to support that, we introduced a new gem object called
> > gvtbuffer. It is a special object with guest framebuffer's pages as
> > its backing storage. Meanwhile, it could behave normally like other
> > gem objects. It can be mapped, exported and used by EGL APIs.
> > 
> > Although we say guest fb pages for gvtbuffer, the solution itself is
> > safe. Because gvtbuffer gets entries from physical GGTT which cannot
> > be accessed by guest VM directly. igvt-g device model is responsible
> > for filling physical GGTT after translating the iova from guest GGTT
> > table. Even if a malicious guest uses a bad framebuffer, the pages
> > filled in GGTT are always valid. Then when gvtbuffer tries to get some
> > entries, they are always valid address not causing hardware problems.
> > 
> > It is possible, however, that the guest VM performs page flip while
> > gvtbuffer is attached with the framebuffer, and is being used for
> > rendering. That may cause some tearing in theory. But in practice, we
> > did not see that. If that is a concern, we can consider to delay the
> > VBLANK irq injection to guest as a solution.
> > 
> > So in general, do you think it is OK to introduce the gvtbuffer gem
> > object, or there could be better way to handle it in gem framework?
> > 
> > Currently we have a new IOCTL added for the gvtbuffer, and we also
> > added some data structures to describe the framebuffer format for user
> > mode. Do you think that is fine? Thanks again!
> > 
> 
> Hi, Zhiyuan,
> 
> After reading the patchwork link, is my below understanding correct
> regarding to the key logic of this new IOCTL?
> 
> - It's similar to stolen memory, i.e. the backing storage may not be
> directly accessed by the driver (it's other VM's memory) so no
> 'page struct' is available;
> 
> - The sg_dma_address of the backing storage is retrieved directly
> from GGTT entries corresponding to the gmadr of guest framebuffer,
> (those entries are audited by GVT-g device model before programming 
> GGTT);
> 
> - Then the gem object can be pinned to either GGTT or PPGTT, upon
> request from user-level compositor;

Above are all correct.

> 
> - A notification will be sent by GVT-g device model, upon any change
> of the guest framebuffer location,  (including change of underlying GGTT 
> entry), but this notification is not implemented yet in this RFC patch;
> 
> - Upon such notification, user-level compositor is expected to destroy
> previous gem object and then recreate a new object according to the
> new information;

That's right. I did not mention this part to make current discussion specific
:-)

> 
> One additional comment here. Since this gem object implies another
> reference to the guest memory page, we need a step to call into
> GVT-g device model to claim such reference (which will then lead to a
> refcnt increment of the guest page through hypervisor specific way). 
> Today it's optional since our device model claims reference of all
> guest memory pages in a batch at boot time, which however might
> be optimized in the future to do selective claim so within device model
> we need clearly mark out all explicit references.

Noted. Thanks for the comments!

Regards,
-Zhiyuan

> 
> Thanks
> Kevin