Re: [Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree)

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 07:47:00PM +0100, Aurelien Jarno wrote:
> I am still waiting for you to list patches from Anthony or me,

Why should I go look for them?  I thought you basically agreed it's a
good idea to post patches publically either before or - for trivial
patches - after commit?

-- 
MST

Re: [Qemu-devel] [PATCH 11/19] use a bottom half to run timers

[Jamie Lokier]

Anthony Liguori wrote:
> introduces a subtle semantic change.  Previously, timers always ran 
> before bottom halves whereas after this change, timers may run after 
> some bottoms halves but before others.  While this should be okay in 
> principle, in practice, I'm sure it'll introduce regressions.  I'd be 
> very surprised if cris wasn't affected by this.

In principle, if it does affect something, it seems likely there is
already a buggy race condition.  After all, if the timer and bottom
half could trigger at the same time, which is the condition where the
order is significant, then in principle the timer could have triggered
slightly later because it depends on the host alarm behaviour.

> But more importantly, I think timer dispatch needs to be part of the 
> select loop.  malc has a git tree that replaces host alarm timers with 
> select() timeouts.  This has a lot of really nice properties like it 
> eliminates the need for signals and EINTR handling.  A move like this 
> would likely make this more difficult.

I agree that select loop is better, but don't you still need a host
alarm signal for when the guest is running for a long time without
trapping?

-- Jamie

Re: [Qemu-devel] [PATCH V4 04/18] support NEC PC-9821 architecture

[Anthony Liguori]

On 12/22/2009 11:40 AM, TAKEDA, toshiya wrote:

Signed-off-by: TAKEDA, toshiya
---
  hw/pc98.c |  317 +
  hw/pc98.h |   65 +
  2 files changed, 382 insertions(+), 0 deletions(-)
  create mode 100644 hw/pc98.c
  create mode 100644 hw/pc98.h

diff --git a/hw/pc98.c b/hw/pc98.c
new file mode 100644
index 000..4bc4805
--- /dev/null
+++ b/hw/pc98.c
@@ -0,0 +1,317 @@
+/*
+ * QEMU NEC PC-9821 System Emulator
+ *
+ * Copyright (c) 2009 TAKEDA, toshiya
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */


I don't think we are really that far from being able to create an NEC 
PC-9821 based on careful use of -device and disabling default devices. 
I think the main issues some of the platform devices that aren't present 
in this machine (like the rtc) that aren't qdev-ified yet.  I think 
that's probably an easier path to take though.


BTW, this patch won't link on it's own because it relies on 
pc98_init_i8259 which this patch doesn't introduce.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH V4 02/18] sysemu: support qemu_cpu_reset()

[Anthony Liguori]

Hi,

Why is this needed verses qemu_system_reset()?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 19/19] split out qemu-timer.c

[Anthony Liguori]

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:

Signed-off-by: Paolo Bonzini
---
  Makefile.target |1 +
  cpu-all.h   |2 +
  qemu-timer.c| 1218 +++
  qemu-timer.h|   11 +
  vl.c| 1164 
  5 files changed, 1232 insertions(+), 1164 deletions(-)
  create mode 100644 qemu-timer.c



All in all, a nice cleanup.  Good work!

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 11/19] use a bottom half to run timers

[Anthony Liguori]

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:

Make the timer subsystem register its own bottom half instead of
placing the bottom half code in the heart of the main loop.  To
test if an alarm timer is pending, just check if the bottom half is
scheduled.

Signed-off-by: Paolo Bonzini


I'm not a huge fan of this for a couple reasons.  The first is that it 
introduces a subtle semantic change.  Previously, timers always ran 
before bottom halves whereas after this change, timers may run after 
some bottoms halves but before others.  While this should be okay in 
principle, in practice, I'm sure it'll introduce regressions.  I'd be 
very surprised if cris wasn't affected by this.


But more importantly, I think timer dispatch needs to be part of the 
select loop.  malc has a git tree that replaces host alarm timers with 
select() timeouts.  This has a lot of really nice properties like it 
eliminates the need for signals and EINTR handling.  A move like this 
would likely make this more difficult.


I think the opposite sort of move makes more sense.  Treating bottom 
halves as 0-duration timer events.  Unfortunately, things like cris do 
not handle this sort of change very well.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c

[Anthony Liguori]

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:

As a side exercise, move 200 lines out of vl.c already into common
code that only needs to be compiled once.



Maybe input.c instead of event.c?

Please split this out of the series and resubmit.  It's a pretty obvious 
patch to include on it's own.


Regards,

Anthony Liguori

[Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree)

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 07:04:38PM +, Blue Swirl wrote:
> On Mon, Jan 4, 2010 at 6:33 PM, Michael S. Tsirkin  wrote:
> >> On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote:
> >> > Likewise, if you see a patch go in that you think would have benefited
> >> > from being on the list, point it out.  People are reasonable and if you
> >> > have a good suggestion, they'll value your input and be likely to seek
> >> > it out in the future.
> >
> > Here is another patch that would have benefitted from review
> > before commit:
> >
> >> commit cf616802171905a9b6d087a69caa3b978b9cd741
> >> Author: Blue Swirl 
> >> Date:   Sun Dec 27 20:52:36 2009 +
> >>
> >>     PCI: Fix bus address conversion
> >>
> >>     Pass physical addresses to map functions instead of PCI bus addresses.
> >>
> >>     Signed-off-by: Blue Swirl 
> >
> > and previous related patches.  The issues here that I see are:
> >
> > - IMO mem_base should really be pci_bus_t, as pci address might be
> >  64 bit mapped into 32 bit target
> >
> > - I think pci to pci bridges need mem_base copied from parent to child,
> >  this does not seem to be done?
> >
> > - map functions need to get pci_bus_t (for io), and now they get
> >  a cpu address there. The real fix IMO is moving the mapping
> >  to within pci.c. I think Avi had a patch to do this -
> >  any objections to refreshing it?
> >
> > Blue Swirl, could you comment please?
> 
> The issues you point out (which may well be valid) are not related to
> the change made by the patch and should be addressed by new patches.

Yes, there's no harm in fixing them separately.  The point I was making
is it is better to post patches on list so issues can be pointed out and
discussed without the need to dig through git history.

> IIRC Avi promised to refresh his patch but never delivered. I think
> Paul also wanted that the bus translation would be handled in a more
> generic way (eliminate map functions).

I'd like to eliminate map functions as well. Do you have a link to the original 
patch
btw?

-- 
MST

[Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree)

[Blue Swirl]

On Mon, Jan 4, 2010 at 6:33 PM, Michael S. Tsirkin  wrote:
>> On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote:
>> > Likewise, if you see a patch go in that you think would have benefited
>> > from being on the list, point it out.  People are reasonable and if you
>> > have a good suggestion, they'll value your input and be likely to seek
>> > it out in the future.
>
> Here is another patch that would have benefitted from review
> before commit:
>
>> commit cf616802171905a9b6d087a69caa3b978b9cd741
>> Author: Blue Swirl 
>> Date:   Sun Dec 27 20:52:36 2009 +
>>
>>     PCI: Fix bus address conversion
>>
>>     Pass physical addresses to map functions instead of PCI bus addresses.
>>
>>     Signed-off-by: Blue Swirl 
>
> and previous related patches.  The issues here that I see are:
>
> - IMO mem_base should really be pci_bus_t, as pci address might be
>  64 bit mapped into 32 bit target
>
> - I think pci to pci bridges need mem_base copied from parent to child,
>  this does not seem to be done?
>
> - map functions need to get pci_bus_t (for io), and now they get
>  a cpu address there. The real fix IMO is moving the mapping
>  to within pci.c. I think Avi had a patch to do this -
>  any objections to refreshing it?
>
> Blue Swirl, could you comment please?

The issues you point out (which may well be valid) are not related to
the change made by the patch and should be addressed by new patches.

IIRC Avi promised to refresh his patch but never delivered. I think
Paul also wanted that the bus translation would be handled in a more
generic way (eliminate map functions).

Re: [Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree)

[Aurelien Jarno]

On Mon, Jan 04, 2010 at 08:33:56PM +0200, Michael S. Tsirkin wrote:
> > On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote:
> > > Likewise, if you see a patch go in that you think would have benefited  
> > > from being on the list, point it out.  People are reasonable and if you  
> > > have a good suggestion, they'll value your input and be likely to seek  
> > > it out in the future.
> 
> Here is another patch that would have benefitted from review
> before commit:
> 

This is the kind of patch I send to the mailing first first.

I am still waiting for you to list patches from Anthony or me, that is 
the only two persons that have been pointed into your original mail.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer

[Paolo Bonzini]

On 01/04/2010 08:34 PM, Anthony Liguori wrote:

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:

The code is initializing an unsigned int to UINT_MAX using "-1", so that
the following always-true comparison seems to be always-false at a
first look. Just remove the if.


This really needs a better comment message as at first it wasn't obvious
to me that this was correct.

start_timer is only ever called once and as such, the check here is
unnecessary. It's not incorrect, it's just more robust than it needs to be.

I'm not sure it's really worth removing to be honest.


I didn't like the comparison with -1 being always true.  I'll update 
with a comment.


Paolo

[Qemu-devel] Re: PCI: Fix bus address conversion (was Re: commit rules for common git tree)

[Michael S. Tsirkin]

> On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote:
> > Likewise, if you see a patch go in that you think would have benefited  
> > from being on the list, point it out.  People are reasonable and if you  
> > have a good suggestion, they'll value your input and be likely to seek  
> > it out in the future.

Here is another patch that would have benefitted from review
before commit:

> commit cf616802171905a9b6d087a69caa3b978b9cd741
> Author: Blue Swirl 
> Date:   Sun Dec 27 20:52:36 2009 +
> 
> PCI: Fix bus address conversion
> 
> Pass physical addresses to map functions instead of PCI bus addresses.
> 
> Signed-off-by: Blue Swirl 

and previous related patches.  The issues here that I see are:

- IMO mem_base should really be pci_bus_t, as pci address might be
  64 bit mapped into 32 bit target

- I think pci to pci bridges need mem_base copied from parent to child,
  this does not seem to be done?

- map functions need to get pci_bus_t (for io), and now they get
  a cpu address there. The real fix IMO is moving the mapping
  to within pci.c. I think Avi had a patch to do this -
  any objections to refreshing it?

Blue Swirl, could you comment please?

-- 
MST

Re: [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer

[Anthony Liguori]

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:

The code is initializing an unsigned int to UINT_MAX using "-1", so that
the following always-true comparison seems to be always-false at a
first look.  Just remove the if.


This really needs a better comment message as at first it wasn't obvious 
to me that this was correct.


start_timer is only ever called once and as such, the check here is 
unnecessary.  It's not incorrect, it's just more robust than it needs to be.


I'm not sure it's really worth removing to be honest.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o

[Anthony Liguori]

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:

This series makes a few cleanup in the timer handling code and
splits out ~1500 lines out of the huge vl.o file.  So far I've tested
it by booting a live CD both under Linux and by cross-compiling to
Windows.  If the series is considered helpful, I can test further
including actually running the Windows version (Wine doesn't work).



Wine usually works although it's been a while since I've tested.

I haven't reviewed the series yet but I love the idea.  It's been 
something I've wanted to do for a while.  More detailed review of the 
patches will follow.


Regards,

Anthony Liguori

Re: [Qemu-devel] need help getting a USB audio device to work

[David S. Ahern]

On 01/03/2010 11:19 AM, David S. Ahern wrote:
> 
> After weeks of fruitless effort I could use some help getting a USB
> audio device to work. I have instrumented the hell out of the guest
> driver and uhci code, qemu's linux and uhci code, and the host side usb
> code. Near as I can tell data from the device makes its way into qemu
> (async_complete shows a urb length equal to the data the host OS
> receives from the device), but the data does not appear to make its way
> to the guest OS. I have tried a variety of guests -- Fedora 12, RHEL5.3,
> and RHEL3U8, and none work.
> 
> How do I determine in fact the data pulled into qemu from
> ioctl(USBDEVFS_REAPURBNDELAY) is getting pushed to the guest?
> 
> Thanks,

Upon further review the problem appears to be that the TD's are still
active when received in the VM. In the bare metal case the usb-uhci code
shows (my instrumentation in a RHEL3U8 guest):

process_iso @ 36912: urb f74bd03c, mode 0, frame 1191, npkt 5, nerr 0,
status 0357
  TD @ c8281480, link 8281180, status 20007ff, info c610369: MaxLen=63
DT0 EP=2 Dev=3 PID=( IN  )
Len=7ff e0
Link points to TD @ 08281180, Breadth first
TD NOT active (20007ff)
process_iso: 0 len 0 020007ff status 0
  TD @ c82814c0, link 8281200, status 20007ff, info c610369: MaxLen=63
DT0 EP=2 Dev=3 PID=( IN  )
Len=7ff e0
Link points to TD @ 08281200, Breadth first
TD NOT active (20007ff)
process_iso: 1 len 0 020007ff status 0
  TD @ c8281500, link 8281180, status 259, info c610369: MaxLen=63
DT0 EP=2 Dev=3 PID=( IN  )
Len=59 e0
Link points to TD @ 08281180, Breadth first
TD NOT active (259)
process_iso: 2 len 90 0259 status 0
  TD @ c8281540, link 82811c0, status 257, info c610369: MaxLen=63
DT0 EP=2 Dev=3 PID=( IN  )
Len=57 e0
Link points to TD @ 082811c0, Breadth first
TD NOT active (257)
process_iso: 3 len 88 0257 status 0
  TD @ c82815c0, link 8281180, status 357, info c610369: MaxLen=63
DT0 EP=2 Dev=3 PID=( IN  )
Len=57 e0 IOC
Link points to TD @ 08281180, Breadth first
TD NOT active (357)
process_iso: 4 len 88 0357 status 0
process_iso: exit 5 (0), transfer length 500 actual len 266


The same case in a VM shows:

process_iso: td still active
  TD @ c1980580, link 1980200, status 380, info c610269: MaxLen=63
DT0 EP=2 Dev=2 PID=( IN  )
Len=00 e0 IOC Active
Link points to TD @ 01980200, Breadth first
process_urb: type 0
process_iso @ 8376: urb dee5c0f0, mode 0, frame 1033, npkt 5, nerr 0,
status 030007ff
  TD @ c19805c0, link 1980180, status 280, info c610269: MaxLen=63
DT0 EP=2 Dev=2 PID=( IN  )
Len=00 e0 Active
Link points to TD @ 01980180, Breadth first
TD still active (200)- grrr. paranoia!


Does this ring a bell with anyone?

David

[Qemu-devel] [PATCH 7/8] virtio-serial: Add 'virtserialport' device for generic serial port support

[Amit Shah]

This patch adds generic serial ports over the virtio serial bus.
These ports have a few more options that are not relevant for
virtio console ports: the ability to cache buffers that are
received for a port while it's disconnected, setting of limits
to the bytes that are cached so as to prevent OOM conditions,
etc.

Sample uses for such a device can be obtaining info from the
guest like the file systems used, apps installed, etc. for
offline usage and logged-in users, clipboard copy-paste, etc.
for online usage.

Signed-off-by: Amit Shah 
---
 hw/virtio-serial.c |   36 
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial.c b/hw/virtio-serial.c
index fd27c33..f417929 100644
--- a/hw/virtio-serial.c
+++ b/hw/virtio-serial.c
@@ -113,3 +113,39 @@ static void virtconsole_register(void)
 virtio_serial_port_qdev_register(&virtconsole_info);
 }
 device_init(virtconsole_register)
+
+/* Generic Virtio Serial Ports */
+static int virtserialport_initfn(VirtIOSerialDevice *dev)
+{
+VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+port->info = dev->info;
+
+if (vcon->chr) {
+qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
+  vcon);
+}
+return 0;
+}
+
+static VirtIOSerialPortInfo virtserialport_info = {
+.qdev.name = "virtserialport",
+.qdev.size = sizeof(VirtConsole),
+.init  = virtserialport_initfn,
+.exit  = virtconsole_exitfn,
+.have_data = flush_buf,
+.qdev.props = (Property[]) {
+DEFINE_PROP_CHR("chardev", VirtConsole, chr),
+DEFINE_PROP_STRING("name", VirtConsole, port.name),
+DEFINE_PROP_UINT8("cache_buffers", VirtConsole, port.cache_buffers, 1),
+DEFINE_PROP_UINT64("byte_limit", VirtConsole, port.byte_limit, 0),
+DEFINE_PROP_END_OF_LIST(),
+},
+};
+
+static void virtserialport_register(void)
+{
+virtio_serial_port_qdev_register(&virtserialport_info);
+}
+device_init(virtserialport_register)
-- 
1.6.2.5

[Qemu-devel] [PATCH 8/8] Move virtio-serial and virtio-serial-bus to Makefile.hw

[Amit Shah]

There's nothing target-dependent in the virtio-serial code so allow it
to be compiled just once for all the targets.

Signed-off-by: Amit Shah 
---
 Makefile.hw |2 +-
 Makefile.target |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.hw b/Makefile.hw
index bd252f5..60b954e 100644
--- a/Makefile.hw
+++ b/Makefile.hw
@@ -13,7 +13,7 @@ QEMU_CFLAGS+=-I.. -I$(SRC_PATH)/fpu
 
 obj-y =
 obj-y += loader.o
-obj-y += virtio.o
+obj-y += virtio.o virtio-serial.o virtio-serial-bus.o
 obj-y += fw_cfg.o
 obj-y += watchdog.o
 obj-$(CONFIG_ECC) += ecc.o
diff --git a/Makefile.target b/Makefile.target
index d217f07..d7e1702 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -156,7 +156,7 @@ ifdef CONFIG_SOFTMMU
 obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial.o 
virtio-serial-bus.o virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 LIBS+=-lz
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests

[Amit Shah]

Guests send us one buffer at a time. Current guests send buffers sized
4K bytes. If guest userspace applications sent out > 4K bytes in one
write() syscall, the write request actually sends out multiple buffers,
each of 4K in size.

This usually isn't a problem but for some apps, like VNC, the entire
data has to be sent in one go to make copy/paste work fine. So if an app
on the guest sends out guest clipboard contents, it has to be sent to
the vnc server in one go as the guest app sent it.

For this to be done, we need the guest to send us START and END markers
for each write request so that we can find out complete buffers and send
them off to ports.

This needs us to buffer all the data that comes in from the guests, hold
it off till we see all the data corresponding to one write request,
merge it all in one buffer and then send it to the port the data was
destined for.

Also, we add support for caching of these buffers till a port indicates
it's ready to receive data.

We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.

We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we open
tell the guest to restart sending data.

Signed-off-by: Amit Shah 
---
 hw/virtio-serial-bus.c |  305 +++-
 hw/virtio-serial.c |   11 +-
 hw/virtio-serial.h |   39 ++
 3 files changed, 348 insertions(+), 7 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 0a953cd..bc47629 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -46,6 +46,20 @@ struct VirtIOSerial {
 uint32_t guest_features;
 };
 
+/* This struct holds individual buffers received for each port */
+typedef struct VirtIOSerialPortBuffer {
+QTAILQ_ENTRY(VirtIOSerialPortBuffer) next;
+
+uint8_t *buf;
+
+size_t len; /* length of the buffer */
+size_t offset; /* offset from which to consume data in the buffer */
+
+uint32_t flags; /* Sent by guest (start of data stream, end of stream) */
+
+bool previous_failure; /* Did sending out this buffer fail previously? */
+} VirtIOSerialPortBuffer;
+
 static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
 {
 VirtIOSerialPort *port;
@@ -159,6 +173,197 @@ static size_t send_control_event(VirtIOSerialPort *port, 
uint16_t event,
 return send_control_msg(port, &cpkt, sizeof(cpkt));
 }
 
+static void init_buf(VirtIOSerialPortBuffer *buf, uint8_t *buffer, size_t len)
+{
+buf->buf = buffer;
+buf->len = len;
+buf->offset = 0;
+buf->flags = 0;
+buf->previous_failure = false;
+}
+
+static VirtIOSerialPortBuffer *alloc_buf(size_t len)
+{
+VirtIOSerialPortBuffer *buf;
+
+buf = qemu_malloc(sizeof(*buf));
+buf->buf = qemu_malloc(len);
+
+init_buf(buf, buf->buf, len);
+
+return buf;
+}
+
+static void free_buf(VirtIOSerialPortBuffer *buf)
+{
+qemu_free(buf->buf);
+qemu_free(buf);
+}
+
+static size_t get_complete_data_size(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf;
+size_t size;
+bool is_complete, start_seen;
+
+size = 0;
+is_complete = false;
+start_seen = false;
+QTAILQ_FOREACH(buf, &port->unflushed_buffers, next) {
+size += buf->len - buf->offset;
+
+if (buf->flags & VIRTIO_CONSOLE_HDR_END_DATA) {
+is_complete = true;
+break;
+}
+if (buf == QTAILQ_FIRST(&port->unflushed_buffers)
+&& !(buf->flags & VIRTIO_CONSOLE_HDR_START_DATA)) {
+
+/* There's some data that arrived without a START flag. Flush it. 
*/
+is_complete = true;
+break;
+}
+
+if (buf->flags & VIRTIO_CONSOLE_HDR_START_DATA) {
+if (start_seen) {
+/*
+ * There's some data that arrived without an END
+ * flag. Flush it.
+ */
+size -= buf->len + buf->offset;
+is_complete = true;
+break;
+}
+start_seen = true;
+}
+}
+return is_complete ? size : 0;
+}
+
+/*
+ * The guest could have sent the data corresponding to one write
+ * request split up in multiple buffers. The first buffer has the
+ * VIRTIO_CONSOLE_HDR_START_DATA flag set and the last buffer has the
+ * VIRTIO_CONSOLE_HDR_END_DATA flag set. Using this information, merge
+ * the parts into one buf here to process it for output.
+ */
+static VirtIOSerialPortBuffer *get_complete_buf(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf, *buf2;
+uint8_t *outbuf;
+size_t out_offset, out_size;
+
+out_size = get_complete_

[Qemu-devel] [PATCH 6/8] virtio-serial-bus: Add ability to hot-unplug ports

[Amit Shah]

Signed-off-by: Amit Shah 
---
 hw/virtio-serial-bus.c |2 ++
 hw/virtio-serial.h |1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index bc47629..cf532f6 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -843,6 +843,8 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
 VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
 VirtIOSerial *vser = port->vser;
 
+send_control_event(port, VIRTIO_CONSOLE_PORT_REMOVE, 1);
+
 /*
  * Don't decrement nr_ports here; thus we keep a linearly
  * increasing port id. Not utilising an id again saves us a couple
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index acb601d..13f6989 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -61,6 +61,7 @@ struct virtio_console_header {
 #define VIRTIO_CONSOLE_PORT_NAME   4
 #define VIRTIO_CONSOLE_THROTTLE_PORT   5
 #define VIRTIO_CONSOLE_CACHE_BUFFERS   6
+#define VIRTIO_CONSOLE_PORT_REMOVE 7
 
 /* == In-qemu interface == */
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/8] virtio-serial-bus: Add a port 'name' property for port discovery in guests

[Amit Shah]

The port 'id' or number is internal state between the guest kernel and
our bus implementation. This is invocation-dependent and isn't part of
the guest-host ABI.

To correcly enumerate and map ports between the host and the guest, the
'name' property is used.

Example:

-device virtserialport,name=org.qemu.port.0

This invocation will get us a char device in the guest at:

/dev/virtio-ports/org.qemu.port.0

which can be a symlink to

/dev/vport0p3

This 'name' property is exposed by the guest kernel in a sysfs
attribute:

/sys/kernel/virtio-ports/vport0p3/name

A simple udev script can pick up this name and create the symlink
mentioned above.

Signed-off-by: Amit Shah 
---
 hw/virtio-serial-bus.c |   16 
 hw/virtio-serial.c |1 +
 hw/virtio-serial.h |8 
 3 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index b961375..0a953cd 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -228,6 +228,8 @@ static void handle_control_message(VirtIOSerial *vser, void 
*buf)
 {
 struct VirtIOSerialPort *port;
 struct virtio_console_control *cpkt;
+uint8_t *buffer;
+size_t buffer_len;
 
 cpkt = buf;
 port = find_port_by_id(vser, le32_to_cpu(cpkt->id));
@@ -249,6 +251,20 @@ static void handle_control_message(VirtIOSerial *vser, 
void *buf)
 if (port->is_console) {
 send_control_event(port, VIRTIO_CONSOLE_CONSOLE_PORT, 1);
 }
+if (port->name) {
+cpkt->event = cpu_to_le16(VIRTIO_CONSOLE_PORT_NAME);
+cpkt->value = cpu_to_le16(1);
+
+buffer_len = sizeof(*cpkt) + strlen(port->name) + 1;
+buffer = qemu_malloc(buffer_len);
+
+memcpy(buffer, cpkt, sizeof(*cpkt));
+memcpy(buffer + sizeof(*cpkt), port->name, strlen(port->name));
+buffer[buffer_len - 1] = 0;
+
+send_control_msg(port, buffer, buffer_len);
+qemu_free(buffer);
+}
 if (port->host_connected) {
 send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
 }
diff --git a/hw/virtio-serial.c b/hw/virtio-serial.c
index d9a6f32..470446b 100644
--- a/hw/virtio-serial.c
+++ b/hw/virtio-serial.c
@@ -102,6 +102,7 @@ static VirtIOSerialPortInfo virtconsole_info = {
 .qdev.props = (Property[]) {
 DEFINE_PROP_UINT8("is_console", VirtConsole, port.is_console, 1),
 DEFINE_PROP_CHR("chardev", VirtConsole, chr),
+DEFINE_PROP_STRING("name", VirtConsole, port.name),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index b855375..5505841 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -54,6 +54,7 @@ struct virtio_console_header {
 #define VIRTIO_CONSOLE_CONSOLE_PORT1
 #define VIRTIO_CONSOLE_RESIZE  2
 #define VIRTIO_CONSOLE_PORT_OPEN   3
+#define VIRTIO_CONSOLE_PORT_NAME   4
 
 /* == In-qemu interface == */
 
@@ -88,6 +89,13 @@ struct VirtIOSerialPort {
 VirtQueue *ivq, *ovq;
 
 /*
+ * This name is sent to the guest and exported via sysfs.
+ * The guest could create symlinks based on this information.
+ * The name is in the reverse fqdn format, like org.qemu.console.0
+ */
+char *name;
+
+/*
  * This id helps identify ports between the guest and the host.
  * The guest sends a "header" with this id with each data packet
  * that it sends and the host can then find out which associated
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/8] virtio-serial-bus: Maintain guest and host port open/close state

[Amit Shah]

Via control channel messages, the guest can tell us whether a port got
opened or closed. Similarly, we can also indicate to the guest of host
port open/close events.

Signed-off-by: Amit Shah 
---
 hw/virtio-serial-bus.c |   73 
 hw/virtio-serial.c |6 
 hw/virtio-serial.h |6 
 3 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 83bc691..b961375 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -162,11 +162,22 @@ static size_t send_control_event(VirtIOSerialPort *port, 
uint16_t event,
 /* Functions for use inside qemu to open and read from/write to ports */
 int virtio_serial_open(VirtIOSerialPort *port)
 {
+/* Don't allow opening an already-open port */
+if (port->host_connected) {
+return 0;
+}
+/* Send port open notification to the guest */
+port->host_connected = true;
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
 return 0;
 }
 
 int virtio_serial_close(VirtIOSerialPort *port)
 {
+port->host_connected = false;
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
 return 0;
 }
 
@@ -174,6 +185,9 @@ int virtio_serial_close(VirtIOSerialPort *port)
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
 size_t size)
 {
+if (!port || !port->host_connected || !port->guest_connected) {
+return 0;
+}
 return write_to_port(port, buf, size);
 }
 
@@ -194,6 +208,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
 virtio_queue_empty(vq)) {
 return 0;
 }
+if (use_multiport(port->vser) && !port->guest_connected) {
+return 0;
+}
 
 size = 4096;
 if (virtqueue_avail_bytes(vq, size, 0)) {
@@ -232,6 +249,9 @@ static void handle_control_message(VirtIOSerial *vser, void 
*buf)
 if (port->is_console) {
 send_control_event(port, VIRTIO_CONSOLE_CONSOLE_PORT, 1);
 }
+if (port->host_connected) {
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+}
 /*
  * When the guest has asked us for this information it means
  * the guest is all setup and has its virtqueues
@@ -242,6 +262,17 @@ static void handle_control_message(VirtIOSerial *vser, 
void *buf)
 port->info->guest_ready(port);
 }
 break;
+case VIRTIO_CONSOLE_PORT_OPEN:
+port->guest_connected = cpkt->value;
+if (cpkt->value && port->info->guest_open) {
+/* Send the guest opened notification if an app is interested */
+port->info->guest_open(port);
+}
+if (!cpkt->value && port->info->guest_close) {
+/* Send the guest closed notification if an app is interested */
+port->info->guest_close(port);
+}
+break;
 }
 }
 
@@ -342,6 +373,8 @@ static void set_config(VirtIODevice *vdev, const uint8_t 
*config_data)
 static void virtio_serial_save(QEMUFile *f, void *opaque)
 {
 VirtIOSerial *s = opaque;
+VirtIOSerialPort *port;
+uint32_t nr_active_ports;
 
 /* The virtio device */
 virtio_save(&s->vdev, f);
@@ -352,11 +385,35 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
 
 /* Items in struct VirtIOSerial */
 qemu_put_be32s(f, &s->guest_features);
+
+/* Do this because we might have hot-unplugged some ports */
+nr_active_ports = 0;
+QTAILQ_FOREACH(port, &s->ports, next)
+nr_active_ports++;
+
+qemu_put_be32s(f, &nr_active_ports);
+
+/*
+ * Items in struct VirtIOSerialPort.
+ */
+QTAILQ_FOREACH(port, &s->ports, next) {
+/*
+ * We put the port number because we may not have an active
+ * port at id 0 that's reserved for a console port, or in case
+ * of ports that might have gotten unplugged
+ */
+qemu_put_be32s(f, &port->id);
+qemu_put_byte(f, port->guest_connected);
+
+}
 }
 
 static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
 VirtIOSerial *s = opaque;
+VirtIOSerialPort *port;
+uint32_t nr_active_ports;
+unsigned int i;
 
 if (version_id > 2) {
 return -EINVAL;
@@ -375,6 +432,18 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
int version_id)
 /* Items in struct VirtIOSerial */
 qemu_get_be32s(f, &s->guest_features);
 
+qemu_get_be32s(f, &nr_active_ports);
+
+/* Items in struct VirtIOSerialPort */
+for (i = 0; i < nr_active_ports; i++) {
+uint32_t id;
+
+id = qemu_get_be32(f);
+port = find_port_by_id(s, id);
+
+port->guest_connected = qemu_get_byte(f);
+
+}
 return 0;
 }
 
@@ -406,6 +475,10 @@ static void virtser_bus_dev_print(Monitor *mon, 
DeviceState *qdev, int indent)
indent, "", port->id);
 monitor_printf(mon, "%*s dev-prop-int: is_console: %d\n",

[Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus

[Amit Shah]

This commit converts the virtio-console device to create a new
virtio-serial bus that can host console and generic serial ports. The
file hosting this code is now called virtio-serial-bus.c.

The virtio console is now a very simple qdev device that sits on the
virtio-serial-bus and communicates between the bus and qemu's chardevs.

This commit also includes a few changes to the virtio backing code for
pci and s390 to spawn the virtio-serial bus.

As a result of the qdev conversion, we get rid of a lot of legacy code.
The old-style way of instantiating a virtio console using

-virtioconsole ...

is maintained, but the new, preferred way is to use

-device virtio-serial -device virtconsole,chardev=...

With this commit, multiple devices as well as multiple ports with a
single device can be supported.

For multiple ports support, each port gets an IO vq pair. Since the
guest needs to know in advance how many vqs a particular device will
need, we have to set this number as a property of the virtio-serial
device and also as a config option.

In addition, we also spawn a pair of control IO vqs. This is an internal
channel meant for guest-host communication for things like port
open/close, sending port properties over to the guest, etc.

This commit is a part of a series of other commits to get the full
implementation of multiport support. Future commits will add other
support as well as ride on the savevm version that we bump up here.

Signed-off-by: Amit Shah 
---
 Makefile.target|2 +-
 hw/pc.c|   11 +-
 hw/ppc440_bamboo.c |7 -
 hw/qdev.c  |8 +-
 hw/s390-virtio-bus.c   |   17 +-
 hw/s390-virtio-bus.h   |2 +
 hw/s390-virtio.c   |8 -
 hw/virtio-console.c|  143 -
 hw/virtio-console.h|   19 --
 hw/virtio-pci.c|   13 +-
 hw/virtio-serial-bus.c |  554 
 hw/virtio-serial.c |  107 ++
 hw/virtio-serial.h |  173 +++
 hw/virtio.h|2 +-
 qemu-options.hx|4 +
 sysemu.h   |6 -
 vl.c   |   17 ++-
 17 files changed, 879 insertions(+), 214 deletions(-)
 delete mode 100644 hw/virtio-console.c
 delete mode 100644 hw/virtio-console.h
 create mode 100644 hw/virtio-serial-bus.c
 create mode 100644 hw/virtio-serial.c
 create mode 100644 hw/virtio-serial.h

diff --git a/Makefile.target b/Makefile.target
index 7c1f30c..d217f07 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -156,7 +156,7 @@ ifdef CONFIG_SOFTMMU
 obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o 
virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial.o 
virtio-serial-bus.o virtio-pci.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 LIBS+=-lz
diff --git a/hw/pc.c b/hw/pc.c
index db7d58e..c5709e8 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1242,15 +1242,6 @@ static void pc_init1(ram_addr_t ram_size,
 }
 }
 
-/* Add virtio console devices */
-if (pci_enabled) {
-for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pci_bus, -1, "virtio-console-pci");
-}
-}
-}
-
 rom_load_fw(fw_cfg);
 }
 
@@ -1308,7 +1299,7 @@ static QEMUMachine pc_machine_v0_10 = {
 .property = "class",
 .value= stringify(PCI_CLASS_STORAGE_OTHER),
 },{
-.driver   = "virtio-console-pci",
+.driver   = "virtio-serial-pci",
 .property = "class",
 .value= stringify(PCI_CLASS_DISPLAY_OTHER),
 },{
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index a488240..1ab9872 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -108,13 +108,6 @@ static void bamboo_init(ram_addr_t ram_size,
 env = ppc440ep_init(&ram_size, &pcibus, pci_irq_nrs, 1, cpu_model);
 
 if (pcibus) {
-/* Add virtio console devices */
-for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pcibus, -1, "virtio-console-pci");
-}
-}
-
 /* Register network interfaces. */
 for (i = 0; i < nb_nics; i++) {
 /* There are no PCI NICs on the Bamboo board, but there are
diff --git a/hw/qdev.c b/hw/qdev.c
index b6bd4ae..38c6e15 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -321,13 +321,9 @@ void qdev_machine_creation_done(void)
 CharDriverState *qdev_init_chardev(DeviceState *dev)
 {
 static int next_serial;
-static int next_virtconsole;
+
 /* FIXME: This is a nasty hack that needs to go away.  */
-if (strncmp(dev->info->name, "virtio", 6) == 0) {
-return virtcon_hds[next_virtconsole++];
-} else {
-  

[Qemu-devel] [PATCH 0/8] virtio-console: Move to qdev, multiple devices, generic ports

[Amit Shah]

Hello,

This series addresses some comments from last time:
- locking is removed
- send/receive struct elements in endian-safe way
- appropriate device is created based on machine being used
  (virtio-serial-pci vs virtio-serial-s390).

This series splits up the patches by functionality. Note, however,
that patches 2-6 introduce some functionality that's advertised to the
guest as having to work all at once or not at all. Also, the savevm
version is bumped only once but save/restore state is added in each of
the patches. They are split only for easier reviewability.

The older virtio-console.c file is completely removed and a new
virtio-serial.c is introduced so that reviewing is easier. I can send a
later patch to rename virtio-serial.c back to virtio-console.c.

Amit Shah (8):
  virtio: Remove duplicate macro definition for max. virtqueues, bump
up the max
  virtio-console: qdev conversion, new virtio-serial-bus
  virtio-serial-bus: Maintain guest and host port open/close state
  virtio-serial-bus: Add a port 'name' property for port discovery in
guests
  virtio-serial-bus: Add support for buffering guest output, throttling
guests
  virtio-serial-bus: Add ability to hot-unplug ports
  virtio-serial: Add 'virtserialport' device for generic serial port
support
  Move virtio-serial and virtio-serial-bus to Makefile.hw

 Makefile.hw|2 +-
 Makefile.target|2 +-
 hw/pc.c|   11 +-
 hw/ppc440_bamboo.c |7 -
 hw/qdev.c  |8 +-
 hw/s390-virtio-bus.c   |   17 +-
 hw/s390-virtio-bus.h   |2 +
 hw/s390-virtio.c   |8 -
 hw/virtio-console.c|  143 
 hw/virtio-console.h|   19 -
 hw/virtio-pci.c|   13 +-
 hw/virtio-serial-bus.c |  946 
 hw/virtio-serial.c |  151 
 hw/virtio-serial.h |  227 
 hw/virtio.c|2 -
 hw/virtio.h|4 +-
 qemu-options.hx|4 +
 sysemu.h   |6 -
 vl.c   |   17 +-
 19 files changed, 1371 insertions(+), 218 deletions(-)
 delete mode 100644 hw/virtio-console.c
 delete mode 100644 hw/virtio-console.h
 create mode 100644 hw/virtio-serial-bus.c
 create mode 100644 hw/virtio-serial.c
 create mode 100644 hw/virtio-serial.h

[Qemu-devel] [PATCH 1/8] virtio: Remove duplicate macro definition for max. virtqueues, bump up the max

[Amit Shah]

VIRTIO_PCI_QUEUE_MAX is redefined in hw/virtio.c. Let's just keep it in
hw/virtio.h.

Also, bump up the value of the maximum allowed virtqueues to 64. This is
in preparation to allow multiple ports per virtio-console device.

Signed-off-by: Amit Shah 
---
 hw/virtio.c |2 --
 hw/virtio.h |2 +-
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index cecd0dc..88f4e78 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -75,8 +75,6 @@ struct VirtQueue
 void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
 };
 
-#define VIRTIO_PCI_QUEUE_MAX16
-
 /* virt queue functions */
 static void virtqueue_init(VirtQueue *vq)
 {
diff --git a/hw/virtio.h b/hw/virtio.h
index 35532a6..051910a 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -90,7 +90,7 @@ typedef struct {
 unsigned (*get_features)(void * opaque);
 } VirtIOBindings;
 
-#define VIRTIO_PCI_QUEUE_MAX 16
+#define VIRTIO_PCI_QUEUE_MAX 64
 
 #define VIRTIO_NO_VECTOR 0x
 
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH V4 00/18] support NEC PC-9821

[Roy Tam]

2009/12/23 TAKEDA, toshiya :
> Dear members,
>
> Sorry too late reply for comments of previous patch.
> (I was busy in these months for my business...)
> This is new patch.
>
> Change from v3 to v4:
> - use vmstate for save/load state
> - remove sound device with MAME opna sound core
>

+1 for adding it to git head.
but it still need real ITF and BASIC ROM in order to work.
I think a set of pseudo ROMs is needed too.

Anthony, what do you think?

Best regards,
Roy

> Thanks,
> TAKEDA, toshiya
>
>
>
>

[Qemu-devel] [PATCHv6 2/3] virtio: rename features -> guest_features

[Michael S. Tsirkin]

Rename features->guest_features. This is what they are, avoid confusion
with host features which we also need to keep around.

Signed-off-by: Michael S. Tsirkin 
---
 hw/s390-virtio-bus.c |2 +-
 hw/syborg_virtio.c   |4 ++--
 hw/virtio-net.c  |   10 +-
 hw/virtio-pci.c  |4 ++--
 hw/virtio.c  |8 
 hw/virtio.h  |2 +-
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index dc154ed..6c0da11 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -251,7 +251,7 @@ void s390_virtio_device_update_status(VirtIOS390Device *dev)
 if (vdev->set_features) {
 vdev->set_features(vdev, features);
 }
-vdev->features = features;
+vdev->guest_features = features;
 }
 
 VirtIOS390Device *s390_virtio_bus_console(VirtIOS390Bus *bus)
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index a84206a..fe6fc23 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -90,7 +90,7 @@ static uint32_t syborg_virtio_readl(void *opaque, 
target_phys_addr_t offset)
 ret |= vdev->binding->get_features(s);
 break;
 case SYBORG_VIRTIO_GUEST_FEATURES:
-ret = vdev->features;
+ret = vdev->guest_features;
 break;
 case SYBORG_VIRTIO_QUEUE_BASE:
 ret = virtio_queue_get_addr(vdev, vdev->queue_sel);
@@ -132,7 +132,7 @@ static void syborg_virtio_writel(void *opaque, 
target_phys_addr_t offset,
 case SYBORG_VIRTIO_GUEST_FEATURES:
 if (vdev->set_features)
 vdev->set_features(vdev, value);
-vdev->features = value;
+vdev->guest_features = value;
 break;
 case SYBORG_VIRTIO_QUEUE_BASE:
 if (value == 0)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 2f201ff..ab20a33 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -768,11 +768,11 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int 
version_id)
 if (n->has_vnet_hdr) {
 tap_using_vnet_hdr(n->nic->nc.peer, 1);
 tap_set_offload(n->nic->nc.peer,
-(n->vdev.features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
-(n->vdev.features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
-(n->vdev.features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
-(n->vdev.features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
-(n->vdev.features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
+(n->vdev.guest_features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
+(n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
+(n->vdev.guest_features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
+(n->vdev.guest_features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
+(n->vdev.guest_features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
 }
 }
 
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 62b46bd..4e1d5e1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -181,7 +181,7 @@ static void virtio_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
}
 if (vdev->set_features)
 vdev->set_features(vdev, val);
-vdev->features = val;
+vdev->guest_features = val;
 break;
 case VIRTIO_PCI_QUEUE_PFN:
 pa = (target_phys_addr_t)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
@@ -239,7 +239,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, 
uint32_t addr)
 ret |= vdev->binding->get_features(proxy);
 break;
 case VIRTIO_PCI_GUEST_FEATURES:
-ret = vdev->features;
+ret = vdev->guest_features;
 break;
 case VIRTIO_PCI_QUEUE_PFN:
 ret = virtio_queue_get_addr(vdev, vdev->queue_sel)
diff --git a/hw/virtio.c b/hw/virtio.c
index bb78ca8..f01548e 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -441,7 +441,7 @@ void virtio_reset(void *opaque)
 if (vdev->reset)
 vdev->reset(vdev);
 
-vdev->features = 0;
+vdev->guest_features = 0;
 vdev->queue_sel = 0;
 vdev->status = 0;
 vdev->isr = 0;
@@ -596,7 +596,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
 mb();
 /* Always notify when queue is empty (when feature acknowledge) */
 if ((vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT) &&
-(!(vdev->features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) ||
+(!(vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) ||
  (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
 return;
 
@@ -623,7 +623,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 qemu_put_8s(f, &vdev->status);
 qemu_put_8s(f, &vdev->isr);
 qemu_put_be16s(f, &vdev->queue_sel);
-qemu_put_be32s(f, &vdev->features);
+qemu_put_be32s(f, &vdev->guest_features);
 qemu_put_be32(f, vdev->config_len);
 qemu_put_buffer(f, vdev->config, vdev->config_len);
 
@@ -668,7 +668,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
  

[Qemu-devel] [PATCHv6 3/3] virtio: add features as qdev properties

[Michael S. Tsirkin]

Add feature bits as properties to virtio. This makes it possible to e.g. define
machine without indirect buffer support, which is required for 0.10
compatibility, or without hardware checksum support, which is required for 0.11
compatibility.  Since default values for optional features are now set by qdev,
get_features callback has been modified: it sets non-optional bits, and clears
bits not supported by host.

Signed-off-by: Michael S. Tsirkin 
Acked-by: Gerd Hoffmann 

---
 hw/s390-virtio-bus.c |   12 +---
 hw/s390-virtio-bus.h |1 +
 hw/syborg_virtio.c   |   12 +++-
 hw/virtio-balloon.c  |4 ++--
 hw/virtio-blk.c  |6 +-
 hw/virtio-blk.h  |8 
 hw/virtio-console.c  |4 ++--
 hw/virtio-net.c  |   39 ---
 hw/virtio-net.h  |   20 
 hw/virtio-pci.c  |   25 +
 hw/virtio.c  |2 +-
 hw/virtio.h  |7 ++-
 12 files changed, 90 insertions(+), 50 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 6c0da11..980e7eb 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -101,6 +101,7 @@ static int s390_virtio_device_init(VirtIOS390Device *dev, 
VirtIODevice *vdev)
 bus->dev_offs += dev_len;
 
 virtio_bind_device(vdev, &virtio_s390_bindings, dev);
+dev->host_features = vdev->get_features(vdev, dev->host_features);
 s390_virtio_device_sync(dev);
 
 return 0;
@@ -222,9 +223,7 @@ static void s390_virtio_device_sync(VirtIOS390Device *dev)
 cur_offs += num_vq * VIRTIO_VQCONFIG_LEN;
 
 /* Sync feature bitmap */
-if (dev->vdev->get_features) {
-stl_phys(cur_offs, dev->vdev->get_features(dev->vdev));
-}
+stl_phys(cur_offs, dev->host_features);
 
 dev->feat_offs = cur_offs + dev->feat_len;
 cur_offs += dev->feat_len * 2;
@@ -310,10 +309,17 @@ static void virtio_s390_notify(void *opaque, uint16_t 
vector)
 kvm_s390_virtio_irq(s390_cpu_addr2state(0), 0, token);
 }
 
+static unsigned virtio_s390_get_features(void *opaque)
+{
+VirtIOS390Device *dev = (VirtIOS390Device*)opaque;
+return dev->host_features;
+}
+
 / S390 Virtio Bus Device Descriptions ***/
 
 static const VirtIOBindings virtio_s390_bindings = {
 .notify = virtio_s390_notify,
+.get_features = virtio_s390_get_features,
 };
 
 static VirtIOS390DeviceInfo s390_virtio_net = {
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index ef36714..8ae2065 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -40,6 +40,7 @@ typedef struct VirtIOS390Device {
 VirtIODevice *vdev;
 DriveInfo *dinfo;
 NICConf nic;
+uint32_t host_features;
 } VirtIOS390Device;
 
 typedef struct VirtIOS390Bus {
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index fe6fc23..ca026ee 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -66,6 +66,7 @@ typedef struct {
 uint32_t int_enable;
 uint32_t id;
 NICConf nic;
+uint32_t host_features;
 } SyborgVirtIOProxy;
 
 static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
@@ -86,8 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, 
target_phys_addr_t offset)
 ret = s->id;
 break;
 case SYBORG_VIRTIO_HOST_FEATURES:
-ret = vdev->get_features(vdev);
-ret |= vdev->binding->get_features(s);
+ret = s->host_features;
 break;
 case SYBORG_VIRTIO_GUEST_FEATURES:
 ret = vdev->guest_features;
@@ -244,9 +244,8 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t 
vector)
 
 static unsigned syborg_virtio_get_features(void *opaque)
 {
-unsigned ret = 0;
-ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
-return ret;
+SyborgVirtIOProxy *proxy = opaque;
+return proxy->host_features;
 }
 
 static VirtIOBindings syborg_virtio_bindings = {
@@ -272,6 +271,8 @@ static int syborg_virtio_init(SyborgVirtIOProxy *proxy, 
VirtIODevice *vdev)
 qemu_register_reset(virtio_reset, vdev);
 
 virtio_bind_device(vdev, &syborg_virtio_bindings, proxy);
+proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+proxy->host_features = vdev->get_features(vdev, proxy->host_features);
 return 0;
 }
 
@@ -292,6 +293,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 .qdev.size  = sizeof(SyborgVirtIOProxy),
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
+DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index cfd3b41..e17880f 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -124,9 +124,9 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 dev->actual = config.actual;
 }
 
-static uint32_t virtio_balloon_get_features(VirtIODevice *vdev)
+static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint

[Qemu-devel] [PATCHv6 1/3] qdev: add bit property type

[Michael S. Tsirkin]

This adds "bit" property type, which is a boolean stored in a 32 bit
integer field, with legal values on and off.  Will be used by virtio for
feature bits.

Signed-off-by: Michael S. Tsirkin 
Acked-by: Gerd Hoffmann 
---
 hw/qdev-properties.c |   62 -
 hw/qdev.h|   11 +
 2 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 217ddc0..9e123ae 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -9,6 +9,59 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 return ptr;
 }
 
+static uint32_t qdev_get_prop_mask(Property *prop)
+{
+assert(prop->info->type == PROP_TYPE_BIT);
+return 0x1 << prop->bitnr;
+}
+
+static void bit_prop_set(DeviceState *dev, Property *props, bool val)
+{
+uint32_t *p = qdev_get_prop_ptr(dev, props);
+uint32_t mask = qdev_get_prop_mask(props);
+if (val)
+*p |= ~mask;
+else
+*p &= ~mask;
+}
+
+static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
+{
+if (props->info->type == PROP_TYPE_BIT) {
+bool *defval = src;
+bit_prop_set(dev, props, *defval);
+} else {
+char *dst = qdev_get_prop_ptr(dev, props);
+memcpy(dst, src, props->info->size);
+}
+}
+
+/* Bit */
+static int parse_bit(DeviceState *dev, Property *prop, const char *str)
+{
+if (!strncasecmp(str, "on", 2))
+bit_prop_set(dev, prop, true);
+else if (!strncasecmp(str, "off", 3))
+bit_prop_set(dev, prop, false);
+else
+return -1;
+return 0;
+}
+
+static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+uint8_t *p = qdev_get_prop_ptr(dev, prop);
+return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
+}
+
+PropertyInfo qdev_prop_bit = {
+.name  = "on/off",
+.type  = PROP_TYPE_BIT,
+.size  = sizeof(uint32_t),
+.parse = parse_bit,
+.print = print_bit,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
@@ -511,7 +564,6 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
const char *value)
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum 
PropertyType type)
 {
 Property *prop;
-void *dst;
 
 prop = qdev_prop_find(dev, name);
 if (!prop) {
@@ -524,8 +576,7 @@ void qdev_prop_set(DeviceState *dev, const char *name, void 
*src, enum PropertyT
 __FUNCTION__, dev->info->name, name);
 abort();
 }
-dst = qdev_get_prop_ptr(dev, prop);
-memcpy(dst, src, prop->info->size);
+qdev_prop_cpy(dev, prop, src);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
@@ -585,14 +636,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char 
*name, void *value)
 
 void qdev_prop_set_defaults(DeviceState *dev, Property *props)
 {
-char *dst;
-
 if (!props)
 return;
 while (props->name) {
 if (props->defval) {
-dst = qdev_get_prop_ptr(dev, props);
-memcpy(dst, props->defval, props->info->size);
+qdev_prop_cpy(dev, props, props->defval);
 }
 props++;
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index bbcdba1..07b9603 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -64,6 +64,7 @@ struct Property {
 const char   *name;
 PropertyInfo *info;
 int  offset;
+int  bitnr;
 void *defval;
 };
 
@@ -82,6 +83,7 @@ enum PropertyType {
 PROP_TYPE_NETDEV,
 PROP_TYPE_VLAN,
 PROP_TYPE_PTR,
+PROP_TYPE_BIT,
 };
 
 struct PropertyInfo {
@@ -173,6 +175,7 @@ void do_device_del(Monitor *mon, const QDict *qdict);
 
 /*** qdev-properties.c ***/
 
+extern PropertyInfo qdev_prop_bit;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -202,6 +205,14 @@ extern PropertyInfo qdev_prop_pci_devfn;
 + type_check(_type,typeof_field(_state, _field)),   \
 .defval= (_type[]) { _defval }, \
 }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
+.name  = (_name),\
+.info  = &(qdev_prop_bit),   \
+.bitnr= (_bit),  \
+.offset= offsetof(_state, _field)\
++ type_check(uint32_t,typeof_field(_state, _field)), \
+.defval= (bool[]) { (_defval) }, \
+}
 
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)   \
 DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-- 
1.6.6.rc1.43.gf55cc

[Qemu-devel] [PATCHv6 0/3] qemu: properties for feature compatibility

[Michael S. Tsirkin]

Here's what I came up with for solving the problem of differences in
features in virtio between 0.12 and 0.11.  This also enables migration
between different backends, e.g.  between host where tap supports virtio
net header and where it does not: management just needs to set features
appropriately.
Compat bits are yet to be set, will be done when this patch is applied.

Changes since v5:
- added patch to rename features->guest_features

Changes since v4:
- Save bit offset in a separate field, offset is in bytes

Changes since RFC v2:
- we already have a flag to control write cache feature,
  let's not add another one

Changes since RFC:
- add symbolic names for properties
- only optional features can be changed

Acked-by: Gerd Hoffmann 


Michael S. Tsirkin (3):
  qdev: add bit property type
  virtio: rename features -> guest_features
  virtio: add features as qdev properties

 hw/qdev-properties.c |   62 -
 hw/qdev.h|   11 +
 hw/s390-virtio-bus.c |   14 ---
 hw/s390-virtio-bus.h |1 +
 hw/syborg_virtio.c   |   16 +++-
 hw/virtio-balloon.c  |4 +-
 hw/virtio-blk.c  |6 +
 hw/virtio-blk.h  |8 ++
 hw/virtio-console.c  |4 +-
 hw/virtio-net.c  |   49 +--
 hw/virtio-net.h  |   20 
 hw/virtio-pci.c  |   29 +++
 hw/virtio.c  |   10 
 hw/virtio.h  |9 +-
 14 files changed, 171 insertions(+), 72 deletions(-)

Re: [Qemu-devel] [PATCH 2/3] virtio-console: Add a virtio-serial bus, support for multiple devices and ports

[Gerd Hoffmann]

On 12/24/09 09:26, Markus Armbruster wrote:

We could declare that -virtioconsole always creates a bus a feature, but
I can do that in good conscience only if the console created along with
it always gets attached to it, even when other suitable buses exist.


I'd just discourage mixing -virtioconsole and -device virtio-serial-$bus

The whole point of -virtioconsole is backward compatibility for 
management apps / start scripts and the like and it should create a 
device which can be used by the guest like the qemu 0.11 virtioconsole.


If you are using device virtio-serial-$bus you are obviously not needing 
the backward compatibility stuff ;)


cheers,
  Gerd

[Qemu-devel] Re: [PATCHv5 0/2] qemu: properties for feature compatibility

[Gerd Hoffmann]

On 01/04/10 16:36, Michael S. Tsirkin wrote:

Here's what I came up with for solving the problem of differences in
features in virtio between 0.12 and 0.11.  This also enables migration
between different backends, e.g.  between host where tap supports virtio
net header and where it does not: management just needs to set features
appropriately.


Looks good to me now.

Acked-by: Gerd Hoffmann 

cheers,
  Gerd

[Qemu-devel] Re: [ANNOUNCE] qemu-kvm-0.12.1.1

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 04:22:25PM +0100, Gerd Hoffmann wrote:
> On 12/23/09 12:37, Avi Kivity wrote:
>> On 12/23/2009 12:58 PM, Thomas Treutner wrote:
>>> On Wednesday 23 December 2009 11:24:04 Avi Kivity wrote:
 Please post a full log, after 'make clean'.
>>> http://pastebin.com/f404c8648
>>>
>>
>> Oh, I missed it at first - looks like libxenguest and libxenctrl conflict.
>
> Indeed, there are (un)lock_pages functions in both libraries.  It is  
> fixed in xen 3.3+, where libxenguest doesn't has these functions any 
> more.
>
> /me also wonders why Debian seems to have only static xen libraries.
> I think when linking against the shared libraries avoids this too as the  
> functions are supposed to be library-internal.
>
>> Copying Gerd for an opinion.
>
> I think there isn't much we can do about this, it is clearly a xen bug.
>
> Uhm, well, while thinking about it:  The test app compiled and linked by  
> configure should have failed in a simliar way, thereby automatically  
> disabling xen support.  I have no idea why it didn't ...

"static library" is in fact just an archive of objects.
If you happen not to pull in the conflicting objects,
you won't see the conflict.
So test file would have to call lock_pages/unlock_pages
for config to detect the issue.

> cheers,
>   Gerd
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Qemu-devel] [PATCHv5 2/2] virtio: add features as qdev properties

[Michael S. Tsirkin]

Add feature bits as properties to virtio. This makes it possible to e.g. define
machine without indirect buffer support, which is required for 0.10
compatibility, or without hardware checksum support, which is required for 0.11
compatibility.  Since default values for optional features are now set by qdev,
get_features callback has been modified: it sets non-optional bits, and clears
bits not supported by host.

Signed-off-by: Michael S. Tsirkin 
---
 hw/s390-virtio-bus.c |   12 +---
 hw/s390-virtio-bus.h |1 +
 hw/syborg_virtio.c   |   12 +++-
 hw/virtio-balloon.c  |4 ++--
 hw/virtio-blk.c  |6 +-
 hw/virtio-blk.h  |8 
 hw/virtio-console.c  |4 ++--
 hw/virtio-net.c  |   39 ---
 hw/virtio-net.h  |   20 
 hw/virtio-pci.c  |   25 +
 hw/virtio.c  |2 +-
 hw/virtio.h  |7 ++-
 12 files changed, 90 insertions(+), 50 deletions(-)

diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 734a579..ac756c8 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -101,6 +101,7 @@ static int s390_virtio_device_init(VirtIOS390Device *dev, 
VirtIODevice *vdev)
 bus->dev_offs += dev_len;
 
 virtio_bind_device(vdev, &virtio_s390_bindings, dev);
+dev->host_features = vdev->get_features(vdev, dev->host_features);
 s390_virtio_device_sync(dev);
 
 return 0;
@@ -222,9 +223,7 @@ static void s390_virtio_device_sync(VirtIOS390Device *dev)
 cur_offs += num_vq * VIRTIO_VQCONFIG_LEN;
 
 /* Sync feature bitmap */
-if (dev->vdev->get_features) {
-stl_phys(cur_offs, dev->vdev->get_features(dev->vdev));
-}
+stl_phys(cur_offs, dev->host_features);
 
 dev->feat_offs = cur_offs + dev->feat_len;
 cur_offs += dev->feat_len * 2;
@@ -310,10 +309,17 @@ static void virtio_s390_notify(void *opaque, uint16_t 
vector)
 kvm_s390_virtio_irq(s390_cpu_addr2state(0), 1, token);
 }
 
+static unsigned virtio_s390_get_features(void *opaque)
+{
+VirtIOS390Device *dev = (VirtIOS390Device*)opaque;
+return dev->host_features;
+}
+
 / S390 Virtio Bus Device Descriptions ***/
 
 static const VirtIOBindings virtio_s390_bindings = {
 .notify = virtio_s390_notify,
+.get_features = virtio_s390_get_features,
 };
 
 static VirtIOS390DeviceInfo s390_virtio_net = {
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index ef36714..8ae2065 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -40,6 +40,7 @@ typedef struct VirtIOS390Device {
 VirtIODevice *vdev;
 DriveInfo *dinfo;
 NICConf nic;
+uint32_t host_features;
 } VirtIOS390Device;
 
 typedef struct VirtIOS390Bus {
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index fe6fc23..ca026ee 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -66,6 +66,7 @@ typedef struct {
 uint32_t int_enable;
 uint32_t id;
 NICConf nic;
+uint32_t host_features;
 } SyborgVirtIOProxy;
 
 static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
@@ -86,8 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, 
target_phys_addr_t offset)
 ret = s->id;
 break;
 case SYBORG_VIRTIO_HOST_FEATURES:
-ret = vdev->get_features(vdev);
-ret |= vdev->binding->get_features(s);
+ret = s->host_features;
 break;
 case SYBORG_VIRTIO_GUEST_FEATURES:
 ret = vdev->guest_features;
@@ -244,9 +244,8 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t 
vector)
 
 static unsigned syborg_virtio_get_features(void *opaque)
 {
-unsigned ret = 0;
-ret |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
-return ret;
+SyborgVirtIOProxy *proxy = opaque;
+return proxy->host_features;
 }
 
 static VirtIOBindings syborg_virtio_bindings = {
@@ -272,6 +271,8 @@ static int syborg_virtio_init(SyborgVirtIOProxy *proxy, 
VirtIODevice *vdev)
 qemu_register_reset(virtio_reset, vdev);
 
 virtio_bind_device(vdev, &syborg_virtio_bindings, proxy);
+proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+proxy->host_features = vdev->get_features(vdev, proxy->host_features);
 return 0;
 }
 
@@ -292,6 +293,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 .qdev.size  = sizeof(SyborgVirtIOProxy),
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
+DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index cfd3b41..e17880f 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -124,9 +124,9 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 dev->actual = config.actual;
 }
 
-static uint32_t virtio_balloon_get_features(VirtIODevice *vdev)
+static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-return 0;

[Qemu-devel] Re: [PATCHv4 1/2] qdev: add bit property type

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 03:22:36PM +0100, Gerd Hoffmann wrote:
> On 12/24/09 13:43, Michael S. Tsirkin wrote:
>> This adds "bit" property type, which is a boolean stored in a 32 bit
>> integer field, with legal values on and off.  Will be used by virtio for
>> feature bits.
>
>> +static void *qdev_bit_prop_ptr(DeviceState *dev, Property *prop)
>> +{
>> +void *ptr = dev;
>> +assert(prop->info->type == PROP_TYPE_BIT);
>> +ptr += prop->offset / 32;
>> +return ptr;
>> +}
>
> I don't like that idea.  IMHO prop->offset should be specified in bytes  
> no matter what the property is.  Better add a new prop->bitnr field to  
> specify which bit of the 32bit word should be toggled.  Then you don't  
> need this function in the first place.
>
> cheers,
>   Gerd

OK, I reworked the patches in this way.

[Qemu-devel] Re: [PATCHv4 2/2] virtio: add features as qdev properties

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 03:24:18PM +0100, Gerd Hoffmann wrote:
> On 12/24/09 13:44, Michael S. Tsirkin wrote:
>> Add feature bits as properties to virtio. This makes it possible to e.g. 
>> define
>> machine without indirect buffer support, which is required for 0.10
>> compatibility, or without hardware checksum support, which is required for 
>> 0.11
>> compatibility.  Since default values for optional features are now set by 
>> qdev,
>> get_features callback has been modified: it sets non-optional bits, and 
>> clears
>> bits not supported by host.
>
> I guess you'll need a bunch of entries for pc-0.10 and pc-0.11 to switch  
> the virtio devices into the correct compatibility modes?  Should  
> probably go as patch #3 of this series.
>
> cheers,
>   Gerd

Yes, but this will be done separately, after these are applied.

-- 
MST

[Qemu-devel] [PATCHv5 1/2] qdev: add bit property type

[Michael S. Tsirkin]

This adds "bit" property type, which is a boolean stored in a 32 bit
integer field, with legal values on and off.  Will be used by virtio for
feature bits.

Signed-off-by: Michael S. Tsirkin 
---
 hw/qdev-properties.c |   62 -
 hw/qdev.h|   11 +
 2 files changed, 66 insertions(+), 7 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index fb07279..9463762 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -9,6 +9,59 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 return ptr;
 }
 
+static uint32_t qdev_get_prop_mask(Property *prop)
+{
+assert(prop->info->type == PROP_TYPE_BIT);
+return 0x1 << prop->bitnr;
+}
+
+static void bit_prop_set(DeviceState *dev, Property *props, bool val)
+{
+uint32_t *p = qdev_get_prop_ptr(dev, props);
+uint32_t mask = qdev_get_prop_mask(props);
+if (val)
+*p |= ~mask;
+else
+*p &= ~mask;
+}
+
+static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
+{
+if (props->info->type == PROP_TYPE_BIT) {
+bool *defval = src;
+bit_prop_set(dev, props, *defval);
+} else {
+char *dst = qdev_get_prop_ptr(dev, props);
+memcpy(dst, src, props->info->size);
+}
+}
+
+/* Bit */
+static int parse_bit(DeviceState *dev, Property *prop, const char *str)
+{
+if (!strncasecmp(str, "on", 2))
+bit_prop_set(dev, prop, true);
+else if (!strncasecmp(str, "off", 3))
+bit_prop_set(dev, prop, false);
+else
+return -1;
+return 0;
+}
+
+static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+uint8_t *p = qdev_get_prop_ptr(dev, prop);
+return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
+}
+
+PropertyInfo qdev_prop_bit = {
+.name  = "on/off",
+.type  = PROP_TYPE_BIT,
+.size  = sizeof(uint32_t),
+.parse = parse_bit,
+.print = print_bit,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
@@ -506,7 +559,6 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
const char *value)
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum 
PropertyType type)
 {
 Property *prop;
-void *dst;
 
 prop = qdev_prop_find(dev, name);
 if (!prop) {
@@ -519,8 +571,7 @@ void qdev_prop_set(DeviceState *dev, const char *name, void 
*src, enum PropertyT
 __FUNCTION__, dev->info->name, name);
 abort();
 }
-dst = qdev_get_prop_ptr(dev, prop);
-memcpy(dst, src, prop->info->size);
+qdev_prop_cpy(dev, prop, src);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
@@ -580,14 +631,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char 
*name, void *value)
 
 void qdev_prop_set_defaults(DeviceState *dev, Property *props)
 {
-char *dst;
-
 if (!props)
 return;
 while (props->name) {
 if (props->defval) {
-dst = qdev_get_prop_ptr(dev, props);
-memcpy(dst, props->defval, props->info->size);
+qdev_prop_cpy(dev, props, props->defval);
 }
 props++;
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index bbcdba1..07b9603 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -64,6 +64,7 @@ struct Property {
 const char   *name;
 PropertyInfo *info;
 int  offset;
+int  bitnr;
 void *defval;
 };
 
@@ -82,6 +83,7 @@ enum PropertyType {
 PROP_TYPE_NETDEV,
 PROP_TYPE_VLAN,
 PROP_TYPE_PTR,
+PROP_TYPE_BIT,
 };
 
 struct PropertyInfo {
@@ -173,6 +175,7 @@ void do_device_del(Monitor *mon, const QDict *qdict);
 
 /*** qdev-properties.c ***/
 
+extern PropertyInfo qdev_prop_bit;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -202,6 +205,14 @@ extern PropertyInfo qdev_prop_pci_devfn;
 + type_check(_type,typeof_field(_state, _field)),   \
 .defval= (_type[]) { _defval }, \
 }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
+.name  = (_name),\
+.info  = &(qdev_prop_bit),   \
+.bitnr= (_bit),  \
+.offset= offsetof(_state, _field)\
++ type_check(uint32_t,typeof_field(_state, _field)), \
+.defval= (bool[]) { (_defval) }, \
+}
 
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)   \
 DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-- 
1.6.6.rc1.43.gf55cc

[Qemu-devel] [PATCHv5 0/2] qemu: properties for feature compatibility

[Michael S. Tsirkin]

Here's what I came up with for solving the problem of differences in
features in virtio between 0.12 and 0.11.  This also enables migration
between different backends, e.g.  between host where tap supports virtio
net header and where it does not: management just needs to set features
appropriately.
Compat bits are yet to be set, will be done when this patch is applied.

Changes since v4:
- Save bit offset in a separate field, offset is in bytes

Changes since RFC v2:
- we already have a flag to control write cache feature,
  let's not add another one

Changes since RFC:
- add symbolic names for properties
- only optional features can be changed
Comments? Gerd, what do you think?



Michael S. Tsirkin (2):
  qdev: add bit property type
  virtio: add features as qdev properties

 hw/qdev-properties.c |   62 -
 hw/qdev.h|   11 +
 hw/s390-virtio-bus.c |   12 +++--
 hw/s390-virtio-bus.h |1 +
 hw/syborg_virtio.c   |   12 +
 hw/virtio-balloon.c  |4 +-
 hw/virtio-blk.c  |6 +
 hw/virtio-blk.h  |8 ++
 hw/virtio-console.c  |4 +-
 hw/virtio-net.c  |   39 +--
 hw/virtio-net.h  |   20 
 hw/virtio-pci.c  |   25 +--
 hw/virtio.c  |2 +-
 hw/virtio.h  |7 +-
 14 files changed, 156 insertions(+), 57 deletions(-)

[Qemu-devel] Re: [ANNOUNCE] qemu-kvm-0.12.1.1

[Gerd Hoffmann]

On 12/23/09 12:37, Avi Kivity wrote:

On 12/23/2009 12:58 PM, Thomas Treutner wrote:

On Wednesday 23 December 2009 11:24:04 Avi Kivity wrote:

Please post a full log, after 'make clean'.

http://pastebin.com/f404c8648



Oh, I missed it at first - looks like libxenguest and libxenctrl conflict.


Indeed, there are (un)lock_pages functions in both libraries.  It is 
fixed in xen 3.3+, where libxenguest doesn't has these functions any more.


/me also wonders why Debian seems to have only static xen libraries.
I think when linking against the shared libraries avoids this too as the 
functions are supposed to be library-internal.



Copying Gerd for an opinion.


I think there isn't much we can do about this, it is clearly a xen bug.

Uhm, well, while thinking about it:  The test app compiled and linked by 
configure should have failed in a simliar way, thereby automatically 
disabling xen support.  I have no idea why it didn't ...


cheers,
  Gerd

[Qemu-devel] Re: [PATCH V10 00/27] split out piix specific part from pc emulator and some clean ups

[Gerd Hoffmann]

On 12/24/09 09:08, Isaku Yamahata wrote:

Anthony. Can you please merge those patches as Gerd has agreed
with the direction of this patch series.
Gerd, can you provide acked-by or reviewed-by for patches
you like?


Whole series looks pretty good to me, except for two minor nits (see 
replies to individual patches).


cheers,
  Gerd

[Qemu-devel] Re: [PATCH V10 22/27] pci hotplug: add argument to pci hot plug callback.

[Gerd Hoffmann]

On 12/24/09 09:09, Isaku Yamahata wrote:

The argument will be used later to remove global variable.



-static int piix4_device_hotplug(PCIDevice *dev, int state);
+static int piix4_device_hotplug(void *opaque, PCIDevice *dev, int state);


You are using that to pass the piix4-pm device state.  How about using 
"DeviceState*" instead of "void*" then?


cheers,
  Gerd

[Qemu-devel] Re: [PATCH V10 01/27] acpi: split out pc smbus routines from acpi.c into pc_smbus.c

[Gerd Hoffmann]

+void pc_smbus_init(PCSMBus *smb)
+{
+smb->smbus = i2c_init_bus(NULL, "i2c");
+}


This function should take a additional "DeviceState *parent" argument 
and pass it down to i2c_init_bus() instead of NULL.  Patch #21 
(acpi_piix4: qdevfy) should pass the piix4-pm DeviceState to 
pc_smbus_init().  Then the smbus and all connected devices should be 
correctly hooked up in the device tree and visible in 'info qtree'.


cheers,
  Gerd

[Qemu-devel] Re: [PATCHv4 2/2] virtio: add features as qdev properties

[Gerd Hoffmann]

On 12/24/09 13:44, Michael S. Tsirkin wrote:

Add feature bits as properties to virtio. This makes it possible to e.g. define
machine without indirect buffer support, which is required for 0.10
compatibility, or without hardware checksum support, which is required for 0.11
compatibility.  Since default values for optional features are now set by qdev,
get_features callback has been modified: it sets non-optional bits, and clears
bits not supported by host.


I guess you'll need a bunch of entries for pc-0.10 and pc-0.11 to switch 
the virtio devices into the correct compatibility modes?  Should 
probably go as patch #3 of this series.


cheers,
  Gerd

[Qemu-devel] Re: [PATCHv4 1/2] qdev: add bit property type

[Gerd Hoffmann]

On 12/24/09 13:43, Michael S. Tsirkin wrote:

This adds "bit" property type, which is a boolean stored in a 32 bit
integer field, with legal values on and off.  Will be used by virtio for
feature bits.



+static void *qdev_bit_prop_ptr(DeviceState *dev, Property *prop)
+{
+void *ptr = dev;
+assert(prop->info->type == PROP_TYPE_BIT);
+ptr += prop->offset / 32;
+return ptr;
+}


I don't like that idea.  IMHO prop->offset should be specified in bytes 
no matter what the property is.  Better add a new prop->bitnr field to 
specify which bit of the 32bit word should be toggled.  Then you don't 
need this function in the first place.


cheers,
  Gerd

[Qemu-devel] Re: [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)

[Gerd Hoffmann]

On 12/29/09 22:51, H. Peter Anvin wrote:

Add generic support for debugging consoles (simple I/O ports which
when written to cause debugging output to be written to a target.)
The current implementation matches Bochs' port 0xe9, allowing the same
debugging code to be used for both Bochs and Qemu.


Looks good to me.

Acked-by: Gerd Hoffmann 

cheers,
  Gerd

Re: [Qemu-devel] [Bug] qemu-system-ppc: "Bad clock read" when started with -icount auto

[Stefan Weil]

Tristan Gingold schrieb:
> On Dec 29, 2009, at 4:30 PM, Stefan Weil wrote:
>
>   
>> Hi,
>>
>> When qemu-system-ppc is started with argument "-icount auto"
>> (needed for running qemu in the emulated system),
>> it will print lots of "Bad clock read" on stderr.
>>
>> env->can_do_io toggles between 0 and 1.
>> 
>
> I have a patch to enable -icount on powerpc.  I plan to submit it shortly.  
> Are you in the hurry ?
>   

No. When you run qemu in qemu, you never are in a hurry :-)

Stefan

Re: [Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable

[Alexander Graf]

On 04.01.2010, at 12:07, Michael S. Tsirkin wrote:

> On Mon, Jan 04, 2010 at 07:45:16PM +0900, Isaku Yamahata wrote:
>> +static PCIDevice *pci_host_dev_find_fn_noswap(PCIHostState *s, uint32_t 
>> addr)
>> +{
>> +return pci_host_find_dev_active(s->bus, pci_host_pci_addr(s, addr));
>> +}
>> +
>> +static PCIDevice *pci_host_dev_find_fn(PCIHostState *s, uint32_t addr)
>> +{
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> +addr = bswap32(addr);
>> +#endif
>> +return pci_host_find_dev_active(s->bus, pci_host_pci_addr(s, addr));
>> +}
>> +
> 
> BTW, I think we really should think about the right way to address the
> swap/noswap issue without using a preprocessor. Maybe make pci host
> bridge explicitly specify whether to swap bytes?  How about adding a
> field in PCIHostState to make it do this?

Sounds reasonable. But let's take baby steps here. I don't want to end up with 
a 1 lines patch :-).


Alex

Re: [Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable

[Michael S. Tsirkin]

On Mon, Jan 04, 2010 at 07:45:16PM +0900, Isaku Yamahata wrote:
> +static PCIDevice *pci_host_dev_find_fn_noswap(PCIHostState *s, uint32_t addr)
> +{
> +return pci_host_find_dev_active(s->bus, pci_host_pci_addr(s, addr));
> +}
> +
> +static PCIDevice *pci_host_dev_find_fn(PCIHostState *s, uint32_t addr)
> +{
> +#ifdef TARGET_WORDS_BIGENDIAN
> +addr = bswap32(addr);
> +#endif
> +return pci_host_find_dev_active(s->bus, pci_host_pci_addr(s, addr));
> +}
> +

BTW, I think we really should think about the right way to address the
swap/noswap issue without using a preprocessor. Maybe make pci host
bridge explicitly specify whether to swap bytes?  How about adding a
field in PCIHostState to make it do this?

-- 
MST

Re: [Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable

[Isaku Yamahata]

On Mon, Jan 04, 2010 at 11:55:10AM +0100, Alexander Graf wrote:
> 
> On 04.01.2010, at 11:45, Isaku Yamahata wrote:
> 
> > On Mon, Jan 04, 2010 at 04:26:46AM +0100, Alexander Graf wrote:
> >> 
> >> On 03.01.2010, at 21:50, Benjamin Herrenschmidt wrote:
> >> 
> >>> On Sun, 2010-01-03 at 21:27 +0100, Alexander Graf wrote:
> >>> 
>  I think if unin_pci is the only user, it'd be better to do it hacky
>  inside unin_pci.c. But if there's a chance there's another user, it'd
>  be better to make it generic.
>  
>  Since this is the first time I ever stumbled across type 0 and type 1
>  PCI config space addresses, I simply don't know if there are any. Blue
>  Swirls comment indicated that there are. Ben also sounded as if it's
>  rather common to not use the x86 format. On the other hand, it looks
>  like nobody in qemu needed it so far - and we're implementing ARM,
>  MIPS and all other sorts of platforms.
>  
>  So if anyone with knowledge in PCI could shed some light here, please
>  do so.
> >>> 
> >>> My feeling is that what you're better off doing is to have the qemu core
> >>> take an abstract struct to identify a device config space location, that
> >>> consists of separate fields for:
> >>> 
> >>> - The PCI domain (which is what host bridge it hangs off since bus
> >>> numbers are not unique between domains)
> >>> 
> >>> - The bus number
> >> 
> >> Hm, I think it'd make more sense to just store a PCIBus pointer in there. 
> >> We could then fetch the bus and domain id from there.
> >> 
> >> I'll write something up :-).
> >> 
> >> 
> >> Alex
> >> 
> > 
> > Does the following patch help?
> > I did only compile test though.
> 
> I sent out v2 already, which contains a complete resolution framework. It 
> also allows for incremental cleanup of the users, as I'd rather like to see 
> everyone use pci_host instead of hacky homegrown functions. But I don't think 
> changing all at once is going to fly wrt reviewablity.

Agreed. Anyway that patch is just for discussion, not for commit.
If wanted, I'm willing to split it up into a series of patches or
rebase it on top of your patches. (Or throw it away)


> I'd be really glad if you could take a glimpse at my version. You're 
> definitely more knowledgable in the PCI areas than me :-). I verified that it 
> fixes Uninorth and x86 still works.

I'll have a look at it tomorrow.

-- 
yamahata

Re: [Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable

[Alexander Graf]

On 04.01.2010, at 11:45, Isaku Yamahata wrote:

> On Mon, Jan 04, 2010 at 04:26:46AM +0100, Alexander Graf wrote:
>> 
>> On 03.01.2010, at 21:50, Benjamin Herrenschmidt wrote:
>> 
>>> On Sun, 2010-01-03 at 21:27 +0100, Alexander Graf wrote:
>>> 
 I think if unin_pci is the only user, it'd be better to do it hacky
 inside unin_pci.c. But if there's a chance there's another user, it'd
 be better to make it generic.
 
 Since this is the first time I ever stumbled across type 0 and type 1
 PCI config space addresses, I simply don't know if there are any. Blue
 Swirls comment indicated that there are. Ben also sounded as if it's
 rather common to not use the x86 format. On the other hand, it looks
 like nobody in qemu needed it so far - and we're implementing ARM,
 MIPS and all other sorts of platforms.
 
 So if anyone with knowledge in PCI could shed some light here, please
 do so.
>>> 
>>> My feeling is that what you're better off doing is to have the qemu core
>>> take an abstract struct to identify a device config space location, that
>>> consists of separate fields for:
>>> 
>>> - The PCI domain (which is what host bridge it hangs off since bus
>>> numbers are not unique between domains)
>>> 
>>> - The bus number
>> 
>> Hm, I think it'd make more sense to just store a PCIBus pointer in there. We 
>> could then fetch the bus and domain id from there.
>> 
>> I'll write something up :-).
>> 
>> 
>> Alex
>> 
> 
> Does the following patch help?
> I did only compile test though.

I sent out v2 already, which contains a complete resolution framework. It also 
allows for incremental cleanup of the users, as I'd rather like to see everyone 
use pci_host instead of hacky homegrown functions. But I don't think changing 
all at once is going to fly wrt reviewablity.

I'd be really glad if you could take a glimpse at my version. You're definitely 
more knowledgable in the PCI areas than me :-). I verified that it fixes 
Uninorth and x86 still works.


Alex

Re: [Qemu-devel] Re: [PATCH 1/6] Make config space accessor host bus trapable

[Isaku Yamahata]

On Mon, Jan 04, 2010 at 04:26:46AM +0100, Alexander Graf wrote:
> 
> On 03.01.2010, at 21:50, Benjamin Herrenschmidt wrote:
> 
> > On Sun, 2010-01-03 at 21:27 +0100, Alexander Graf wrote:
> > 
> >> I think if unin_pci is the only user, it'd be better to do it hacky
> >> inside unin_pci.c. But if there's a chance there's another user, it'd
> >> be better to make it generic.
> >> 
> >> Since this is the first time I ever stumbled across type 0 and type 1
> >> PCI config space addresses, I simply don't know if there are any. Blue
> >> Swirls comment indicated that there are. Ben also sounded as if it's
> >> rather common to not use the x86 format. On the other hand, it looks
> >> like nobody in qemu needed it so far - and we're implementing ARM,
> >> MIPS and all other sorts of platforms.
> >> 
> >> So if anyone with knowledge in PCI could shed some light here, please
> >> do so.
> > 
> > My feeling is that what you're better off doing is to have the qemu core
> > take an abstract struct to identify a device config space location, that
> > consists of separate fields for:
> > 
> > - The PCI domain (which is what host bridge it hangs off since bus
> > numbers are not unique between domains)
> > 
> > - The bus number
> 
> Hm, I think it'd make more sense to just store a PCIBus pointer in there. We 
> could then fetch the bus and domain id from there.
> 
> I'll write something up :-).
> 
> 
> Alex
> 

Does the following patch help?
I did only compile test though.

>From 9c62b4846f95ebe84e182f76295016e1fe980699 Mon Sep 17 00:00:00 2001
From: Isaku Yamahata 
Date: Mon, 4 Jan 2010 19:39:36 +0900
Subject: [PATCH] pci: pcihost clean up.

remove some codes by introduce callback to calculate pci device and offset
in configuration space.

Signed-off-by: Isaku Yamahata 
---
 hw/gt64xxx.c   |9 +---
 hw/pci_host.c  |  115 
 hw/pci_host.h  |   17 ++-
 hw/pci_host_template.h |   21 +++--
 hw/prep_pci.c  |   68 +++--
 hw/sh_pci.c|   92 +-
 hw/versatile_pci.c |   74 +--
 7 files changed, 150 insertions(+), 246 deletions(-)

diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index fb7f5bd..b399f0c 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -530,8 +530,7 @@ static void gt64120_writel (void *opaque, 
target_phys_addr_t addr,
 case GT_PCI0_CFGDATA:
 if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800))
 val = bswap32(val);
-if (s->pci->config_reg & (1u << 31))
-pci_data_write(s->pci->bus, s->pci->config_reg, val, 4);
+pci_data_write(s->pci, 0, val, 4);
 break;
 
 /* Interrupts */
@@ -768,10 +767,7 @@ static uint32_t gt64120_readl (void *opaque,
 val = s->pci->config_reg;
 break;
 case GT_PCI0_CFGDATA:
-if (!(s->pci->config_reg & (1 << 31)))
-val = 0x;
-else
-val = pci_data_read(s->pci->bus, s->pci->config_reg, 4);
+val = pci_data_read(s->pci, 0, 4);
 if (!(s->regs[GT_PCI0_CMD] & 1) && (s->pci->config_reg & 0x00fff800))
 val = bswap32(val);
 break;
@@ -1119,6 +1115,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
 
 s = qemu_mallocz(sizeof(GT64120State));
 s->pci = qemu_mallocz(sizeof(GT64120PCIState));
+pci_host_init(s->pci, NULL, NULL);
 
 s->pci->bus = pci_register_bus(NULL, "pci",
pci_gt64120_set_irq, pci_gt64120_map_irq,
diff --git a/hw/pci_host.c b/hw/pci_host.c
index eeb8dee..6677175 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -40,6 +40,49 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while 
(0)
  */
 
 /* the helper functio to get a PCIDeice* for a given pci address */
+PCIDevice *pci_host_find_dev(PCIBus *bus, uint32_t addr)
+{
+uint8_t bus_num = addr >> 16;
+uint8_t devfn = addr >> 8;
+
+return pci_find_device(bus, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
+}
+
+uint32_t pci_host_config_offset(uint32_t addr)
+{
+return addr & (PCI_CONFIG_SPACE_SIZE - 1);
+}
+
+static uint32_t pci_host_pci_addr(PCIHostState *s, uint32_t addr)
+{
+return s->config_reg | (addr & 3);
+}
+static PCIDevice *pci_host_find_dev_active(PCIBus *bus, uint32_t pci_addr)
+{
+if (!(pci_addr & (1u << 31)))
+return NULL;
+
+return pci_host_find_dev(bus, pci_addr);
+}
+
+static PCIDevice *pci_host_dev_find_fn_noswap(PCIHostState *s, uint32_t addr)
+{
+return pci_host_find_dev_active(s->bus, pci_host_pci_addr(s, addr));
+}
+
+static PCIDevice *pci_host_dev_find_fn(PCIHostState *s, uint32_t addr)
+{
+#ifdef TARGET_WORDS_BIGENDIAN
+addr = bswap32(addr);
+#endif
+return pci_host_find_dev_active(s->bus, pci_host_pci_addr(s, addr));
+}
+
+static uint32_t pci_host_config_offset_fn(PCIHostState *s, uint32_t addr)
+{
+return pci_host_config_offset(pci_host_pci_addr(s, addr));
+}

Re: [Qemu-devel] [Bug] qemu-system-ppc: "Bad clock read" when started with -icount auto

[Tristan Gingold]

On Dec 29, 2009, at 4:30 PM, Stefan Weil wrote:

> Hi,
> 
> When qemu-system-ppc is started with argument "-icount auto"
> (needed for running qemu in the emulated system),
> it will print lots of "Bad clock read" on stderr.
> 
> env->can_do_io toggles between 0 and 1.

I have a patch to enable -icount on powerpc.  I plan to submit it shortly.  Are 
you in the hurry ?

[Qemu-devel] [PATCH] finish VPATH -> vpath translation

[Paolo Bonzini]

This adds a few more vpath suffixes and points the remaining two paths
explicitly to $(SRC_PATH) in order to eliminate the VPATH assignment
from config-host.mak.

Signed-off-by: Paolo Bonzini 
Cc: Juan Quintela 
---
 Makefile|2 +-
 Makefile.target |2 +-
 configure   |3 ---
 rules.mak   |3 ++-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 10c04a8..5c4e256 100644
--- a/Makefile
+++ b/Makefile
@@ -8,7 +8,7 @@ ifneq ($(wildcard config-host.mak),)
 all: build-all
 include config-host.mak
 include $(SRC_PATH)/rules.mak
-config-host.mak: configure
+config-host.mak: $(SRC_PATH)/configure
@echo $@ is out-of-date, running configure
@sed -n "/.*Configured with/s/[^:]*: //p" $@ | sh
 else
diff --git a/Makefile.target b/Makefile.target
index edcd2e5..5999da7 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -316,7 +316,7 @@ $(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) 
$(ARLIBS)
$(call LINK,$(obj-y) $(obj-$(TARGET_BASE_ARCH)-y))
 
 
-gdbstub-xml.c: $(TARGET_XML_FILES) feature_to_c.sh
+gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/feature_to_c.sh
$(call quiet-command,rm -f $@ && $(SHELL) $(SRC_PATH)/feature_to_c.sh 
$@ $(TARGET_XML_FILES),"  GEN   $(TARGET_DIR)$@")
 
 qemu-options.h: $(SRC_PATH)/qemu-options.hx
diff --git a/configure b/configure
index 18aed43..b7c6519 100755
--- a/configure
+++ b/configure
@@ -2022,9 +2022,6 @@ qemu_version=`head $source_path/VERSION`
 echo "VERSION=$qemu_version" >>$config_host_mak
 echo "PKGVERSION=$pkgversion" >>$config_host_mak
 echo "SRC_PATH=$source_path" >> $config_host_mak
-if [ "$source_path_used" = "yes" ]; then
-  echo "VPATH=$source_path" >> $config_host_mak
-fi
 echo "TARGET_DIRS=$target_list" >> $config_host_mak
 if [ "$docs" = "yes" ] ; then
   echo "BUILD_DOCS=yes" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 9cd67f0..82e8e3d 100644
--- a/rules.mak
+++ b/rules.mak
@@ -39,7 +39,8 @@ quiet-command = $(if $(V),$1,$(if $(2),@echo $2 && $1, @$1))
 cc-option = $(if $(shell $(CC) $1 $2 -S -o /dev/null -xc /dev/null \
   >/dev/null 2>&1 && echo OK), $2, $3)
 
-set-vpath = $(if $1,$(foreach PATTERN,%.c %.h %.S, $(eval vpath $(PATTERN) 
$1)))
+VPATH_SUFFIXES = %.c %.h %.S %.m %.mak %.texi
+set-vpath = $(if $1,$(foreach PATTERN,$(VPATH_SUFFIXES), $(eval vpath 
$(PATTERN) $1)))
 
 # Generate timestamp files for .h include files
 
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 2/3] virtio-console: Add a virtio-serial bus, support for multiple devices and ports

[Gerd Hoffmann]

  Hi,


@@ -321,13 +321,9 @@ void qdev_machine_creation_done(void)
  CharDriverState *qdev_init_chardev(DeviceState *dev)
  {
  static int next_serial;
-static int next_virtconsole;
+
  /* FIXME: This is a nasty hack that needs to go away.  */
-if (strncmp(dev->info->name, "virtio", 6) == 0) {
-return virtcon_hds[next_virtconsole++];
-} else {
-return serial_hds[next_serial++];
-}
+return serial_hds[next_serial++];
  }


I believe the FIXME is about the nasty special case for "virtio".  Since
you fix that, better remove the FIXME.


I did that in a previous submission and Gerd asked me to keep it. Even
the serial init can be changed, I guess.


Okay, Gerd's the authority on this.


Yes, serial drivers should use a chardev property instead of 
qdev_init_chardev().  You can try to zap this function altogether, but I 
think there are still serial drivers using this so this would break the 
(full, all archs) build.


cheers,
  Gerd

Re: [Qemu-devel] [PATCH] linux-user: adapt uname machine to emulated CPU

[Riku Voipio]

Hi,

Works great, thanks.

Acked-By: Riku Voipio 

On Tue, Dec 29, 2009 at 10:39:21PM +0100, Loïc Minier wrote:
> Hey there
> 
>  This patch for linux-user adapts the output of the emulated uname()
>  syscall to match the configured CPU.  Tested with x86, x86-64 and arm
>  emulation.
> 
>Thanks,
> 
> 
> ---
>  Makefile.target|2 +-
>  linux-user/cpu-uname.c |   72 
> 
>  linux-user/cpu-uname.h |1 +
>  linux-user/syscall.c   |3 +-
>  4 files changed, 76 insertions(+), 2 deletions(-)
>  create mode 100644 linux-user/cpu-uname.c
>  create mode 100644 linux-user/cpu-uname.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index 7c1f30c..6414472 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -90,7 +90,7 @@ ifdef CONFIG_LINUX_USER
>  VPATH+=:$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
>  QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user 
> -I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
>  obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
> -  elfload.o linuxload.o uaccess.o gdbstub.o
> +  elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o
>  
>  obj-$(TARGET_HAS_BFLT) += flatload.o
>  obj-$(TARGET_HAS_ELFLOAD32) += elfload32.o
> diff --git a/linux-user/cpu-uname.c b/linux-user/cpu-uname.c
> new file mode 100644
> index 000..ddc37be
> --- /dev/null
> +++ b/linux-user/cpu-uname.c
> @@ -0,0 +1,72 @@
> +/*
> + *  cpu to uname machine name map
> + *
> + *  Copyright (c) 2009 Loïc Minier
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see .
> + */
> +
> +#include 
> +
> +#include "qemu.h"
> +//#include "qemu-common.h"
> +#include "cpu-uname.h"
> +
> +/* return highest utsname machine name for emulated instruction set
> + *
> + * NB: the default emulated CPU ("any") might not match any existing CPU, 
> e.g.
> + * on ARM it has all features turned on, so there is no perfect arch string 
> to
> + * return here */
> +const char *cpu_to_uname_machine(void *cpu_env)
> +{
> +#ifdef TARGET_ARM
> +/* utsname machine name on linux arm is CPU arch name + endianness, e.g.
> + * armv7l; to get a list of CPU arch names from the linux source, use:
> + * grep arch_name: -A1 linux/arch/arm/mm/proc-*.S
> + * see arch/arm/kernel/setup.c: setup_processor()
> + *
> + * to test by CPU id, compare cpu_env->cp15.c0_cpuid to ARM_CPUID_*
> + * defines and to test by CPU feature, use arm_feature(cpu_env,
> + * ARM_FEATURE_*) */
> +
> +/* in theory, endianness is configurable on some ARM CPUs, but this isn't
> + * used in user mode emulation */
> +#ifdef TARGET_WORDS_BIGENDIAN
> +#define utsname_suffix "b"
> +#else
> +#define utsname_suffix "l"
> +#endif
> +if (arm_feature(cpu_env, ARM_FEATURE_V7))
> +return "armv7" utsname_suffix;
> +if (arm_feature(cpu_env, ARM_FEATURE_V6))
> +return "armv6" utsname_suffix;
> +/* earliest emulated CPU is ARMv5TE; qemu can emulate the 1026, but not 
> its
> + * Jazelle support */
> +return "armv5te" utsname_suffix;
> +#elif defined(TARGET_X86_64)
> +return "x86-64";
> +#elif defined(TARGET_I386)
> +/* see arch/x86/kernel/cpu/bugs.c: check_bugs(), 386, 486, 586, 686 */
> +uint32_t cpuid_version = ((CPUX86State *)cpu_env)->cpuid_version;
> +int family = ((cpuid_version >> 8) & 0x0f) + ((cpuid_version >> 20) & 
> 0xff);
> +if (family == 4)
> +return "i486";
> +if (family == 5)
> +return "i586";
> +return "i686";
> +#else
> +/* default is #define-d in each arch/ subdir */
> +return UNAME_MACHINE;
> +#endif
> +}
> diff --git a/linux-user/cpu-uname.h b/linux-user/cpu-uname.h
> new file mode 100644
> index 000..32492de
> --- /dev/null
> +++ b/linux-user/cpu-uname.h
> @@ -0,0 +1 @@
> +const char *cpu_to_uname_machine(void *cpu_env);
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1acf1f5..9e2e89a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -82,6 +82,7 @@
>  #include 
>  #include 
>  #include "linux_loop.h"
> +#include "cpu-uname.h"
>  
>  #include "qemu.h"
>  #include "qemu-common.h"
> @@ -5739,7 +5740,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (!is_error(ret)) {
>  /* Overrite the native machine name with whatever is being
> 

Re: [Qemu-devel] [PATCH RFC] Advertise IDE physical block size as 4K

[Christoph Hellwig]

On Tue, Dec 29, 2009 at 12:07:58PM +0200, Avi Kivity wrote:
> Guests use this number as a hint for alignment and I/O request sizes.  Given
> that modern disks have 4K block sizes, and cached file-backed images also
> have 4K block sizes, this hint can improve guest performance.
> 
> We probably need to make this configurable depending on machine type.  It
> should be the default for -M 0.13 only as it can affect guest code paths.

The information is correct per the ATA spec, but:

 (a) as mentioned above it should not be used for old machine types
 (b) we need to sort out passing through the first block alignment bits
 that are also in IDENTIFY word 106 if using a raw block device
 underneat
 (b) probably need to adjust the physical blocks size depending on the
 underlying storage topology.

I have a patch in my queue for a while now dealing with (b) and parts of
(c), but it's been preempted by more urgent work.

Re: [Qemu-devel] [PATCH RFC] Advertise IDE physical block size as 4K

[Christoph Hellwig]

On Tue, Dec 29, 2009 at 02:39:38PM +0100, Luca Tettamanti wrote:
> Linux tools put the first partition at sector 63 (512-byte) to retain
> compatibility with Windows;

Well, some of them, and depending on the exact disks.  It's all rather
complicated.

> > It has been discussed for hardware disk design with 4k sectors, and
> > somehow there were plans to map sectors so that the Linux partition
> > scheme results in nicely aligned filesystem blocks
> 
> Ugh, I hope you're wrong ;-) AFAICS remapping will lead only to
> headaches... Linux does not have any problem with aligned partitions.

Linux doesn't care.  As doesn't windows.  But performance on mis-aligned
partitions will suck badly - both on 4k sector drives, SSDs or probably
various copy on write layers in virtualization once you hit the worst
case.  Fortunately the block topology information present in recent
ATA and SCSI standards allows the storage hardware to tell about the
required alignment, and Linux now has a topology API to expose it, which
is used by the most recent versions of the partitioning tools and
filesystem creation tools.

Re: [Qemu-devel] Re: [PATCH v2] virtio-blk physical block size

[Christoph Hellwig]

On Mon, Jan 04, 2010 at 01:38:51PM +1030, Rusty Russell wrote:
> I thought this was what I was doing, but I have shown over and over that
> I have no idea about block devices.
> 
> Our current driver treats BLK_SIZE as the logical and physical size (see
> blk_queue_logical_block_size).
> 
> I have no idea what "logical" vs. "physical" actually means.  Anyone?  Most
> importantly, is it some Linux-internal difference or a real I/O-visible
> distinction?

Those should be the same for any sane interface.  They are for classical
disk devices with larger block sizes (MO, s390 dasd) and also for the
now appearing 4k sector scsi disks.  But in the ide world people are
concerned about dos/window legacy compatiblity so they came up with a
nasty hack:

 - there is a physical block size as used by the disk internally
   (4k initially)
 - all the interfaces to the operating system still happen in the
   traditional 512 byte blocks to not break any existing assumptions
 - to make sure modern operating systems can optimize for the larger
   physical sectors the disks expose this size, too.
 - even worse disks can also have alignment hacks for the traditional
   DOS partitions tables, so that the 512 byte block zero might even
   have an offset into the first larger physical block.  This is also
   exposed in the ATA identify information.

All in all I don't think this mess is a good idea to replicate in
virtio.  Virtio by defintion requires virtualization aware guests, so we
should just follow the SCSI way of larger real block sizes here.

> 
> Rusty.
> 
---end quoted text---

Re: [Qemu-devel] Re: Help on "Could not initialize SDL - exiting"

[Dejun.Liu]

Hi, 

I will do it tomorrow!
sorry for that ,today i prepare for a ppt for my boss.

sorry

Steven Liu

On Mon, 2010-01-04 at 15:17 +0700, Mulyadi Santosa wrote:
> On Mon, Jan 4, 2010 at 3:10 PM, Dejun.Liu  wrote:
> >>
> >> I will land a fix in a second. What you want to change in
> > libsdl-native as
> >>  a quick solution is to add DEPENDS += "libx11-native" and change
> >>  --disable- video-x11" to --enable-video-x11".
> >
> > I(Holger) have comitted two patches for it (one with a crappy username
> > though).
> > --
> > I will post it to Qemu wiki tomorrow.
> >
> > Steven Liu
> 
> Great! I believe lots of people will be thankful for that! :)
> 
> NB: I mean, Qemu forum, not qemu wiki. But anyway, at least you could
> always just post the patch here (to this list).
> 

---
Confidentiality Notice: The information contained in this e-mail and any 
accompanying attachment(s) 
is intended only for the use of the intended recipient and may be confidential 
and/or privileged of 
Neusoft Corporation, its subsidiaries and/or its affiliates. If any reader of 
this communication is 
not the intended recipient, unauthorized use, forwarding, printing,  storing, 
disclosure or copying 
is strictly prohibited, and may be unlawful.If you have received this 
communication in error,please 
immediately notify the sender by return e-mail, and delete the original message 
and all copies from 
your system. Thank you. 
---

Re: [Qemu-devel] Re: Help on "Could not initialize SDL - exiting"

[Dejun.Liu]

Hi,
OE's Holger has patch OE with this problem

---
> On Monday 04 January 2010 04:11:33 Dejun.Liu wrote:
> > -->I used the OE build Qemu which at the staging
> > dir(angstrom-dev/staging/i686-linux/usr/bin).
> > and i executed the command in the buildserver not using ssh
> 
> The good thing is, I was just trying the same and see the error you
see.
>  The error is printed by qemu when it can not use the SetVideoMode
call of
>  SDL.
> 
> I will land a fix in a second. What you want to change in
libsdl-native as
>  a quick solution is to add DEPENDS += "libx11-native" and change
>  --disable- video-x11" to --enable-video-x11".

I(Holger) have comitted two patches for it (one with a crappy username
though).
--
I will post it to Qemu wiki tomorrow.

Steven Liu


On Mon, 2010-01-04 at 14:43 +0700, Mulyadi Santosa wrote:
> On Mon, Jan 4, 2010 at 8:06 AM, Dejun.Liu  wrote:
> > Hi
> >
> > yes, I hava find that OE build SDL not support x11 ,so hava the error
> > No available video device.
> >
> > thanks all these helps
> 
> Glad you spot the error. Let us know how you solve it further. Or
> maybe you'll be kind enough and post your howto to Qemu wiki.
> 

---
Confidentiality Notice: The information contained in this e-mail and any 
accompanying attachment(s) 
is intended only for the use of the intended recipient and may be confidential 
and/or privileged of 
Neusoft Corporation, its subsidiaries and/or its affiliates. If any reader of 
this communication is 
not the intended recipient, unauthorized use, forwarding, printing,  storing, 
disclosure or copying 
is strictly prohibited, and may be unlawful.If you have received this 
communication in error,please 
immediately notify the sender by return e-mail, and delete the original message 
and all copies from 
your system. Thank you. 
---

Re: [Qemu-devel] Re: Help on "Could not initialize SDL - exiting"

[Mulyadi Santosa]

On Mon, Jan 4, 2010 at 3:10 PM, Dejun.Liu  wrote:
>>
>> I will land a fix in a second. What you want to change in
> libsdl-native as
>>  a quick solution is to add DEPENDS += "libx11-native" and change
>>  --disable- video-x11" to --enable-video-x11".
>
> I(Holger) have comitted two patches for it (one with a crappy username
> though).
> --
> I will post it to Qemu wiki tomorrow.
>
> Steven Liu

Great! I believe lots of people will be thankful for that! :)

NB: I mean, Qemu forum, not qemu wiki. But anyway, at least you could
always just post the patch here (to this list).

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com