Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-17 Thread Paolo Bonzini

Il 17/03/2014 05:36, Dave Airlie ha scritto:

I steered away from using config space for anything for the normal
operation of the GPU after looking at overheads and hearing from S390
people that config space has some special properties on their hw,


Right, but this is only for the old s390-virtio implementation, and it 
means that the cursor would not work on s390-virtio either.  The new 
one, which looks like real s390 hardware, doesn't have the problem.



The number of these events should be small in a running system, and
I'm not sure how we'd lose one.


Are they few even if you resize the viewer window, and this causes a 
stream of display-size changes?


Paolo



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-17 Thread Paolo Bonzini

Il 17/03/2014 06:21, Dave Airlie ha scritto:

Oh I was also going to use this queue to report HW error events from
the guest to the host,

like if the guest tries an illegal operation,


What fields would be present for such an error?

Paolo



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-17 Thread Michael S. Tsirkin
On Mon, Mar 17, 2014 at 02:36:37PM +1000, Dave Airlie wrote:
 On Thu, Mar 13, 2014 at 8:40 PM, Paolo Bonzini pbonz...@redhat.com wrote:
  Il 12/03/2014 21:26, Michael S. Tsirkin ha scritto:
 
  +Event queue:
  +The only current event passed is a message to denote the host
  +wants to update the layout of the screens. It contains the same
  +info as the response to VIRTGPU_CMD_GET_DISPLAY_INFO.
 
 
  I wonder if an event queue is the best mechanism if you can get the same
  info from a command anyway.  For virtio-scsi I used a queue because I needed
  to specify which target or LUN the event applied to, but here you do not
  need it and a queue is susceptible to dropped events.
 
  Perhaps a configuration field is better, like this:
 
  u32 events_read;
  u32 events_clear;
 
  A new event sets a bit in events_read and generates a configuration change
  interrupt.  The guest should never write to events_read.
 
  Writing to events_clear has the side effect of the device doing events_read
  = ~events_clear.  We cannot have R/W1C fields in virtio, but this
  approximation is good enough.
 
  When the guest receives a configuration change interrupt, it reads
  event_read.  If it is nonzero, it writes the same value it read to
  events_clear, and sends the necessary commands to the card in order to
  retrieve the event data.  It can then read again event_read, and loop if it
  is again nonzero.
 
 
 I steered away from using config space for anything for the normal
 operation of the GPU after looking at overheads and hearing from S390
 people that config space has some special properties on their hw,
 
 The number of these events should be small in a running system, and
 I'm not sure how we'd lose one.
 
 Dave.

I think Paolo refers to a case where these events
arrive faster than guest can handle them.

As long as there's a single event like this,
it seems possible to avoid such under-runs by guest,
by first making a new buffer available, then processing
a used buffer.

If we want this extendable to multiple events,
make guest prepend an out buffer with a header, saying what kind of
event is expected in a given buffer.

Yes, config space is slow and should be avoided for
normal operation. I'm not sure how common this
specific operation is. For reporting guest errors
config space has some advantages as it can
be read even if driver/device communication
is completely wedged.

-- 
MST



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-16 Thread Michael S. Tsirkin
On Fri, Mar 14, 2014 at 12:13:09PM +0100, Gerd Hoffmann wrote:
 On Do, 2014-03-13 at 10:08 +0100, Gerd Hoffmann wrote:
Hi,
  
   One thing to do is add this to the virtio spec.
   Need help with that?
  
  What is the authoritative spec these days?
  The virtio 1.0 draft?
  Where can I find the latest revision?
 
 Seems to be this:
 
 https://github.com/rustyrussell/virtio-wip.git
 
 After figthing with texlive package splitting for a while I seem to have
 all needed *.sty files installed now.  Now I'm facing this:
 
 kpathsea: Running mktextfm Arial
 /usr/share/texlive/texmf/web2c/mktexnam: Could not map source
 abbreviation  for Arial.
 /usr/share/texlive/texmf/web2c/mktexnam: Need to update ?
 mktextfm: Running mf-nowin -progname=mf \mode:=ljfour; mag:=1;
 nonstopmode; input Arial
 This is METAFONT, Version 2.718281 (TeX Live 2013)
 
 
 kpathsea: Running mktexmf Arial
 ! I can't find file `Arial'.
 * ...e:=ljfour; mag:=1; nonstopmode; input Arial
 
 Suggestions?

Do you have tex4ht installed?

I don't think I saw this specific error, but check out
t4ht-workaround/README for a description of a known Fedora
bug and workarounds.

  Where should I send patches?
 
 virtio-comments list?
 
 cheers,
   Gerd
 



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-16 Thread Michael S. Tsirkin
On Fri, Mar 14, 2014 at 12:18:38PM +0100, Gerd Hoffmann wrote:
   Hi,
 
  Perhaps a configuration field is better, like this:
  
   u32 events_read;
   u32 events_clear;
  
  A new event sets a bit in events_read and generates a configuration 
  change interrupt.  The guest should never write to events_read.
 
 Makes sense to me.  I think I'll go write up things as diff against
 virtio-wip, then post it for review.
 
 cheers,
   Gerd


Net device has a similar problem with gratitious packet
sending. What we are doing there is VIRTIO_NET_S_ANNOUNCE bit
in the status field in the configuration, coupled with
VIRTIO_NET_CTRL_ANNOUNCE_ACK command to clear it.


-- 
MST



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-16 Thread Dave Airlie
On Thu, Mar 13, 2014 at 8:40 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 12/03/2014 21:26, Michael S. Tsirkin ha scritto:

 +Event queue:
 +The only current event passed is a message to denote the host
 +wants to update the layout of the screens. It contains the same
 +info as the response to VIRTGPU_CMD_GET_DISPLAY_INFO.


 I wonder if an event queue is the best mechanism if you can get the same
 info from a command anyway.  For virtio-scsi I used a queue because I needed
 to specify which target or LUN the event applied to, but here you do not
 need it and a queue is susceptible to dropped events.

 Perhaps a configuration field is better, like this:

 u32 events_read;
 u32 events_clear;

 A new event sets a bit in events_read and generates a configuration change
 interrupt.  The guest should never write to events_read.

 Writing to events_clear has the side effect of the device doing events_read
 = ~events_clear.  We cannot have R/W1C fields in virtio, but this
 approximation is good enough.

 When the guest receives a configuration change interrupt, it reads
 event_read.  If it is nonzero, it writes the same value it read to
 events_clear, and sends the necessary commands to the card in order to
 retrieve the event data.  It can then read again event_read, and loop if it
 is again nonzero.


I steered away from using config space for anything for the normal
operation of the GPU after looking at overheads and hearing from S390
people that config space has some special properties on their hw,

The number of these events should be small in a running system, and
I'm not sure how we'd lose one.

Dave.



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-16 Thread Dave Airlie
On Mon, Mar 17, 2014 at 2:36 PM, Dave Airlie airl...@gmail.com wrote:
 On Thu, Mar 13, 2014 at 8:40 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 12/03/2014 21:26, Michael S. Tsirkin ha scritto:

 +Event queue:
 +The only current event passed is a message to denote the host
 +wants to update the layout of the screens. It contains the same
 +info as the response to VIRTGPU_CMD_GET_DISPLAY_INFO.


 I wonder if an event queue is the best mechanism if you can get the same
 info from a command anyway.  For virtio-scsi I used a queue because I needed
 to specify which target or LUN the event applied to, but here you do not
 need it and a queue is susceptible to dropped events.

 Perhaps a configuration field is better, like this:

 u32 events_read;
 u32 events_clear;

 A new event sets a bit in events_read and generates a configuration change
 interrupt.  The guest should never write to events_read.

 Writing to events_clear has the side effect of the device doing events_read
 = ~events_clear.  We cannot have R/W1C fields in virtio, but this
 approximation is good enough.

 When the guest receives a configuration change interrupt, it reads
 event_read.  If it is nonzero, it writes the same value it read to
 events_clear, and sends the necessary commands to the card in order to
 retrieve the event data.  It can then read again event_read, and loop if it
 is again nonzero.


 I steered away from using config space for anything for the normal
 operation of the GPU after looking at overheads and hearing from S390
 people that config space has some special properties on their hw,

 The number of these events should be small in a running system, and
 I'm not sure how we'd lose one.

Oh I was also going to use this queue to report HW error events from
the guest to the host,

like if the guest tries an illegal operation,

Dave.



Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-14 Thread Gerd Hoffmann
On Do, 2014-03-13 at 10:08 +0100, Gerd Hoffmann wrote:
   Hi,
 
  One thing to do is add this to the virtio spec.
  Need help with that?
 
 What is the authoritative spec these days?
 The virtio 1.0 draft?
 Where can I find the latest revision?

Seems to be this:

https://github.com/rustyrussell/virtio-wip.git

After figthing with texlive package splitting for a while I seem to have
all needed *.sty files installed now.  Now I'm facing this:

kpathsea: Running mktextfm Arial
/usr/share/texlive/texmf/web2c/mktexnam: Could not map source
abbreviation  for Arial.
/usr/share/texlive/texmf/web2c/mktexnam: Need to update ?
mktextfm: Running mf-nowin -progname=mf \mode:=ljfour; mag:=1;
nonstopmode; input Arial
This is METAFONT, Version 2.718281 (TeX Live 2013)


kpathsea: Running mktexmf Arial
! I can't find file `Arial'.
* ...e:=ljfour; mag:=1; nonstopmode; input Arial

Suggestions?

 Where should I send patches?

virtio-comments list?

cheers,
  Gerd





Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-14 Thread Gerd Hoffmann
  Hi,

 Perhaps a configuration field is better, like this:
 
  u32 events_read;
  u32 events_clear;
 
 A new event sets a bit in events_read and generates a configuration 
 change interrupt.  The guest should never write to events_read.

Makes sense to me.  I think I'll go write up things as diff against
virtio-wip, then post it for review.

cheers,
  Gerd





Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-13 Thread Gerd Hoffmann
  Hi,

 One thing to do is add this to the virtio spec.
 Need help with that?

What is the authoritative spec these days?
The virtio 1.0 draft?
Where can I find the latest revision?
Where should I send patches?

   docs/specs/virtio-gpu.txt  |  89 +

Some docs are here.

cheers,
  Gerd





Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-13 Thread Paolo Bonzini

Il 12/03/2014 21:26, Michael S. Tsirkin ha scritto:

+Event queue:
+The only current event passed is a message to denote the host
+wants to update the layout of the screens. It contains the same
+info as the response to VIRTGPU_CMD_GET_DISPLAY_INFO.


I wonder if an event queue is the best mechanism if you can get the same 
info from a command anyway.  For virtio-scsi I used a queue because I 
needed to specify which target or LUN the event applied to, but here you 
do not need it and a queue is susceptible to dropped events.


Perhaps a configuration field is better, like this:

u32 events_read;
u32 events_clear;

A new event sets a bit in events_read and generates a configuration 
change interrupt.  The guest should never write to events_read.


Writing to events_clear has the side effect of the device doing 
events_read = ~events_clear.  We cannot have R/W1C fields in virtio, 
but this approximation is good enough.


When the guest receives a configuration change interrupt, it reads 
event_read.  If it is nonzero, it writes the same value it read to 
events_clear, and sends the necessary commands to the card in order to 
retrieve the event data.  It can then read again event_read, and loop if 
it is again nonzero.


Paolo



[Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-12 Thread Gerd Hoffmann
From: Dave Airlie airl...@redhat.com

This is the basic virtio-gpu which is

multi-head capable,
ARGB cursor support,
unaccelerated.

Some more info is in docs/specs/virtio-gpu.txt.

Signed-off-by: Dave Airlie airl...@redhat.com

changes by kraxel:
 * adapt to changes in master.
 * move from hw/virtio to hw/display.
 * codestyle cleanups.
 * misc minor fixes.

Signed-off-by: Gerd Hoffmann kra...@redhat.com

virtio-gpu codestyle
---
 default-configs/x86_64-softmmu.mak |   1 +
 docs/specs/virtio-gpu.txt  |  89 +
 hw/display/Makefile.objs   |   3 +
 hw/display/virtgpu_hw.h| 149 
 hw/display/virtio-gpu-pci.c|  80 +
 hw/display/virtio-gpu.c| 689 +
 hw/virtio/virtio-pci.h |  15 +
 include/hw/pci/pci.h   |   1 +
 include/hw/virtio/virtio-gpu.h |  91 +
 9 files changed, 1118 insertions(+)
 create mode 100644 docs/specs/virtio-gpu.txt
 create mode 100644 hw/display/virtgpu_hw.h
 create mode 100644 hw/display/virtio-gpu-pci.c
 create mode 100644 hw/display/virtio-gpu.c
 create mode 100644 include/hw/virtio/virtio-gpu.h

diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index 31bddce..1a00b78 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -9,6 +9,7 @@ CONFIG_VGA_PCI=y
 CONFIG_VGA_ISA=y
 CONFIG_VGA_CIRRUS=y
 CONFIG_VMWARE_VGA=y
+CONFIG_VIRTIO_GPU=y
 CONFIG_VMMOUSE=y
 CONFIG_SERIAL=y
 CONFIG_PARALLEL=y
diff --git a/docs/specs/virtio-gpu.txt b/docs/specs/virtio-gpu.txt
new file mode 100644
index 000..987d807
--- /dev/null
+++ b/docs/specs/virtio-gpu.txt
@@ -0,0 +1,89 @@
+Initial info on virtio-gpu:
+
+virtio-gpu is a virtio based graphics adapter. The initial version provides 
support for:
+
+- ARGB Hardware cursors
+- multiple outputs
+
+There are no acceleration features available in the first release of this 
device, 3D acceleration features will be provided later via an interface to an 
external renderer.
+
+The virtio-gpu exposes 3 vqs to the guest,
+
+1) control vq - guest-host queue for sending commands and getting responses 
when required.
+2) cursor vq - guest-host queue for sending cursor position and resource 
updates
+3) event vq - host-guest queue for sending async events like display 
topography updates and errors (todo).
+
+How to use the virtio-gpu:
+
+The virtio-gpu is based around the concept of resources private to the host, 
the guest must DMA transfer into these resources. This is a design requirement 
in order to interface with future 3D rendering. In the unaccelerated there is 
no support for DMA transfers from resources, just to them.
+
+Resources are initially simple 2D resources, consisting of a width, height and 
format along with an identifier. The guest must then attach backing store to 
the resources in order for DMA transfers to work. This is like a GART in a real 
GPU.
+
+A typical guest user would create a 2D resource using 
VIRTGPU_CMD_RESOURCE_CREATE_2D, attach backing store using 
VIRTGPU_CMD_RESOURCE_ATTACH_BACKING, then attach the resource to a scanout 
using VIRTGPU_CMD_SET_SCANOUT, then use VIRTGPU_CMD_TRANSFER_SEND_2D to send 
updates to the resource, and finally VIRTGPU_CMD_RESOURCE_FLUSH to flush the 
scanout buffers to screen.
+
+Command queue:
+
+VIRTGPU_CMD_GET_DISPLAY_INFO:
+Retrieve the current output configuration.
+
+This sends a response in the same queue slot. The response contains the max 
number of scanouts the host can support,
+along with a list of per-scanout information. The info contains whether the
+scanout is enabled, what its preferred x, y, width and height are and some 
future flags.
+
+VIRTGPU_CMD_RESOURCE_CREATE_2D:
+Create a 2D resource on the host.
+
+This creates a 2D resource on the host with the specified width, height and 
format. Only a small subset of formats are support. The resource ids are 
generated by the guest.
+
+VIRTGPU_CMD_RESOURCE_UNREF:
+Destroy a resource.
+
+This informs the host that a resource is no longer required by the guest.
+
+VIRTGPU_CMD_SET_SCANOUT:
+Set the scanout parameters for a single output.
+
+This sets the scanout parameters for a single scanout. The resource_id is the
+resource to be scanned out from, along with a rectangle specified by x, y, 
width and height.
+
+VIRTGPU_CMD_RESOURCE_FLUSH:
+Flush a scanout resource
+
+This flushes a resource to screen, it takes a rectangle and a resource id,
+and flushes any scanouts the resource is being used on.
+
+VIRTGPU_CMD_TRANSFER_TO_HOST_2D:
+Transfer from guest memory to host resource.
+
+This takes a resource id along with an destination offset into the resource,
+and a box to transfer from the host backing for the resource.
+
+VIRTGPU_CMD_RESOURCE_ATTACH_BACKING:
+Assign backing pages to a resource.
+
+This assign an array of guest pages as the backing store for a resource. These
+pages are then used for the transfer operations for that resource from that

Re: [Qemu-devel] [PATCH 3/4] virtio-gpu: v0.3 of the virtio based GPU code.

2014-03-12 Thread Michael S. Tsirkin
On Wed, Mar 12, 2014 at 01:55:13PM +0100, Gerd Hoffmann wrote:
 From: Dave Airlie airl...@redhat.com
 
 This is the basic virtio-gpu which is
 
 multi-head capable,
 ARGB cursor support,
 unaccelerated.
 
 Some more info is in docs/specs/virtio-gpu.txt.
 
 Signed-off-by: Dave Airlie airl...@redhat.com
 
 changes by kraxel:
  * adapt to changes in master.
  * move from hw/virtio to hw/display.
  * codestyle cleanups.
  * misc minor fixes.
 
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 
 virtio-gpu codestyle

Still reviewing.
One thing to do is add this to the virtio spec.
Need help with that?

 ---
  default-configs/x86_64-softmmu.mak |   1 +
  docs/specs/virtio-gpu.txt  |  89 +
  hw/display/Makefile.objs   |   3 +
  hw/display/virtgpu_hw.h| 149 
  hw/display/virtio-gpu-pci.c|  80 +
  hw/display/virtio-gpu.c| 689 
 +
  hw/virtio/virtio-pci.h |  15 +
  include/hw/pci/pci.h   |   1 +
  include/hw/virtio/virtio-gpu.h |  91 +
  9 files changed, 1118 insertions(+)
  create mode 100644 docs/specs/virtio-gpu.txt
  create mode 100644 hw/display/virtgpu_hw.h
  create mode 100644 hw/display/virtio-gpu-pci.c
  create mode 100644 hw/display/virtio-gpu.c
  create mode 100644 include/hw/virtio/virtio-gpu.h
 
 diff --git a/default-configs/x86_64-softmmu.mak 
 b/default-configs/x86_64-softmmu.mak
 index 31bddce..1a00b78 100644
 --- a/default-configs/x86_64-softmmu.mak
 +++ b/default-configs/x86_64-softmmu.mak
 @@ -9,6 +9,7 @@ CONFIG_VGA_PCI=y
  CONFIG_VGA_ISA=y
  CONFIG_VGA_CIRRUS=y
  CONFIG_VMWARE_VGA=y
 +CONFIG_VIRTIO_GPU=y
  CONFIG_VMMOUSE=y
  CONFIG_SERIAL=y
  CONFIG_PARALLEL=y
 diff --git a/docs/specs/virtio-gpu.txt b/docs/specs/virtio-gpu.txt
 new file mode 100644
 index 000..987d807
 --- /dev/null
 +++ b/docs/specs/virtio-gpu.txt
 @@ -0,0 +1,89 @@
 +Initial info on virtio-gpu:
 +
 +virtio-gpu is a virtio based graphics adapter. The initial version provides 
 support for:
 +
 +- ARGB Hardware cursors
 +- multiple outputs
 +
 +There are no acceleration features available in the first release of this 
 device, 3D acceleration features will be provided later via an interface to 
 an external renderer.
 +
 +The virtio-gpu exposes 3 vqs to the guest,
 +
 +1) control vq - guest-host queue for sending commands and getting responses 
 when required.
 +2) cursor vq - guest-host queue for sending cursor position and resource 
 updates
 +3) event vq - host-guest queue for sending async events like display 
 topography updates and errors (todo).
 +
 +How to use the virtio-gpu:
 +
 +The virtio-gpu is based around the concept of resources private to the host, 
 the guest must DMA transfer into these resources. This is a design 
 requirement in order to interface with future 3D rendering. In the 
 unaccelerated there is no support for DMA transfers from resources, just to 
 them.
 +
 +Resources are initially simple 2D resources, consisting of a width, height 
 and format along with an identifier. The guest must then attach backing store 
 to the resources in order for DMA transfers to work. This is like a GART in a 
 real GPU.
 +
 +A typical guest user would create a 2D resource using 
 VIRTGPU_CMD_RESOURCE_CREATE_2D, attach backing store using 
 VIRTGPU_CMD_RESOURCE_ATTACH_BACKING, then attach the resource to a scanout 
 using VIRTGPU_CMD_SET_SCANOUT, then use VIRTGPU_CMD_TRANSFER_SEND_2D to send 
 updates to the resource, and finally VIRTGPU_CMD_RESOURCE_FLUSH to flush the 
 scanout buffers to screen.
 +
 +Command queue:
 +
 +VIRTGPU_CMD_GET_DISPLAY_INFO:
 +Retrieve the current output configuration.
 +
 +This sends a response in the same queue slot. The response contains the max 
 number of scanouts the host can support,
 +along with a list of per-scanout information. The info contains whether the
 +scanout is enabled, what its preferred x, y, width and height are and some 
 future flags.
 +
 +VIRTGPU_CMD_RESOURCE_CREATE_2D:
 +Create a 2D resource on the host.
 +
 +This creates a 2D resource on the host with the specified width, height and 
 format. Only a small subset of formats are support. The resource ids are 
 generated by the guest.
 +
 +VIRTGPU_CMD_RESOURCE_UNREF:
 +Destroy a resource.
 +
 +This informs the host that a resource is no longer required by the guest.
 +
 +VIRTGPU_CMD_SET_SCANOUT:
 +Set the scanout parameters for a single output.
 +
 +This sets the scanout parameters for a single scanout. The resource_id is the
 +resource to be scanned out from, along with a rectangle specified by x, y, 
 width and height.
 +
 +VIRTGPU_CMD_RESOURCE_FLUSH:
 +Flush a scanout resource
 +
 +This flushes a resource to screen, it takes a rectangle and a resource id,
 +and flushes any scanouts the resource is being used on.
 +
 +VIRTGPU_CMD_TRANSFER_TO_HOST_2D:
 +Transfer from guest memory to host resource.
 +
 +This takes a resource id along with an destination offset into the resource,
 +and a