Re: [Qemu-devel] [PATCH 01/15] pci: add new bus functions

2010-02-10 Thread Isaku Yamahata
On Tue, Feb 09, 2010 at 04:01:25PM -0600, Anthony Liguori wrote:
>  - mapping and managing io regions
>  - reading and writing physical memory
> 
> Signed-off-by: Anthony Liguori 
> ---
>  hw/pci.c |  172 
> +++---
>  hw/pci.h |   18 ++-
>  2 files changed, 181 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..5460f27 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -664,6 +664,9 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
>   r->addr),
>   r->filtered_size,
>   IO_MEM_UNASSIGNED);
> +if (!r->map_func && r->read && r->write) {
> +cpu_unregister_io_memory(r->io_memory_addr);
> +}
>  }
>  }
>  }
> @@ -687,16 +690,15 @@ static int pci_unregister_device(DeviceState *dev)
>  return 0;
>  }
>  
> -void pci_register_bar(PCIDevice *pci_dev, int region_num,
> -pcibus_t size, int type,
> -PCIMapIORegionFunc *map_func)
> +static PCIIORegion *do_pci_register_bar(PCIDevice *pci_dev, int region_num,
> +pcibus_t size, int type)
>  {
>  PCIIORegion *r;
>  uint32_t addr;
>  pcibus_t wmask;
>  
>  if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> -return;
> +return NULL;
>  
>  if (size & (size-1)) {
>  fprintf(stderr, "ERROR: PCI region size must be pow2 "
> @@ -709,7 +711,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>  r->size = size;
>  r->filtered_size = size;
>  r->type = type;
> -r->map_func = map_func;
>  
>  wmask = ~(size - 1);
>  addr = pci_bar(pci_dev, region_num);
> @@ -726,6 +727,100 @@ void pci_register_bar(PCIDevice *pci_dev, int 
> region_num,
>  pci_set_long(pci_dev->wmask + addr, wmask & 0x);
>  pci_set_long(pci_dev->cmask + addr, 0x);
>  }
> +
> +return r;
> +}
> +
> +void pci_register_bar(PCIDevice *pci_dev, int region_num,
> +  pcibus_t size, int type,
> +  PCIMapIORegionFunc *map_func)
> +{
> +PCIIORegion *r;
> +r = do_pci_register_bar(pci_dev, region_num, size, type);
> +if (r) {
> +r->map_func = map_func;
> +}
> +}
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len)
> +{
> +cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> +  const void *buf, int len)
> +{
> +cpu_physical_memory_write(addr, buf, len);
> +}
> +

Isn't something like cpu_to_pci_addr() needed in theory?


> +static void pci_io_region_writeb(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +PCIIORegion *r = opaque;
> +r->write(r->dev, addr, 1, value);
> +}
> +
> +static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +PCIIORegion *r = opaque;
> +r->write(r->dev, addr, 2, value);
> +}
> +
> +static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +PCIIORegion *r = opaque;
> +r->write(r->dev, addr, 4, value);
> +}
> +
> +static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
> +{
> +PCIIORegion *r = opaque;
> +return r->read(r->dev, addr, 1);
> +}
> +
> +static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
> +{
> +PCIIORegion *r = opaque;
> +return r->read(r->dev, addr, 2);
> +}
> +
> +static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
> +{
> +PCIIORegion *r = opaque;
> +return r->read(r->dev, addr, 4);
> +}

They pass addr, not offset from base address.
pci_io_region_ioport_ {read, write}[bwl]() pass offset.
It's inconsistent. offset should be passed.

Isn't the inverse of pci_to_cpu_addr() necessary in theory?



> +
> +static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
> +pci_io_region_readb,
> +pci_io_region_readw,
> +pci_io_region_readl,
> +};
> +
> +static CPUWriteMemoryFunc * const pci_io_region_writefn[3] = {
> +pci_io_region_writeb,
> +pci_io_region_writew,
> +pci_io_region_writel,
> +};
> +
> +void pci_register_io_region(PCIDevice *d, int region_num,
> +pcibus_t size, int type,
> +PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb)
> +{
> +PCIIORegion *r;
> +r = do_pci_register_bar(d, region_num, size, type);
> +if (r) {
> +r->map_func = NULL;
> +r->dev = d;
> +r->read = readcb;
> +r->write = writecb;
> +if (r->type != PCI_BASE_ADDRESS_SPACE_IO && r->read && r->write) {
> +r

Re: [Qemu-devel] [PATCH v1 0/4]: QMP related fixes

2010-02-10 Thread Markus Armbruster
Luiz Capitulino  writes:

>  Should be applied on top of feature negotiation series.
>
> changelog:
> --
>
> v0 -> v1:
>
> - Document qobject_from_jsonf() new semantics

Looks good.




[Qemu-devel] Re: [PATCH 01/15] pci: add new bus functions

2010-02-10 Thread Michael S. Tsirkin
On Tue, Feb 09, 2010 at 04:01:25PM -0600, Anthony Liguori wrote:
>  - mapping and managing io regions
>  - reading and writing physical memory
> 
> Signed-off-by: Anthony Liguori 

One thing I'm concerned with here is that this adds
another level of indirection on datapath operations.
Note that rwhandler is only used for config cycles,
which aren't normally datapath.

Can the patch be split to separate mapping and read/write parts?
I think mapping is the harder part, and also where we have actual
emulation bugs now. It would thus probably be uncontroversial.

Also, maybe it's time to bite the bullet and change memory/io callbacks
in exec.c (that rwhandler currently layers above): pass in length
and use some generic type like addr_t for address?
This would let us get rid of both rwhandler and most of this patch.

> ---
>  hw/pci.c |  172 
> +++---
>  hw/pci.h |   18 ++-
>  2 files changed, 181 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..5460f27 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -664,6 +664,9 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
>   r->addr),
>   r->filtered_size,
>   IO_MEM_UNASSIGNED);
> +if (!r->map_func && r->read && r->write) {
> +cpu_unregister_io_memory(r->io_memory_addr);
> +}
>  }
>  }
>  }
> @@ -687,16 +690,15 @@ static int pci_unregister_device(DeviceState *dev)
>  return 0;
>  }
>  
> -void pci_register_bar(PCIDevice *pci_dev, int region_num,
> -pcibus_t size, int type,
> -PCIMapIORegionFunc *map_func)
> +static PCIIORegion *do_pci_register_bar(PCIDevice *pci_dev, int region_num,
> +pcibus_t size, int type)
>  {
>  PCIIORegion *r;
>  uint32_t addr;
>  pcibus_t wmask;
>  
>  if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> -return;
> +return NULL;
>  
>  if (size & (size-1)) {
>  fprintf(stderr, "ERROR: PCI region size must be pow2 "
> @@ -709,7 +711,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>  r->size = size;
>  r->filtered_size = size;
>  r->type = type;
> -r->map_func = map_func;
>  
>  wmask = ~(size - 1);
>  addr = pci_bar(pci_dev, region_num);
> @@ -726,6 +727,100 @@ void pci_register_bar(PCIDevice *pci_dev, int 
> region_num,
>  pci_set_long(pci_dev->wmask + addr, wmask & 0x);
>  pci_set_long(pci_dev->cmask + addr, 0x);
>  }
> +
> +return r;
> +}
> +
> +void pci_register_bar(PCIDevice *pci_dev, int region_num,
> +  pcibus_t size, int type,
> +  PCIMapIORegionFunc *map_func)
> +{
> +PCIIORegion *r;
> +r = do_pci_register_bar(pci_dev, region_num, size, type);
> +if (r) {
> +r->map_func = map_func;
> +}
> +}
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len)
> +{
> +cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> +  const void *buf, int len)
> +{
> +cpu_physical_memory_write(addr, buf, len);
> +}
> +
> +static void pci_io_region_writeb(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +PCIIORegion *r = opaque;
> +r->write(r->dev, addr, 1, value);
> +}
> +
> +static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +PCIIORegion *r = opaque;
> +r->write(r->dev, addr, 2, value);
> +}
> +
> +static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> +PCIIORegion *r = opaque;
> +r->write(r->dev, addr, 4, value);
> +}
> +
> +static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
> +{
> +PCIIORegion *r = opaque;
> +return r->read(r->dev, addr, 1);
> +}
> +
> +static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
> +{
> +PCIIORegion *r = opaque;
> +return r->read(r->dev, addr, 2);
> +}
> +
> +static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
> +{
> +PCIIORegion *r = opaque;
> +return r->read(r->dev, addr, 4);
> +}
> +
> +static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
> +pci_io_region_readb,
> +pci_io_region_readw,
> +pci_io_region_readl,
> +};
> +
> +static CPUWriteMemoryFunc * const pci_io_region_writefn[3] = {
> +pci_io_region_writeb,
> +pci_io_region_writew,
> +pci_io_region_writel,
> +};
> +
> +void pci_register_io_region(PCIDevice *d, int region_num,
> +pcibus_t size, int type,
> +  

[Qemu-devel] Re: [PATCH 0/15][RFC] New PCI interfaces

2010-02-10 Thread Michael S. Tsirkin
On Tue, Feb 09, 2010 at 04:01:24PM -0600, Anthony Liguori wrote:
> This is a work in progress that I wanted to share giving some of the 
> discussions
> around rwhandlers.  The idea is to make PCI devices have a common set of
> functions to interact with the CPU that is driven entirely through the PCI 
> bus.
> 
> I've tested the network card conversions, but have not yet tested the other
> bits.

It would definitely be an improvement when all mapping would be done in
pci core. We could thus finally fix bridge filtering.
The hardest part with this change would be legacy vga class.

I am not so sure about adding an extra level of indirection
for all memory/io transactions. What we do now is perform
setup during bus scan, afterwards cpu calls device directly,
which looks nicer on the surface. Maybe we just need to
make callbacks more generic.

-- 
MST




[Qemu-devel] Re: [PATCH] pci: introduce get_dev_dict callback.

2010-02-10 Thread Michael S. Tsirkin
On Wed, Feb 10, 2010 at 03:59:30PM +0900, Isaku Yamahata wrote:
> This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5

Could you please clarify what exactly is the bug,
and how this patch fixes it?

> by introducing device specific get_dev_dict callback.
> pci host bridge doesn't always have header type of bridge.
> 
> Especially PBM which is emulated doesn't conform to PCI spec
> at the moment. So by introducing get_dev_dict, allow each pci device
> to return specific infos.
> This patch also makes it easier to enhance info pci command in the future.
> For example returning pcie stuff.
> 
> Cc: Blue Swirl 
> Cc: "Michael S. Tsirkin" 
> Signed-off-by: Isaku Yamahata 

If the idea is to allow quirk mode, how about we call the hook
get_info_quirk, and only use it for non-spec compliant devices?
The point being making it clear that most devices should not use it.

> ---
>  hw/apb_pci.c |1 +
>  hw/dec_pci.c |1 +
>  hw/pci.c |   74 ++---
>  hw/pci.h |3 ++
>  4 files changed, 44 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/apb_pci.c b/hw/apb_pci.c
> index 46d5b0e..693f99e 100644
> --- a/hw/apb_pci.c
> +++ b/hw/apb_pci.c
> @@ -479,6 +479,7 @@ static PCIDeviceInfo pbm_pci_host_info = {
>  .qdev.name = "pbm",
>  .qdev.size = sizeof(PCIDevice),
>  .init  = pbm_pci_host_init,
> +.get_dev_dict = pci_bridge_get_dev_dict,
>  .header_type  = PCI_HEADER_TYPE_BRIDGE,
>  };
>  
> diff --git a/hw/dec_pci.c b/hw/dec_pci.c
> index 8d059f1..824fafc 100644
> --- a/hw/dec_pci.c
> +++ b/hw/dec_pci.c
> @@ -91,6 +91,7 @@ static PCIDeviceInfo dec_21154_pci_host_info = {
>  .qdev.name = "dec-21154",
>  .qdev.size = sizeof(PCIDevice),
>  .init  = dec_21154_pci_host_init,
> +.get_dev_dict = pci_bridge_get_dev_dict,
>  .header_type  = PCI_HEADER_TYPE_BRIDGE,
>  };
>  
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..9510aee 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1271,10 +1271,43 @@ static QObject *pci_get_regions_list(const PCIDevice 
> *dev)
>  
>  static QObject *pci_get_devices_list(PCIBus *bus, int bus_num);
>  
> +void pci_bridge_get_dev_dict(PCIDevice *dev, PCIBus *bus, QDict *qdict)
> +{
> +QObject *pci_bridge;
> +
> +pci_bridge = qobject_from_jsonf("{ 'bus': "
> +"{ 'number': %d, 'secondary': %d, 'subordinate': %d }, "
> +"'io_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
> +"'memory_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "}, "
> +"'prefetchable_range': { 'base': %" PRId64 ", 'limit': %" PRId64 "} }",
> +dev->config[PCI_PRIMARY_BUS], dev->config[PCI_SECONDARY_BUS],
> +dev->config[PCI_SUBORDINATE_BUS],
> +pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_IO),
> +pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_IO),
> +pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
> +pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY),
> +pci_bridge_get_base(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
> +PCI_BASE_ADDRESS_MEM_PREFETCH),
> +pci_bridge_get_limit(dev, PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_PREFETCH));
> +
> +if (dev->config[PCI_SECONDARY_BUS] != 0) {
> +PCIBus *child_bus = pci_find_bus(bus, 
> dev->config[PCI_SECONDARY_BUS]);
> +if (child_bus) {
> +qdict_put_obj(qobject_to_qdict(pci_bridge), "devices",
> +  pci_get_devices_list(child_bus,
> +   
> dev->config[PCI_SECONDARY_BUS]));
> +}
> +}
> +
> +qdict_put_obj(qdict, "pci_bridge", pci_bridge);
> +}
> +
>  static QObject *pci_get_dev_dict(PCIDevice *dev, PCIBus *bus, int bus_num)
>  {
> -int class;
> +PCIDeviceInfo *info = DO_UPCAST(PCIDeviceInfo, qdev, dev->qdev.info);
>  QObject *obj;
> +QDict *qdict;
>  
>  obj = qobject_from_jsonf("{ 'bus': %d, 'slot': %d, 'function': %d,"  
>  "'class_info': %p, 'id': %p, 'regions': %p,"
>" 'qdev_id': %s }",
> @@ -1283,45 +1316,15 @@ static QObject *pci_get_dev_dict(PCIDevice *dev, 
> PCIBus *bus, int bus_num)
>pci_get_dev_class(dev), pci_get_dev_id(dev),
>pci_get_regions_list(dev),
>dev->qdev.id ? dev->qdev.id : "");
> +qdict = qobject_to_qdict(obj);
>  
>  if (dev->config[PCI_INTERRUPT_PIN] != 0) {
> -QDict *qdict = qobject_to_qdict(obj);
>  qdict_put(qdict, "irq", 
> qint_from_int(dev->config[PCI_INTERRUPT_LINE]));
>  }
>  
> -class = pci_get_word(dev->config + PCI_CLASS_DEVICE);
> -if (class == PCI_CLASS_BRIDGE_HOST || class == PCI_CLASS_BRIDGE_PCI) {
> -QDict *qdict;
> -QObject *pci_bridge;
> -
> -pci_bridge = qobject_from_jsonf("{ 'bus': "
> -"{ 'number': %d, 'secondary': %d, 'subordinate': %

[Qemu-devel] Re: [PATCH 09/15] eepro100: convert to new pci interface

2010-02-10 Thread Michael S. Tsirkin
On Wed, Feb 10, 2010 at 07:32:10AM +0100, Stefan Weil wrote:
> See my inline comments.
> 
> 
> Anthony Liguori schrieb:
> >  - Removed some dead defines for TARGET_I386 so that we could build once
> >
> > Signed-off-by: Anthony Liguori 
> > ---
> >  hw/eepro100.c |  238 
> > ++---
> >  1 files changed, 57 insertions(+), 181 deletions(-)
> >
> > diff --git a/hw/eepro100.c b/hw/eepro100.c
> > index b33dbb8..16230c9 100644
> > --- a/hw/eepro100.c
> > +++ b/hw/eepro100.c
> > @@ -33,10 +33,6 @@
> >   * Open Source Software Developer Manual
> >   */
> >  
> > -#if defined(TARGET_I386)
> > -# warning "PXE boot still not working!"
> > -#endif
> > -
> >   
> 
> You did not fix PXE boot here, did you?
> So the warning or a comment should stay there.
> 
> >  #include  /* offsetof */
> >  #include 
> >  #include "hw.h"
> >   
> ...
> 
> 
> > -static void ioport_write2(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > -EEPRO100State *s = opaque;
> > -eepro100_write2(s, addr - s->region[1], val);
> > -}
> > -
> > -static void ioport_write4(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > -EEPRO100State *s = opaque;
> > -eepro100_write4(s, addr - s->region[1], val);
> > -}
> > -
> >  /***/
> >  /* PCI EEPRO100 definitions */
> >  
> > -static void pci_map(PCIDevice * pci_dev, int region_num,
> > -pcibus_t addr, pcibus_t size, int type)
> > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
> > + uint32_t value)
> >  {
> > -EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
> > -
> > -TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
> > -  "size=0x%08"FMT_PCIBUS", type=%d\n",
> > -  region_num, addr, size, type));
> > +EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
> >   
> 
> Please don't change the name of the PCIDevice pointer argument
> from pci_dev to dev.
> 
> This dev, dev in DO_UPCAST is ugly and misleading.

It's a matter of taste: I actually think it's pretty: I never remember
which argument of DO_UPCAST is which, and if both are dev it doesn't
matter.

> 
> >  
> > -assert(region_num == 1);
> > -register_ioport_write(addr, size, 1, ioport_write1, s);
> > -register_ioport_read(addr, size, 1, ioport_read1, s);
> > -register_ioport_write(addr, size, 2, ioport_write2, s);
> > -register_ioport_read(addr, size, 2, ioport_read2, s);
> > -register_ioport_write(addr, size, 4, ioport_write4, s);
> > -register_ioport_read(addr, size, 4, ioport_read4, s);
> > -
> > -s->region[region_num] = addr;
> > -}
> > -
> > -/*
> > - *
> > - * Memory mapped I/O.
> > - *
> > - 
> > /
> > -
> > -static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, 
> > uint32_t val)
> > -{
> > -EEPRO100State *s = opaque;
> > -//~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> > -eepro100_write1(s, addr, val);
> > -}
> > -
> > -static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, 
> > uint32_t val)
> > -{
> > -EEPRO100State *s = opaque;
> > -//~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> > -eepro100_write2(s, addr, val);
> > -}
> > -
> > -static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, 
> > uint32_t val)
> > -{
> > -EEPRO100State *s = opaque;
> > -//~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> > -eepro100_write4(s, addr, val);
> > -}
> > -
> > -static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
> > -{
> > -EEPRO100State *s = opaque;
> > -//~ logout("addr=%s\n", regname(addr));
> > -return eepro100_read1(s, addr);
> > -}
> > -
> > -static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
> > -{
> > -EEPRO100State *s = opaque;
> > -//~ logout("addr=%s\n", regname(addr));
> > -return eepro100_read2(s, addr);
> > -}
> > -
> > -static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr)
> > -{
> > -EEPRO100State *s = opaque;
> > -//~ logout("addr=%s\n", regname(addr));
> > -return eepro100_read4(s, addr);
> > +if (size == 1) {
> > +eepro100_write1(s, addr, value);
> > +} else if (size == 2) {
> > +eepro100_write2(s, addr, value);
> > +} else {
> > +eepro100_write4(s, addr, value);
> > +}
> >  }
> >  
> > -static CPUWriteMemoryFunc * const pci_mmio_write[] = {
> > -pci_mmio_writeb,
> > -pci_mmio_writew,
> > -pci_mmio_writel
> > -};
> > -
> > -static CPUReadMemoryFunc * const pci_mmio_read[] = {
> > -pci_mmio_readb,
> > -pci_mmio_readw,
> > -pci_mmio_readl
> > -};
> > -
> > -static void pci_mmio_map(PCIDevice * pci_dev, int region_num,
> > - pcibus_t addr

[Qemu-devel] Re: [PATCH] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread OHMURA Kei
> Please reuse the changelog when reposing a patch, this makes it easier
> for me to apply it.

Thanks.  Will follow it from next time.


> Should be a host long size, not guest. This will fail when running a
> 32-bit qemu-system-x86_64 binary.

Sorry. That was our mistake.


> Instead of using a nested loop if bitmap_ul[i] != 0, it is possible to
> use just a single loop (while (c>  0)), and process a long's worth of data.
>
> The only trickery is with big endian hosts, where the conversion from
> bit number to page number is a bit complicated.

To convert the bitmap from big endian to little endian, le_bswap macro in
bswap.h seems useful, which is now undefined.  What do you think about this
approach?

This is an example bitmap-traveling code using le_bswap:
 /* 
  * bitmap-traveling is faster than memory-traveling (for addr...) 
  * especially when most of the memory is not dirty.
  */
 for (i = 0; i < len; i++) {
if (bitmap_ul[i] != 0) {
c = le_bswap(bitmap_ul[i], HOST_LONG_BITS);
while (c > 0) {
j = ffsl(c) - 1;
c &= ~(1ul << j);
page_number = i * HOST_LONG_BITS + j;
addr1 = page_number * TARGET_PAGE_SIZE;
addr = offset + addr1;
ram_addr = cpu_get_physical_page_desc(addr);
cpu_physical_memory_set_dirty(ram_addr);
}
 }
 }








Re: [Qemu-devel] qemu 0.12.x / -git PS/2 mouse not working under Win3.11 / Win9x

2010-02-10 Thread Klaus Rechert

Hi,

I know from myself that in 0.9.x it worked so you should start testing 
older versions until it works and then test each commit to the ps2 
emulator onward that.




Win3.11 worked well with 0.11.x releases, while win9x crashed when 
switching to VGA (IIRC). 0.10.x worked well for both.


Cheers
   Klaus




[Qemu-devel] Re: [PATCH] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Avi Kivity
On 02/10/2010 11:55 AM, OHMURA Kei wrote:
>
>> Instead of using a nested loop if bitmap_ul[i] != 0, it is possible to
>> use just a single loop (while (c>  0)), and process a long's worth of data.
>>
>> The only trickery is with big endian hosts, where the conversion from
>> bit number to page number is a bit complicated.
>> 
> To convert the bitmap from big endian to little endian, le_bswap macro in
> bswap.h seems useful, which is now undefined.  What do you think about this
> approach?
>
> This is an example bitmap-traveling code using le_bswap:
>  /* 
>   * bitmap-traveling is faster than memory-traveling (for addr...) 
>   * especially when most of the memory is not dirty.
>   */
>  for (i = 0; i < len; i++) {
> if (bitmap_ul[i] != 0) {
> c = le_bswap(bitmap_ul[i], HOST_LONG_BITS);
> while (c > 0) {
> j = ffsl(c) - 1;
> c &= ~(1ul << j);
> page_number = i * HOST_LONG_BITS + j;
> addr1 = page_number * TARGET_PAGE_SIZE;
> addr = offset + addr1;
> ram_addr = cpu_get_physical_page_desc(addr);
> cpu_physical_memory_set_dirty(ram_addr);
> }
>  }
>  }
>   

Yes, that solves the problem very neatly.

-- 
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] Re: [PATCH 09/15] eepro100: convert to new pci interface

2010-02-10 Thread Markus Armbruster
"Michael S. Tsirkin"  writes:

> On Wed, Feb 10, 2010 at 07:32:10AM +0100, Stefan Weil wrote:
>> See my inline comments.
>> 
>> 
>> Anthony Liguori schrieb:
[...]
>> > -static void pci_map(PCIDevice * pci_dev, int region_num,
>> > -pcibus_t addr, pcibus_t size, int type)
>> > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
>> > + uint32_t value)
>> >  {
>> > -EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
>> > -
>> > -TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
>> > -  "size=0x%08"FMT_PCIBUS", type=%d\n",
>> > -  region_num, addr, size, type));
>> > +EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
>> >   
>> 
>> Please don't change the name of the PCIDevice pointer argument
>> from pci_dev to dev.
>> 
>> This dev, dev in DO_UPCAST is ugly and misleading.
>
> It's a matter of taste: I actually think it's pretty: I never remember
> which argument of DO_UPCAST is which, and if both are dev it doesn't
> matter.

I also have trouble remembering how to use DO_UPCAST(), but playing
games with identifiers is a poor fix for that.  I find the use of the
same name "dev" for two different things in the same expression rather
confusing.  Could we improve DO_UPCAST() instead?

For what it's worth, I have no trouble with container_of().  DO_UPCAST()
takes the same arguments in a different order, which I find irritating.

[...]




Re: [Qemu-devel] Re: [PATCH 09/15] eepro100: convert to new pci interface

2010-02-10 Thread Michael S. Tsirkin
On Wed, Feb 10, 2010 at 11:43:29AM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin"  writes:
> 
> > On Wed, Feb 10, 2010 at 07:32:10AM +0100, Stefan Weil wrote:
> >> See my inline comments.
> >> 
> >> 
> >> Anthony Liguori schrieb:
> [...]
> >> > -static void pci_map(PCIDevice * pci_dev, int region_num,
> >> > -pcibus_t addr, pcibus_t size, int type)
> >> > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
> >> > + uint32_t value)
> >> >  {
> >> > -EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
> >> > -
> >> > -TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
> >> > -  "size=0x%08"FMT_PCIBUS", type=%d\n",
> >> > -  region_num, addr, size, type));
> >> > +EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
> >> >   
> >> 
> >> Please don't change the name of the PCIDevice pointer argument
> >> from pci_dev to dev.
> >> 
> >> This dev, dev in DO_UPCAST is ugly and misleading.
> >
> > It's a matter of taste: I actually think it's pretty: I never remember
> > which argument of DO_UPCAST is which, and if both are dev it doesn't
> > matter.
> 
> I also have trouble remembering how to use DO_UPCAST(), but playing
> games with identifiers is a poor fix for that.  I find the use of the
> same name "dev" for two different things in the same expression rather
> confusing.  Could we improve DO_UPCAST() instead?
> 
> For what it's worth, I have no trouble with container_of().  DO_UPCAST()
> takes the same arguments in a different order, which I find irritating.
> 
> [...]

IMO it would be ideal to remove offset assumptions where possible
and then we can use container_of.

-- 
MST




[Qemu-devel] [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread OHMURA Kei
dirty-bitmap-traveling is carried out by byte size in qemu-kvm.c.
But We think that dirty-bitmap-traveling by long size is faster than by byte
size especially when most of memory is not dirty.

Signed-off-by: OHMURA Kei 
---
 bswap.h|1 -
 qemu-kvm.c |   30 --
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/bswap.h b/bswap.h
index 4558704..d896f01 100644
--- a/bswap.h
+++ b/bswap.h
@@ -209,7 +209,6 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
 #define cpu_to_32wu cpu_to_le32wu
 #endif
 
-#undef le_bswap
 #undef be_bswap
 #undef le_bswaps
 #undef be_bswaps
diff --git a/qemu-kvm.c b/qemu-kvm.c
index a305907..ea07912 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -2438,27 +2438,29 @@ static int kvm_get_dirty_pages_log_range(unsigned long 
start_addr,
  unsigned long offset,
  unsigned long mem_size)
 {
-unsigned int i, j, n = 0;
-unsigned char c;
-unsigned long page_number, addr, addr1;
+unsigned int i, j;
+unsigned long page_number, addr, addr1, c;
 ram_addr_t ram_addr;
-unsigned int len = ((mem_size / TARGET_PAGE_SIZE) + 7) / 8;
+unsigned int len = ((mem_size / TARGET_PAGE_SIZE) + HOST_LONG_BITS - 1) /
+HOST_LONG_BITS;
+unsigned long *bitmap_ul = (unsigned long *)bitmap;
 
 /* 
  * bitmap-traveling is faster than memory-traveling (for addr...) 
  * especially when most of the memory is not dirty.
  */
 for (i = 0; i < len; i++) {
-c = bitmap[i];
-while (c > 0) {
-j = ffsl(c) - 1;
-c &= ~(1u << j);
-page_number = i * 8 + j;
-addr1 = page_number * TARGET_PAGE_SIZE;
-addr = offset + addr1;
-ram_addr = cpu_get_physical_page_desc(addr);
-cpu_physical_memory_set_dirty(ram_addr);
-n++;
+if (bitmap_ul[i] != 0) {
+c = le_bswap(bitmap_ul[i], HOST_LONG_BITS);
+while (c > 0) {
+j = ffsl(c) - 1;
+c &= ~(1ul << j);
+page_number = i * HOST_LONG_BITS + j;
+addr1 = page_number * TARGET_PAGE_SIZE;
+addr = offset + addr1;
+ram_addr = cpu_get_physical_page_desc(addr);
+cpu_physical_memory_set_dirty(ram_addr);
+}
 }
 }
 return 0;
-- 
1.6.3.3






Re: [Qemu-devel] [PATCH] alpha-linux-user: Implement signals.

2010-02-10 Thread Riku Voipio
On Tue, Feb 09, 2010 at 10:46:32AM -0800, Richard Henderson wrote:
> Ping?

The linux-user side of the patch seems fine, but the target-alpha
code doesn't apply. Either something has changed upstream or this
depends on one of your other alpha patches. If the latter case, I
don't mind if this is applied together with the rest of alpha
patches.

> r~
>
> On 01/04/2010 03:17 PM, Richard Henderson wrote:
>> Move userland PALcode handling into linux-user main loop so that
>> we can send signals from there.  This also makes alpha_palcode.c
>> system-level only, so don't build it for userland.  Add defines
>> for GENTRAP PALcall mapping to signals.
>>
>> Signed-off-by: Richard Henderson
>> ---
>>   Makefile.target  |3 +-
>>   hw/alpha_palcode.c   |   81 +---
>>   linux-user/alpha/target_signal.h |   27 
>>   linux-user/main.c|  137 
>>   linux-user/signal.c  |  267 
>> ++
>>   linux-user/syscall.c |   61 -
>>   linux-user/syscall_defs.h|   23 +++-
>>   target-alpha/cpu.h   |4 +-
>>   target-alpha/translate.c |3 +-
>>   9 files changed, 489 insertions(+), 117 deletions(-)
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 7c1f30c..0ecfe76 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -47,7 +47,6 @@ libobj-$(CONFIG_NOSOFTFLOAT) += fpu/softfloat-native.o
>>   libobj-y += op_helper.o helper.o
>>   libobj-$(CONFIG_NEED_MMU) += mmu.o
>>   libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
>> -libobj-$(TARGET_ALPHA) += alpha_palcode.o
>>
>>   # NOTE: the disassembler code is only needed for debugging
>>   libobj-y += disas.o
>> @@ -295,6 +294,8 @@ obj-m68k-y += m68k-semi.o dummy_m68k.o
>>
>>   obj-s390x-y = s390-virtio-bus.o s390-virtio.o
>>
>> +obj-alpha-y = alpha_palcode.o
>> +
>>   main.o vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>>
>>   vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
>> diff --git a/hw/alpha_palcode.c b/hw/alpha_palcode.c
>> index 843bd14..c1220ad 100644
>> --- a/hw/alpha_palcode.c
>> +++ b/hw/alpha_palcode.c
>> @@ -21,11 +21,9 @@
>>   #include
>>   #include
>>
>> -#include "qemu.h"
>>   #include "cpu.h"
>>   #include "exec-all.h"
>>
>> -#if !defined (CONFIG_USER_ONLY)
>>   /* Shared handlers */
>>   static void pal_reset (CPUState *env);
>>   /* Console handlers */
>> @@ -997,12 +995,9 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, uint32_t 
>> address, int rw,
>>   uint64_t physical, page_size, end;
>>   int prot, zbits, ret;
>>
>> -#if defined(CONFIG_USER_ONLY)
>> -ret = 2;
>> -#else
>> -ret = virtual_to_physical(env,&physical,&zbits,&prot,
>> -  address, mmu_idx, rw);
>> -#endif
>> +ret = virtual_to_physical(env,&physical,&zbits,&prot,
>> +  address, mmu_idx, rw);
>> +
>>   switch (ret) {
>>   case 0:
>>   /* No fault */
>> @@ -1050,73 +1045,3 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, uint32_t 
>> address, int rw,
>>   return ret;
>>   }
>>   #endif
>> -
>> -#else /* !defined (CONFIG_USER_ONLY) */
>> -void pal_init (CPUState *env)
>> -{
>> -}
>> -
>> -void call_pal (CPUState *env, int palcode)
>> -{
>> -target_long ret;
>> -
>> -switch (palcode) {
>> -case 0x80:
>> -/* BPT */
>> -qemu_log("BPT\n");
>> -/* FIXME: Sends SIGTRAP, si_code=TRAP_BRKPT.  */
>> -exit(1);
>> -case 0x81:
>> -/* BUGCHK */
>> -qemu_log("BUGCHK\n");
>> -/* FIXME: Sends SIGTRAP, si_code=SI_FAULT.  */
>> -exit(1);
>> -case 0x83:
>> -/* CALLSYS */
>> -qemu_log("CALLSYS n " TARGET_FMT_ld "\n", env->ir[0]);
>> -ret = do_syscall(env, env->ir[IR_V0], env->ir[IR_A0], 
>> env->ir[IR_A1],
>> - env->ir[IR_A2], env->ir[IR_A3], env->ir[IR_A4],
>> - env->ir[IR_A5]);
>> -if (ret>= 0) {
>> -env->ir[IR_A3] = 0;
>> -env->ir[IR_V0] = ret;
>> -} else {
>> -env->ir[IR_A3] = 1;
>> -env->ir[IR_V0] = -ret;
>> -}
>> -break;
>> -case 0x86:
>> -/* IMB */
>> -qemu_log("IMB\n");
>> -/* ??? We can probably elide the code using page_unprotect that is
>> -   checking for self-modifying code.  Instead we could simply call
>> -   tb_flush here.  Until we work out the changes required to turn
>> -   off the extra write protection, this can be a no-op.  */
>> -break;
>> -case 0x9E:
>> -/* RDUNIQUE */
>> -qemu_log("RDUNIQUE: " TARGET_FMT_lx "\n", env->unique);
>> -/* Handled in the translator for usermode.  */
>> -abort();
>> -case 0x9F:
>> -/* WRUNIQUE */
>> -qemu_log("WRUNIQUE: " TARGET_FMT_lx "\n", env->ir[IR_A0]);
>> -/* Handled in the translator for usermode.  */
>> -abort();
>> -case 0xAA

Re: [Qemu-devel] [PATCH] block: saner flags filtering in bdrv_open2

2010-02-10 Thread Christoph Hellwig
ping?

On Thu, Jan 28, 2010 at 03:19:12PM +0100, Christoph Hellwig wrote:
> Clean up the current mess about figuring out which flags to pass to the
> driver.  BDRV_O_FILE, BDRV_O_SNAPSHOT and BDRV_O_NO_BACKING are flags
> only used by the block layer internally so filter them out directly.
> Previously BDRV_O_NO_BACKING could accidentally be passed to the drivers,
> but wasn't ever used.
> 
> Signed-off-by: Christoph Hellwig 
> 
> Index: qemu/block.c
> ===
> --- qemu.orig/block.c 2010-01-28 15:12:52.316024386 +0100
> +++ qemu/block.c  2010-01-28 15:13:33.419004083 +0100
> @@ -451,13 +451,20 @@ int bdrv_open2(BlockDriverState *bs, con
>  bs->enable_write_cache = 1;
>  
>  bs->read_only = (flags & BDRV_O_RDWR) == 0;
> -if (!(flags & BDRV_O_FILE)) {
> -open_flags = (flags & (BDRV_O_RDWR | 
> BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
> -if (bs->is_temporary) { /* snapshot should be writeable */
> -open_flags |= BDRV_O_RDWR;
> -}
> -} else {
> -open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
> +
> +/*
> + * Clear flags that are internal to the block layer before opening the
> + * image.
> + */
> +open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT | 
> BDRV_O_NO_BACKING);
> +
> +/*
> + * Snapshots should be writeable.
> + *
> + * XXX(hch): and what is the point of a snapshot during a read-only open?
> + */
> +if (!(flags & BDRV_O_FILE) && bs->is_temporary) {
> +open_flags |= BDRV_O_RDWR;
>  }
>  
>  ret = drv->bdrv_open(bs, filename, open_flags);
> 
---end quoted text---




[Qemu-devel] [PATCH] virtio-spec: document indirect descriptors

2010-02-10 Thread Michael S. Tsirkin
Add documentation for indirect descriptors

Signed-off-by: Michael S. Tsirkin 
---
 virtio-spec.lyx |  118 +--
 1 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 8062e11..b5a8fbd 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -1,4 +1,4 @@
-#LyX 1.6.4 created this file. For more info see http://www.lyx.org/
+#LyX 1.6.5 created this file. For more info see http://www.lyx.org/
 \lyxformat 345
 \begin_document
 \begin_header
@@ -36,8 +36,7 @@
 \paperpagestyle default
 \tracking_changes true
 \output_changes true
-\author "" 
-\author "" 
+\author "Michael S. Tsirkin" 
 \author "" 
 \end_header
 
@@ -1441,7 +1440,28 @@ struct vring_desc {
 
 \begin_layout Plain Layout
 
-#define VRING_DESC_F_WRITE 2 
+#define VRING_DESC_F_WRITE 2
+\change_deleted 0 1265802057
+ 
+\change_inserted 0 1265802048
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1265802054
+
+/* This means the buffer contains a list of buffer descriptors.
+ */
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1265802049
+
+#define VRING_DESC_F_INDIRECT   4 
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout
@@ -1478,6 +1498,96 @@ struct vring_desc {
 \begin_layout Standard
 The number of descriptors in the table is specified by the Queue Size field
  for this virtqueue.
+\change_deleted 0 1265801065
+
+\end_layout
+
+\begin_layout Subsubsection
+
+\change_inserted 0 1265801341
+Indirect Descriptors
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 0 1265805736
+Some devices benefit by concurrently dispatching a large number of large
+ requests.
+ To increase ring capacity it is possible to store a table of 
+\emph on
+indirect descriptors
+\emph default
+ anywhere in memory, and insert a descriptor in main virtqueue (with 
flags&INDIRECT on) that refers to memory buffer containing this 
+\emph on
+indirect descriptor table
+\emph default
+; fields 
+\emph on
+addr
+\emph default
+ and 
+\emph on
+len
+\emph default
+ refer to the indirect table address and length in bytes, respectively.
+ The indirect table layout structure looks like this (len is the length
+ of the descriptor that refers to this table, which is a variable, so this
+ code won't compile):
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 0 1265804397
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1265804526
+
+struct indirect_descriptor_table {
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1265804397
+
+/* The actual descriptors (16 bytes each) */
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1265804473
+
+struct vring_desc desc[len / 16];
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 0 1265804397
+
+};
+\end_layout
+
+\end_inset
+
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 0 1265805219
+The first indirect descriptor is located at start of the indirect descriptor
+ table (index 0), additional indirect descriptors are chained by next field.
+ An indirect descriptor without next field (with flags&NEXT off) signals
+ the end of the indirect descriptor table, and transfers control back to
+ the main virtqueue.
+ An indirect descriptor can not refer to another indirect descriptor table
+ (flags&INDIRECT must be off).
+ A single indirect descriptor table can include both read-only and write-only
+ descriptors; write-only flag (flags&WRITE) in the descriptor that refers
+ to it is ignored.
 \end_layout
 
 \begin_layout Subsection
-- 
1.6.6.144.g5c3af




[Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Avi Kivity
On 02/10/2010 12:52 PM, OHMURA Kei wrote:
> dirty-bitmap-traveling is carried out by byte size in qemu-kvm.c.
> But We think that dirty-bitmap-traveling by long size is faster than by byte
> size especially when most of memory is not dirty.
>
> --- a/bswap.h
> +++ b/bswap.h
> @@ -209,7 +209,6 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
>  #define cpu_to_32wu cpu_to_le32wu
>  #endif
>  
> -#undef le_bswap
>  #undef be_bswap
>  #undef le_bswaps
>   


Anthony, is it okay to export le_bswap this way, or will you want
leul_to_cpu()?

-- 
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface

2010-02-10 Thread Anthony Liguori

On 02/10/2010 12:32 AM, Stefan Weil wrote:

See my inline comments.


Anthony Liguori schrieb:
   

  - Removed some dead defines for TARGET_I386 so that we could build once

Signed-off-by: Anthony Liguori
---
  hw/eepro100.c |  238 ++---
  1 files changed, 57 insertions(+), 181 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index b33dbb8..16230c9 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -33,10 +33,6 @@
   * Open Source Software Developer Manual
   */

-#if defined(TARGET_I386)
-# warning "PXE boot still not working!"
-#endif
-

 

You did not fix PXE boot here, did you?
So the warning or a comment should stay there.
   


A comment is fine, but the TARGET_I386 makes this file unnecessarily 
dependent on TARGET.  With this change, we only need to build eepro100.o 
once.

  /***/
  /* PCI EEPRO100 definitions */

-static void pci_map(PCIDevice * pci_dev, int region_num,
-pcibus_t addr, pcibus_t size, int type)
+static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
  {
-EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
-
-TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
-  "size=0x%08"FMT_PCIBUS", type=%d\n",
-  region_num, addr, size, type));
+EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);

 

Please don't change the name of the PCIDevice pointer argument
from pci_dev to dev.

This dev, dev in DO_UPCAST is ugly and misleading.
   


It's very common and I changed it for consistency.  I honestly don't 
care though either way.


Regards,

Anthony Liguori




[Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Ulrich Drepper
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 02/10/2010 02:52 AM, OHMURA Kei wrote:

>  for (i = 0; i < len; i++) {
> -c = bitmap[i];
> -while (c > 0) {
> -j = ffsl(c) - 1;
> -c &= ~(1u << j);
> -page_number = i * 8 + j;
> -addr1 = page_number * TARGET_PAGE_SIZE;
> -addr = offset + addr1;
> -ram_addr = cpu_get_physical_page_desc(addr);
> -cpu_physical_memory_set_dirty(ram_addr);
> -n++;
> +if (bitmap_ul[i] != 0) {
> +c = le_bswap(bitmap_ul[i], HOST_LONG_BITS);
> +while (c > 0) {
> +j = ffsl(c) - 1;
> +c &= ~(1ul << j);
> +page_number = i * HOST_LONG_BITS + j;
> +addr1 = page_number * TARGET_PAGE_SIZE;
> +addr = offset + addr1;
> +ram_addr = cpu_get_physical_page_desc(addr);
> +cpu_physical_memory_set_dirty(ram_addr);
> +}

If you're optimizing this code you might want to do it all.  The
compiler might not see through the bswap call and create unnecessary
data dependencies.  Especially problematic if the bitmap is really
sparse.  Also, the outer test is != while the inner test is >.  Be
consistent.  I suggest to replace the inner loop with

  do {
...
  } while (c != 0);

Depending on how sparse the bitmap is populated this might reduce the
number of data dependencies quite a bit.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAktysDoACgkQ2ijCOnn/RHS2zwCfcj+G0S5ZAEA8MjGAVI/rKjJJ
+0oAnA4njIrwx3/5+o43ekYeYXSNyei0
=ukkz
-END PGP SIGNATURE-




[Qemu-devel] Training request

2010-02-10 Thread Jean-Christophe Voisin
French version at the end of mail.
Please excuse our English.

Hello,

We are two apprentice in the last year of our training as engineers.
We would like to offer our services for the development of qemu. We
have 256 hours of school project to accompling, helped by a teacher
and researcher at ESEO (Ecole Supérieure d'Electronique de l'Ouest, in
Angers, France). This project starts in March and ends in July.

Your project interests us, we are highly motivated by the integration
or the improvement of a target platform in your software, especially
PowerPC 405, 440, ARM7 or Cortex-M3. We already worked with these architectures
in our companies. These propositions are not restrictive, we are open
to any proposition that could allow us to acquire experience in the
field of emulation.

We rely on your knowledge of the qemu project to advise us on what
kind of projects we could achieve in the time we have. We hope this
project will fulfill our objectives of learning and being useful to
the community.

Kind regards,


Antoine Chagneau (antoine.chagn...@gmail.com) and Jean-Christophe
Voisin (jc.voi...@gmail.com)

-- French version --

Bonjour,


Apprentis en 3ème année d'école d'ingénieur à l' ESEO-ITII (Ecole
Supérieure d'Electronique de l'Ouest, à Angers en partenariat avec
l'Institut des Techniques d'Ingénieur de l'Industrie). Nous vous
contactons pour vous proposer notre collaboration au développement de
qemu. Dans le cadre de notre cursus, nous devons effectuer en binôme
un projet de 128 heures chacun (soit 256 heures au total) avec l'appui
d'un enseignant de l'ESEO. Ce projet commence en mars et se termine en
juillet.

Votre projet nous intéresse, nous sommes fortement motivés par
l'intégration dans votre logiciel ou l'amélioration d'une plateforme
cible, en particulier PowerPC 405, 440, ARM7 or Cortex-M3. Ce sont des
architectures avec lesquels nous avons eu l'occasion de travailler
dans nos projets en entreprise. Ces propositions ne sont pas
restrictives, nous sommes ouvert à toute proposition qui nous
permettrait d'acquérir des compétences dans le domaine de l'émulation
de systèmes.

Nous comptons sur votre connaissance du projet qemu pour nous
aiguiller sur le type de sujet réalisable dans le temps imparti qui,
outre un intérêt pédagogique pour nous, répondra au besoin de la
communauté.

Cordialement,


Antoine Chagneau (antoine.chagn...@gmail.com) et Jean-Christophe
Voisin (jc.voi...@gmail.com)




[Qemu-devel] Re: [PATCH] pci: introduce get_dev_dict callback.

2010-02-10 Thread Isaku Yamahata
On Wed, Feb 10, 2010 at 11:45:48AM +0200, Michael S. Tsirkin wrote:
> On Wed, Feb 10, 2010 at 03:59:30PM +0900, Isaku Yamahata wrote:
> > This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5
> 
> Could you please clarify what exactly is the bug,
> and how this patch fixes it?

How about the following?

The change set of 525e05147d5a3bdc08caa422d108c1ef71b584b5
wrongly assumes that pci host bridge class device has
header type of pci-pci bridge. But this isn't true. In fact i440fx
pci host bridge has header type of normal device.
Header type should be checked, instead of device class.

The change set's purpose is to show PBM pci host bridge
info which doesn't conform to PCI specification.
So introduce get_info_quirk callback and use it for PBM.


> > by introducing device specific get_dev_dict callback.
> > pci host bridge doesn't always have header type of bridge.
> > 
> > Especially PBM which is emulated doesn't conform to PCI spec
> > at the moment. So by introducing get_dev_dict, allow each pci device
> > to return specific infos.
> > This patch also makes it easier to enhance info pci command in the future.
> > For example returning pcie stuff.
> > 
> > Cc: Blue Swirl 
> > Cc: "Michael S. Tsirkin" 
> > Signed-off-by: Isaku Yamahata 
> 
> If the idea is to allow quirk mode, how about we call the hook
> get_info_quirk, and only use it for non-spec compliant devices?
> The point being making it clear that most devices should not use it.

Ok. I'll respin the patch with get_info_quirk.


-- 
yamahata




[Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Anthony Liguori
On 02/10/2010 07:20 AM, Avi Kivity wrote:
> On 02/10/2010 12:52 PM, OHMURA Kei wrote:
>   
>> dirty-bitmap-traveling is carried out by byte size in qemu-kvm.c.
>> But We think that dirty-bitmap-traveling by long size is faster than by byte
>> size especially when most of memory is not dirty.
>>
>> --- a/bswap.h
>> +++ b/bswap.h
>> @@ -209,7 +209,6 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
>>  #define cpu_to_32wu cpu_to_le32wu
>>  #endif
>>  
>> -#undef le_bswap
>>  #undef be_bswap
>>  #undef le_bswaps
>>   
>> 
>
> Anthony, is it okay to export le_bswap this way, or will you want
> leul_to_cpu()?
>   

kvm_get_dirty_pages_log_range() is kvm-specific code. We're guaranteed
that when we're using kvm, target byte order == host byte order.

So is it really necessary to use a byte swapping function at all?

Regards,

Anthony Liguori






[Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Anthony Liguori
On 02/10/2010 07:20 AM, Avi Kivity wrote:
> On 02/10/2010 12:52 PM, OHMURA Kei wrote:
>   
>> dirty-bitmap-traveling is carried out by byte size in qemu-kvm.c.
>> But We think that dirty-bitmap-traveling by long size is faster than by byte
>> size especially when most of memory is not dirty.
>>
>> --- a/bswap.h
>> +++ b/bswap.h
>> @@ -209,7 +209,6 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t v)
>>  #define cpu_to_32wu cpu_to_le32wu
>>  #endif
>>  
>> -#undef le_bswap
>>  #undef be_bswap
>>  #undef le_bswaps
>>   
>> 
>
> Anthony, is it okay to export le_bswap this way, or will you want
> leul_to_cpu()?
>   

Oh, I see what's happening here. Yes, I think a leul_to_cpu() makes more
sense.

Regards,

Anthony Liguori





[Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Avi Kivity
On 02/10/2010 05:54 PM, Anthony Liguori wrote:
> On 02/10/2010 07:20 AM, Avi Kivity wrote:
>   
>> On 02/10/2010 12:52 PM, OHMURA Kei wrote:
>>   
>> 
>>> dirty-bitmap-traveling is carried out by byte size in qemu-kvm.c.
>>> But We think that dirty-bitmap-traveling by long size is faster than by byte
>>> size especially when most of memory is not dirty.
>>>
>>> --- a/bswap.h
>>> +++ b/bswap.h
>>> @@ -209,7 +209,6 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t 
>>> v)
>>>  #define cpu_to_32wu cpu_to_le32wu
>>>  #endif
>>>  
>>> -#undef le_bswap
>>>  #undef be_bswap
>>>  #undef le_bswaps
>>>   
>>> 
>>>   
>> Anthony, is it okay to export le_bswap this way, or will you want
>> leul_to_cpu()?
>>   
>> 
> kvm_get_dirty_pages_log_range() is kvm-specific code. We're guaranteed
> that when we're using kvm, target byte order == host byte order.
>
> So is it really necessary to use a byte swapping function at all?
>   

The dirty log bitmap is always little endian. This is so we don't have
to depend on sizeof(long) (which can vary between kernel and userspace)
or mandate some other access size.

(if native endian worked, then the previous byte-based code would have
been broken on big endian).

Seriously, those who say that big vs little endian is a matter of taste
are missing something.


-- 
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] Training request

2010-02-10 Thread Alexander Graf
Hi Jean-Christophe,


Jean-Christophe Voisin wrote:
> French version at the end of mail.
> Please excuse our English.
>
> Hello,
>
> We are two apprentice in the last year of our training as engineers.
> We would like to offer our services for the development of qemu. We
> have 256 hours of school project to accompling, helped by a teacher
> and researcher at ESEO (Ecole Supérieure d'Electronique de l'Ouest, in
> Angers, France). This project starts in March and ends in July.
>
> Your project interests us, we are highly motivated by the integration
> or the improvement of a target platform in your software, especially
> PowerPC 405, 440, ARM7 or Cortex-M3. We already worked with these 
> architectures
> in our companies. These propositions are not restrictive, we are open
> to any proposition that could allow us to acquire experience in the
> field of emulation.
>   

What exactly are you looking forward to emulate? CPUs? Devices?
Also, what background do you have in mind? Speed? Accuracy? Just getting
something to work?

There are quite some fields in Qemu that could use improvement, in all
of the above areas. It usually works out best when you pick something
you're really happy with though ;)

Alex




Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Alexander Graf
Anthony Liguori wrote:
> On 02/10/2010 07:20 AM, Avi Kivity wrote:
>   
>> On 02/10/2010 12:52 PM, OHMURA Kei wrote:
>>   
>> 
>>> dirty-bitmap-traveling is carried out by byte size in qemu-kvm.c.
>>> But We think that dirty-bitmap-traveling by long size is faster than by byte
>>> size especially when most of memory is not dirty.
>>>
>>> --- a/bswap.h
>>> +++ b/bswap.h
>>> @@ -209,7 +209,6 @@ static inline void cpu_to_be32wu(uint32_t *p, uint32_t 
>>> v)
>>>  #define cpu_to_32wu cpu_to_le32wu
>>>  #endif
>>>  
>>> -#undef le_bswap
>>>  #undef be_bswap
>>>  #undef le_bswaps
>>>   
>>> 
>>>   
>> Anthony, is it okay to export le_bswap this way, or will you want
>> leul_to_cpu()?
>>   
>> 
>
> kvm_get_dirty_pages_log_range() is kvm-specific code. We're guaranteed
> that when we're using kvm, target byte order == host byte order.
>
> So is it really necessary to use a byte swapping function at all?
>   

On PPC the bitmap is Little Endian.


Alex




Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Anthony Liguori
On 02/10/2010 10:00 AM, Alexander Graf wrote:
> On PPC the bitmap is Little Endian.
>   

Out of curiousity, why? It seems like an odd interface.

Regards,

Anthony Liguori





Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Alexander Graf
Anthony Liguori wrote:
> On 02/10/2010 10:00 AM, Alexander Graf wrote:
>   
>> On PPC the bitmap is Little Endian.
>>   
>> 
>
> Out of curiousity, why? It seems like an odd interface.
>   

Because on PPC, you usually run PPC32 userspace code on a PPC64 kernel.
Unlike with x86, there's no real benefit in using 64 bit userspace.

So thanks to the nature of big endianness, that breaks our set_bit
helpers, because they assume you're using "long" data types for the
bits. While that's no real issue on little endian, since the next int is
just the high part of a u64, it messes everything up on ppc.

For more details, please just look in the archives on my patches to make
it little endian.


Alex




Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Avi Kivity
On 02/10/2010 06:35 PM, Anthony Liguori wrote:
> On 02/10/2010 10:00 AM, Alexander Graf wrote:
>   
>> On PPC the bitmap is Little Endian.
>>   
>> 
> Out of curiousity, why? It seems like an odd interface.
>
>   

Exactly this issue. If you specify it as unsigned long native endian,
there is ambiguity between 32-bit and 64-bit userspace. If you specify
it as uint64_t native endian, you have an inefficient implementation on
32-bit userspace. So we went for unsigned byte native endian, which is
the same as any size little endian.

(well I think the real reason is that it just grew that way out of x86,
but the above is quite plausible).

-- 
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Avi Kivity
On 02/10/2010 06:43 PM, Alexander Graf wrote:
>
>> Out of curiousity, why? It seems like an odd interface.
>>   
>> 
> Because on PPC, you usually run PPC32 userspace code on a PPC64 kernel.
> Unlike with x86, there's no real benefit in using 64 bit userspace.
>   

btw, does 32-bit ppc qemu support large memory guests? It doesn't on
x86, and I don't remember any hacks to support large memory guests
elsewhere.

-- 
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Alexander Graf
Avi Kivity wrote:
> On 02/10/2010 06:43 PM, Alexander Graf wrote:
>   
>>> Out of curiousity, why? It seems like an odd interface.
>>>   
>>> 
>>>   
>> Because on PPC, you usually run PPC32 userspace code on a PPC64 kernel.
>> Unlike with x86, there's no real benefit in using 64 bit userspace.
>>   
>> 
>
> btw, does 32-bit ppc qemu support large memory guests? It doesn't on
> x86, and I don't remember any hacks to support large memory guests
> elsewhere.
>   


It doesn't :-). In fact, the guest we virtualize wouldn't work with > 2
GB anyways, because that needs an iommu implementation.


Alex




Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Avi Kivity
On 02/10/2010 06:47 PM, Alexander Graf wrote:
>>> Because on PPC, you usually run PPC32 userspace code on a PPC64 kernel.
>>> Unlike with x86, there's no real benefit in using 64 bit userspace.
>>>   
>>> 
>>>   
>> btw, does 32-bit ppc qemu support large memory guests? It doesn't on
>> x86, and I don't remember any hacks to support large memory guests
>> elsewhere.
>>   
>> 
>
> It doesn't :-). In fact, the guest we virtualize wouldn't work with > 2
> GB anyways, because that needs an iommu implementation.
>   

Oh, so you may want to revisit the "there's no real benefit in using 64
bit userspace".

Seriously, that looks like a big deficiency. What would it take to
implement an iommu?

I imagine Anthony's latest patches are a first step in that journey.

-- 
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] Re: [PATCH v2] qemu-kvm: Speed up of the dirty-bitmap-traveling

2010-02-10 Thread Alexander Graf
Avi Kivity wrote:
> On 02/10/2010 06:47 PM, Alexander Graf wrote:
>   
 Because on PPC, you usually run PPC32 userspace code on a PPC64 kernel.
 Unlike with x86, there's no real benefit in using 64 bit userspace.
   
 
   
 
>>> btw, does 32-bit ppc qemu support large memory guests? It doesn't on
>>> x86, and I don't remember any hacks to support large memory guests
>>> elsewhere.
>>>   
>>> 
>>>   
>> It doesn't :-). In fact, the guest we virtualize wouldn't work with > 2
>> GB anyways, because that needs an iommu implementation.
>>   
>> 
>
> Oh, so you may want to revisit the "there's no real benefit in using 64
> bit userspace".
>   

Well, for normal users they don't. SLES11 is 64-bit only, so we're good
on that. But openSUSE uses 32-bit userland.

> Seriously, that looks like a big deficiency. What would it take to
> implement an iommu?
>
> I imagine Anthony's latest patches are a first step in that journey.
>   

All reads/writes from PCI devices would need to go through a wrapper.
Maybe we could also define a per-device offset for memory accesses. That
way the overhead might be less.

Yes, Anthony's patches look like they are a really big step in that
direction.


Alex




[Qemu-devel] [RFC] qcow2: Rewrite alloc_refcount_block

2010-02-10 Thread Kevin Wolf
The current implementation of alloc_refcount_block and grow_refcount_table has
fundamental problems regarding error handling. There are some places where an
I/O error means that the image is going to be corrupted.

I have found that the only way to fix this is to completely rewrite the thing.
This is a first version that passes qemu-iotests but hasn't undergone a lot of
testing otherwise.

For the real submission I think I'm going to split it into maybe three patches,
but I want to get an early review as this is really critical code and tests
alone won't be enough to verify it.

Thorough reviews would really be appreciated.
---
 block/qcow2-refcount.c |  333 +++-
 1 files changed, 243 insertions(+), 90 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 2fdc26b..66a7912 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -27,7 +27,7 @@
 #include "block/qcow2.h"
 
 static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size);
-static int update_refcount(BlockDriverState *bs,
+static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
 int64_t offset, int64_t length,
 int addend);
 
@@ -123,124 +123,265 @@ static int get_refcount(BlockDriverState *bs, int64_t 
cluster_index)
 return be16_to_cpu(s->refcount_block_cache[block_index]);
 }
 
-static int grow_refcount_table(BlockDriverState *bs, int min_size)
+/*
+ * Rounds the refcount table size up to avoid growing the table for each single
+ * refcount block that is allocated.
+ */
+static unsigned int next_refcount_table_size(BDRVQcowState *s,
+unsigned int min_size)
 {
-BDRVQcowState *s = bs->opaque;
-int new_table_size, new_table_size2, refcount_table_clusters, i, ret;
-uint64_t *new_table;
-int64_t table_offset;
-uint8_t data[12];
-int old_table_size;
-int64_t old_table_offset;
+unsigned int refcount_table_clusters = 0;
+unsigned int new_table_size = 1;
 
-if (min_size <= s->refcount_table_size)
-return 0;
-/* compute new table size */
-refcount_table_clusters = s->refcount_table_size >> (s->cluster_bits - 3);
-for(;;) {
+while (min_size > new_table_size) {
 if (refcount_table_clusters == 0) {
 refcount_table_clusters = 1;
 } else {
 refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2;
 }
 new_table_size = refcount_table_clusters << (s->cluster_bits - 3);
-if (min_size <= new_table_size)
-break;
 }
-#ifdef DEBUG_ALLOC2
-printf("grow_refcount_table from %d to %d\n",
-   s->refcount_table_size,
-   new_table_size);
-#endif
-new_table_size2 = new_table_size * sizeof(uint64_t);
-new_table = qemu_mallocz(new_table_size2);
-memcpy(new_table, s->refcount_table,
-   s->refcount_table_size * sizeof(uint64_t));
-for(i = 0; i < s->refcount_table_size; i++)
-cpu_to_be64s(&new_table[i]);
-/* Note: we cannot update the refcount now to avoid recursion */
-table_offset = alloc_clusters_noref(bs, new_table_size2);
-ret = bdrv_pwrite(s->hd, table_offset, new_table, new_table_size2);
-if (ret != new_table_size2)
-goto fail;
-for(i = 0; i < s->refcount_table_size; i++)
-be64_to_cpus(&new_table[i]);
 
-cpu_to_be64w((uint64_t*)data, table_offset);
-cpu_to_be32w((uint32_t*)(data + 8), refcount_table_clusters);
-ret = bdrv_pwrite(s->hd, offsetof(QCowHeader, refcount_table_offset),
-data, sizeof(data));
-if (ret != sizeof(data)) {
-goto fail;
-}
+return new_table_size;
+}
 
-qemu_free(s->refcount_table);
-old_table_offset = s->refcount_table_offset;
-old_table_size = s->refcount_table_size;
-s->refcount_table = new_table;
-s->refcount_table_size = new_table_size;
-s->refcount_table_offset = table_offset;
+/* Checks if two offsets are described by the same refcount block */
+static int in_same_refcount_block(BDRVQcowState *s, uint64_t offset_a,
+uint64_t offset_b)
+{
+uint64_t block_a = offset_a >> (2 * s->cluster_bits - REFCOUNT_SHIFT);
+uint64_t block_b = offset_b >> (2 * s->cluster_bits - REFCOUNT_SHIFT);
 
-update_refcount(bs, table_offset, new_table_size2, 1);
-qcow2_free_clusters(bs, old_table_offset, old_table_size * 
sizeof(uint64_t));
-return 0;
- fail:
-qemu_free(new_table);
-return ret < 0 ? ret : -EIO;
+return (block_a == block_b);
 }
 
-
+/*
+ * Loads a refcount block. If it doesn't exist yet, it is allocated first
+ * (including growing the refcount table if needed).
+ *
+ * Returns the offset of the refcount block on success or -errno in error case
+ */
 static int64_t alloc_refcount_block(BlockDriverState *bs, int64_t 
cluster_index)
 {
 BDRVQcowState *s = bs->opaque;
-int64_t offset, refcount_block_offset;
 unsigned int refcount_

Re: [Qemu-devel] [PATCH] alpha-linux-user: Implement signals.

2010-02-10 Thread Richard Henderson

On 02/10/2010 04:04 AM, Riku Voipio wrote:

On Tue, Feb 09, 2010 at 10:46:32AM -0800, Richard Henderson wrote:

Ping?


The linux-user side of the patch seems fine, but the target-alpha
code doesn't apply. Either something has changed upstream or this
depends on one of your other alpha patches. If the latter case, I
don't mind if this is applied together with the rest of alpha
patches.


Really?  I just cherry-picked the patch out of my submitted branch onto 
a new branch off mainline and it applied just fine.  If there was some 
kind of conflict, it must be trivial.


I'll re-generate the patch.


r~




Re: [Qemu-devel] [PATCH] Fix lost serial TX interrupts. Report receive overruns.

2010-02-10 Thread Anthony Liguori

On 01/28/2010 05:13 PM, Justin T. Gibbs wrote:

This patch compliments the patch submitted by Jergen Lock and further
improves the performance of QEMU's serial emulation with FreeBSD's
uart(9) driver.

 o Implement receive overrun status.  The FreeBSD uart driver
   relies on this status in it's probe routine to determine the size
   of the FIFO supported.
 o As per the 16550 spec, do not overwrite the RX FIFO on an RX overrun.
 o Do not allow TX or RX FIFO overruns to increment the data valid count
   beyond the size of the FIFO.
 o For reads of the IIR register, only clear the "TX holding register
   emtpy interrupt" if the read reports this interrupt.  This is required
   by the specification and avoids losing TX interrupts when other, 
higher

   priority interrupts (usually RX) are reported first.

Signed-off-by: Justin T. Gibbs 


This patch is malformed.  Please export the patch from a git tree via 
git-format-patch.


Regards,

Anthony Liguori



--- serial.c.origThu Jan 14 15:18:00 2010
+++ serial.cThu Jan 28 15:36:04 2010
@@ -169,11 +169,19 @@
 {
 SerialFIFO *f = (fifo) ? &s->recv_fifo : &s->xmit_fifo;

-f->data[f->head++] = chr;
+/* Receive overruns do not overwrite FIFO contents. */
+if (fifo == XMIT_FIFO || f->count < UART_FIFO_LENGTH) {

-if (f->head == UART_FIFO_LENGTH)
-f->head = 0;
-f->count++;
+f->data[f->head++] = chr;
+
+if (f->head == UART_FIFO_LENGTH)
+f->head = 0;
+}
+
+if (f->count < UART_FIFO_LENGTH)
+f->count++;
+else if (fifo == RECV_FIFO)
+s->lsr |= UART_LSR_OE;

 return 1;
 }
@@ -533,8 +541,10 @@
 break;
 case 2:
 ret = s->iir;
+if (ret & UART_IIR_THRI) {
 s->thr_ipending = 0;
-serial_update_irq(s);
+serial_update_irq(s);
+}
 break;
 case 3:
 ret = s->lcr;
@@ -544,9 +554,9 @@
 break;
 case 5:
 ret = s->lsr;
-/* Clear break interrupt */
-if (s->lsr & UART_LSR_BI) {
-s->lsr &= ~UART_LSR_BI;
+/* Clear break and overrun interrupts */
+if (s->lsr & (UART_LSR_BI|UART_LSR_OE)) {
+s->lsr &= ~(UART_LSR_BI|UART_LSR_OE);
 serial_update_irq(s);
 }
 break;
@@ -629,6 +639,8 @@
 /* call the timeout receive callback in 4 char transmit time */
 qemu_mod_timer(s->fifo_timeout_timer, qemu_get_clock 
(vm_clock) + s->char_transmit_time * 4);

 } else {
+if (s->lsr & UART_LSR_DR)
+s->lsr |= UART_LSR_OE;
 s->rbr = buf[0];
 s->lsr |= UART_LSR_DR;
 }










[Qemu-devel] (no subject)

2010-02-10 Thread Stephen Isard

Hello,

I hope you will excuse a naive question here.  I'm not getting any 
response on the users' lists.


I'm wondering whether shutting down a guest machine with the quit 
command in the monitor can corrupt the snapshots saved with savevm.  As 
I understand it,  quit will have an effect on the virtual guest similar 
to pulling the plug on a physical machine, so could leave it in a funny 
state.  Since the snapshots appear to be stored on the guest's hard 
disk, it seems conceivable (in my state of ignorance) that they could be 
put at risk.  Is that in fact possible?


Thanks for your attention.




[Qemu-devel] [PATCH] alpha-linux-user: Implement signals.

2010-02-10 Thread Richard Henderson
Move userland PALcode handling into linux-user main loop so that
we can send signals from there.  This also makes alpha_palcode.c
system-level only, so don't build it for userland.  Add defines
for GENTRAP PALcall mapping to signals.

Signed-off-by: Richard Henderson 
---
 Makefile.target  |3 +-
 hw/alpha_palcode.c   |   81 +---
 linux-user/alpha/target_signal.h |   27 
 linux-user/main.c|  137 
 linux-user/signal.c  |  267 ++
 linux-user/syscall.c |   61 -
 linux-user/syscall_defs.h|   23 +++-
 target-alpha/cpu.h   |4 +-
 target-alpha/translate.c |3 +-
 9 files changed, 489 insertions(+), 117 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index f498574..f752210 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -51,7 +51,6 @@ libobj-$(CONFIG_NOSOFTFLOAT) += fpu/softfloat-native.o
 libobj-y += op_helper.o helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o
 libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
-libobj-$(TARGET_ALPHA) += alpha_palcode.o
 
 # NOTE: the disassembler code is only needed for debugging
 libobj-y += disas.o
@@ -311,6 +310,8 @@ obj-m68k-y += m68k-semi.o dummy_m68k.o
 
 obj-s390x-y = s390-virtio-bus.o s390-virtio.o
 
+obj-alpha-y = alpha_palcode.o
+
 main.o vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
 vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
diff --git a/hw/alpha_palcode.c b/hw/alpha_palcode.c
index 843bd14..c1220ad 100644
--- a/hw/alpha_palcode.c
+++ b/hw/alpha_palcode.c
@@ -21,11 +21,9 @@
 #include 
 #include 
 
-#include "qemu.h"
 #include "cpu.h"
 #include "exec-all.h"
 
-#if !defined (CONFIG_USER_ONLY)
 /* Shared handlers */
 static void pal_reset (CPUState *env);
 /* Console handlers */
@@ -997,12 +995,9 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, uint32_t 
address, int rw,
 uint64_t physical, page_size, end;
 int prot, zbits, ret;
 
-#if defined(CONFIG_USER_ONLY)
-ret = 2;
-#else
-ret = virtual_to_physical(env, &physical, &zbits, &prot,
-  address, mmu_idx, rw);
-#endif
+ret = virtual_to_physical(env, &physical, &zbits, &prot,
+  address, mmu_idx, rw);
+
 switch (ret) {
 case 0:
 /* No fault */
@@ -1050,73 +1045,3 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, uint32_t 
address, int rw,
 return ret;
 }
 #endif
-
-#else /* !defined (CONFIG_USER_ONLY) */
-void pal_init (CPUState *env)
-{
-}
-
-void call_pal (CPUState *env, int palcode)
-{
-target_long ret;
-
-switch (palcode) {
-case 0x80:
-/* BPT */
-qemu_log("BPT\n");
-/* FIXME: Sends SIGTRAP, si_code=TRAP_BRKPT.  */
-exit(1);
-case 0x81:
-/* BUGCHK */
-qemu_log("BUGCHK\n");
-/* FIXME: Sends SIGTRAP, si_code=SI_FAULT.  */
-exit(1);
-case 0x83:
-/* CALLSYS */
-qemu_log("CALLSYS n " TARGET_FMT_ld "\n", env->ir[0]);
-ret = do_syscall(env, env->ir[IR_V0], env->ir[IR_A0], env->ir[IR_A1],
- env->ir[IR_A2], env->ir[IR_A3], env->ir[IR_A4],
- env->ir[IR_A5]);
-if (ret >= 0) {
-env->ir[IR_A3] = 0;
-env->ir[IR_V0] = ret;
-} else {
-env->ir[IR_A3] = 1;
-env->ir[IR_V0] = -ret;
-}
-break;
-case 0x86:
-/* IMB */
-qemu_log("IMB\n");
-/* ??? We can probably elide the code using page_unprotect that is
-   checking for self-modifying code.  Instead we could simply call
-   tb_flush here.  Until we work out the changes required to turn
-   off the extra write protection, this can be a no-op.  */
-break;
-case 0x9E:
-/* RDUNIQUE */
-qemu_log("RDUNIQUE: " TARGET_FMT_lx "\n", env->unique);
-/* Handled in the translator for usermode.  */
-abort();
-case 0x9F:
-/* WRUNIQUE */
-qemu_log("WRUNIQUE: " TARGET_FMT_lx "\n", env->ir[IR_A0]);
-/* Handled in the translator for usermode.  */
-abort();
-case 0xAA:
-/* GENTRAP */
-qemu_log("GENTRAP: " TARGET_FMT_lx "\n", env->ir[IR_A0]);
-/* FIXME: This is supposed to send a signal:
-   SIGFPE:
- GEN_INTOVF, GEN_INTDIV, GEN_FLTOVF, GEN_FLTDIV,
- GEN_FLTUND, GEN_FLTINV, GEN_FLTINE, GEN_ROPRAND
-   SIGTRAP:
- others
-   with various settings of si_code.  */
-exit(1);
-default:
-qemu_log("%s: unhandled palcode %02x\n", __func__, palcode);
-exit(1);
-}
-}
-#endif
diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
index 2382ffd..cb86402 100644
--- a/linux-user/alpha/target_signal.h
+++ b/linux-user/alpha/target_signal.h
@@ -26,4 +26,31 @@ static inline abi_ulong get_sp_from_cpustate(CPUAlphaState 
*state)
 return stat

Re: [Qemu-devel] [PATCH] block: saner flags filtering in bdrv_open2

2010-02-10 Thread Anthony Liguori

On 01/28/2010 08:19 AM, Christoph Hellwig wrote:

Clean up the current mess about figuring out which flags to pass to the
driver.  BDRV_O_FILE, BDRV_O_SNAPSHOT and BDRV_O_NO_BACKING are flags
only used by the block layer internally so filter them out directly.
Previously BDRV_O_NO_BACKING could accidentally be passed to the drivers,
but wasn't ever used.

Signed-off-by: Christoph Hellwig
   


Applied.  Thanks.

Regards,

Anthony Liguori


Index: qemu/block.c
===
--- qemu.orig/block.c   2010-01-28 15:12:52.316024386 +0100
+++ qemu/block.c2010-01-28 15:13:33.419004083 +0100
@@ -451,13 +451,20 @@ int bdrv_open2(BlockDriverState *bs, con
  bs->enable_write_cache = 1;

  bs->read_only = (flags&  BDRV_O_RDWR) == 0;
-if (!(flags&  BDRV_O_FILE)) {
-open_flags = (flags&  (BDRV_O_RDWR | 
BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
-if (bs->is_temporary) { /* snapshot should be writeable */
-open_flags |= BDRV_O_RDWR;
-}
-} else {
-open_flags = flags&  ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
+
+/*
+ * Clear flags that are internal to the block layer before opening the
+ * image.
+ */
+open_flags = flags&  ~(BDRV_O_FILE | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+
+/*
+ * Snapshots should be writeable.
+ *
+ * XXX(hch): and what is the point of a snapshot during a read-only open?
+ */
+if (!(flags&  BDRV_O_FILE)&&  bs->is_temporary) {
+open_flags |= BDRV_O_RDWR;
  }

  ret = drv->bdrv_open(bs, filename, open_flags);



   






Re: [Qemu-devel] [PATCH] Do not ignore error, if open file failed (-serial /dev/tty)

2010-02-10 Thread Anthony Liguori

On 01/28/2010 12:44 PM, Evgeniy Dushistov wrote:

In case, when qemu is executed with option like
-serial /dev/ttyS0, report if there are problems with
opening of devices. At now errors are silently ignoring.

Signed-off-by: Evgeniy Dushistov
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  qemu-char.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 800ee6c..75dbf66 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1180,6 +1180,9 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
  int fd;

  TFR(fd = open(filename, O_RDWR | O_NONBLOCK));
+if (fd<  0) {
+return NULL;
+}
  tty_serial_init(fd, 115200, 'N', 8, 1);
  chr = qemu_chr_open_fd(fd, fd);
  if (!chr) {
   






Re: [Qemu-devel] [PATCH v2] qemu-img: Fix qemu-img can't create qcow image based on read-only image

2010-02-10 Thread Anthony Liguori

On 01/28/2010 08:15 PM, Sheng Yang wrote:

Commit 03cbdac7 "Disable fall-back to read-only when cannot open drive's
file for read-write" result in read-only image can't be used as backed
image in qemu-img.

Cc: Naphtali Sprei
Signed-off-by: Sheng Yang
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  qemu-img.c |   15 ++-
  1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 3cea8ce..ac4d15e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -188,11 +188,13 @@ static int read_password(char *buf, int buf_size)
  #endif

  static BlockDriverState *bdrv_new_open(const char *filename,
-   const char *fmt)
+   const char *fmt,
+   int readonly)
  {
  BlockDriverState *bs;
  BlockDriver *drv;
  char password[256];
+int flags = BRDV_O_FLAGS;

  bs = bdrv_new("");
  if (!bs)
@@ -204,7 +206,10 @@ static BlockDriverState *bdrv_new_open(const char 
*filename,
  } else {
  drv = NULL;
  }
-if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv)<  0) {
+if (!readonly) {
+flags |= BDRV_O_RDWR;
+}
+if (bdrv_open2(bs, filename, flags, drv)<  0) {
  error("Could not open '%s'", filename);
  }
  if (bdrv_is_encrypted(bs)) {
@@ -343,7 +348,7 @@ static int img_create(int argc, char **argv)
  }
  }

-bs = bdrv_new_open(backing_file->value.s, fmt);
+bs = bdrv_new_open(backing_file->value.s, fmt, 1);
  bdrv_get_geometry(bs,&size);
  size *= 512;
  bdrv_delete(bs);
@@ -627,7 +632,7 @@ static int img_convert(int argc, char **argv)

  total_sectors = 0;
  for (bs_i = 0; bs_i<  bs_n; bs_i++) {
-bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt);
+bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, 1);
  if (!bs[bs_i])
  error("Could not open '%s'", argv[optind + bs_i]);
  bdrv_get_geometry(bs[bs_i],&bs_sectors);
@@ -685,7 +690,7 @@ static int img_convert(int argc, char **argv)
  }
  }

-out_bs = bdrv_new_open(out_filename, out_fmt);
+out_bs = bdrv_new_open(out_filename, out_fmt, 0);

  bs_i = 0;
  bs_offset = 0;
   






Re: [Qemu-devel] [PATCH] doc: Update mingw cross compile instructions

2010-02-10 Thread Anthony Liguori

On 01/29/2010 01:28 PM, Scott Tsai wrote:

The "Cross compilation for Windows with Linux" section of qemu-doc.texi
still instructs the user to use 'configure --enable-mingw32'
even after the option was removed in Aug 2008:
http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=cd01b4a312248dd4e12c3d389d1a349cea4015d8

This documentation only change updates the instructions to:
* Remove use of '--enable-mingw32' in the configure example
* Correct the 'sdl-config' script name
* Remove references to i386-mingw32msvc.tar.gz which no longer exists in
   recent SDL releases
* Document the zlib dependency

Signed-off-by: Scott Tsai
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  qemu-doc.texi |   45 +++--
  1 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 2fb5c0b..4e220d0 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2313,11 +2313,14 @@ instructions in the download section and the FAQ.
  @item Download
  the MinGW development library of SDL 1.2.x
  (@file{SDL-devel-1.2.x-@/mingw32.tar.gz}) from
-...@url{http://www.libsdl.org}. Unpack it in a temporary place, and
-unpack the archive @file{i386-mingw32msvc.tar.gz} in the MinGW tool
-directory. Edit the @file{sdl-config} script so that it gives the
+...@url{http://www.libsdl.org}. Unpack it in a temporary place and
+edit the @file{sdl-config} script so that it gives the
  correct SDL directory when invoked.

+...@item Install the MinGW version of zlib and make sure
+...@file{zlib.h} and @file{libz.dll.a} are in
+MingGW's default header and linker search paths.
+
  @item Extract the current version of QEMU.

  @item Start the MSYS shell (file @file{msys.bat}).
@@ -2340,29 +2343,43 @@ correct SDL directory when invoked.
  Install the MinGW cross compilation tools available at
  @url{http://www.mingw.org/}.

-...@item
-Install the Win32 version of SDL (@url{http://www.libsdl.org}) by
-unpacking @file{i386-mingw32msvc.tar.gz}. Set up the PATH environment
-variable so that @file{i386-mingw32msvc-sdl-config} can be launched by
+...@item Download
+the MinGW development library of SDL 1.2.x
+(@file{SDL-devel-1.2.x-@/mingw32.tar.gz}) from
+...@url{http://www.libsdl.org}. Unpack it in a temporary place and
+edit the @file{sdl-config} script so that it gives the
+correct SDL directory when invoked.  Set up the @code{PATH} environment
+variable so that @file{sdl-config} can be launched by
  the QEMU configuration script.

+...@item Install the MinGW version of zlib and make sure
+...@file{zlib.h} and @file{libz.dll.a} are in
+MingGW's default header and linker search paths.
+
  @item
  Configure QEMU for Windows cross compilation:
  @example
-./configure --enable-mingw32
+PATH=/usr/i686-pc-mingw32/sys-root/mingw/bin:$PATH ./configure 
--cross-prefix='i686-pc-mingw32-'
+...@end example
+The example assumes @file{sdl-config} is installed under 
@file{/usr/i686-pc-mingw32/sys-root/mingw/bin} and
+MinGW cross compilation tools have names like @file{i686-pc-mingw32-gcc} and 
@file{i686-pc-mingw32-strip}.
+We set the @code{PATH} environment variable to ensure the MingW version of 
@file{sdl-config} is used and
+use --cross-prefix to specify the name of the cross compiler.
+You can also use --prefix to set the Win32 install path which defaults to 
@file{c:/Program Files/Qemu}.
+
+Under Fedora Linux, you can run:
+...@example
+yum -y install mingw32-gcc mingw32-SDL mingw32-zlib
  @end example
-If necessary, you can change the cross-prefix according to the prefix
-chosen for the MinGW tools with --cross-prefix. You can also use
---prefix to set the Win32 install path.
+to get a suitable cross compilation environment.

  @item You can install QEMU in the installation directory by typing
-...@file{make install}. Don't forget to copy @file{SDL.dll} in the
+...@code{make install}. Don't forget to copy @file{SDL.dll} and 
@file{zlib1.dll} into the
  installation directory.

  @end itemize

-Note: Currently, Wine does not seem able to launch
-QEMU for Win32.
+Wine can be used to launch the resulting qemu.exe compiled for Win32.

  @node Mac OS X
  @section Mac OS X
   






Re: [Qemu-devel] [PATCH] Documentation: Add build support for documentation in pdf format

2010-02-10 Thread Anthony Liguori

On 01/29/2010 04:16 PM, Stefan Weil wrote:

Makefile already supported dvi, html and info formats,
but pdf was missing.

pdf is especially convenient for printing and for
documentation reviews. I hope it will help to
improve qemu's documentation.

Make now supports the new target 'pdf' which will
create qemu-doc.pdf and qemu-tech.pdf. It is also
possible to build both files individually.

texi2pdf and texi2dvi are rather noisy, so normally
some less important warnings are suppressed.
When make is called with V=1 (verbose mode),
warnings are not suppressed.

The patch also sorts the documentation targets
alphabetically and wraps a line which was too long.

Signed-off-by: Stefan Weil
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  .gitignore |1 +
  Makefile   |   21 +
  2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/.gitignore b/.gitignore
index d7d2146..dfc8e5b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,6 +36,7 @@ qemu-monitor.texi
  *.fn
  *.ky
  *.log
+*.pdf
  *.pg
  *.toc
  *.tp
diff --git a/Makefile b/Makefile
index 3848627..692bd75 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,7 @@ Makefile: ;
  configure: ;

  .PHONY: all clean cscope distclean dvi html info install install-doc \
-   recurse-all speed tar tarbin test build-all
+   pdf recurse-all speed tar tarbin test build-all

  $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)

@@ -160,7 +160,7 @@ distclean: clean
rm -f config-host.mak config-host.h* config-host.ld $(DOCS) 
qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
rm -f config-all-devices.mak
rm -f roms/seabios/config.mak roms/vgabios/config.mak
-   rm -f qemu-{doc,tech}.{info,aux,cp,dvi,fn,info,ky,log,pg,toc,tp,vr}
+   rm -f qemu-{doc,tech}.{info,aux,cp,dvi,fn,info,ky,log,pdf,pg,toc,tp,vr}
for d in $(TARGET_DIRS) libhw32 libhw64 libuser; do \
rm -rf $$d || exit 1 ; \
  done
@@ -224,14 +224,18 @@ cscope:
cscope -b

  # documentation
+TEXIFLAG=$(if $(V),,--quiet)
+%.dvi: %.texi
+   $(call quiet-command,texi2dvi $(TEXIFLAG) -I . $<,"  GEN   $@")
+
  %.html: %.texi
$(call quiet-command,texi2html -I=. -monolithic -number $<,"  GEN   $@")

  %.info: %.texi
$(call quiet-command,makeinfo -I . $<  -o $@,"  GEN   $@")

-%.dvi: %.texi
-   $(call quiet-command,texi2dvi -I . $<,"  GEN   $@")
+%.pdf: %.texi
+   $(call quiet-command,texi2pdf $(TEXIFLAG) -I . $<,"  GEN   $@")

  qemu-options.texi: $(SRC_PATH)/qemu-options.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -t<  $<  >  $@,"  GEN   $@")
@@ -260,13 +264,14 @@ qemu-nbd.8: qemu-nbd.texi
  pod2man --section=8 --center=" " --release=" " qemu-nbd.pod>  $@, \
  "  GEN   $@")

-info: qemu-doc.info qemu-tech.info
-
  dvi: qemu-doc.dvi qemu-tech.dvi
-
  html: qemu-doc.html qemu-tech.html
+info: qemu-doc.info qemu-tech.info
+pdf: qemu-doc.pdf qemu-tech.pdf

-qemu-doc.dvi qemu-doc.html qemu-doc.info: qemu-img.texi qemu-nbd.texi 
qemu-options.texi qemu-monitor.texi qemu-img-cmds.texi
+qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
+   qemu-img.texi qemu-nbd.texi qemu-options.texi \
+   qemu-monitor.texi qemu-img-cmds.texi

  VERSION ?= $(shell cat VERSION)
  FILE = qemu-$(VERSION)
   






Re: [Qemu-devel] [PATCH] qcow2: Fix signedness bugs

2010-02-10 Thread Anthony Liguori

On 02/02/2010 08:20 AM, Kevin Wolf wrote:

Checking for return codes<  0 isn't really going to work with unsigned
types. Use signed types instead.

Signed-off-by: Kevin Wolf
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  block/qcow2-cluster.c |   12 ++--
  block/qcow2.h |6 ++
  2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 4e30d16..3501a94 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -219,7 +219,8 @@ static uint64_t *l2_allocate(BlockDriverState *bs, int 
l1_index)
  BDRVQcowState *s = bs->opaque;
  int min_index;
  uint64_t old_l2_offset;
-uint64_t *l2_table, l2_offset;
+uint64_t *l2_table;
+int64_t l2_offset;

  old_l2_offset = s->l1_table[l1_index];

@@ -560,7 +561,8 @@ uint64_t 
qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
  {
  BDRVQcowState *s = bs->opaque;
  int l2_index, ret;
-uint64_t l2_offset, *l2_table, cluster_offset;
+uint64_t l2_offset, *l2_table;
+int64_t cluster_offset;
  int nb_csectors;

  ret = get_cluster_table(bs, offset,&l2_table,&l2_offset,&l2_index);
@@ -704,10 +706,8 @@ err:
   *
   * Return 0 on success and -errno in error cases
   */
-uint64_t qcow2_alloc_cluster_offset(BlockDriverState *bs,
-uint64_t offset,
-int n_start, int n_end,
-int *num, QCowL2Meta *m)
+int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int n_start, int n_end, int *num, QCowL2Meta *m)
  {
  BDRVQcowState *s = bs->opaque;
  int l2_index, ret;
diff --git a/block/qcow2.h b/block/qcow2.h
index d9ea6ab..de9397a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -192,10 +192,8 @@ void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t 
sector_num,

  uint64_t qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
  int *num);
-uint64_t qcow2_alloc_cluster_offset(BlockDriverState *bs,
-  uint64_t offset,
-  int n_start, int n_end,
-  int *num, QCowL2Meta *m);
+int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int n_start, int n_end, int *num, QCowL2Meta *m);
  uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
   uint64_t offset,
   int compressed_size);
   






Re: [Qemu-devel] [PATCH 1/3] do not loop on an incomplete io_thread_fd read

2010-02-10 Thread Anthony Liguori

On 02/02/2010 01:33 PM, Paolo Bonzini wrote:

No need to loop if less than a full buffer is read, the next
read would return EAGAIN.

Signed-off-by: Paolo Bonzini
   


Applied all.  Thanks.

Regards,

Anthony Liguori

---
  vl.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 6f1e1ab..46c1118 100644
--- a/vl.c
+++ b/vl.c
@@ -3210,12 +3210,12 @@ static void qemu_event_read(void *opaque)
  {
  int fd = (unsigned long)opaque;
  ssize_t len;
+char buffer[512];

  /* Drain the notify pipe */
  do {
-char buffer[512];
  len = read(fd, buffer, sizeof(buffer));
-} while ((len == -1&&  errno == EINTR) || len>  0);
+} while ((len == -1&&  errno == EINTR) || len == sizeof(buffer));
  }

  static int qemu_event_init(void)
   






Re: [Qemu-devel] [PATCH 1/5] QMP: BLOCK_IO_ERROR event handling

2010-02-10 Thread Anthony Liguori

On 02/03/2010 08:41 AM, Luiz Capitulino wrote:

This commit adds the basic definitions for the BLOCK_IO_ERROR
event, but actual event emission will be introduced by the
next commits.

Signed-off-by: Luiz Capitulino
   


Applied all.  Thanks.

Regards,

Anthony Liguori

---
  QMP/qmp-events.txt |   21 +
  monitor.c  |3 +++
  monitor.h  |1 +
  3 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index dc48ccc..d585a8d 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -43,3 +43,24 @@ Data: 'server' and 'client' keys with the same keys as 
'query-vnc'.

  Description: Issued when the VNC session is made active.
  Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
+
+7 BLOCK_IO_ERROR
+
+
+Description: Issued when a disk I/O error occurs
+Data:
+
+- 'device': device name (json-string)
+- 'operation': I/O operation (json-string, "read" or "write")
+- 'action': action that has been taken, it's one of the following:
+"ignore": error has been ignored
+"report": error has been reported to the device
+"stop": error caused VM to be stopped
+
+Example:
+
+{ "event": "BLOCK_IO_ERROR",
+"data": { "device": "ide0-hd1",
+  "operation": "write",
+  "action": "stop" },
+"timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
diff --git a/monitor.c b/monitor.c
index fb7c572..6e688ac 100644
--- a/monitor.c
+++ b/monitor.c
@@ -378,6 +378,9 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
  case QEVENT_VNC_DISCONNECTED:
  event_name = "VNC_DISCONNECTED";
  break;
+case QEVENT_BLOCK_IO_ERROR:
+event_name = "BLOCK_IO_ERROR";
+break;
  default:
  abort();
  break;
diff --git a/monitor.h b/monitor.h
index b0f9270..e35f1e4 100644
--- a/monitor.h
+++ b/monitor.h
@@ -23,6 +23,7 @@ typedef enum MonitorEvent {
  QEVENT_VNC_CONNECTED,
  QEVENT_VNC_INITIALIZED,
  QEVENT_VNC_DISCONNECTED,
+QEVENT_BLOCK_IO_ERROR,
  QEVENT_MAX,
  } MonitorEvent;

   






Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces

2010-02-10 Thread Blue Swirl
On Wed, Feb 10, 2010 at 12:01 AM, Anthony Liguori  wrote:
> This is a work in progress that I wanted to share giving some of the 
> discussions
> around rwhandlers.  The idea is to make PCI devices have a common set of
> functions to interact with the CPU that is driven entirely through the PCI 
> bus.
>
> I've tested the network card conversions, but have not yet tested the other
> bits.

By itself, the patches look OK. But given that the conclusion of the
generic DMA discussion was to aim for mapping approach (in order to
prepare for zero copy DMA), I wonder how the patches fit to the larger
picture. Perhaps instead of pci specific functions, a more generic
memory object with read and write methods could be introduced, which
would be more useful in the longer term?




Re: [Qemu-devel] [PATCH] configure: Add --enable-docs and --disable-docs to --help

2010-02-10 Thread Anthony Liguori

On 02/06/2010 02:48 AM, Dirk Ullrich wrote:

This patch adds the documentation-related options "--enable-docs" and
"--disable-docs" to the help message of "configure".

Signed-off-by: Dirk Ullrich
   


This patch is white space damaged and this is not a valid SoB line.

Regards,

Anthony Liguori


---
  configure |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 42ef628..eac2a15 100755
--- a/configure
+++ b/configure
@@ -794,6 +794,8 @@ echo "  --enable-linux-aio   enable Linux AIO support"
  echo "  --enable-io-thread   enable IO thread"
  echo "  --disable-blobs  disable installing provided firmware blobs"
  echo "  --kerneldir=PATH look for kernel includes in PATH"
+echo "  --enable-docsenable installing of documentation"
+echo "  --disable-docs   disable installing of documentation"
  echo ""
  echo "NOTE: The object files are built at the place where configure
is launched"
  exit 1
   






Re: [Qemu-devel] [PATCH] add close callback for tty-based char device

2010-02-10 Thread Anthony Liguori

On 02/03/2010 10:18 AM, David S. Ahern wrote:

Add a tty close callback. Right now if a guest device that is connected
to a tty-based chardev in the host is removed, the tty is not closed.
With this patch it is closed.

Example use case is connecting an emulated USB serial cable in the guest
to tty0 of the host using the monitor command:

usb_add serial::/dev/tty0

and then removing the device with:

usb_del serial::/dev/tty0

Signed-off-by: David Ahern
   


This patch is whitespace damaged.


diff --git a/qemu-char.c b/qemu-char.c
index 800ee6c..ecd84ec 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1173,6 +1173,20 @@ static int tty_serial_ioctl(CharDriverState *chr,
int cmd
   


Right here and in other places.

Regards,

Anthony Liguori


  return 0;
  }

+static void qemu_chr_close_tty(CharDriverState *chr)
+{
+FDCharDriver *s = chr->opaque;
+int fd = -1;
+
+if (s)
+fd = s->fd_in;
+
+fd_chr_close(chr);
+
+if (fd>= 0)
+close(fd);
+}
+
  static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
  {
  const char *filename = qemu_opt_get(opts, "path");
@@ -1187,6 +1201,7 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts
*opts)
  return NULL;
  }
  chr->chr_ioctl = tty_serial_ioctl;
+chr->chr_close = qemu_chr_close_tty;
  return chr;
  }
  #else  /* ! __linux__&&  ! __sun__ */




   






Re: [Qemu-devel] [PATCH] audio streaming from usb devices

2010-02-10 Thread Anthony Liguori

On 02/03/2010 09:49 AM, David S. Ahern wrote:

I have streaming audio devices working within qemu-kvm. This is a port
of the changes to qemu.

Streaming audio generates a series of isochronous requests that are
repetitive and time sensitive. The URBs need to be submitted in
consecutive USB frames and responses need to be handled in a timely manner.

Summary of the changes for isochronous requests:

1. The initial 'valid' value is increased to 32. It needs to be higher
than its current value of 10 since the host adds a 10 frame delay to the
scheduling of the first request; if valid is set to 10 the first
isochronous request times out and qemu cancels it. 32 was chosen as a
nice round number, and it is used in the path where a TD-async pairing
already exists.

2. The token field in the TD is *not* unique for isochronous requests,
so it is not a good choice for finding a matching async request. The
buffer (where to write the guest data) is unique, so use that value instead.

3. TD's for isochronous request need to be completed in the async
completion handler so that data is pushed to the guest as soon as it is
available. The uhci code currently attempts to process complete
isochronous TDs the next time the UHCI frame with the request is
processed. The results in lost data since the async requests will have
long since timed out based on the valid parameter. Increasing the valid
value is not acceptable as it introduces a 1+ second delay in the data
getting pushed to the guest.

4. The frame timer needs to be run on 1 msec intervals. Currently, the
expire time for the processing the next frame is computed after the
processing of each frame. This regularly causes the scheduling of frames
to shift in time. When this happens the periodic scheduling of the
requests is broken and the subsequent request is seen as a new request
by the host resulting in a 10 msec delay (first isochronous request is
scheduled for 10 frames from when the URB is submitted).


[ For what's worth a small change is needed to the guest driver to have
more outstanding URBs (at least 4 URBs with 5 packets per URB).]

Signed-off-by: David Ahern
   


Applied.  Thanks.

Regards,

Anthony Liguori





Re: [Qemu-devel] [PATCH] segfault due to buffer overrun in usb-serial

2010-02-10 Thread Anthony Liguori

On 02/03/2010 10:00 AM, David S. Ahern wrote:

This fixes a segfault due to buffer overrun in the usb-serial device.
The memcpy was incrementing the start location by recv_used yet, the
computation of first_size (how much to write at the end of the buffer
before wrapping to the front) was not accounting for it. This causes the
next element after the receive buffer (recv_ptr) to get overwritten with
random data.

Signed-off-by: David Ahern
   


Applied.  Thanks.

Regards,

Anthony Liguori

diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index 37293ea..c3f3401 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -497,12 +497,28 @@ static int usb_serial_can_read(void *opaque)
  static void usb_serial_read(void *opaque, const uint8_t *buf, int size)
  {
  USBSerialState *s = opaque;
-int first_size = RECV_BUF - s->recv_ptr;
-if (first_size>  size)
-first_size = size;
-memcpy(s->recv_buf + s->recv_ptr + s->recv_used, buf, first_size);
-if (size>  first_size)
-memcpy(s->recv_buf, buf + first_size, size - first_size);
+int first_size, start;
+
+/* room in the buffer? */
+if (size>  (RECV_BUF - s->recv_used))
+size = RECV_BUF - s->recv_used;
+
+start = s->recv_ptr + s->recv_used;
+if (start<  RECV_BUF) {
+/* copy data to end of buffer */
+first_size = RECV_BUF - start;
+if (first_size>  size)
+first_size = size;
+
+memcpy(s->recv_buf + start, buf, first_size);
+
+/* wrap around to front if needed */
+if (size>  first_size)
+memcpy(s->recv_buf, buf + first_size, size - first_size);
+} else {
+start -= RECV_BUF;
+memcpy(s->recv_buf + start, buf, size);
+}
  s->recv_used += size;
  }





   






Re: [PATCH] JSON: add %I64d support (Was: Re: [Qemu-devel] system_reset command cause assert failed)

2010-02-10 Thread Anthony Liguori

On 02/03/2010 08:30 PM, Roy Tam wrote:

2010/2/4 Roy Tam:
   

2010/2/3 Luiz Capitulino:
 

OK we are fooled by the json lexer and parser. As we use %I64d to
print 'long long' variables in Win32, but lexer and parser only deal
with %lld but not %I64d, this patch add support for %I64d and solve
'info pci', 'powser_reset' and 'power_powerdown' assert failure in
Win32.

P.S.: an assert(state.result != NULL) statement in
qobject_from_jsonv() will be good for asserting failure of parsing
JSON strings.
   



Applied.  Thanks.

Regards,

Anthony Liguori


diff --git a/json-lexer.c b/json-lexer.c
index 53697c5..9d64920 100644
--- a/json-lexer.c
+++ b/json-lexer.c
@@ -54,6 +54,9 @@ enum json_lexer_state {
  IN_ESCAPE,
  IN_ESCAPE_L,
  IN_ESCAPE_LL,
+IN_ESCAPE_I,
+IN_ESCAPE_I6,
+IN_ESCAPE_I64,
  IN_ESCAPE_DONE,
  IN_WHITESPACE,
  IN_OPERATOR_DONE,
@@ -223,6 +226,18 @@ static const uint8_t json_lexer[][256] =  {
  ['l'] = IN_ESCAPE_LL,
  },

+[IN_ESCAPE_I64] = {
+['d'] = IN_ESCAPE_DONE,
+},
+
+[IN_ESCAPE_I6] = {
+['4'] = IN_ESCAPE_I64,
+},
+
+[IN_ESCAPE_I] = {
+['6'] = IN_ESCAPE_I6,
+},
+
  [IN_ESCAPE] = {
  ['d'] = IN_ESCAPE_DONE,
  ['i'] = IN_ESCAPE_DONE,
@@ -230,6 +245,7 @@ static const uint8_t json_lexer[][256] =  {
  ['s'] = IN_ESCAPE_DONE,
  ['f'] = IN_ESCAPE_DONE,
  ['l'] = IN_ESCAPE_L,
+['I'] = IN_ESCAPE_I,
  },

  /* top level rule */
diff --git a/json-parser.c b/json-parser.c
index e04932f..40a5d15 100644
--- a/json-parser.c
+++ b/json-parser.c
@@ -474,7 +474,7 @@ static QObject *parse_escape(JSONParserContext
*ctxt, QList **tokens, va_list *a
  obj = QOBJECT(qint_from_int(va_arg(*ap, int)));
  } else if (token_is_escape(token, "%ld")) {
  obj = QOBJECT(qint_from_int(va_arg(*ap, long)));
-} else if (token_is_escape(token, "%lld")) {
+} else if (token_is_escape(token, "%lld") ||
token_is_escape(token, "%I64d")) {
  obj = QOBJECT(qint_from_int(va_arg(*ap, long long)));
  } else if (token_is_escape(token, "%s")) {
  obj = QOBJECT(qstring_from_str(va_arg(*ap, const char *)));



   






Re: [Qemu-devel] [PATCH] kvm: reduce code duplication in config_iothread

2010-02-10 Thread Anthony Liguori

On 02/04/2010 08:46 AM, Amit Shah wrote:

We have some duplicated code in the CONFIG_IOTHREAD #ifdef and #else
cases. Fix that.

Signed-off-by: Amit Shah
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  kvm-all.c |9 +++--
  1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 15ec38e..a2bd78e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -901,14 +901,11 @@ void kvm_setup_guest_memory(void *start, size_t size)
  static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
  {
  #ifdef CONFIG_IOTHREAD
-if (env == cpu_single_env) {
-func(data);
-return;
+if (env != cpu_single_env) {
+abort();
  }
-abort();
-#else
-func(data);
  #endif
+func(data);
  }

  struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env,
   






Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces

2010-02-10 Thread Anthony Liguori

On 02/10/2010 12:34 PM, Blue Swirl wrote:

On Wed, Feb 10, 2010 at 12:01 AM, Anthony Liguori  wrote:
   

This is a work in progress that I wanted to share giving some of the discussions
around rwhandlers.  The idea is to make PCI devices have a common set of
functions to interact with the CPU that is driven entirely through the PCI bus.

I've tested the network card conversions, but have not yet tested the other
bits.
 

By itself, the patches look OK. But given that the conclusion of the
generic DMA discussion was to aim for mapping approach (in order to
prepare for zero copy DMA), I wonder how the patches fit to the larger
picture. Perhaps instead of pci specific functions, a more generic
memory object with read and write methods could be introduced, which
would be more useful in the longer term?
   


This is a good point and it's something that needs to be addressed for 
the virtio conversion.


I was thinking:

void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int 
is_write);


void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int 
is_write, pcibus_t access_len);


Not too surprising.

Since virtio devices can live on two busses (sysbus with Syborg or PCI), 
we need to introduce a set of virtio specific functions.


void virtio_memory_read(VirtIODevice *dev, uint64_t addr, void *buf, int len);
void virtio_memory_write(VirtIODevice *dev, uint64_t addr, const void *buf, int 
len);
void *virtio_memory_map(VirtIODevice *dev, uint64_t addr, uint64_t *plen, int 
is_write);
void virtio_memory_unmap(VirtIODevice *dev, uint64_t addr, uint64_t *plen, int 
is_write, uint64_t access_len);

Inside the VirtIODevice, there would be corresponding function pointers, and 
depending on whether it was a PCI device or a Syborg device, it would call 
pci_memory_map or cpu_physical_memory_map.

If we introduced a consistent address type, it would be possible to make 
generic memory access functions that used a state variable to mask this from 
the user.

I personally prefer the explicit interfaces though.  It makes it easy to look 
at a PCI device and see that it only uses PCI interfaces.  I also think that 
some bus concepts will be difficult to abstract in a generic way.

Regards,

Anthony Liguori







Re: [Qemu-devel] [PATCH 1/4] QMP: Add QEMU's version to the greeting message

2010-02-10 Thread Anthony Liguori

On 02/04/2010 02:10 PM, Luiz Capitulino wrote:

With capability negotiation support clients will only have a chance
to check QEMU's version (ie. issue 'query-version') after the
negotiation procedure is done.

It might be useful to clients to check QEMU's version before
negotiating features, though.

To allow that, this commit adds the QEMU's version object to the
greeting message.

Not really sure this is needed, but doesn't hurt anyway.

Signed-off-by: Luiz Capitulino
   


Applied all.  Thanks.

Regards,

Anthony Liguori

---
  QMP/README   |6 --
  QMP/qmp-spec.txt |6 --
  monitor.c|   10 +-
  3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/QMP/README b/QMP/README
index 09e7053..9334c25 100644
--- a/QMP/README
+++ b/QMP/README
@@ -52,9 +52,11 @@ $ telnet localhost 
  Trying 127.0.0.1...
  Connected to localhost.
  Escape character is '^]'.
-{"QMP": {"capabilities": []}}
+{"QMP": {"version": {"qemu": "0.12.50", "package": ""}, "capabilities": []}}
+{ "execute": "qmp_capabilities" }
+{"return": {}}
  { "execute": "query-version" }
-{"return": {"qemu": "0.11.50", "package": ""}}
+{"return": {"qemu": "0.12.50", "package": ""}}

  Contact
  ---
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 56f388c..b2617bb 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -48,10 +48,12 @@ waiting for commands.

  The format is:

-{ "QMP": { "capabilities": json-array } }
+{ "QMP": { "version": json-object, "capabilities": json-array } }

   Where,

+- The "version" member contains the Server's version information (the format
+  is the same of the 'query-version' command)
  - The "capabilities" member specify the availability of features beyond the
baseline specification

@@ -152,7 +154,7 @@ This section provides some examples of real QMP usage, in 
all of them
  3.1 Server greeting
  ---

-S: {"QMP": {"capabilities": []}}
+S: {"QMP": {"version": {"qemu": "0.12.50", "package": ""}, "capabilities": []}}

  3.2 Simple 'stop' execution
  ---
diff --git a/monitor.c b/monitor.c
index ff22123..ec1d11e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4365,6 +4365,14 @@ void monitor_resume(Monitor *mon)
  readline_show_prompt(mon->rs);
  }

+static QObject *get_qmp_greeting(void)
+{
+QObject *ver;
+
+do_info_version(NULL,&ver);
+return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities': 
[]}}",ver);
+}
+
  /**
   * monitor_control_event(): Print QMP gretting
   */
@@ -4376,7 +4384,7 @@ static void monitor_control_event(void *opaque, int event)

  json_message_parser_init(&mon->mc->parser, handle_qmp_command);

-data = qobject_from_jsonf("{ 'QMP': { 'capabilities': [] } }");
+data = get_qmp_greeting();
  assert(data != NULL);

  monitor_json_emitter(mon, data);
   






Re: [Qemu-devel] [PATCH v2] vnc: Migrate to using QTAILQ instead of custom implementation

2010-02-10 Thread Anthony Liguori

On 02/05/2010 05:04 AM, Amit Shah wrote:

Just a 1-1 conversion for now.

Signed-off-by: Amit Shah
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
v2:
  - QTAILQ_INIT the queue.

  vnc.c |   74 
  vnc.h |5 ++-
  2 files changed, 31 insertions(+), 48 deletions(-)

diff --git a/vnc.c b/vnc.c
index 92facde..2200fbf 100644
--- a/vnc.c
+++ b/vnc.c
@@ -356,17 +356,14 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
  *ret_data = qobject_from_jsonf("{ 'enabled': false }");
  } else {
  QList *clist;
+VncState *client;

  clist = qlist_new();
-if (vnc_display->clients) {
-VncState *client = vnc_display->clients;
-while (client) {
-if (client->info) {
-/* incref so that it's not freed by upper layers */
-qobject_incref(client->info);
-qlist_append_obj(clist, client->info);
-}
-client = client->next;
+QTAILQ_FOREACH(client,&vnc_display->clients, next) {
+if (client->info) {
+/* incref so that it's not freed by upper layers */
+qobject_incref(client->info);
+qlist_append_obj(clist, client->info);
  }
  }

@@ -519,7 +516,7 @@ static void vnc_dpy_resize(DisplayState *ds)
  {
  int size_changed;
  VncDisplay *vd = ds->opaque;
-VncState *vs = vd->clients;
+VncState *vs;

  /* server surface */
  if (!vd->server)
@@ -540,7 +537,7 @@ static void vnc_dpy_resize(DisplayState *ds)
  *(vd->guest.ds) = *(ds->surface);
  memset(vd->guest.dirty, 0xFF, sizeof(vd->guest.dirty));

-while (vs != NULL) {
+QTAILQ_FOREACH(vs,&vd->clients, next) {
  vnc_colordepth(vs);
  if (size_changed) {
  if (vs->csock != -1&&  vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
@@ -553,7 +550,6 @@ static void vnc_dpy_resize(DisplayState *ds)
  }
  }
  memset(vs->dirty, 0xFF, sizeof(vs->dirty));
-vs = vs->next;
  }
  }

@@ -867,8 +863,7 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int 
src_y, int dst_x, int
  int cmp_bytes;

  vnc_refresh_server_surface(vd);
-for (vs = vd->clients; vs != NULL; vs = vn) {
-vn = vs->next;
+QTAILQ_FOREACH_SAFE(vs,&vd->clients, next, vn) {
  if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
  vs->force_update = 1;
  vnc_update_client(vs, 1);
@@ -912,11 +907,10 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int 
src_y, int dst_x, int
  if (memcmp(src_row, dst_row, cmp_bytes) == 0)
  continue;
  memmove(dst_row, src_row, cmp_bytes);
-vs = vd->clients;
-while (vs != NULL) {
-if (!vnc_has_feature(vs, VNC_FEATURE_COPYRECT))
+QTAILQ_FOREACH(vs,&vd->clients, next) {
+if (!vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
  vnc_set_bit(vs->dirty[y], ((x + dst_x) / 16));
-vs = vs->next;
+}
  }
  }
  src_row += pitch - w * depth;
@@ -924,9 +918,10 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int 
src_y, int dst_x, int
  y += inc;
  }

-for (vs = vd->clients; vs != NULL; vs = vs->next) {
-if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT))
+QTAILQ_FOREACH(vs,&vd->clients, next) {
+if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
  vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h);
+}
  }
  }

@@ -1109,19 +1104,11 @@ static void vnc_disconnect_finish(VncState *vs)
  #endif /* CONFIG_VNC_SASL */
  audio_del(vs);

-VncState *p, *parent = NULL;
-for (p = vs->vd->clients; p != NULL; p = p->next) {
-if (p == vs) {
-if (parent)
-parent->next = p->next;
-else
-vs->vd->clients = p->next;
-break;
-}
-parent = p;
-}
-if (!vs->vd->clients)
+QTAILQ_REMOVE(&vs->vd->clients, vs, next);
+
+if (QTAILQ_EMPTY(&vs->vd->clients)) {
  dcl->idle = 1;
+}

  vnc_remove_timer(vs->vd);
  qemu_free(vs);
@@ -2299,7 +2286,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
  uint8_t *server_row;
  int cmp_bytes;
  uint32_t width_mask[VNC_DIRTY_WORDS];
-VncState *vs = NULL;
+VncState *vs;
  int has_dirty = 0;

  /*
@@ -2328,10 +2315,8 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
  if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0)
  continue;
  memcpy(server_ptr, guest_ptr, cmp_bytes);
-vs = vd->clients;
-while (vs != NULL) {
+QTAILQ_FOREACH(vs,&vd->clients, next) {
  vnc_set_bit(vs->dirty[y], (x / 16));
- 

Re: [Qemu-devel] [PATCH 2/8] Documentation: Add direntry for info format

2010-02-10 Thread Anthony Liguori

On 02/05/2010 04:51 PM, Stefan Weil wrote:

update-info-dir maintains an index of all available
documentation in info format (the file /usr/share/info/dir).

It reads special @direntry tags in info files.

This patch (extracted from a larger patch provided by
Dirk Ullrich) adds these tags for qemu-doc.info and
qemu-tech.info.

Signed-off-by: Stefan Weil
   


Applied 2-8.  I already picked up 1/8 when you submitted it separately.  
Thanks.


Regards,

Anthony Liguori

---
  qemu-doc.texi  |6 ++
  qemu-tech.texi |6 ++
  2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 2fb5c0b..eba6437 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -6,6 +6,12 @@
  @paragraphindent 0
  @c %**end of header

+...@ifinfo
+...@direntry
+* QEMU: (qemu-doc).The QEMU Emulator User Documentation.
+...@end direntry
+...@end ifinfo
+
  @iftex
  @titlepage
  @sp 7
diff --git a/qemu-tech.texi b/qemu-tech.texi
index 97d8dea..52560dc 100644
--- a/qemu-tech.texi
+++ b/qemu-tech.texi
@@ -6,6 +6,12 @@
  @paragraphindent 0
  @c %**end of header

+...@ifinfo
+...@direntry
+* QEMU Internals: (qemu-tech).   The QEMU Emulator Internals.
+...@end direntry
+...@end ifinfo
+
  @iftex
  @titlepage
  @sp 7
   






Re: [Qemu-devel] [PATCH] configure: Add --enable-docs and --disable-docs to --help

2010-02-10 Thread Anthony Liguori

On 02/06/2010 02:48 AM, Dirk Ullrich wrote:

This patch adds the documentation-related options "--enable-docs" and
"--disable-docs" to the help message of "configure".

Signed-off-by: Dirk Ullrich
   


Applied.  But I missed that you weren't using a valid SoB.  Please use a 
proper email address in the future.  Thanks.


Regards,

Anthony Liguori

---
  configure |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 42ef628..eac2a15 100755
--- a/configure
+++ b/configure
@@ -794,6 +794,8 @@ echo "  --enable-linux-aio   enable Linux AIO support"
  echo "  --enable-io-thread   enable IO thread"
  echo "  --disable-blobs  disable installing provided firmware blobs"
  echo "  --kerneldir=PATH look for kernel includes in PATH"
+echo "  --enable-docsenable installing of documentation"
+echo "  --disable-docs   disable installing of documentation"
  echo ""
  echo "NOTE: The object files are built at the place where configure
is launched"
  exit 1
   






[Qemu-devel] Re: Network shutdown under load

2010-02-10 Thread Anthony Liguori

On 02/08/2010 10:10 AM, Tom Lendacky wrote:

Fix a race condition where qemu finds that there are not enough virtio
ring buffers available and the guest make more buffers available before
qemu can enable notifications.

Signed-off-by: Tom Lendacky
Signed-off-by: Anthony Liguori
   


Applied.  Thanks.  We should audit the code for proper barrier support.  
Right now, I think there's a lot of places that we're missing them.


Regards,

Anthony Liguori


  hw/virtio-net.c |   10 +-
  1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6e48997..5c0093e 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -379,7 +379,15 @@ static int virtio_net_has_buffers(VirtIONet *n, int 
bufsize)
  (n->mergeable_rx_bufs&&
   !virtqueue_avail_bytes(n->rx_vq, bufsize, 0))) {
  virtio_queue_set_notification(n->rx_vq, 1);
-return 0;
+
+/* To avoid a race condition where the guest has made some buffers
+ * available after the above check but before notification was
+ * enabled, check for available buffers again.
+ */
+if (virtio_queue_empty(n->rx_vq) ||
+(n->mergeable_rx_bufs&&
+ !virtqueue_avail_bytes(n->rx_vq, bufsize, 0)))
+return 0;
  }

  virtio_queue_set_notification(n->rx_vq, 0);

On Friday 29 January 2010 02:06:41 pm Tom Lendacky wrote:
   

There's been some discussion of this already in the kvm list, but I want to
summarize what I've found and also include the qemu-devel list in an effort
  to find a solution to this problem.

Running a netperf test between two kvm guests results in the guest's
  network interface shutting down. I originally found this using kvm guests
  on two different machines that were connected via a 10GbE link.  However,
  I found this problem can be easily reproduced using two guests on the same
  machine.

I am running the 2.6.32 level of the kvm.git tree and the 0.12.1.2 level of
the qemu-kvm.git tree.

The setup includes two bridges, br0 and br1.

The commands used to start the guests are as follows:
usr/local/bin/qemu-system-x86_64 -name cape-vm001 -m 1024 -drive
file=/autobench/var/tmp/cape-vm001-
raw.img,if=virtio,index=0,media=disk,boot=on -net
nic,model=virtio,vlan=0,macaddr=00:16:3E:00:62:51,netdev=cape-vm001-eth0 -
netdev tap,id=cape-vm001-eth0,script=/autobench/var/tmp/ifup-kvm-
br0,downscript=/autobench/var/tmp/ifdown-kvm-br0 -net
nic,model=virtio,vlan=1,macaddr=00:16:3E:00:62:D1,netdev=cape-vm001-eth1 -
netdev tap,id=cape-vm001-eth1,script=/autobench/var/tmp/ifup-kvm-
br1,downscript=/autobench/var/tmp/ifdown-kvm-br1 -vnc :1 -monitor
telnet::5701,server,nowait -snapshot -daemonize

usr/local/bin/qemu-system-x86_64 -name cape-vm002 -m 1024 -drive
file=/autobench/var/tmp/cape-vm002-
raw.img,if=virtio,index=0,media=disk,boot=on -net
nic,model=virtio,vlan=0,macaddr=00:16:3E:00:62:61,netdev=cape-vm002-eth0 -
netdev tap,id=cape-vm002-eth0,script=/autobench/var/tmp/ifup-kvm-
br0,downscript=/autobench/var/tmp/ifdown-kvm-br0 -net
nic,model=virtio,vlan=1,macaddr=00:16:3E:00:62:E1,netdev=cape-vm002-eth1 -
netdev tap,id=cape-vm002-eth1,script=/autobench/var/tmp/ifup-kvm-
br1,downscript=/autobench/var/tmp/ifdown-kvm-br1 -vnc :2 -monitor
telnet::5702,server,nowait -snapshot -daemonize

The ifup-kvm-br0 script takes the (first) qemu created tap device and
  brings it up and adds it to bridge br0.  The ifup-kvm-br1 script take the
  (second) qemu created tap device and brings it up and adds it to bridge
  br1.

Each ethernet device within a guest is on it's own subnet.  For example:
   guest 1 eth0 has addr 192.168.100.32 and eth1 has addr 192.168.101.32
   guest 2 eth0 has addr 192.168.100.64 and eth1 has addr 192.168.101.64

On one of the guests run netserver:
   netserver -L 192.168.101.32 -p 12000

On the other guest run netperf:
   netperf -L 192.168.101.64 -H 192.168.101.32 -p 12000 -t TCP_STREAM -l 60
  -c -C -- -m 16K -M 16K

It may take more than one netperf run (I find that my second run almost
  always causes the shutdown) but the network on the eth1 links will stop
  working.

I did some debugging and found that in qemu on the guest running netserver:
  - the receive_disabled variable is set and never gets reset
  - the read_poll event handler for the eth1 tap device is disabled and
  never re-enabled
These conditions result in no packets being read from the tap device and
  sent to the guest - effectively shutting down the network.  Network
  connectivity can be restored by shutting down the guest interfaces,
  unloading the virtio_net module, re-loading the virtio_net module and
  re-starting the guest interfaces.

I'm continuing to work on debugging this, but would appreciate if some
  folks with more qemu network experience could try to recreate and debug
  this.

If my kernel config matters, I can provide that.

Thanks,
Tom
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a me

Re: [Qemu-devel] [PATCH] iothread: fix vcpu stop with smp tcg

2010-02-10 Thread Anthony Liguori

On 02/09/2010 08:49 AM, Marcelo Tosatti wrote:

Round robin vcpus in tcg_cpu_next even if the vm stopped. This
allows all cpus to enter stopped state.

Signed-off-by: Marcelo Tosatti
   


Applied.  Thanks.

Regards,

Anthony Liguori

diff --git a/vl.c b/vl.c
index 880bcd5..f61e362 100644
--- a/vl.c
+++ b/vl.c
@@ -3855,14 +3855,15 @@ static void tcg_cpu_exec(void)
  for (; next_cpu != NULL; next_cpu = next_cpu->next_cpu) {
  CPUState *env = cur_cpu = next_cpu;

-if (!vm_running)
-break;
  if (timer_alarm_pending) {
  timer_alarm_pending = 0;
  break;
  }
  if (cpu_can_run(env))
  ret = qemu_cpu_exec(env);
+else if (env->stop)
+break;
+
  if (ret == EXCP_DEBUG) {
  gdb_set_stop_cpu(env);
  debug_requested = 1;



   






Re: [Qemu-devel] [PATCH] fix inet_parse typo

2010-02-10 Thread Anthony Liguori

On 02/09/2010 11:31 AM, Marcelo Tosatti wrote:

qemu_opt_set wants on/off, not yes/no.

Signed-off-by: Marcelo Tosatti
   


Applied.  Thanks.

Regards,

Anthony Liguori

diff --git a/qemu-sockets.c b/qemu-sockets.c
index 8850516..a88b2a7 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -424,7 +424,7 @@ static int inet_parse(QemuOpts *opts, const char *str)
  __FUNCTION__, str);
  return -1;
  }
-qemu_opt_set(opts, "ipv6", "yes");
+qemu_opt_set(opts, "ipv6", "on");
  } else if (qemu_isdigit(str[0])) {
  /* IPv4 addr */
  if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) {
@@ -432,7 +432,7 @@ static int inet_parse(QemuOpts *opts, const char *str)
  __FUNCTION__, str);
  return -1;
  }
-qemu_opt_set(opts, "ipv4", "yes");
+qemu_opt_set(opts, "ipv4", "on");
  } else {
  /* hostname */
  if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) {
@@ -450,9 +450,9 @@ static int inet_parse(QemuOpts *opts, const char *str)
  if (h)
  qemu_opt_set(opts, "to", h+4);
  if (strstr(optstr, ",ipv4"))
-qemu_opt_set(opts, "ipv4", "yes");
+qemu_opt_set(opts, "ipv4", "on");
  if (strstr(optstr, ",ipv6"))
-qemu_opt_set(opts, "ipv6", "yes");
+qemu_opt_set(opts, "ipv6", "on");
  return 0;
  }




   






Re: [Qemu-devel] [PATCH 1/2] versatile_pci: convert to symbolic names

2010-02-10 Thread Anthony Liguori

On 02/08/2010 03:41 PM, Michael S. Tsirkin wrote:

This converts versatile_pci to use symbolic
constants. Verified by comparing binary to
original one.

Signed-off-by: Michael S. Tsirkin
   


Acked-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/versatile_pci.c |   17 ++---
  1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 153c651..e58b7f4 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -147,14 +147,17 @@ static int versatile_pci_host_init(PCIDevice *d)
  pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_XILINX);
  /* Both boards have the same device ID.  Oh well.  */
  pci_config_set_device_id(d->config, PCI_DEVICE_ID_XILINX_XC2VP30);
-d->config[0x04] = 0x00;
-d->config[0x05] = 0x00;
-d->config[0x06] = 0x20;
-d->config[0x07] = 0x02;
-d->config[0x08] = 0x00; // revision
-d->config[0x09] = 0x00; // programming i/f
+/* TODO: no need to clear command */
+pci_set_byte(d->config + PCI_COMMAND, 0x00);
+pci_set_byte(d->config + PCI_COMMAND + 1, 0x00);
+/* TODO: convert to set_word */
+pci_set_byte(d->config + PCI_STATUS, PCI_STATUS_66MHZ);
+pci_set_byte(d->config + PCI_STATUS + 1, PCI_STATUS_DEVSEL_MEDIUM>>  8);
+/* TODO: no need to clear revision/prog ifc */
+pci_set_byte(d->config + PCI_REVISION_ID, 0x00);
+pci_set_byte(d->config + PCI_CLASS_PROG, 0x00);
  pci_config_set_class(d->config, PCI_CLASS_PROCESSOR_CO);
-d->config[0x0D] = 0x10; // latency_timer
+pci_set_byte(d->config + PCI_LATENCY_TIMER, 0x10);
  return 0;
  }

   






Re: [Qemu-devel] [PATCH 2/2] versatile_pci: cleanup

2010-02-10 Thread Anthony Liguori

On 02/08/2010 03:43 PM, Michael S. Tsirkin wrote:

Cleanup versatile_pci: no need to re-set fields
to zero (pci core sets 0 already), use set_word
for status field. Compile-tested only, but seems obvious.

Signed-off-by: Michael S. Tsirkin
   


Acked-by: Anthony Liguori 

Regards,

Anthony Liguori

---
  hw/versatile_pci.c |   11 ++-
  1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index e58b7f4..7f79348 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -147,15 +147,8 @@ static int versatile_pci_host_init(PCIDevice *d)
  pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_XILINX);
  /* Both boards have the same device ID.  Oh well.  */
  pci_config_set_device_id(d->config, PCI_DEVICE_ID_XILINX_XC2VP30);
-/* TODO: no need to clear command */
-pci_set_byte(d->config + PCI_COMMAND, 0x00);
-pci_set_byte(d->config + PCI_COMMAND + 1, 0x00);
-/* TODO: convert to set_word */
-pci_set_byte(d->config + PCI_STATUS, PCI_STATUS_66MHZ);
-pci_set_byte(d->config + PCI_STATUS + 1, PCI_STATUS_DEVSEL_MEDIUM>>  8);
-/* TODO: no need to clear revision/prog ifc */
-pci_set_byte(d->config + PCI_REVISION_ID, 0x00);
-pci_set_byte(d->config + PCI_CLASS_PROG, 0x00);
+pci_set_word(d->config + PCI_STATUS,
+PCI_STATUS_66MHZ | PCI_STATUS_DEVSEL_MEDIUM);
  pci_config_set_class(d->config, PCI_CLASS_PROCESSOR_CO);
  pci_set_byte(d->config + PCI_LATENCY_TIMER, 0x10);
  return 0;
   






Re: [Qemu-devel] [PATCH] Add cpu model configuration support.. (resend)

2010-02-10 Thread Anthony Liguori

On 02/01/2010 01:02 PM, john cooper wrote:

[target-x86_64.conf was unintentionally omitted from the earlier patch]

This is a reimplementation of prior versions which adds
the ability to define cpu models for contemporary processors.
The added models are likewise selected via -cpu,
and are intended to displace the existing convention
of "-cpu qemu64" augmented with a series of feature flags
   


This breaks the arm-softmmu build.

Regards,

Anthony Liguori


A primary motivation was determination of a least common
denominator within a given processor class to simplify guest
migration.  It is still possible to modify an arbitrary model
via additional feature flags however the goal here was to
make doing so unnecessary in typical usage.  The other
consideration was providing models names reflective of
current processors.  Both AMD and Intel have reviewed the
models in terms of balancing generality of migration vs.
excessive feature downgrade relative to released silicon.

This version of the patch replaces the prior hard wired
definitions with a configuration file approach for new
models.  Existing models are thus far left as-is but may
easily be transitioned to (or may be overridden by) the
configuration file representation.

Proposed new model definitions are provided here for current
AMD and Intel processors.  Each model consists of a name
used to select it on the command line (-cpu), and a
model_id which corresponds to a least common denominator
commercial instance of the processor class.

A table of names/model_ids may be queried via "-cpu ?model":

 :
 x86   Opteron_G3  AMD Opteron 23xx (Gen 3 Class Opteron)
 x86   Opteron_G2  AMD Opteron 22xx (Gen 2 Class Opteron)
 x86   Opteron_G1  AMD Opteron 240 (Gen 1 Class Opteron)
 x86  Nehalem  Intel Core i7 9xx (Nehalem Class Core i7)
 x86   Penryn  Intel Core 2 Duo P9xxx (Penryn Class Core 2)
 x86   Conroe  Intel Celeron_4x0 (Conroe/Merom Class Core 2)
 :

Also added is "-cpu ?dump" which exhaustively outputs all config
data for all defined models, and "-cpu ?cpuid" which enumerates
all qemu recognized CPUID feature flags.

The pseudo cpuid flag 'check' when added to the feature flag list
will warn when feature flags (either implicit in a cpu model or
explicit on the command line) would have otherwise been quietly
unavailable to a guest:

 # qemu-system-x86_64 ... -cpu Nehalem,check
 warning: host cpuid _0001 lacks requested flag 'sse4.2|sse4_2' 
[0x0010]
 warning: host cpuid _0001 lacks requested flag 'popcnt' [0x0080]

A similar 'enforce' pseudo flag exists which in addition
to the above causes qemu to error exit if requested flags are
unavailable.

Configuration data for a cpu model resides in the target config
file which by default will be installed as:

 /usr/local/etc/qemu/target-.conf

The format of this file should be self explanatory given the
definitions for the above six models and essentially mimics
the structure of the static x86_def_t x86_defs.

Encoding of cpuid flags names now allows aliases for both the
configuration file and the command line which reconciles some
Intel/AMD/Linux/Qemu naming differences.

This patch was tested relative to qemu.git.

Signed-off-by: john cooper
---

diff --git a/Makefile b/Makefile
index 3848627..b7fa6ef 100644
--- a/Makefile
+++ b/Makefile
@@ -191,7 +191,11 @@ ifdef CONFIG_POSIX
$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
  endif

-install: all $(if $(BUILD_DOCS),install-doc)
+install-sysconfig:
+   $(INSTALL_DIR) "$(sysconfdir)/qemu"
+   $(INSTALL_DATA) sysconfigs/target/target-x86_64.conf 
"$(sysconfdir)/qemu"
+
+install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
  ifneq ($(TOOLS),)
$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
diff --git a/qemu-config.c b/qemu-config.c
index c3203c8..246fae6 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -242,6 +242,54 @@ QemuOptsList qemu_mon_opts = {
  },
  };

+QemuOptsList qemu_cpudef_opts = {
+.name = "cpudef",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_cpudef_opts.head),
+.desc = {
+{
+.name = "name",
+.type = QEMU_OPT_STRING,
+},{
+.name = "level",
+.type = QEMU_OPT_NUMBER,
+},{
+.name = "vendor",
+.type = QEMU_OPT_STRING,
+},{
+.name = "family",
+.type = QEMU_OPT_NUMBER,
+},{
+.name = "model",
+.type = QEMU_OPT_NUMBER,
+},{
+.name = "stepping",
+.type = QEMU_OPT_NUMBER,
+},{
+.name = "feature_edx",  /* cpuid _0001.edx */
+.type = QEMU_OPT_STRING,
+},{
+.name = "feature_ecx",  /* cpuid _0001.ecx */
+.type = QEMU_OPT_STRING,
+},{
+.name = "extfeature_edx",

Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces

2010-02-10 Thread Richard Henderson

On 02/10/2010 11:29 AM, Anthony Liguori wrote:

void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int
is_write);

void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int
is_write, pcibus_t access_len);


Are these functions intended to be controllable by the root bus object? 
 It would be awfully nice if we would design in a hook that allowed 
iommu mapping to be done properly.



r~




Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces

2010-02-10 Thread Anthony Liguori

On 02/10/2010 02:41 PM, Richard Henderson wrote:

On 02/10/2010 11:29 AM, Anthony Liguori wrote:

void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int
is_write);

void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int
is_write, pcibus_t access_len);


Are these functions intended to be controllable by the root bus 
object?  It would be awfully nice if we would design in a hook that 
allowed iommu mapping to be done properly.


Yes, that's the point.  For something like virtio, you would have a call 
chain like:


virtio_memory_map -> pci_memory_map -> sysbus_memory_map[1] -> 
cpu_memory_map.


Each layer has the ability to do things like implement iommu mapping.

[1] I think it might make sense to have a sysbus layer but I'm not 100% 
sure yet.


Regards,

Anthony Liguori



r~






[Qemu-devel] forking i386 binaries on arm linux user mode

2010-02-10 Thread Damion Yates
I've grabbed the latest stable qemu and compiled under scratchbox.  I
hit an issue compiling it, with no __builtin__clear_cache() so linked in
a kludge.c containing a call to __clear_cache() with the params passed
as they would be to __builtin__clear_cache().

Firstly does this sound like it should work as a workaround?

It certainly got me to the next level, which is that I can now run loads
of linux binaries on my armlinux system (a Nokia n900). I've tried tower
toppler (http://toppler.sourceforge.net/) which uses SDL (via X11) and
this was surprisingly fast, in fact it almost felt as fast as the native
toppler that somebody crosscompiled already. Most linux utils work when
I copy then and any dependant libs from my x86 laptop to the phone. I'm
lucky (I guess) that /lib/ld-linux.so.3 is the arm version and I'm using
a slightly older .2 for x86 so I can have both files there. I also
enabled arbitrary execution of binaries via binfmt_misc. The 600 Mhz Arm
V8 Cortex (I think it is), feels like it's running at about Pentium 90
speeds, which I'm hoping is enough for what I really want to get going.

I want to run an old, possibly win16 Windows game under wine. I saw that
user mode qemu-i386 was able to run wine in a post in 2004:
http://lists.terrasoftsolutions.com/pipermail/yellowdog-general/2004-June/014468.html
 - This was on a PPC however.

When I run wine it SEGVs out and the strace of it shows it dies trying
to do clone(). I also can't run things like xterm which can't do fork().
Is this because by default it's trying to go via the arm "/bin/sh" to
invoke whatever it wants to exec() in to?

Should clone()/fork() work?  Has anyone been able to run wine ./blah.exe
under user-linux mode of qemu on arm or indeed any other non x86 based
CPU ?

Damion




Re: [Qemu-devel] [PATCH] qemu-img: use the heap instead of the huge stack array for win32

2010-02-10 Thread Anthony Liguori

On 02/08/2010 02:20 AM, TeLeMan wrote:

The default stack size of PE is 1MB on win32 and IO_BUF_SIZE in
img_convert()&  img_rebase() is 2MB, so qemu-img will crash when doing
"convert"&  "rebase" on win32.
Although we can improve the stack size of PE to resolve it, I think we
should avoid using the huge stack variables.

Signed-off-by: TeLeMan
   


Applied.  Thanks.

Regards,

Anthony Liguori
   

---

  qemu-img.c |   14 +++---
  1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index bbfeea1..9994b3d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -571,7 +571,7 @@ static int img_convert(int argc, char **argv)
  BlockDriverState **bs, *out_bs;
  int64_t total_sectors, nb_sectors, sector_num, bs_offset;
  uint64_t bs_sectors;
-uint8_t buf[IO_BUF_SIZE];
+uint8_t * buf;
  const uint8_t *buf1;
  BlockDriverInfo bdi;
  QEMUOptionParameter *param = NULL;
@@ -690,6 +690,7 @@ static int img_convert(int argc, char **argv)
  bs_i = 0;
  bs_offset = 0;
  bdrv_get_geometry(bs[0],&bs_sectors);
+buf = qemu_malloc(IO_BUF_SIZE);

  if (flags&  BLOCK_FLAG_COMPRESS) {
  if (bdrv_get_info(out_bs,&bdi)<  0)
@@ -822,6 +823,7 @@ static int img_convert(int argc, char **argv)
  }
  }
  }
+qemu_free(buf);
  bdrv_delete(out_bs);
  for (bs_i = 0; bs_i<  bs_n; bs_i++)
  bdrv_delete(bs[bs_i]);
@@ -1178,8 +1180,11 @@ static int img_rebase(int argc, char **argv)
  uint64_t num_sectors;
  uint64_t sector;
  int n, n1;
-uint8_t buf_old[IO_BUF_SIZE];
-uint8_t buf_new[IO_BUF_SIZE];
+uint8_t * buf_old;
+uint8_t * buf_new;
+
+buf_old = qemu_malloc(IO_BUF_SIZE);
+buf_new = qemu_malloc(IO_BUF_SIZE);

  bdrv_get_geometry(bs,&num_sectors);

@@ -1226,6 +1231,9 @@ static int img_rebase(int argc, char **argv)
  written += pnum;
  }
  }
+
+qemu_free(buf_old);
+qemu_free(buf_new);
  }

  /*
   






Re: [Qemu-devel] [PATCH] don't dereference NULL after failed strdup

2010-02-10 Thread Anthony Liguori

On 02/08/2010 12:28 PM, Jim Meyering wrote:

Most of these are obvious NULL-deref bug fixes, for example,
the ones in these files:

   block/curl.c
   net.c
   slirp/misc.c

and the first one in block/vvfat.c.
The others in block/vvfat.c may not lead to an immediate segfault, but I
traced the two schedule_rename(..., strdup(path)) uses, and a failed
strdup would appear to trigger this assertion in handle_renames_and_mkdirs:

assert(commit->path);

The conversion to use qemu_strdup in envlist_to_environ is not technically
needed, but does avoid a theoretical leak in the caller when strdup fails
for one value, but later succeeds in allocating another buffer(plausible,
if one string length is much larger than the others).  The caller does
not know the length of the returned list, and as such can only free
pointers until it hits the first NULL.  If there are non-NULL pointers
beyond the first, their buffers would be leaked.  This one is admittedly
far-fetched.

The two in linux-user/main.c are worth fixing to ensure that an
OOM error is diagnosed up front, rather than letting it provoke some
harder-to-diagnose secondary error, in case of exec failure, or worse, in
case the exec succeeds but with an invalid list of command line options.
However, considering how unlikely it is to encounter a failed strdup early
in main, this isn't a big deal.  Note that adding the required uses of
qemu_strdup here and in envlist.c induce link failures because qemu_strdup
is not currently in any library they're linked with.  So for now, I've
omitted those changes, as well as the fixes in target-i386/helper.c
and target-sparc/helper.c.

If you'd like to see the above discussion (or anything else)
in the commit log, just let me know and I'll be happy to adjust.


> From 9af42864fd1ea666bd25e2cecfdfae74c20aa8c7 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Mon, 8 Feb 2010 18:29:29 +0100
Subject: [PATCH] don't dereference NULL after failed strdup

Handle failing strdup by replacing each use with qemu_strdup,
so as not to dereference NULL or trigger a failing assertion.
* block/curl.c (curl_open): s/\bstrdup\b/qemu_strdup/
* block/vvfat.c (init_directories): Likewise.
(get_cluster_count_for_direntry, check_directory_consistency): Likewise.
* net.c (parse_host_src_port): Likewise.
   



Applied, but this got ugly in git am.  FYI, if you want to include 
comments with a patch that aren't in the git commit message, you can use 
a '---' separator just like with diffstat below.


Regards,

Anthony Liguori


* slirp/misc.c (fork_exec): Likewise.
---
  block/curl.c  |2 +-
  block/vvfat.c |   10 +-
  net.c |2 +-
  slirp/misc.c  |2 +-
  4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index fe08f7b..2cf72cb 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -309,7 +309,7 @@ static int curl_open(BlockDriverState *bs, const char 
*filename, int flags)

  static int inited = 0;

-file = strdup(filename);
+file = qemu_strdup(filename);
  s->readahead_size = READ_AHEAD_SIZE;

  /* Parse a trailing ":readahead=#:" param, if present. */
diff --git a/block/vvfat.c b/block/vvfat.c
index d2787b9..bb707c0 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -883,7 +883,7 @@ static int init_directories(BDRVVVFATState* s,
  mapping->dir_index = 0;
  mapping->info.dir.parent_mapping_index = -1;
  mapping->first_mapping_index = -1;
-mapping->path = strdup(dirname);
+mapping->path = qemu_strdup(dirname);
  i = strlen(mapping->path);
  if (i>  0&&  mapping->path[i - 1] == '/')
mapping->path[i - 1] = '\0';
@@ -1633,10 +1633,10 @@ static uint32_t 
get_cluster_count_for_direntry(BDRVVVFATState* s,

/* rename */
if (strcmp(basename, basename2))
-   schedule_rename(s, cluster_num, strdup(path));
+   schedule_rename(s, cluster_num, qemu_strdup(path));
} else if (is_file(direntry))
/* new file */
-   schedule_new_file(s, strdup(path), cluster_num);
+   schedule_new_file(s, qemu_strdup(path), cluster_num);
else {
assert(0);
return 0;
@@ -1753,10 +1753,10 @@ static int check_directory_consistency(BDRVVVFATState 
*s,
mapping->mode&= ~MODE_DELETED;

if (strcmp(basename, basename2))
-   schedule_rename(s, cluster_num, strdup(path));
+   schedule_rename(s, cluster_num, qemu_strdup(path));
  } else
/* new directory */
-   schedule_mkdir(s, cluster_num, strdup(path));
+   schedule_mkdir(s, cluster_num, qemu_strdup(path));

  lfn_init(&lfn);
  do {
diff --git a/net.c b/net.c
index 6ef93e6..8e951ca 100644
--- a/net.c
+++ b/net.c
@@ -96,7 +96,7 @@ int parse_host_src_port(struct sockaddr_in *haddr,
  struct sockaddr_in *saddr,
  const char *input_str)
  {
-char *str = strdup(input_str);
+char *str = qemu_strdup(inpu

Re: [Qemu-devel] [PATCH 1/4] qjson: Improve debugging

2010-02-10 Thread Anthony Liguori

On 02/08/2010 01:01 PM, Luiz Capitulino wrote:

Add an assert() to qobject_from_jsonf() to assure that the returned
QObject is not NULL. Currently this is duplicated in the callers.

Signed-off-by: Luiz Capitulino
   


Applied all.  Thanks.

Regards,

Anthony Liguori


---
  qjson.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/qjson.c b/qjson.c
index 9ad8a91..483c667 100644
--- a/qjson.c
+++ b/qjson.c
@@ -53,6 +53,10 @@ QObject *qobject_from_json(const char *string)
  return qobject_from_jsonv(string, NULL);
  }

+/*
+ * IMPORTANT: This function aborts on error, thus it must not
+ * be used with untrusted arguments.
+ */
  QObject *qobject_from_jsonf(const char *string, ...)
  {
  QObject *obj;
@@ -62,6 +66,7 @@ QObject *qobject_from_jsonf(const char *string, ...)
  obj = qobject_from_jsonv(string,&ap);
  va_end(ap);

+assert(obj != NULL);
  return obj;
  }

   






Re: [Qemu-devel] [PATCH 1/1] Increase VNC_MAX_WIDTH

2010-02-10 Thread Anthony Liguori

On 02/08/2010 02:22 PM, Brian Jackson wrote:

Increase VNC_MAX_WIDTH to match "commonly available" consumer level monitors
available these days.

This also closes KVM bug 2907597

Signed-off-by: Brian Jackson
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  vnc.h |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vnc.h b/vnc.h
index 1210824..1575af2 100644
--- a/vnc.h
+++ b/vnc.h
@@ -68,7 +68,7 @@ typedef void VncSendHextileTile(VncState *vs,
  void *last_fg,
  int *has_bg, int *has_fg);

-#define VNC_MAX_WIDTH 2048
+#define VNC_MAX_WIDTH 2560
  #define VNC_MAX_HEIGHT 2048
  #define VNC_DIRTY_WORDS (VNC_MAX_WIDTH / (16 * 32))




   






[Qemu-devel] [PATCH] add close callback for tty-based char device

2010-02-10 Thread David Ahern
Add a tty close callback. Right now if a guest device that is connected
to a tty-based chardev in the host is removed, the tty is not closed.
With this patch it is closed. 

Example use case is connecting an emulated USB serial cable in the guest
to ttyS0 of the host using the monitor command:

usb_add serial::/dev/ttyS0

and then removing the device with:

usb_del serial::/dev/ttyS0

Signed-off-by: David Ahern 
---
 qemu-char.c |   15 +++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 75dbf66..77edec9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1173,6 +1173,20 @@ static int tty_serial_ioctl(CharDriverState *chr, int 
cmd, void *arg)
 return 0;
 }
 
+static void qemu_chr_close_tty(CharDriverState *chr)
+{
+FDCharDriver *s = chr->opaque;
+int fd = -1;
+
+if (s)
+fd = s->fd_in;
+
+fd_chr_close(chr);
+
+if (fd >= 0) 
+close(fd);
+}
+
 static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 {
 const char *filename = qemu_opt_get(opts, "path");
@@ -1190,6 +1204,7 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 return NULL;
 }
 chr->chr_ioctl = tty_serial_ioctl;
+chr->chr_close = qemu_chr_close_tty;
 return chr;
 }
 #else  /* ! __linux__ && ! __sun__ */
-- 
1.6.2.5





Re: [Qemu-devel] forking i386 binaries on arm linux user mode

2010-02-10 Thread Laurent Desnogues
On Wed, Feb 10, 2010 at 10:38 PM, Damion Yates  wrote:
> I've grabbed the latest stable qemu and compiled under scratchbox.  I
> hit an issue compiling it, with no __builtin__clear_cache() so linked in
> a kludge.c containing a call to __clear_cache() with the params passed
> as they would be to __builtin__clear_cache().
>
> Firstly does this sound like it should work as a workaround?

That's how I did it (except that I replaced the calls to
__builtin__clear_cache with calls to __clear_cache in
tcg/arm/tcg-target.h and exec-all.h.

> It certainly got me to the next level, which is that I can now run loads
> of linux binaries on my armlinux system (a Nokia n900). I've tried tower
> toppler (http://toppler.sourceforge.net/) which uses SDL (via X11) and
> this was surprisingly fast, in fact it almost felt as fast as the native
> toppler that somebody crosscompiled already. Most linux utils work when
> I copy then and any dependant libs from my x86 laptop to the phone. I'm
> lucky (I guess) that /lib/ld-linux.so.3 is the arm version and I'm using
> a slightly older .2 for x86 so I can have both files there. I also
> enabled arbitrary execution of binaries via binfmt_misc. The 600 Mhz Arm
> V8 Cortex (I think it is), feels like it's running at about Pentium 90
> speeds, which I'm hoping is enough for what I really want to get going.

The N900 has an ARMv7 Cortex-A8 (OMAP3).

P90 is about the speed I'd expect.  Note it will seem
much slower if your program makes heavy use of floating
point.

> I want to run an old, possibly win16 Windows game under wine. I saw that
> user mode qemu-i386 was able to run wine in a post in 2004:
> http://lists.terrasoftsolutions.com/pipermail/yellowdog-general/2004-June/014468.html
>  - This was on a PPC however.
>
> When I run wine it SEGVs out and the strace of it shows it dies trying
> to do clone(). I also can't run things like xterm which can't do fork().
> Is this because by default it's trying to go via the arm "/bin/sh" to
> invoke whatever it wants to exec() in to?
>
> Should clone()/fork() work?  Has anyone been able to run wine ./blah.exe
> under user-linux mode of qemu on arm or indeed any other non x86 based
> CPU ?

I have seen a complex Linux i386 program run on an
ARM platform (with multithreading and Qt;  note it
crashed at some point but it was due to the
application doing access to uninitialized memory).
IIRC it was using chroot from inside QEMU (in main)
to make sure that all calls to exec would happen on
an x86 file system.  You might give that a try.


Laurent




[Qemu-devel] [PATCH 1/5] virtio-blk: revert serial number support

2010-02-10 Thread Christoph Hellwig
The addition of the whole ATA IDENTIY page caused the config space to
go above the allowed size in the PCI spec, and thus the feature was
already reverted in the Linux guest driver and disabled by default in
qemu.

Signed-off-by: Christoph Hellwig 

Index: qemu/hw/virtio-blk.c
===
--- qemu.orig/hw/virtio-blk.c   2010-02-10 20:27:57.845254656 +0100
+++ qemu/hw/virtio-blk.c2010-02-10 23:27:51.067254307 +0100
@@ -25,9 +25,7 @@ typedef struct VirtIOBlock
 BlockDriverState *bs;
 VirtQueue *vq;
 void *rq;
-char serial_str[BLOCK_SERIAL_STRLEN + 1];
 QEMUBH *bh;
-size_t config_size;
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -35,47 +33,6 @@ static VirtIOBlock *to_virtio_blk(VirtIO
 return (VirtIOBlock *)vdev;
 }
 
-/* store identify data in little endian format
- */
-static inline void put_le16(uint16_t *p, unsigned int v)
-{
-*p = cpu_to_le16(v);
-}
-
-/* copy to *dst from *src, nul pad dst tail as needed to len bytes
- */
-static inline void padstr(char *dst, const char *src, int len)
-{
-while (len--)
-*dst++ = *src ? *src++ : '\0';
-}
-
-/* setup simulated identify data as appropriate for virtio block device
- *
- * ref: AT Attachment 8 - ATA/ATAPI Command Set (ATA8-ACS)
- */
-static inline void virtio_identify_template(struct virtio_blk_config *bc)
-{
-uint16_t *p = &bc->identify[0];
-uint64_t lba_sectors = bc->capacity;
-
-memset(p, 0, sizeof(bc->identify));
-put_le16(p + 0, 0x0);/* ATA device */
-padstr((char *)(p + 23), QEMU_VERSION, 8);   /* firmware revision */
-padstr((char *)(p + 27), "QEMU VIRT_BLK", 40);   /* model# */
-put_le16(p + 47, 0x80ff);/* max xfer 255 sectors */
-put_le16(p + 49, 0x0b00);/* support IORDY/LBA/DMA 
*/
-put_le16(p + 59, 0x1ff); /* cur xfer 255 sectors */
-put_le16(p + 80, 0x1f0); /* support ATA8/7/6/5/4 */
-put_le16(p + 81, 0x16);
-put_le16(p + 82, 0x400);
-put_le16(p + 83, 0x400);
-put_le16(p + 100, lba_sectors);
-put_le16(p + 101, lba_sectors >> 16);
-put_le16(p + 102, lba_sectors >> 32);
-put_le16(p + 103, lba_sectors >> 48);
-}
-
 typedef struct VirtIOBlockReq
 {
 VirtIOBlock *dev;
@@ -448,10 +405,7 @@ static void virtio_blk_update_config(Vir
 blkcfg.heads = heads;
 blkcfg.sectors = secs;
 blkcfg.size_max = 0;
-virtio_identify_template(&blkcfg);
-memcpy(&blkcfg.identify[VIRTIO_BLK_ID_SN], s->serial_str,
-VIRTIO_BLK_ID_SN_BYTES);
-memcpy(config, &blkcfg, s->config_size);
+memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
 }
 
 static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
@@ -463,8 +417,6 @@ static uint32_t virtio_blk_get_features(
 
 if (bdrv_enable_write_cache(s->bs))
 features |= (1 << VIRTIO_BLK_F_WCACHE);
-if (strcmp(s->serial_str, "0"))
-features |= 1 << VIRTIO_BLK_F_IDENTIFY;
 
 if (bdrv_is_read_only(s->bs))
 features |= 1 << VIRTIO_BLK_F_RO;
@@ -510,24 +462,16 @@ VirtIODevice *virtio_blk_init(DeviceStat
 VirtIOBlock *s;
 int cylinders, heads, secs;
 static int virtio_blk_id;
-char *ps = (char *)drive_get_serial(dinfo->bdrv);
-size_t size = strlen(ps) ? sizeof(struct virtio_blk_config) :
-   offsetof(struct virtio_blk_config, _blk_size);
 
 s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-  size,
+  sizeof(struct virtio_blk_config),
   sizeof(VirtIOBlock));
 
-s->config_size = size;
 s->vdev.get_config = virtio_blk_update_config;
 s->vdev.get_features = virtio_blk_get_features;
 s->vdev.reset = virtio_blk_reset;
 s->bs = dinfo->bdrv;
 s->rq = NULL;
-if (strlen(ps))
-strncpy(s->serial_str, ps, sizeof(s->serial_str));
-else
-snprintf(s->serial_str, sizeof(s->serial_str), "0");
 bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
 bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
 
Index: qemu/hw/virtio-blk.h
===
--- qemu.orig/hw/virtio-blk.h   2010-02-10 20:27:52.756275049 +0100
+++ qemu/hw/virtio-blk.h2010-02-10 23:27:51.067254307 +0100
@@ -30,13 +30,9 @@
 #define VIRTIO_BLK_F_RO 5   /* Disk is read-only */
 #define VIRTIO_BLK_F_BLK_SIZE   6   /* Block size of disk is available*/
 #define VIRTIO_BLK_F_SCSI   7   /* Supports scsi command passthru */
-#define VIRTIO_BLK_F_IDENTIFY   8   /* ATA IDENTIFY supported */
+/* #define VIRTIO_BLK_F_IDENTIFY   8   ATA IDENTIFY supported, DEPRECATED 
*/
 #define VIRTIO_BLK_F_WCACHE 9   /* write cache enabled */
 
-#define VIRTIO_BL

[Qemu-devel] [PATCH 2/5] block: add topology qdev properties

2010-02-10 Thread Christoph Hellwig
Add three new qdev properties to export block topology information to
the guest.  This is needed to get optimal I/O alignment for RAID arrays
or SSDs.

The options are:

 - physical_block_size to specify the physical block size of the device,
   this is going to increase from 512 bytes to 4096 kilobytes for many
   modern storage devices
 - min_io_size to specify the minimal I/O size without performance impact,
   this is typically set to the RAID chunk size for arrays.
 - opt_io_size to specify the optimal sustained I/O size, this is
   typically the RAID stripe width for arrays.

I decided to not auto-probe these values from blkid which might easily
be possible as I don't know how to deal with these issues on migration.

Note that we specificly only set the physical_block_size, and not the
logial one which is the unit all I/O is described in.  The reason for
that is that IDE does not support increasing the logical block size and
at last for now I want to stick to one meachnisms in queue and allow
for easy switching of transports for a given backing image which would
not be possible if scsi and virtio use real 4k sectors, while ide only
uses the physical block exponent.

To make this more common for the different block drivers introduce a
new BlockConf structure holding all common block properties and a
DEFINE_BLOCK_PROPERTIES macro to add them all together, mirroring
what is done for network drivers.  Also switch over all block drivers
to use it, except for the floppy driver which has weird driveA/driveB
properties and probably won't require any advanced block options ever.

Example usage for a virtio device with 4k physical block size and
8k optimal I/O size:

  -drive file=scratch.img,media=disk,cache=none,id=scratch \
  -device virtio-blk-pci,drive=scratch,physical_block_size=4096,opt_io_size=8192

Signed-off-by: Christoph Hellwig 

Index: qemu/block_int.h
===
--- qemu.orig/block_int.h   2010-02-10 20:27:57.836254726 +0100
+++ qemu/block_int.h2010-02-10 23:27:55.101003645 +0100
@@ -202,4 +202,31 @@ extern BlockDriverState *bdrv_first;
 int is_windows_drive(const char *filename);
 #endif
 
+struct DriveInfo;
+
+typedef struct BlockConf {
+struct DriveInfo *dinfo;
+uint16_t physical_block_size;
+uint16_t min_io_size;
+uint32_t opt_io_size;
+} BlockConf;
+
+static inline unsigned int get_physical_block_exp(BlockConf *conf)
+{
+unsigned int exp = 0, size;
+
+for (size = conf->physical_block_size; size > 512; size >>= 1) {
+exp++;
+}
+
+return exp;
+}
+
+#define DEFINE_BLOCK_PROPERTIES(_state, _conf)  \
+DEFINE_PROP_DRIVE("drive", _state, _conf.dinfo),\
+DEFINE_PROP_UINT16("physical_block_size", _state,   \
+   _conf.physical_block_size, 512), \
+DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 512),  \
+DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 512)
+
 #endif /* BLOCK_INT_H */
Index: qemu/hw/s390-virtio-bus.c
===
--- qemu.orig/hw/s390-virtio-bus.c  2010-02-10 20:27:52.575275957 +0100
+++ qemu/hw/s390-virtio-bus.c   2010-02-10 23:27:55.102012375 +0100
@@ -123,7 +123,7 @@ static int s390_virtio_blk_init(VirtIOS3
 {
 VirtIODevice *vdev;
 
-vdev = virtio_blk_init((DeviceState *)dev, dev->dinfo);
+vdev = virtio_blk_init((DeviceState *)dev, dev->block.dinfo);
 if (!vdev) {
 return -1;
 }
@@ -337,7 +337,7 @@ static VirtIOS390DeviceInfo s390_virtio_
 .qdev.name = "virtio-blk-s390",
 .qdev.size = sizeof(VirtIOS390Device),
 .qdev.props = (Property[]) {
-DEFINE_PROP_DRIVE("drive", VirtIOS390Device, dinfo),
+DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, block),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
Index: qemu/hw/s390-virtio-bus.h
===
--- qemu.orig/hw/s390-virtio-bus.h  2010-02-10 20:27:52.583254306 +0100
+++ qemu/hw/s390-virtio-bus.h   2010-02-10 23:27:55.104003295 +0100
@@ -38,7 +38,7 @@ typedef struct VirtIOS390Device {
 ram_addr_t feat_offs;
 uint8_t feat_len;
 VirtIODevice *vdev;
-DriveInfo *dinfo;
+BlockConf block;
 NICConf nic;
 uint32_t host_features;
 /* Max. number of ports we can have for a the virtio-serial device */
Index: qemu/hw/virtio-blk.c
===
--- qemu.orig/hw/virtio-blk.c   2010-02-10 23:27:51.067254307 +0100
+++ qemu/hw/virtio-blk.c2010-02-10 23:27:55.105016426 +0100
@@ -457,7 +457,7 @@ static int virtio_blk_load(QEMUFile *f, 
 return 0;
 }
 
-VirtIODevice *virtio_blk_init(DeviceState *dev, DriveInfo *dinfo)
+VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf)
 {
 VirtIOBlock *s;
 int cylinders, heads, secs;
@@ -470,7 +470,7 @@

[Qemu-devel] [PATCH 3/5] virtio-blk: add topology support

2010-02-10 Thread Christoph Hellwig
Export all topology information in the block config structure,
guarded by a new VIRTIO_BLK_F_TOPOLOGY feature flag.

Signed-off-by: Christoph Hellwig 

Index: qemu/hw/virtio-blk.c
===
--- qemu.orig/hw/virtio-blk.c   2010-02-10 23:27:55.105016426 +0100
+++ qemu/hw/virtio-blk.c2010-02-10 23:31:30.927004343 +0100
@@ -26,6 +26,7 @@ typedef struct VirtIOBlock
 VirtQueue *vq;
 void *rq;
 QEMUBH *bh;
+BlockConf *conf;
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -405,6 +406,10 @@ static void virtio_blk_update_config(Vir
 blkcfg.heads = heads;
 blkcfg.sectors = secs;
 blkcfg.size_max = 0;
+blkcfg.physical_block_exp = get_physical_block_exp(s->conf);
+blkcfg.alignment_offset = 0;
+blkcfg.min_io_size = s->conf->min_io_size / 512;
+blkcfg.opt_io_size = s->conf->opt_io_size / 512;
 memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
 }
 
@@ -414,6 +419,7 @@ static uint32_t virtio_blk_get_features(
 
 features |= (1 << VIRTIO_BLK_F_SEG_MAX);
 features |= (1 << VIRTIO_BLK_F_GEOMETRY);
+features |= (1 << VIRTIO_BLK_F_TOPOLOGY);
 
 if (bdrv_enable_write_cache(s->bs))
 features |= (1 << VIRTIO_BLK_F_WCACHE);
@@ -471,6 +477,7 @@ VirtIODevice *virtio_blk_init(DeviceStat
 s->vdev.get_features = virtio_blk_get_features;
 s->vdev.reset = virtio_blk_reset;
 s->bs = conf->dinfo->bdrv;
+s->conf = conf;
 s->rq = NULL;
 bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
 bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
Index: qemu/hw/virtio-blk.h
===
--- qemu.orig/hw/virtio-blk.h   2010-02-10 23:27:51.067254307 +0100
+++ qemu/hw/virtio-blk.h2010-02-10 23:31:30.928004413 +0100
@@ -32,6 +32,7 @@
 #define VIRTIO_BLK_F_SCSI   7   /* Supports scsi command passthru */
 /* #define VIRTIO_BLK_F_IDENTIFY   8   ATA IDENTIFY supported, DEPRECATED 
*/
 #define VIRTIO_BLK_F_WCACHE 9   /* write cache enabled */
+#define VIRTIO_BLK_F_TOPOLOGY   10  /* Topology information is available */
 
 struct virtio_blk_config
 {
@@ -42,6 +43,10 @@ struct virtio_blk_config
 uint8_t heads;
 uint8_t sectors;
 uint32_t _blk_size;/* structure pad, currently unused */
+uint8_t physical_block_exp;
+uint8_t alignment_offset;
+uint16_t min_io_size;
+uint32_t opt_io_size;
 } __attribute__((packed));
 
 /* These two define direction. */




[Qemu-devel] [PATCH 4/5] scsi: add topology support

2010-02-10 Thread Christoph Hellwig
Export the physical block size in the READ CAPACITY (16) command,
and add the new block limits VPD page to export the minimum and
optiomal I/O sizes.

Note that we also need to bump the scsi revision level to SPC-2
as that is the minimum requirement by at least the Linux kernel
to try READ CAPACITY (16) first and look at the block limits VPD
page.

Signed-off-by: Christoph Hellwig 

Index: qemu/hw/scsi-disk.c
===
--- qemu.orig/hw/scsi-disk.c2010-02-10 23:27:55.123255565 +0100
+++ qemu/hw/scsi-disk.c 2010-02-10 23:31:45.355004273 +0100
@@ -349,10 +349,11 @@ static int scsi_disk_emulate_inquiry(SCS
 case 0x00: /* Supported page codes, mandatory */
 DPRINTF("Inquiry EVPD[Supported pages] "
 "buffer size %zd\n", req->cmd.xfer);
-outbuf[buflen++] = 3;// number of pages
+outbuf[buflen++] = 4;// number of pages
 outbuf[buflen++] = 0x00; // list of supported pages (this page)
 outbuf[buflen++] = 0x80; // unit serial number
 outbuf[buflen++] = 0x83; // device identification
+outbuf[buflen++] = 0xb0; // block device characteristics
 break;
 
 case 0x80: /* Device serial number, optional */
@@ -394,6 +395,27 @@ static int scsi_disk_emulate_inquiry(SCS
 buflen += id_len;
 break;
 }
+case 0xb0: /* block device characteristics */
+{
+unsigned int min_io_size = s->qdev.conf.min_io_size >> 9;
+unsigned int opt_io_size = s->qdev.conf.opt_io_size >> 9;
+
+/* required VPD size with unmap support */
+outbuf[3] = buflen = 0x3c;
+
+memset(outbuf + 4, 0, buflen - 4);
+
+/* optimal transfer length granularity */
+outbuf[6] = (min_io_size >> 8) & 0xff;
+outbuf[7] = min_io_size & 0xff;
+
+/* optimal transfer length */
+outbuf[12] = (opt_io_size >> 24) & 0xff;
+outbuf[13] = (opt_io_size >> 16) & 0xff;
+outbuf[14] = (opt_io_size >> 8) & 0xff;
+outbuf[15] = opt_io_size & 0xff;
+break;
+}
 default:
 BADF("Error: unsupported Inquiry (EVPD[%02X]) "
  "buffer size %zd\n", page_code, req->cmd.xfer);
@@ -440,7 +462,7 @@ static int scsi_disk_emulate_inquiry(SCS
 memcpy(&outbuf[32], s->version ? s->version : QEMU_VERSION, 4);
 /* Identify device as SCSI-3 rev 1.
Some later commands are also implemented. */
-outbuf[2] = 3;
+outbuf[2] = 5;
 outbuf[3] = 2; /* Format 2 */
 
 if (buflen > 36) {
@@ -794,6 +816,8 @@ static int scsi_disk_emulate_command(SCS
 outbuf[9] = 0;
 outbuf[10] = s->cluster_size * 2;
 outbuf[11] = 0;
+outbuf[12] = 0;
+outbuf[13] = get_physical_block_exp(&s->qdev.conf);
 /* Protection, exponent and lowest lba field left blank. */
 buflen = req->cmd.xfer;
 break;




[Qemu-devel] [PATCH 5/5] ide: add topology support

2010-02-10 Thread Christoph Hellwig
Export the physical block size in the ATA IDENTIFY command.  The
other topology values are not supported in ATA so skip them.

Signed-off-by: Christoph Hellwig 

Index: qemu/hw/ide/core.c
===
--- qemu.orig/hw/ide/core.c 2010-02-10 20:08:51.328254308 +0100
+++ qemu/hw/ide/core.c  2010-02-10 20:10:09.546152407 +0100
@@ -164,6 +164,8 @@ static void ide_identify(IDEState *s)
 put_le16(p + 101, s->nb_sectors >> 16);
 put_le16(p + 102, s->nb_sectors >> 32);
 put_le16(p + 103, s->nb_sectors >> 48);
+if (s->conf && s->conf->physical_block_size)
+put_le16(p + 106, 0x6000 | get_physical_block_exp(s->conf));
 
 memcpy(s->identify_data, p, sizeof(s->identify_data));
 s->identify_set = 1;




[Qemu-devel] sparc32 fix spurious dma interrupts

2010-02-10 Thread Artyom Tarasenko
Don't raise interrupt when not enabled.
Don't set DMA_INTR bit spuriously.
Don't print misleading debug messages "Raise IRQ" when not raising any.

Signed-off-by: Artyom Tarasenko 
---
diff --git a/hw/sparc32_dma.c b/hw/sparc32_dma.c
index 6e991e0..b2992ca 100644
--- a/hw/sparc32_dma.c
+++ b/hw/sparc32_dma.c
@@ -125,9 +125,11 @@ static void dma_set_irq(void *opaque, int irq, int level)
 {
 DMAState *s = opaque;
 if (level) {
-DPRINTF("Raise IRQ\n");
 s->dmaregs[0] |= DMA_INTR;
-qemu_irq_raise(s->irq);
+if (s->dmaregs[0] & DMA_INTREN) {
+DPRINTF("Raise IRQ\n");
+qemu_irq_raise(s->irq);
+}
 } else {
 s->dmaregs[0] &= ~DMA_INTR;
 DPRINTF("Lower IRQ\n");
@@ -142,8 +145,6 @@ void espdma_memory_read(void *opaque, uint8_t *buf, int len)
 DPRINTF("DMA read, direction: %c, addr 0x%8.8x\n",
 s->dmaregs[0] & DMA_WRITE_MEM ? 'w': 'r', s->dmaregs[1]);
 sparc_iommu_memory_read(s->iommu, s->dmaregs[1], buf, len);
-DPRINTF("Raise IRQ\n");
-s->dmaregs[0] |= DMA_INTR;
 s->dmaregs[1] += len;
 }
 
@@ -154,8 +155,6 @@ void espdma_memory_write(void *opaque, uint8_t *buf, int 
len)
 DPRINTF("DMA write, direction: %c, addr 0x%8.8x\n",
 s->dmaregs[0] & DMA_WRITE_MEM ? 'w': 'r', s->dmaregs[1]);
 sparc_iommu_memory_write(s->iommu, s->dmaregs[1], buf, len);
-DPRINTF("Raise IRQ\n");
-s->dmaregs[0] |= DMA_INTR;
 s->dmaregs[1] += len;
 }
 




Re: [Qemu-devel] [PATCH] add close callback for tty-based char device

2010-02-10 Thread Anthony Liguori

On 02/10/2010 04:00 PM, David Ahern wrote:

Add a tty close callback. Right now if a guest device that is connected
to a tty-based chardev in the host is removed, the tty is not closed.
With this patch it is closed.

Example use case is connecting an emulated USB serial cable in the guest
to ttyS0 of the host using the monitor command:

usb_add serial::/dev/ttyS0

and then removing the device with:

usb_del serial::/dev/ttyS0

Signed-off-by: David Ahern
---
  qemu-char.c |   15 +++
  1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 75dbf66..77edec9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1173,6 +1173,20 @@ static int tty_serial_ioctl(CharDriverState *chr, int 
cmd, void *arg)
  return 0;
  }

+static void qemu_chr_close_tty(CharDriverState *chr)
+{
+FDCharDriver *s = chr->opaque;
+int fd = -1;
+
+if (s)
+fd = s->fd_in;

   


You've got trailing spaces and this doesn't adhere to CODING_STYLE 
(needs curly braces around the if clause).


Regards,

Anthony Liguori





[Qemu-devel] [PATCH 0/4] qemu-kvm: prepare for adding eventfd usage to upstream

2010-02-10 Thread Paolo Bonzini
This patch series morphs the code in qemu-kvm's eventfd so that it looks
like the code in upstream qemu.  Patch 4 is not yet in upstream QEMU,
I'm submitting it first to qemu-kvm to avoid conflicts.

Paolo Bonzini (4):
  morph qemu_kvm_notify_work into qemu.git's qemu_event_increment
  morph io_thread_wakeup into qemu.git's qemu_event_read
  fix placement of config-host.h inclusion
  move qemu_eventfd to osdep.c

 compatfd.c|   26 --
 compatfd.h|2 --
 osdep.c   |   37 +++--
 qemu-common.h |1 +
 qemu-kvm.c|   50 +-
 5 files changed, 53 insertions(+), 63 deletions(-)





[Qemu-devel] [PATCH 4/4] qemu-kvm: move qemu_eventfd to osdep.c

2010-02-10 Thread Paolo Bonzini
Upstream has no compatfd.[ch], so move the code to the most similar
place.

Signed-off-by: Paolo Bonzini 
---
 compatfd.c|   26 --
 compatfd.h|2 --
 osdep.c   |   32 
 qemu-common.h |1 +
 4 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/compatfd.c b/compatfd.c
index 4a00d95..a7cebc4 100644
--- a/compatfd.c
+++ b/compatfd.c
@@ -115,29 +115,3 @@ int qemu_signalfd(const sigset_t *mask)
 
 return qemu_signalfd_compat(mask);
 }
-
-int qemu_eventfd(int *fds)
-{
-int ret;
-
-#if defined(CONFIG_EVENTFD)
-ret = syscall(SYS_eventfd, 0);
-if (ret >= 0) {
-fds[0] = ret;
-qemu_set_cloexec(ret);
-if ((fds[1] = dup(ret)) == -1) {
-close(ret);
-return -1;
-}
-qemu_set_cloexec(fds[1]);
-return 0;
-}
-#endif
-
-ret = pipe(fds);
-if (ret != -1) {
-qemu_set_cloexec(fds[0]);
-qemu_set_cloexec(fds[1]);
-}
-return ret;
-}
diff --git a/compatfd.h b/compatfd.h
index 06b0b6b..fc37915 100644
--- a/compatfd.h
+++ b/compatfd.h
@@ -40,6 +40,4 @@ struct qemu_signalfd_siginfo {
 
 int qemu_signalfd(const sigset_t *mask);
 
-int qemu_eventfd(int *fds);
-
 #endif
diff --git a/osdep.c b/osdep.c
index 616e821..1b1e593 100644
--- a/osdep.c
+++ b/osdep.c
@@ -37,6 +37,10 @@
 #include 
 #endif
 
+#ifdef CONFIG_EVENTFD
+#include 
+#endif
+
 #ifdef _WIN32
 #include 
 #elif defined(CONFIG_BSD)
@@ -285,6 +289,34 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t 
count)
 
 #ifndef _WIN32
 /*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+int ret;
+
+#ifdef CONFIG_EVENTFD
+ret = eventfd(0, 0);
+if (ret >= 0) {
+fds[0] = ret;
+qemu_set_cloexec(ret);
+if ((fds[1] = dup(ret)) == -1) {
+close(ret);
+return -1;
+}
+qemu_set_cloexec(fds[1]);
+return 0;
+}
+
+if (errno != ENOSYS) {
+return -1;
+}
+#endif
+
+return qemu_pipe(fds);
+}
+
+/*
  * Creates a pipe with FD_CLOEXEC set on both file descriptors
  */
 int qemu_pipe(int pipefd[2])
diff --git a/qemu-common.h b/qemu-common.h
index bf14a22..f59d5f5 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -170,6 +170,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t 
count)
 void qemu_set_cloexec(int fd);
 
 #ifndef _WIN32
+int qemu_eventfd(int pipefd[2]);
 int qemu_pipe(int pipefd[2]);
 #endif
 
-- 
1.6.6





[Qemu-devel] [PATCH 3/4] qemu-kvm: fix placement of config-host.h inclusion

2010-02-10 Thread Paolo Bonzini
Cherry-picked from upstream f582af5.

Signed-off-by: Paolo Bonzini 
---
 osdep.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/osdep.c b/osdep.c
index e613e4b..616e821 100644
--- a/osdep.c
+++ b/osdep.c
@@ -28,14 +28,15 @@
 #include 
 #include 
 #include 
+
+/* Needed early for CONFIG_BSD etc. */
+#include "config-host.h"
+
 #ifdef CONFIG_SOLARIS
 #include 
 #include 
 #endif
 
-/* Needed early for CONFIG_BSD etc. */
-#include "config-host.h"
-
 #ifdef _WIN32
 #include 
 #elif defined(CONFIG_BSD)
-- 
1.6.6






[Qemu-devel] [PATCH 1/4] qemu-kvm: morph qemu_kvm_notify_work into qemu.git's qemu_event_increment

2010-02-10 Thread Paolo Bonzini
No need to loop if < 8 bytes are written, since that will happen only
for pipes and is harmless.  eventfd writes of 8 bytes will always succeed
or fail with EAGAIN.

Signed-off-by: Paolo Bonzini 
---
 qemu-kvm.c |   34 --
 1 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index a305907..669a784 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1991,32 +1991,22 @@ int kvm_init_ap(void)
 
 void qemu_kvm_notify_work(void)
 {
-uint64_t value = 1;
-char buffer[8];
-size_t offset = 0;
+/* Write 8 bytes to be compatible with eventfd.  */
+static uint64_t val = 1;
+ssize_t ret;
 
 if (io_thread_fd == -1)
 return;
 
-memcpy(buffer, &value, sizeof(value));
-
-while (offset < 8) {
-ssize_t len;
-
-len = write(io_thread_fd, buffer + offset, 8 - offset);
-if (len == -1 && errno == EINTR)
-continue;
-
-/* In case we have a pipe, there is not reason to insist writing
- * 8 bytes
- */
-if (len == -1 && errno == EAGAIN)
-break;
-
-if (len <= 0)
-break;
-
-offset += len;
+do {
+ret = write(io_thread_fd, &val, sizeof(val));
+} while (ret < 0 && errno == EINTR);
+
+/* EAGAIN is fine in case we have a pipe.  */
+if (ret < 0 && errno != EAGAIN) {
+ fprintf(stderr, "qemu_kvm_notify_work: write() filed: %s\n",
+ strerror(errno));
+ exit (1);
 }
 }
 
-- 
1.6.6






[Qemu-devel] [PATCH 2/4] qemu-kvm: morph io_thread_wakeup into qemu.git's qemu_event_read

2010-02-10 Thread Paolo Bonzini
Again, no need to loop if less than a full buffer is read, the next
read would return EAGAIN.

Signed-off-by: Paolo Bonzini 
---
 qemu-kvm.c |   16 +---
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 669a784..50e1303 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -2049,19 +2049,13 @@ static void sigfd_handler(void *opaque)
 static void io_thread_wakeup(void *opaque)
 {
 int fd = (unsigned long) opaque;
-char buffer[4096];
-
-/* Drain the pipe/(eventfd) */
-while (1) {
-ssize_t len;
+ssize_t len;
+char buffer[512];
 
+/* Drain the notify pipe.  For eventfd, only 8 bytes will be read.  */
+do {
 len = read(fd, buffer, sizeof(buffer));
-if (len == -1 && errno == EINTR)
-continue;
-
-if (len <= 0)
-break;
-}
+} while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
 }
 
 int kvm_main_loop(void)
-- 
1.6.6






[Qemu-devel] Re: [PATCH] virtio-spec: document indirect descriptors

2010-02-10 Thread Rusty Russell
On Wed, 10 Feb 2010 11:30:39 pm Michael S. Tsirkin wrote:
> Add documentation for indirect descriptors

Thanks, that's awesome!

I added an entry to the Reserved Feature table in appendix B, and applied
it.  We're now at 0.8.5.

Cheers,
Rusty.




[Qemu-devel] [PATCH 0/2] simplify global register save/restore

2010-02-10 Thread Paolo Bonzini
Since b567b38 (target-arm: remove T0 and T1, 2009-10-16) the only global
register that is actually used is AREG0, so the complexity of
hostregs_helper.h is unwarranted.

Let's just say that env should be the only global register.  AREG1 and
AREG2 in principle could still be used to work around bad register
allocation in GCC, so I'm leaving them in dyngen-exec.h.

Blue Swirl, can you check whether changing AREG0 to another register
in dyngen-exec.h would fix the "annoying glibc bugs mangling global
register variables"?  Or maybe we can remove the workaround altogether,
considering the bug was fixed in version 2.3 of glibc dated 2001-11-29
(at least that's what I'd guess from the history)?

Paolo Bonzini (2):
  remove dead m68k global register definitions
  get rid of hostregs_helper.h

 cpu-exec.c |   15 +++-
 hostregs_helper.h  |   61 
 qemu-common.h  |2 +
 target-m68k/exec.h |4 ---
 4 files changed, 11 insertions(+), 71 deletions(-)
 delete mode 100644 hostregs_helper.h





[Qemu-devel] [PATCH 1/2] remove dead m68k global register definitions

2010-02-10 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 target-m68k/exec.h |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index 1267bb6..ece9aa0 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -20,10 +20,6 @@
 #include "dyngen-exec.h"
 
 register struct CPUM68KState *env asm(AREG0);
-/* This is only used for tb lookup.  */
-register uint32_t T0 asm(AREG1);
-/* ??? We don't use T1, but common code expects it to exist  */
-#define T1 env->t1
 
 #include "cpu.h"
 #include "exec-all.h"
-- 
1.6.6






[Qemu-devel] [PATCH 2/2] get rid of hostregs_helper.h

2010-02-10 Thread Paolo Bonzini
Since b567b38 (target-arm: remove T0 and T1, 2009-10-16) the only global
register that is used is AREG0, so the complexity of hostregs_helper.h
is unused.  Use regular assignments and a compiler optimization barrier.

Signed-off-by: Paolo Bonzini 
---
 cpu-exec.c|   15 +++-
 hostregs_helper.h |   61 -
 qemu-common.h |2 +
 3 files changed, 11 insertions(+), 67 deletions(-)
 delete mode 100644 hostregs_helper.h

diff --git a/cpu-exec.c b/cpu-exec.c
index 0256edf..2553d1d 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -210,8 +210,7 @@ static void cpu_handle_debug_exception(CPUState *env)
 
 int cpu_exec(CPUState *env1)
 {
-#define DECLARE_HOST_REGS 1
-#include "hostregs_helper.h"
+host_reg_t saved_env_reg;
 int ret, interrupt_request;
 TranslationBlock *tb;
 uint8_t *tc_ptr;
@@ -222,9 +221,12 @@ int cpu_exec(CPUState *env1)
 
 cpu_single_env = env1;
 
-/* first we save global registers */
-#define SAVE_HOST_REGS 1
-#include "hostregs_helper.h"
+/* the access to env below is actually saving the global register's
+   value, so that files not including target-xyz/exec.h are free to
+   use it.  */
+BUILD_BUG_ON (sizeof (saved_env_reg) != sizeof (env));
+saved_env_reg = (host_reg_t) env;
+asm("");
 env = env1;
 
 #if defined(TARGET_I386)
@@ -668,7 +670,8 @@ int cpu_exec(CPUState *env1)
 #endif
 
 /* restore global registers */
-#include "hostregs_helper.h"
+asm("");
+env = (void *) saved_env_reg;
 
 /* fail safe : never use cpu_single_env outside cpu_exec() */
 cpu_single_env = NULL;
diff --git a/hostregs_helper.h b/hostregs_helper.h
deleted file mode 100644
index 3a0bece..000
--- a/hostregs_helper.h
+++ /dev/null
@@ -1,61 +0,0 @@
-/*
- *  Save/restore host registers.
- *
- *  Copyright (c) 2007 CodeSourcery
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library 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
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see .
- */
-
-/* The GCC global register variable extension is used to reserve some
-   host registers for use by generated code.  However only the core parts of
-   the translation engine are compiled with these settings.  We must manually
-   save/restore these registers when called from regular code.
-   It is not sufficient to save/restore T0 et. al. as these may be declared
-   with a datatype smaller than the actual register.  */
-
-#if defined(DECLARE_HOST_REGS)
-
-#define DO_REG(REG)\
-register host_reg_t reg_AREG##REG asm(AREG##REG);  \
-volatile host_reg_t saved_AREG##REG;
-
-#elif defined(SAVE_HOST_REGS)
-
-#define DO_REG(REG)\
-__asm__ __volatile__ ("" : "=r" (reg_AREG##REG));  \
-saved_AREG##REG = reg_AREG##REG;
-
-#else
-
-#define DO_REG(REG) \
-reg_AREG##REG = saved_AREG##REG;   \
-__asm__ __volatile__ ("" : : "r" (reg_AREG##REG));
-
-#endif
-
-#ifdef AREG0
-DO_REG(0)
-#endif
-
-#ifdef AREG1
-DO_REG(1)
-#endif
-
-#ifdef AREG2
-DO_REG(2)
-#endif
-
-#undef SAVE_HOST_REGS
-#undef DECLARE_HOST_REGS
-#undef DO_REG
diff --git a/qemu-common.h b/qemu-common.h
index b09f717..b791148 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -11,6 +11,8 @@
 #define QEMU_WARN_UNUSED_RESULT
 #endif
 
+#define BUILD_BUG_ON(x) typedef char __build_bug_on__##__LINE__[(x)?-1:1];
+
 /* Hack around the mess dyngen-exec.h causes: We need QEMU_NORETURN in files 
that
cannot include the following headers without conflicts. This condition has
to be removed once dyngen is gone. */
-- 
1.6.6





[Qemu-devel] [PATCH] [uq/master] use eventfd for iothread

2010-02-10 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 osdep.c   |   32 
 qemu-common.h |1 +
 vl.c  |9 +
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/osdep.c b/osdep.c
index 9059f01..9e4b17b 100644
--- a/osdep.c
+++ b/osdep.c
@@ -37,6 +37,10 @@
 #include 
 #endif
 
+#ifdef CONFIG_EVENTFD
+#include 
+#endif
+
 #ifdef _WIN32
 #include 
 #elif defined(CONFIG_BSD)
@@ -281,6 +285,34 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t 
count)
 
 #ifndef _WIN32
 /*
+ * Creates an eventfd that looks like a pipe and has EFD_CLOEXEC set.
+ */
+int qemu_eventfd(int fds[2])
+{
+int ret;
+
+#ifdef CONFIG_EVENTFD
+ret = eventfd(0, 0);
+if (ret >= 0) {
+fds[0] = ret;
+qemu_set_cloexec(ret);
+if ((fds[1] = dup(ret)) == -1) {
+close(ret);
+return -1;
+}
+qemu_set_cloexec(fds[1]);
+return 0;
+}
+
+if (errno != ENOSYS) {
+return -1;
+}
+#endif
+
+return qemu_pipe(fds);
+}
+
+/*
  * Creates a pipe with FD_CLOEXEC set on both file descriptors
  */
 int qemu_pipe(int pipefd[2])
diff --git a/qemu-common.h b/qemu-common.h
index b09f717..c941006 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -170,6 +170,7 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t 
count)
 void qemu_set_cloexec(int fd);
 
 #ifndef _WIN32
+int qemu_eventfd(int pipefd[2]);
 int qemu_pipe(int pipefd[2]);
 #endif
 
diff --git a/vl.c b/vl.c
index 98918ac..1957018 100644
--- a/vl.c
+++ b/vl.c
@@ -3211,14 +3211,15 @@ static int io_thread_fd = -1;
 
 static void qemu_event_increment(void)
 {
-static const char byte = 0;
+/* Write 8 bytes to be compatible with eventfd.  */
+static uint64_t val = 1;
 ssize_t ret;
 
 if (io_thread_fd == -1)
 return;
 
 do {
-ret = write(io_thread_fd, &byte, sizeof(byte));
+ret = write(io_thread_fd, &val, sizeof(val));
 } while (ret < 0 && errno == EINTR);
 
 /* EAGAIN is fine, a read must be pending.  */
@@ -3235,7 +3236,7 @@ static void qemu_event_read(void *opaque)
 ssize_t len;
 char buffer[512];
 
-/* Drain the notify pipe */
+/* Drain the notify pipe.  For eventfd, only 8 bytes will be read.  */
 do {
 len = read(fd, buffer, sizeof(buffer));
 } while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
@@ -3246,7 +3247,7 @@ static int qemu_event_init(void)
 int err;
 int fds[2];
 
-err = qemu_pipe(fds);
+err = qemu_eventfd(fds);
 if (err == -1)
 return -errno;
 
-- 
1.6.6





[Qemu-devel] [PATCH 1/3] use lazy initialization for display_state

2010-02-10 Thread Paolo Bonzini
Ensure initialization of a dumb display, if needed, by making
all accesses go through get_displaystate.

Signed-off-by: Paolo Bonzini 
---
 vl.c |   29 +++--
 1 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/vl.c b/vl.c
index 5ddf1fe..94aeb5e 100644
--- a/vl.c
+++ b/vl.c
@@ -2568,6 +2568,16 @@ struct DisplayAllocator default_allocator = {
 defaultallocator_free_displaysurface
 };
 
+/* dumb display */
+
+static void dumb_display_init(void)
+{
+DisplayState *ds = qemu_mallocz(sizeof(DisplayState));
+ds->allocator = &default_allocator;
+ds->surface = qemu_create_displaysurface(ds, 640, 480);
+register_displaystate(ds);
+}
+
 void register_displaystate(DisplayState *ds)
 {
 DisplayState **s;
@@ -2580,6 +2590,9 @@ void register_displaystate(DisplayState *ds)
 
 DisplayState *get_displaystate(void)
 {
+if (!display_state) {
+dumb_display_init();
+}
 return display_state;
 }
 
@@ -2589,16 +2602,6 @@ DisplayAllocator *register_displayallocator(DisplayState 
*ds, DisplayAllocator *
 return ds->allocator;
 }
 
-/* dumb display */
-
-static void dumb_display_init(void)
-{
-DisplayState *ds = qemu_mallocz(sizeof(DisplayState));
-ds->allocator = &default_allocator;
-ds->surface = qemu_create_displaysurface(ds, 640, 480);
-register_displaystate(ds);
-}
-
 /***/
 /* I/O handling */
 
@@ -5871,10 +5874,8 @@ int main(int argc, char **argv, char **envp)
 if (qemu_opts_foreach(&qemu_device_opts, device_init_func, NULL, 1) != 0)
 exit(1);
 
-if (!display_state)
-dumb_display_init();
 /* just use the first displaystate for the moment */
-ds = display_state;
+ds = get_displaystate();
 
 if (display_type == DT_DEFAULT) {
 #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
@@ -5932,7 +5933,7 @@ int main(int argc, char **argv, char **envp)
 qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
 }
 
-text_consoles_set_display(display_state);
+text_consoles_set_display(ds);
 
 if (qemu_opts_foreach(&qemu_mon_opts, mon_init_func, NULL, 1) != 0)
 exit(1);
-- 
1.6.6






[Qemu-devel] [PATCH 2/3] remove knowledge of defaultallocator_free_displaysurface from sdl.c

2010-02-10 Thread Paolo Bonzini
Let register_displayallocator hand over the old width/height to the new
allocator.

Signed-off-by: Paolo Bonzini 
---
So these functions will be made static when moved to console.c.

 sdl.c |4 
 vl.c  |8 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/sdl.c b/sdl.c
index cf27ad2..a9b4323 100644
--- a/sdl.c
+++ b/sdl.c
@@ -872,10 +872,6 @@ void sdl_display_init(DisplayState *ds, int full_screen, 
int no_frame)
 da->resize_displaysurface = sdl_resize_displaysurface;
 da->free_displaysurface = sdl_free_displaysurface;
 if (register_displayallocator(ds, da) == da) {
-DisplaySurface *surf;
-surf = sdl_create_displaysurface(ds_get_width(ds), ds_get_height(ds));
-defaultallocator_free_displaysurface(ds->surface);
-ds->surface = surf;
 dpy_resize(ds);
 }
 
diff --git a/vl.c b/vl.c
index 94aeb5e..8e8b4d1 100644
--- a/vl.c
+++ b/vl.c
@@ -2598,7 +2598,13 @@ DisplayState *get_displaystate(void)
 
 DisplayAllocator *register_displayallocator(DisplayState *ds, DisplayAllocator 
*da)
 {
-if(ds->allocator ==  &default_allocator) ds->allocator = da;
+if(ds->allocator ==  &default_allocator) {
+DisplaySurface *surf;
+surf = da->create_displaysurface(ds_get_width(ds), ds_get_height(ds));
+defaultallocator_free_displaysurface(ds->surface);
+ds->surface = surf;
+ds->allocator = da;
+}
 return ds->allocator;
 }
 
-- 
1.6.6






[Qemu-devel] [PATCH 3/3] move default allocator to console.c

2010-02-10 Thread Paolo Bonzini
Moving stuff in console.c to avoid the need for prototypes makes
this patch a bit bigger, but there's no change in the code.

Signed-off-by: Paolo Bonzini 
---
 console.c |  176 +++--
 console.h |4 --
 vl.c  |   50 -
 3 files changed, 112 insertions(+), 118 deletions(-)

diff --git a/console.c b/console.c
index 8086bd6..8bcd00b 100644
--- a/console.c
+++ b/console.c
@@ -154,6 +154,7 @@ struct TextConsole {
 QEMUTimer *kbd_timer;
 };
 
+static DisplayState *display_state;
 static TextConsole *active_console;
 static TextConsole *consoles[MAX_CONSOLES];
 static int nb_consoles = 0;
@@ -1265,6 +1266,117 @@ static TextConsole *new_console(DisplayState *ds, 
console_type_t console_type)
 return s;
 }
 
+static DisplaySurface* defaultallocator_create_displaysurface(int width, int 
height)
+{
+DisplaySurface *surface = (DisplaySurface*) 
qemu_mallocz(sizeof(DisplaySurface));
+
+surface->width = width;
+surface->height = height;
+surface->linesize = width * 4;
+surface->pf = qemu_default_pixelformat(32);
+#ifdef HOST_WORDS_BIGENDIAN
+surface->flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
+#else
+surface->flags = QEMU_ALLOCATED_FLAG;
+#endif
+surface->data = (uint8_t*) qemu_mallocz(surface->linesize * 
surface->height);
+
+return surface;
+}
+
+static DisplaySurface* defaultallocator_resize_displaysurface(DisplaySurface 
*surface,
+  int width, int height)
+{
+surface->width = width;
+surface->height = height;
+surface->linesize = width * 4;
+surface->pf = qemu_default_pixelformat(32);
+if (surface->flags & QEMU_ALLOCATED_FLAG)
+surface->data = (uint8_t*) qemu_realloc(surface->data, 
surface->linesize * surface->height);
+else
+surface->data = (uint8_t*) qemu_malloc(surface->linesize * 
surface->height);
+#ifdef HOST_WORDS_BIGENDIAN
+surface->flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
+#else
+surface->flags = QEMU_ALLOCATED_FLAG;
+#endif
+
+return surface;
+}
+
+DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp,
+  int linesize, uint8_t *data)
+{
+DisplaySurface *surface = (DisplaySurface*) 
qemu_mallocz(sizeof(DisplaySurface));
+
+surface->width = width;
+surface->height = height;
+surface->linesize = linesize;
+surface->pf = qemu_default_pixelformat(bpp);
+#ifdef HOST_WORDS_BIGENDIAN
+surface->flags = QEMU_BIG_ENDIAN_FLAG;
+#endif
+surface->data = data;
+
+return surface;
+}
+
+static void defaultallocator_free_displaysurface(DisplaySurface *surface)
+{
+if (surface == NULL)
+return;
+if (surface->flags & QEMU_ALLOCATED_FLAG)
+qemu_free(surface->data);
+qemu_free(surface);
+}
+
+static struct DisplayAllocator default_allocator = {
+defaultallocator_create_displaysurface,
+defaultallocator_resize_displaysurface,
+defaultallocator_free_displaysurface
+};
+
+static void dumb_display_init(void)
+{
+DisplayState *ds = qemu_mallocz(sizeof(DisplayState));
+ds->allocator = &default_allocator;
+ds->surface = qemu_create_displaysurface(ds, 640, 480);
+register_displaystate(ds);
+}
+
+/***/
+/* register display */
+
+void register_displaystate(DisplayState *ds)
+{
+DisplayState **s;
+s = &display_state;
+while (*s != NULL)
+s = &(*s)->next;
+ds->next = NULL;
+*s = ds;
+}
+
+DisplayState *get_displaystate(void)
+{
+if (!display_state) {
+dumb_display_init ();
+}
+return display_state;
+}
+
+DisplayAllocator *register_displayallocator(DisplayState *ds, DisplayAllocator 
*da)
+{
+if(ds->allocator ==  &default_allocator) {
+DisplaySurface *surf;
+surf = da->create_displaysurface(ds_get_width(ds), ds_get_height(ds));
+defaultallocator_free_displaysurface(ds->surface);
+ds->surface = surf;
+ds->allocator = da;
+}
+return ds->allocator;
+}
+
 DisplayState *graphic_console_init(vga_hw_update_ptr update,
vga_hw_invalidate_ptr invalidate,
vga_hw_screen_dump_ptr screen_dump,
@@ -1559,67 +1671,3 @@ PixelFormat qemu_default_pixelformat(int bpp)
 }
 return pf;
 }
-
-DisplaySurface* defaultallocator_create_displaysurface(int width, int height)
-{
-DisplaySurface *surface = (DisplaySurface*) 
qemu_mallocz(sizeof(DisplaySurface));
-
-surface->width = width;
-surface->height = height;
-surface->linesize = width * 4;
-surface->pf = qemu_default_pixelformat(32);
-#ifdef HOST_WORDS_BIGENDIAN
-surface->flags = QEMU_ALLOCATED_FLAG | QEMU_BIG_ENDIAN_FLAG;
-#else
-surface->flags = QEMU_ALLOCATED_FLAG;
-#endif
-surface->data = (uint8_t*) qemu_mallocz(surface->linesize * 
surface->height);
-
-return

[Qemu-devel] [PATCH 0/3] move stuff out from vl.c to console.c

2010-02-10 Thread Paolo Bonzini
Moving stuff around to console.c; video does not belong in a target-dependent
file.

Paolo Bonzini (3):
  use lazy initialization for display_state
  remove knowledge of defaultallocator_free_displaysurface from sdl.c
  move default allocator to console.c

 console.c |  176 +++--
 console.h |4 --
 sdl.c |4 --
 vl.c  |   47 +
 4 files changed, 114 insertions(+), 117 deletions(-)





[Qemu-devel] Audio latency

2010-02-10 Thread Alberich de megres
Hi!

I'm running qemu on suse, and i got a very high audio latency. For
example, using windows latency is about 4s and audio gets distorted
with little cuts.
On the host side (suse), i changed the audio pci device latency to 99
but this only improves a little bit.

Other thing i noticed is when lowering video resolution, the audio
latency gets better and better.

I started played with source code, but i'm just a newbie on qemu.
Anyone can point some light to me please?

thanks guys!!




Re: [Qemu-devel] Audio latency

2010-02-10 Thread malc
On Thu, 11 Feb 2010, Alberich de megres wrote:

> Hi!
> 
> I'm running qemu on suse, and i got a very high audio latency. For
> example, using windows latency is about 4s and audio gets distorted
> with little cuts.
> On the host side (suse), i changed the audio pci device latency to 99
> but this only improves a little bit.
> 
> Other thing i noticed is when lowering video resolution, the audio
> latency gets better and better.
> 
> I started played with source code, but i'm just a newbie on qemu.
> Anyone can point some light to me please?
> 
> thanks guys!!

-audio-help is your friend, from there you can deduce (at the very least)
which audio driver your qemu uses.

-- 
mailto:av1...@comtv.ru




[Qemu-devel] [PATCH v2] add close callback for tty-based char device

2010-02-10 Thread David Ahern
v1 -> v2  coding style changes

Add a tty close callback. Right now if a guest device that is connected
to a tty-based chardev in the host is removed, the tty is not closed.
With this patch it is closed.

Example use case is connecting an emulated USB serial cable in the guest
to ttyS0 of the host using the monitor command:

usb_add serial::/dev/ttyS0

and then removing the device with:

usb_del serial::/dev/ttyS0

Signed-off-by: David Ahern 
---
 qemu-char.c |   17 +
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 75dbf66..4169492 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1173,6 +1173,22 @@ static int tty_serial_ioctl(CharDriverState *chr, int 
cmd, void *arg)
 return 0;
 }
 
+static void qemu_chr_close_tty(CharDriverState *chr)
+{
+FDCharDriver *s = chr->opaque;
+int fd = -1;
+
+if (s) {
+fd = s->fd_in;
+}
+
+fd_chr_close(chr);
+
+if (fd >= 0) {
+close(fd);
+}
+}
+
 static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 {
 const char *filename = qemu_opt_get(opts, "path");
@@ -1190,6 +1206,7 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 return NULL;
 }
 chr->chr_ioctl = tty_serial_ioctl;
+chr->chr_close = qemu_chr_close_tty;
 return chr;
 }
 #else  /* ! __linux__ && ! __sun__ */
-- 
1.6.2.5





[Qemu-devel] [PATCH v0 00/21]: Monitor: improve handlers error handling

2010-02-10 Thread Luiz Capitulino
 Hi there,

 When I started converting handlers to the QObject style, I thought that
returning an error code wouldn't be needed. That is, we have an error object
already, so if the handler returns the error object it has failed, otherwise
it has succeeded.

 This was also very convenient, because handlers have never returned an error
code, and thus it would be easier to add a call to qemu_error_new() in the
right place instead of returning error codes.

 Turns out we need both. Actually, I should not have abused the error object
this way because (as Markus says) this is too fragile and we can end up
reporting bogus errors to clients (among other problems).

 The good news is that it's easy to fix.

 All we have to do is to change cmd_new() (the handler callback) to return an
error code and convert existing QObject handlers to it. This series does that
and most of the patches are really straightforward conversions.

 Additionally, Markus has designed an excellent debug mechanism for QMP, which
is implemented by the last patches in this series, and will allow us to catch
important QObject conversion and error handling bugs in handlers.

 Thanks.




[Qemu-devel] [PATCH 01/21] Monitor: Introduce cmd_new_ret()

2010-02-10 Thread Luiz Capitulino
In order to implement the new error handling and debugging
mechanism for command handlers, we need to change the cmd_new()
callback to return a value.

This commit introduces cmd_new_ret(), which returns a value and
will be used only temporarily to handle the transition from
cmd_new().

That is, as soon as all command handlers are ported to cmd_new_ret(),
it will be renamed back to cmd_new() and the new error handling
and debugging mechanism will be added on top of it.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index ae125b8..63c62fb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -98,6 +98,7 @@ typedef struct mon_cmd_t {
 const char *params;
 const char *help;
 void (*user_print)(Monitor *mon, const QObject *data);
+int (*cmd_new_ret)(Monitor *mon, const QDict *params, QObject **ret_data);
 union {
 void (*info)(Monitor *mon);
 void (*info_new)(Monitor *mon, QObject **ret_data);
@@ -3801,7 +3802,11 @@ static void monitor_call_handler(Monitor *mon, const 
mon_cmd_t *cmd,
 {
 QObject *data = NULL;
 
-cmd->mhandler.cmd_new(mon, params, &data);
+if (cmd->cmd_new_ret) {
+cmd->cmd_new_ret(mon, params, &data);
+} else {
+cmd->mhandler.cmd_new(mon, params, &data);
+}
 
 if (is_async_return(data)) {
 /*
-- 
1.6.6





[Qemu-devel] [PATCH 03/21] Monitor: Convert do_cont() to cmd_new_ret()

2010-02-10 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 monitor.c   |8 ++--
 qemu-monitor.hx |2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index c1e0af8..cede368 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1148,14 +1148,18 @@ struct bdrv_iterate_context {
 /**
  * do_cont(): Resume emulation.
  */
-static void do_cont(Monitor *mon, const QDict *qdict, QObject **ret_data)
+static int do_cont(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 struct bdrv_iterate_context context = { mon, 0 };
 
 bdrv_iterate(encrypted_bdrv_it, &context);
 /* only resume the vm if all keys are set and valid */
-if (!context.err)
+if (!context.err) {
 vm_start();
+return 0;
+} else {
+return -1;
+}
 }
 
 static void bdrv_key_cb(void *opaque, int err)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 8e51df0..0eab6db 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -318,7 +318,7 @@ ETEXI
 .params = "",
 .help   = "resume emulation",
 .user_print = monitor_user_noop,
-.mhandler.cmd_new = do_cont,
+.cmd_new_ret = do_cont,
 },
 
 STEXI
-- 
1.6.6





[Qemu-devel] [PATCH 02/21] Monitor: Convert simple handlers to cmd_new_ret()

2010-02-10 Thread Luiz Capitulino
The following handlers always succeed and hence can be converted
to cmd_new_ret() in the same commit.

- do_stop()
- do_quit()
- do_system_reset()
- do_system_powerdown()
- do_migrate_cancel()
- do_qmp_capabilities()
- do_migrate_set_speed()
- do_migrate_set_downtime()

Signed-off-by: Luiz Capitulino 
---
 migration.c |   14 ++
 migration.h |8 
 monitor.c   |   22 ++
 qemu-monitor.hx |   16 
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/migration.c b/migration.c
index 2320c5f..557bec4 100644
--- a/migration.c
+++ b/migration.c
@@ -98,15 +98,17 @@ void do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 }
 }
 
-void do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
+int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 MigrationState *s = current_migration;
 
 if (s)
 s->cancel(s);
+
+return 0;
 }
 
-void do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
+int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 double d;
 FdMigrationState *s;
@@ -119,6 +121,8 @@ void do_migrate_set_speed(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 if (s && s->file) {
 qemu_file_set_rate_limit(s->file, max_throttle);
 }
+
+return 0;
 }
 
 /* amount of nanoseconds we are willing to wait for migration to be down.
@@ -132,14 +136,16 @@ uint64_t migrate_max_downtime(void)
 return max_downtime;
 }
 
-void do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
- QObject **ret_data)
+int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
+QObject **ret_data)
 {
 double d;
 
 d = qdict_get_double(qdict, "value") * 1e9;
 d = MAX(0, MIN(UINT64_MAX, d));
 max_downtime = (uint64_t)d;
+
+return 0;
 }
 
 static void migrate_print_status(Monitor *mon, const char *name,
diff --git a/migration.h b/migration.h
index 65572c1..9345d97 100644
--- a/migration.h
+++ b/migration.h
@@ -54,14 +54,14 @@ void qemu_start_incoming_migration(const char *uri);
 
 void do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
-void do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
-void do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject 
**ret_data);
+int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 uint64_t migrate_max_downtime(void);
 
-void do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
- QObject **ret_data);
+int do_migrate_set_downtime(Monitor *mon, const QDict *qdict,
+QObject **ret_data);
 
 void do_info_migrate_print(Monitor *mon, const QObject *data);
 
diff --git a/monitor.c b/monitor.c
index 63c62fb..c1e0af8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -417,13 +417,15 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 QDECREF(qmp);
 }
 
-static void do_qmp_capabilities(Monitor *mon, const QDict *params,
-QObject **ret_data)
+static int do_qmp_capabilities(Monitor *mon, const QDict *params,
+   QObject **ret_data)
 {
 /* Will setup QMP capabilities in the future */
 if (monitor_ctrl_mode(mon)) {
 mon->mc->command_mode = 1;
 }
+
+return 0;
 }
 
 static int compare_cmd(const char *name, const char *list)
@@ -962,9 +964,10 @@ static void do_info_cpu_stats(Monitor *mon)
 /**
  * do_quit(): Quit QEMU execution
  */
-static void do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
+static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 exit(0);
+return 0;
 }
 
 static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
@@ -1129,9 +1132,10 @@ static void do_singlestep(Monitor *mon, const QDict 
*qdict)
 /**
  * do_stop(): Stop VM execution
  */
-static void do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data)
+static int do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 vm_stop(EXCP_INTERRUPT);
+return 0;
 }
 
 static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs);
@@ -1829,19 +1833,21 @@ static void do_boot_set(Monitor *mon, const QDict 
*qdict)
 /**
  * do_system_reset(): Issue a machine reset
  */
-static void do_system_reset(Monitor *mon, const QDict *qdict,
-QObject **ret_data)
+static int do_system_reset(Monitor *mon, const QDict *qdict,
+   QObject **ret_data)
 {
 qemu_system_reset_request();
+return 0;
 }
 
 /**
  * do_system_powerdown(): Issue a machine powerdown
  */
-static void do_system_powerdown(Monitor *mon, const QDict *qdict,
-QObject **ret_data)
+static int do_system_powerdown(

  1   2   >