2010-11-01 11:53:52

by Alon Levy

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport


----- "Anthony Liguori" <[email protected]> wrote:

> On 10/29/2010 06:18 AM, Rusty Russell wrote:
> >> Fixed - updated patch tested and attached.
> >>
> > OK. FWIW, I think this is an awesome idea.
>
> Paravirtual OpenGL or the actual proposed implementation? Have you
> looked at the actual code?
>
> If I understand correctly, the implementation is an RPC of opengl
> commands across virtio that are then rendered on the host to an
> offscreen buffer. The buffer is then sent back to the guest in
> rendered
> form.
>
> But it's possible to mess around with other guests because the opengl
>
> commands are not scrubbed. There have been other proposals in the past
>
> that include a mini JIT that would translate opengl commands into a
> safe
> form.
>
> Exposing just an opengl RPC transport doesn't seem that useful either.
>
> It ought to be part of a VGA card in order for it to be integrated
> with
> the rest of the graphics system without a round trip.
>
> The alternative proposal is Spice which so far noone has mentioned.
> Right now, Spice seems to be taking the right approach to guest 3d
> support.
>

While we (speaking as part of the SPICE developers) want to have the same
support in our virtual GPU for 3d as we have for 2d, we just don't at this
point of time.

> Regards,
>
> Anthony Liguori
>
> > I understand others are skeptical,
> > but this seems simple and if it works and you're happy to maintain
> it I'm
> > happy to let you do it :)
> >
> > Cheers,
> > Rusty.
> >
> >


2010-11-01 13:28:54

by Anthony Liguori

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 11/01/2010 06:53 AM, Alon Levy wrote:
>> The alternative proposal is Spice which so far noone has mentioned.
>> Right now, Spice seems to be taking the right approach to guest 3d
>> support.
>>
>>
> While we (speaking as part of the SPICE developers) want to have the same
> support in our virtual GPU for 3d as we have for 2d, we just don't at this
> point of time.
>

Yes, but I think the point is that are two general approaches to
supporting 3d that are being proposed. One approach is to an RPC layer
at the OpenGL level which essentially passes through the host OpenGL
stack. That's what virtio-gl is. This has existed for quite a while
and there are multiple transports for it. It supports serial ports, TCP
sockets, a custom ISA extension for x86, and now a custom virtio transport.

Another approach would be to have a virtual GPU and to implement
GPU-level commands for 3d. I have been repeated told that much of the
complexity of Spice is absolutely needed for 3d and that that's a major
part of the design.

GPU-level support for 3d operations has a number of advantages mainly
that it's more reasonably portable to things like Windows since the 3d
commands can be a superset of both OpenGL and Direct3d.

Also, Spice has an abstraction layer that doesn't simply passthrough
graphics commands, but translates/sanitizes them first. That's another
advantage over OpenGL passthrough. Without a layer to sanitize
commands, a guest can do funky things with the host or other guests.

I think a Spice-like approach is the best thing long term. In the short
term, I think doing the GL marshalling over virtio-serial makes a ton of
sense since the kernel driver is already present upstream. It exists
exactly for things like this.

In the very, very short term, I think an external backend to QEMU also
makes a lot of sense because that's something that Just Works today. I
think we can consider integrating it into QEMU (or at least simplifying
the execution of the backend) but integrating into QEMU is going to
require an awful lot of the existing code to be rewritten. Keeping it
separate has the advantage of allowing something to Just Work as an
interim solution as we wait for proper support in Spice.

Regards,

Anthony Liguori

>
>> Regards,
>>
>> Anthony Liguori
>>
>>
>>> I understand others are skeptical,
>>> but this seems simple and if it works and you're happy to maintain
>>>
>> it I'm
>>
>>> happy to let you do it :)
>>>
>>> Cheers,
>>> Rusty.
>>>
>>>
>>>

2010-11-03 18:09:27

by Ian Molton

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 01/11/10 13:28, Anthony Liguori wrote:
> On 11/01/2010 06:53 AM, Alon Levy wrote:

>> While we (speaking as part of the SPICE developers) want to have the same
>> support in our virtual GPU for 3d as we have for 2d, we just don't at
>> this point of time.

Would it be helpful to you to have /something/ that works in the
interim? I'm happy to work with you guys so that we dont need to
reinvent the wheel ;-)

> Yes, but I think the point is that are two general approaches to
> supporting 3d that are being proposed. One approach is to an RPC layer
> at the OpenGL level which essentially passes through the host OpenGL
> stack. That's what virtio-gl is. This has existed for quite a while and
> there are multiple transports for it.

Well, sort of. this version is heavily modified and only the virtio
transport is supported in it. Its quite a large code cleanup.

> It supports serial ports, TCP sockets, a custom ISA extension for x86,

The custom ISA idea is cut too since it fails to play nice with KVM.
Virtio-gl offers better security too.

> Another approach would be to have a virtual GPU and to implement
> GPU-level commands for 3d. I have been repeated told that much of the
> complexity of Spice is absolutely needed for 3d and that that's a major
> part of the design.

I've not seen any implementations of this type that are even close to
useable.

> GPU-level support for 3d operations has a number of advantages mainly
> that it's more reasonably portable to things like Windows since the 3d
> commands can be a superset of both OpenGL and Direct3d.

Agreed.

> Also, Spice has an abstraction layer that doesn't simply passthrough
> graphics commands, but translates/sanitizes them first. That's another
> advantage over OpenGL passthrough.

I'm not sure that thats actually needed if you are careful about what
commands you implement on the virtual GPU (and how)

> Without a layer to sanitize commands,
> a guest can do funky things with the host or other guests.

It shouldnt be possible for the guest to hurt other guests actual GL
rendering (or the hosts) as each PID is handed a context (or contexts)
of its own. It is possible for the guest to cause the host problems
though. this is a design flaw in the whole idea of GL RPC. It *isnt*
reasonable to make it secure, but it certainly is useful right now since
theres no alternative available.

> I think a Spice-like approach is the best thing long term. In the short
> term, I think doing the GL marshalling over virtio-serial makes a ton of
> sense since the kernel driver is already present upstream. It exists
> exactly for things like this.

The virtio driver enfoces the PID field and understands the packet
format used. Its better than using serial. Its also just one driver -
which doesnt have any special interdependencies and can be extended or
got rid of in future if and when better things come along.

> In the very, very short term, I think an external backend to QEMU also
> makes a lot of sense because that's something that Just Works today.

Whos written that? The 2007 patch I've been working on and updating
simply fails to work altogether without huge alterations on current qemu.

My current patch touches a tiny part of the qemu sources. It works today.

> I
> think we can consider integrating it into QEMU (or at least simplifying
> the execution of the backend) but integrating into QEMU is going to
> require an awful lot of the existing code to be rewritten. Keeping it
> separate has the advantage of allowing something to Just Work as an
> interim solution as we wait for proper support in Spice.

I dont know why you think integrating it into qemu is hard? I've already
done it. I added one virtio driver and a seperate offscreen renderer. it
touches the qemu code in *one* place. There should be no need to rewrite
anything.

2010-11-03 18:17:36

by Anthony Liguori

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 11/03/2010 01:03 PM, Ian Molton wrote:
>
> The virtio driver enfoces the PID field and understands the packet
> format used. Its better than using serial. Its also just one driver -
> which doesnt have any special interdependencies and can be extended or
> got rid of in future if and when better things come along.

Why is it better than using virtio-serial?

>
>> In the very, very short term, I think an external backend to QEMU also
>> makes a lot of sense because that's something that Just Works today.
>
> Whos written that? The 2007 patch I've been working on and updating
> simply fails to work altogether without huge alterations on current qemu.
>
> My current patch touches a tiny part of the qemu sources. It works today.

But it's not at all mergable in the current form. If you want to do the
work of getting it into a mergable state (cleaning up the coding style,
moving it to hw/, etc.) than I'm willing to consider it. But I don't
think a custom virtio transport is the right thing to do here.

However, if you want something that Just Works with the least amount of
code possible, just split it into a separate process and we can stick it
in a contrib/ directory or something.

>
>> I
>> think we can consider integrating it into QEMU (or at least simplifying
>> the execution of the backend) but integrating into QEMU is going to
>> require an awful lot of the existing code to be rewritten. Keeping it
>> separate has the advantage of allowing something to Just Work as an
>> interim solution as we wait for proper support in Spice.
>
> I dont know why you think integrating it into qemu is hard? I've
> already done it.

Adding a file that happens to compile as part of qemu even though it
doesn't actually integrate with qemu in any meaningful way is not
integrating. That's just build system manipulation.

Regards,

Anthony Liguori

> I added one virtio driver and a seperate offscreen renderer. it
> touches the qemu code in *one* place. There should be no need to
> rewrite anything.

2010-11-04 09:14:25

by Alon Levy

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On Wed, Nov 03, 2010 at 06:03:50PM +0000, Ian Molton wrote:
> On 01/11/10 13:28, Anthony Liguori wrote:
> >On 11/01/2010 06:53 AM, Alon Levy wrote:
>
> >>While we (speaking as part of the SPICE developers) want to have the same
> >>support in our virtual GPU for 3d as we have for 2d, we just don't at
> >>this point of time.
>
> Would it be helpful to you to have /something/ that works in the
> interim? I'm happy to work with you guys so that we dont need to
> reinvent the wheel ;-)
>

In case it wasn't clear, I think putting virtio-gl is a good idea, exactly because it works right now.

[Snip]

2010-11-05 18:03:16

by Ian Molton

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 04/11/10 09:13, Alon Levy wrote:
> On Wed, Nov 03, 2010 at 06:03:50PM +0000, Ian Molton wrote:
>> On 01/11/10 13:28, Anthony Liguori wrote:
>>> On 11/01/2010 06:53 AM, Alon Levy wrote:
>>
>>>> While we (speaking as part of the SPICE developers) want to have the same
>>>> support in our virtual GPU for 3d as we have for 2d, we just don't at
>>>> this point of time.
>>
>> Would it be helpful to you to have /something/ that works in the
>> interim? I'm happy to work with you guys so that we dont need to
>> reinvent the wheel ;-)
>
> In case it wasn't clear, I think putting virtio-gl is a good idea, exactly because it works right now.

Thanks :)

-Ian

2010-11-05 18:10:57

by Ian Molton

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 03/11/10 18:17, Anthony Liguori wrote:
> On 11/03/2010 01:03 PM, Ian Molton wrote:

> Why is it better than using virtio-serial?

For one thing, it enforces the PID in kernel so the guests processes
cant screw each other over by forging the PID passed to qemu.

>> My current patch touches a tiny part of the qemu sources. It works today.
>
> But it's not at all mergable in the current form. If you want to do the
> work of getting it into a mergable state (cleaning up the coding style,
> moving it to hw/, etc.) than I'm willing to consider it. But I don't
> think a custom virtio transport is the right thing to do here.

Hm, I thought I'd indented everything in qemus odd way... Is there a
codingstyle document or a checkpatch-like tool for qemu?

I'm happy to make the code meet qemus coding style.

> However, if you want something that Just Works with the least amount of
> code possible, just split it into a separate process and we can stick it
> in a contrib/ directory or something.

I dont see what splitting it into a seperate process buys us given we
still need the virtio-gl driver in order to enforce the PID. The virtio
driver is probably quite a bit more efficient at marshalling the data
too, given that it was designed alongside the userspace code.

>>> I
>>> think we can consider integrating it into QEMU (or at least simplifying
>>> the execution of the backend) but integrating into QEMU is going to
>>> require an awful lot of the existing code to be rewritten.

Why? aside from codingstyle, whats massively wrong with it thats going
to demand a total rewrite?

>>> Keeping it
>>> separate has the advantage of allowing something to Just Work as an
>>> interim solution as we wait for proper support in Spice.

Why does keeping it seperate make life easier? qemu is in a git repo.
when the time comes, if it reall is a total replacement, git-rm will
nuke it all.

>> I dont know why you think integrating it into qemu is hard? I've
>> already done it.
>
> Adding a file that happens to compile as part of qemu even though it
> doesn't actually integrate with qemu in any meaningful way is not
> integrating. That's just build system manipulation.

Uh? Either it compiles and works as part of qemu (which it does) or it
doesnt. How is that not integrated? I've added it to the configure
script too.

-Ian

2010-11-10 17:28:15

by Ian Molton

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

Ping ?

On 05/11/10 18:05, Ian Molton wrote:
> On 03/11/10 18:17, Anthony Liguori wrote:
>> On 11/03/2010 01:03 PM, Ian Molton wrote:
>
>> Why is it better than using virtio-serial?
>
> For one thing, it enforces the PID in kernel so the guests processes
> cant screw each other over by forging the PID passed to qemu.
>
>>> My current patch touches a tiny part of the qemu sources. It works
>>> today.
>>
>> But it's not at all mergable in the current form. If you want to do the
>> work of getting it into a mergable state (cleaning up the coding style,
>> moving it to hw/, etc.) than I'm willing to consider it. But I don't
>> think a custom virtio transport is the right thing to do here.
>
> Hm, I thought I'd indented everything in qemus odd way... Is there a
> codingstyle document or a checkpatch-like tool for qemu?
>
> I'm happy to make the code meet qemus coding style.
>
>> However, if you want something that Just Works with the least amount of
>> code possible, just split it into a separate process and we can stick it
>> in a contrib/ directory or something.
>
> I dont see what splitting it into a seperate process buys us given we
> still need the virtio-gl driver in order to enforce the PID. The virtio
> driver is probably quite a bit more efficient at marshalling the data
> too, given that it was designed alongside the userspace code.
>
>>>> I
>>>> think we can consider integrating it into QEMU (or at least simplifying
>>>> the execution of the backend) but integrating into QEMU is going to
>>>> require an awful lot of the existing code to be rewritten.
>
> Why? aside from codingstyle, whats massively wrong with it thats going
> to demand a total rewrite?
>
>>>> Keeping it
>>>> separate has the advantage of allowing something to Just Work as an
>>>> interim solution as we wait for proper support in Spice.
>
> Why does keeping it seperate make life easier? qemu is in a git repo.
> when the time comes, if it reall is a total replacement, git-rm will
> nuke it all.
>
>>> I dont know why you think integrating it into qemu is hard? I've
>>> already done it.
>>
>> Adding a file that happens to compile as part of qemu even though it
>> doesn't actually integrate with qemu in any meaningful way is not
>> integrating. That's just build system manipulation.
>
> Uh? Either it compiles and works as part of qemu (which it does) or it
> doesnt. How is that not integrated? I've added it to the configure
> script too.
>
> -Ian
>

2010-11-10 17:48:07

by Anthony Liguori

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 11/10/2010 11:22 AM, Ian Molton wrote:
> Ping ?

I think the best way forward is to post patches.

To summarize what I was trying to express in the thread, I think this is
not the right long term architecture but am not opposed to it as a short
term solution. I think having a new virtio device is a bad design
choice but am not totally opposed to it.

My advice is that using virtio-serial + an external tool is probably the
least amount of work to get something working and usable with QEMU. If
you want to go for the path of integration, you're going to have to fix
all of the coding style issues and make the code fit into QEMU.
Dropping a bunch of junk into target-i386/ is not making the code fit
into QEMU.

If you post just what you have now in patch form, I can try to provide
more concrete advice ignoring the coding style problems.

Regards,

Anthony Liguori

2010-11-12 12:20:14

by Ian Molton

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 10/11/10 17:47, Anthony Liguori wrote:
> On 11/10/2010 11:22 AM, Ian Molton wrote:
>> Ping ?
>
> I think the best way forward is to post patches.

I posted links to the git trees. I can post patches, but they are
*large*. Do you really want me to post them?

> To summarize what I was trying to express in the thread, I think this is
> not the right long term architecture but am not opposed to it as a short
> term solution. I think having a new virtio device is a bad design choice
> but am not totally opposed to it.

Ok! (I agree (that this should be a short term solution) :) )

> you want to go for the path of integration, you're going to have to fix
> all of the coding style issues and make the code fit into QEMU. Dropping
> a bunch of junk into target-i386/ is not making the code fit into QEMU.

I agree. how about hw/gl for the renderer and hw/ for the virtio module?

> If you post just what you have now in patch form, I can try to provide
> more concrete advice ignoring the coding style problems.

I can post patches, although I dont think LKML would appreciate the
volume! I can post them to the qemu list if you do.

-Ian

2010-11-12 13:21:58

by Anthony Liguori

[permalink] [raw]
Subject: Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport

On 11/12/2010 06:14 AM, Ian Molton wrote:
> On 10/11/10 17:47, Anthony Liguori wrote:
>> On 11/10/2010 11:22 AM, Ian Molton wrote:
>>> Ping ?
>>
>> I think the best way forward is to post patches.
>
> I posted links to the git trees. I can post patches, but they are
> *large*. Do you really want me to post them?

Yes, and they have to be split up into something reviewable.

>
>> To summarize what I was trying to express in the thread, I think this is
>> not the right long term architecture but am not opposed to it as a short
>> term solution. I think having a new virtio device is a bad design choice
>> but am not totally opposed to it.
>
> Ok! (I agree (that this should be a short term solution) :) )
>
>> you want to go for the path of integration, you're going to have to fix
>> all of the coding style issues and make the code fit into QEMU. Dropping
>> a bunch of junk into target-i386/ is not making the code fit into QEMU.
>
> I agree. how about hw/gl for the renderer and hw/ for the virtio module?

That would be fine.

>> If you post just what you have now in patch form, I can try to provide
>> more concrete advice ignoring the coding style problems.
>
> I can post patches, although I dont think LKML would appreciate the
> volume! I can post them to the qemu list if you do.

Yes, qemu is where I was suggesting you post them.

Regards,

Anthony Liguori

> -Ian