[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus |
Date: |
Tue, 5 Jan 2010 22:46:03 +0530 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
Hey Anthony
[skipping the part Gerd already answered]
On (Tue) Jan 05 2010 [10:42:39], Anthony Liguori wrote:
>> +static size_t send_control_msg(VirtIOSerialPort *port, void *buf, size_t
>> len)
>> +{
>> + VirtQueueElement elem;
>> + VirtQueue *vq;
>> + struct virtio_console_control *cpkt;
>> +
>> + vq = port->vser->c_ivq;
>> + if (!virtio_queue_ready(vq)) {
>> + return 0;
>> + }
>> + if (!virtqueue_pop(vq,&elem)) {
>> + return 0;
>> + }
>> +
>> + cpkt = (struct virtio_console_control *)buf;
>> + cpkt->id = cpu_to_le32(port->id);
>>
>
> This is not the right way to deal with endianness. The guest should
> write these fields in whatever their native endianness is. From within
> qemu, we should access this fields with ldl_p/stl_p. For x86-on-x86,
> the effect is a nop. For TCG with le-on-be, it becomes a byte
> swap.
Yes, as indicated in a separate thread, I've done that in my dev tree.
I'm just waiting for comments here before sending a new version.
>> +static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>> +{
>> + VirtIOSerial *s = opaque;
>> +
>> + if (version_id> 2) {
>> + return -EINVAL;
>> + }
>> + /* The virtio device */
>> + virtio_load(&s->vdev, f);
>> +
>> + if (version_id< 2) {
>> + return 0;
>> + }
>> + /* The config space */
>> + qemu_get_be16s(f,&s->config.cols);
>> + qemu_get_be16s(f,&s->config.rows);
>> + s->config.nr_ports = qemu_get_be32(f);
>> +
>> + /* Items in struct VirtIOSerial */
>> + qemu_get_be32s(f,&s->guest_features);
>>
>
> Why save a copy of this when you can access it through the virtio device?
Hm, not needed here I guess.
>> +static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int
>> indent)
>> +{
>> + VirtIOSerialDevice *dev = DO_UPCAST(VirtIOSerialDevice, qdev, qdev);
>> + VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev,&dev->qdev);
>> +
>> + monitor_printf(mon, "%*s dev-prop-int: id: %u\n",
>> + indent, "", port->id);
>> + monitor_printf(mon, "%*s dev-prop-int: is_console: %d\n",
>> + indent, "", port->is_console);
>> +}
>>
>
> This will break the build since it's not referenced anywhere.
Again, as mentioned in the other thread, it gets used here:
static struct BusInfo virtser_bus_info = {
.name = "virtio-serial-bus",
.size = sizeof(VirtIOSerialBus),
.print_dev = virtser_bus_dev_print,
};
(I've compile- and run- tested these patches.)
Amit
- [Qemu-devel] [PATCH 0/8] virtio-console: Move to qdev, multiple devices, generic ports, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 1/8] virtio: Remove duplicate macro definition for max. virtqueues, bump up the max, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 3/8] virtio-serial-bus: Maintain guest and host port open/close state, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 4/8] virtio-serial-bus: Add a port 'name' property for port discovery in guests, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 6/8] virtio-serial-bus: Add ability to hot-unplug ports, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 7/8] virtio-serial: Add 'virtserialport' device for generic serial port support, Amit Shah, 2010/01/04
- [Qemu-devel] [PATCH 8/8] Move virtio-serial and virtio-serial-bus to Makefile.hw, Amit Shah, 2010/01/04
- Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Anthony Liguori, 2010/01/05
- Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus,
Amit Shah <=
- Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Anthony Liguori, 2010/01/05
- Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Gerd Hoffmann, 2010/01/05
- Re: [Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Anthony Liguori, 2010/01/05
- [Qemu-devel] Re: [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Gerd Hoffmann, 2010/01/05
- [Qemu-devel] Re: [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus, Amit Shah, 2010/01/05