Re: [Qemu-devel] qcow2: online snasphots : internal vs external ?

[Stefan Hajnoczi]

On Sat, Aug 25, 2012 at 6:06 PM, Alexandre DERUMIER  wrote:
> I work since many years with snapshot on zfs or netapp,
> and on these system like ceph, I can rollback at the time of the snapshot, 
> and have a view of when the snapshot was taken.
>
> exemple :
> image1 : empty dir /
> take a snapshot (snap1)
> touch /file1
> now rollback to snap1
> ls /   ->empty dir, like when snap1 was taken

Using snapshot capabilities of the underlying storage is a good idea.
LVM snapshots have been used with KVM for a long time.  The same works
for zfs, btrfs, etc.  They are probably more efficient than using
QEMU's external snapshots, which must copy data between image files
when flattening the image chain.

> now,example with qemu:
> image1 : empty dir /

Did you shut down the guest here?

> take a snapshot: (qemu-img snapshot -c snap1 image1)
> touch /file1

Did you shut down the guest here?

> now rollback to snap1 (qemu-img snapshot -a snap1 image1)
> ls /file1
>
>
> the behaviour is completly different. Did I miss something ?

It should work if you shut down the guest before manipulating snapshots.

Stefan

Re: [Qemu-devel] [Bug 1036363] Re: Major network performance problems on AMD hardware

[Stefan Hajnoczi]

On Sun, Aug 26, 2012 at 1:08 AM, Ziemowit Pierzycki
 wrote:
> Executed another test:
>
> F16 KVM <-- 15 gbps --> F17 VM
>
> So why is F16 much faster?

You could try "bisecting" to find the change that slowed down networking:

$ git clone git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git
$ cd qemu-kvm

# Test qemu-kvm 0.15.0, which F16 qemu-kvm is based on
$ git checkout qemu-kvm-0.15.0
$ ./configure && make
$ ...run test...

# Test qemu-kvm 1.0, which F17 qemu-kvm is based on
$ git checkout qemu-kvm-1.0
$ ./configure && make
$ ...run test...

If the 0.15.0 vs 1.0 results show the same change as F16 vs F17, then
you can use git-bisect(1):

$ git bisect start qemu-kvm-1.0 qemu-kvm-0.15.0

http://www.kernel.org/pub/software/scm/git/docs/git-bisect.html
http://git-scm.com/book/en/Git-Tools-Debugging-with-Git#Binary-Search

The bisect will leave you at a commit where the performance regression
was introduced.

Stefan

[Qemu-devel] [PATCH for 1.2] memory: Fix copy&paste mistake in memory_region_iorange_write

[Jan Kiszka]

From: Jan Kiszka 

The last argument of find_portio is "write", so this must be true here.

Signed-off-by: Jan Kiszka 
---

We were likely lucky so far and didn't hit this - it would have caused
an assertion. However, there are also rarely used devices...

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

diff --git a/memory.c b/memory.c
index 643871b..d528d1f 100644
--- a/memory.c
+++ b/memory.c
@@ -426,7 +426,7 @@ static void memory_region_iorange_write(IORange *iorange,
 if (mrp) {
 mrp->write(mr->opaque, offset, data);
 } else if (width == 2) {
-mrp = find_portio(mr, offset - mrio->offset, 1, false);
+mrp = find_portio(mr, offset - mrio->offset, 1, true);
 assert(mrp);
 mrp->write(mr->opaque, offset, data & 0xff);
 mrp->write(mr->opaque, offset + 1, data >> 8);
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH V5 2/8] hw/acpi_piix4.c: replace register_ioport*

[Jan Kiszka]

On 2012-08-22 14:27, Julien Grall wrote:
> This patch replaces all register_ioport* with the new memory API. It permits
> to use the new Memory stuff like listener.
> 
> Signed-off-by: Julien Grall 
> ---
>  hw/acpi_piix4.c |  160 ++
>  1 files changed, 124 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 0aace60..26d5559 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -28,6 +28,7 @@
>  #include "range.h"
>  #include "ioport.h"
>  #include "fw_cfg.h"
> +#include "exec-memory.h"
>  
>  //#define DEBUG
>  
> @@ -41,8 +42,7 @@
>  
>  #define GPE_BASE 0xafe0
>  #define GPE_LEN 4
> -#define PCI_UP_BASE 0xae00
> -#define PCI_DOWN_BASE 0xae04
> +#define PCI_BASE 0xaa00
>  #define PCI_EJ_BASE 0xae08
>  #define PCI_RMV_BASE 0xae0c
>  
> @@ -55,7 +55,7 @@ struct pci_status {
>  
>  typedef struct PIIX4PMState {
>  PCIDevice dev;
> -IORange ioport;
> +MemoryRegion pm_io;
>  ACPIREGS ar;
>  
>  APMState apm;
> @@ -63,6 +63,13 @@ typedef struct PIIX4PMState {
>  PMSMBus smb;
>  uint32_t smb_io_base;
>  
> +MemoryRegion smb_io;
> +MemoryRegion acpi_io;
> +MemoryRegion acpi_hot_io;
> +PortioList pci_hot_port_list;
> +MemoryRegion pciej_hot_io;
> +MemoryRegion pcirmv_hot_io;
> +
>  qemu_irq irq;
>  qemu_irq smi_irq;
>  int kvm_enabled;
> @@ -108,12 +115,12 @@ static void pm_tmr_timer(ACPIREGS *ar)
>  pm_update_sci(s);
>  }
>  
> -static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
> -uint64_t val)
> +static void pm_ioport_write(void *opaque, target_phys_addr_t addr,
> +uint64_t val, unsigned size)
>  {
> -PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
> +PIIX4PMState *s = opaque;
>  
> -if (width != 2) {
> +if (size != 2) {
>  PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
>(unsigned)addr, width, (unsigned)val);
>  }
> @@ -137,11 +144,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t 
> addr, unsigned width,
>(unsigned int)val);
>  }
>  
> -static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
> -uint64_t *data)
> +static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr,
> +   unsigned size)
>  {
> -PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
> -uint32_t val;
> +PIIX4PMState *s = opaque;
> +uint64_t val;
>  
>  switch(addr) {
>  case 0x00:
> @@ -161,12 +168,18 @@ static void pm_ioport_read(IORange *ioport, uint64_t 
> addr, unsigned width,
>  break;
>  }
>  PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned int)addr, 
> val);
> -*data = val;
> +
> +return val;
>  }
>  
> -static const IORangeOps pm_iorange_ops = {
> +static const MemoryRegionOps pm_io_ops = {
>  .read = pm_ioport_read,
>  .write = pm_ioport_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +.impl = {
> +.min_access_size = 2,
> +.max_access_size = 2,
> +},
>  };
>  
>  static void apm_ctrl_changed(uint32_t val, void *arg)
> @@ -183,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
>  }
>  }
>  
> -static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
> +static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr,
> +uint64_t val, unsigned size)
>  {
>  PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
>  }
> @@ -198,8 +212,10 @@ static void pm_io_space_update(PIIX4PMState *s)
>  
>  /* XXX: need to improve memory and ioport allocation */
>  PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
> -iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
> -ioport_register(&s->ioport);
> +
> +memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
> +memory_region_add_subregion(pci_address_space_io(&s->dev),
> +pm_io_base, &s->pm_io);
>  }
>  }
>  
> @@ -381,6 +397,25 @@ static void piix4_pm_machine_ready(Notifier *n, void 
> *opaque)
>  
>  }
>  
> +static const MemoryRegionOps smb_io_ops = {
> +.read = smb_ioport_readb,
> +.write = smb_ioport_writeb,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +.impl = {
> +.min_access_size = 1,
> +.max_access_size = 1,
> +},
> +};
> +
> +static const MemoryRegionOps acpi_io_ops = {
> +.write = acpi_dbg_writel,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +.impl = {
> +.min_access_size = 4,
> +.max_access_size = 4,
> +},
> +};
> +
>  static int piix4_pm_initfn(PCIDevice *dev)
>  {
>  PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
> @@ -395,9 +430,11 @@ static int piix4_pm_initfn(PCIDevice *dev)
>  pci_conf[0x40] = 0x01; /* PM io base 

Re: [Qemu-devel] [PATCH V5 3/8] hw/cirrus_vga.c: replace register_ioport*

[Jan Kiszka]

On 2012-08-22 14:27, Julien Grall wrote:
> This patch replaces all register_ioport* with portio_*. It permits to
> use the new Memory stuff like listener.
> 
> Signed-off-by: Julien Grall 
> ---
>  hw/cirrus_vga.c |   42 --
>  1 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index e8dcc6b..adfc855 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
>  typedef struct CirrusVGAState {
>  VGACommonState vga;
>  
> +MemoryRegion cirrus_vga_io;
>  MemoryRegion cirrus_linear_io;
>  MemoryRegion cirrus_linear_bitblt_io;
>  MemoryRegion cirrus_mmio_io;
> @@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, 
> uint32_t addr)
>  return val;
>  }
>  
> -static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t 
> val)
> +static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr,
> +uint64_t val, unsigned size)
>  {
>  CirrusVGAState *c = opaque;
>  VGACommonState *s = &c->vga;
>  int index;
>  
> +addr += 0x3b0;
> +
>  /* check port range access depending on color/monochrome mode */
>  if (vga_ioport_invalid(s, addr)) {
>   return;
> @@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, 
> target_phys_addr_t addr,
>  if (addr >= 0x100) {
>   cirrus_mmio_blt_write(s, addr - 0x100, val);
>  } else {
> -cirrus_vga_ioport_write(s, addr + 0x3c0, val);
> +cirrus_vga_ioport_write(s, addr + 0x10, val, size);
>  }
>  }
>  
> @@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
>  },
>  };
>  
> +static const MemoryRegionOps cirrus_vga_io_ops = {
> +.write = cirrus_vga_ioport_write,

Missing .read. Crashes immediately when you test it.

Jan




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH V5 2/8] hw/acpi_piix4.c: replace register_ioport*

[Jan Kiszka]

On 2012-08-26 11:10, Jan Kiszka wrote:
> On 2012-08-22 14:27, Julien Grall wrote:
>> This patch replaces all register_ioport* with the new memory API. It permits
>> to use the new Memory stuff like listener.
>>
>> Signed-off-by: Julien Grall 
>> ---
>>  hw/acpi_piix4.c |  160 
>> ++
>>  1 files changed, 124 insertions(+), 36 deletions(-)
>>
>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>> index 0aace60..26d5559 100644
>> --- a/hw/acpi_piix4.c
>> +++ b/hw/acpi_piix4.c
>> @@ -28,6 +28,7 @@
>>  #include "range.h"
>>  #include "ioport.h"
>>  #include "fw_cfg.h"
>> +#include "exec-memory.h"
>>  
>>  //#define DEBUG
>>  
>> @@ -41,8 +42,7 @@
>>  
>>  #define GPE_BASE 0xafe0
>>  #define GPE_LEN 4
>> -#define PCI_UP_BASE 0xae00
>> -#define PCI_DOWN_BASE 0xae04
>> +#define PCI_BASE 0xaa00
>>  #define PCI_EJ_BASE 0xae08
>>  #define PCI_RMV_BASE 0xae0c
>>  
>> @@ -55,7 +55,7 @@ struct pci_status {
>>  
>>  typedef struct PIIX4PMState {
>>  PCIDevice dev;
>> -IORange ioport;
>> +MemoryRegion pm_io;
>>  ACPIREGS ar;
>>  
>>  APMState apm;
>> @@ -63,6 +63,13 @@ typedef struct PIIX4PMState {
>>  PMSMBus smb;
>>  uint32_t smb_io_base;
>>  
>> +MemoryRegion smb_io;
>> +MemoryRegion acpi_io;
>> +MemoryRegion acpi_hot_io;
>> +PortioList pci_hot_port_list;
>> +MemoryRegion pciej_hot_io;
>> +MemoryRegion pcirmv_hot_io;
>> +
>>  qemu_irq irq;
>>  qemu_irq smi_irq;
>>  int kvm_enabled;
>> @@ -108,12 +115,12 @@ static void pm_tmr_timer(ACPIREGS *ar)
>>  pm_update_sci(s);
>>  }
>>  
>> -static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
>> -uint64_t val)
>> +static void pm_ioport_write(void *opaque, target_phys_addr_t addr,
>> +uint64_t val, unsigned size)
>>  {
>> -PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>> +PIIX4PMState *s = opaque;
>>  
>> -if (width != 2) {
>> +if (size != 2) {
>>  PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
>>(unsigned)addr, width, (unsigned)val);
>>  }
>> @@ -137,11 +144,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t 
>> addr, unsigned width,
>>(unsigned int)val);
>>  }
>>  
>> -static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
>> -uint64_t *data)
>> +static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr,
>> +   unsigned size)
>>  {
>> -PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>> -uint32_t val;
>> +PIIX4PMState *s = opaque;
>> +uint64_t val;
>>  
>>  switch(addr) {
>>  case 0x00:
>> @@ -161,12 +168,18 @@ static void pm_ioport_read(IORange *ioport, uint64_t 
>> addr, unsigned width,
>>  break;
>>  }
>>  PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned int)addr, 
>> val);
>> -*data = val;
>> +
>> +return val;
>>  }
>>  
>> -static const IORangeOps pm_iorange_ops = {
>> +static const MemoryRegionOps pm_io_ops = {
>>  .read = pm_ioport_read,
>>  .write = pm_ioport_write,
>> +.endianness = DEVICE_NATIVE_ENDIAN,
>> +.impl = {
>> +.min_access_size = 2,
>> +.max_access_size = 2,
>> +},
>>  };
>>  
>>  static void apm_ctrl_changed(uint32_t val, void *arg)
>> @@ -183,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
>>  }
>>  }
>>  
>> -static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
>> +static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr,
>> +uint64_t val, unsigned size)
>>  {
>>  PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
>>  }
>> @@ -198,8 +212,10 @@ static void pm_io_space_update(PIIX4PMState *s)
>>  
>>  /* XXX: need to improve memory and ioport allocation */
>>  PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
>> -iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
>> -ioport_register(&s->ioport);
>> +
>> +memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
>> +memory_region_add_subregion(pci_address_space_io(&s->dev),
>> +pm_io_base, &s->pm_io);
>>  }
>>  }
>>  
>> @@ -381,6 +397,25 @@ static void piix4_pm_machine_ready(Notifier *n, void 
>> *opaque)
>>  
>>  }
>>  
>> +static const MemoryRegionOps smb_io_ops = {
>> +.read = smb_ioport_readb,
>> +.write = smb_ioport_writeb,
>> +.endianness = DEVICE_NATIVE_ENDIAN,
>> +.impl = {
>> +.min_access_size = 1,
>> +.max_access_size = 1,
>> +},
>> +};
>> +
>> +static const MemoryRegionOps acpi_io_ops = {
>> +.write = acpi_dbg_writel,
>> +.endianness = DEVICE_NATIVE_ENDIAN,
>> +.impl = {
>> +.min_access_size = 4,
>> +.max_access_size = 4,
>> +},
>> +};
>> +
>>  static int piix4_

Re: [Qemu-devel] qcow2: online snasphots : internal vs external ?

[Alexandre DERUMIER]

>>Using snapshot capabilities of the underlying storage is a good idea.
>>LVM snapshots have been used with KVM for a long time. The same works
>>for zfs, btrfs, etc. They are probably more efficient than using
>>QEMU's external snapshots, which must copy data between image files
>>when flattening the image chain.
Yes, indeed ;)  But I'll try to add feature to proxmox for all kind of storage.



>>It should work if you shut down the guest before manipulating snapshots. 
Oh, sorry, I have redone test with stopping the vm before manipulate, and it's 
work fine now.

It is possible to achieve the same behaviour with external snapshot ? (I would 
like to do it online)
I don't see how I can rollback to the point of time of the snapshot.

Also I see that snapshot_blkdev qmp command give in his description:
"Otherwise the snapshot will be internal! (currently unsupported)."

is Live internal snapshots on the roadmap ?

Thanks Again,

Alexandre Derumier
- Mail original -

De: "Stefan Hajnoczi" 
À: "Alexandre DERUMIER" 
Cc: "Jeff Cody" , "qemu-devel" 
Envoyé: Dimanche 26 Août 2012 09:07:44
Objet: Re: [Qemu-devel] qcow2: online snasphots : internal vs external ?

On Sat, Aug 25, 2012 at 6:06 PM, Alexandre DERUMIER  wrote:
> I work since many years with snapshot on zfs or netapp,
> and on these system like ceph, I can rollback at the time of the snapshot, 
> and have a view of when the snapshot was taken.
>
> exemple :
> image1 : empty dir /
> take a snapshot (snap1)
> touch /file1
> now rollback to snap1
> ls / ->empty dir, like when snap1 was taken

Using snapshot capabilities of the underlying storage is a good idea.
LVM snapshots have been used with KVM for a long time. The same works
for zfs, btrfs, etc. They are probably more efficient than using
QEMU's external snapshots, which must copy data between image files
when flattening the image chain.

> now,example with qemu:
> image1 : empty dir /

Did you shut down the guest here?

> take a snapshot: (qemu-img snapshot -c snap1 image1)
> touch /file1

Did you shut down the guest here?

> now rollback to snap1 (qemu-img snapshot -a snap1 image1)
> ls /file1
>
>
> the behaviour is completly different. Did I miss something ?

It should work if you shut down the guest before manipulating snapshots.

Stefan



--

--





Alexandre D e rumier

Ingénieur Systèmes et Réseaux


Fixe : 03 20 68 88 85

Fax : 03 20 68 90 88


45 Bvd du Général Leclerc 59100 Roubaix
12 rue Marivaux 75002 Paris

Re: [Qemu-devel] Windows slow boot: contractor wanted

[Richard Davies]

Rik van Riel wrote:
> Can you get a backtrace to that _raw_spin_lock_irqsave, to see
> from where it is running into lock contention?
>
> It would be good to know whether it is isolate_freepages_block,
> yield_to, kvm_vcpu_on_spin or something else...

Hi Rik,

I got into a slow boot situation on 3.6.0-rc3, ran "perf record -g -a" for a
while, then ran perf report with the output below.

This trace looks more like the second perf top trace that I sent on Saturday
(there were two in my email and they were different from each other as well
as different from on 3.5.2).

The symptoms were a bit different too - the VM boots appeared to be
completely locked up rather than just slow, and I couldn't quit qemu-kvm at
the monitor - I had to restart the host.

So perhaps this one is actually a deadlock rather than just slow?

Cheers,

Richard.


# 
# captured on: Sun Aug 26 10:08:28 2012
# os release : 3.6.0-rc3-elastic
# perf version : 3.5.2
# arch : x86_64
# nrcpus online : 16
# nrcpus avail : 16
# cpudesc : AMD Opteron(tm) Processor 6128
# cpuid : AuthenticAMD,16,9,1
# total memory : 131971760 kB
# cmdline : /home/root/bin/perf record -g -a 
# event : name = cycles, type = 0, config = 0x0, config1 = 0x0, config2 = 0x0, 
excl_usr = 0, excl_kern = 0, id = { 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 
60, 61, 62, 63, 64 }
# HEADER_CPU_TOPOLOGY info available, use -I to display
# HEADER_NUMA_TOPOLOGY info available, use -I to display
# 
#
# Samples: 2M of event 'cycles'
# Event count (approx.): 1040676441385
#
# Overhead  Command Shared Object   
   Symbol
#   ...    
..
#
90.01% qemu-kvm  [kernel.kallsyms] [k] _raw_spin_lock_irqsave   
 
   |
   --- _raw_spin_lock_irqsave
  |  
  |--99.99%-- isolate_migratepages_range
  |  compact_zone
  |  compact_zone_order
  |  try_to_compact_pages
  |  __alloc_pages_direct_compact
  |  __alloc_pages_nodemask
  |  alloc_pages_vma
  |  do_huge_pmd_anonymous_page
  |  handle_mm_fault
  |  __get_user_pages
  |  get_user_page_nowait
  |  hva_to_pfn.isra.33
  |  __gfn_to_pfn
  |  gfn_to_pfn_async
  |  try_async_pf
  |  tdp_page_fault
  |  kvm_mmu_page_fault
  |  pf_interception
  |  handle_exit
  |  kvm_arch_vcpu_ioctl_run
  |  kvm_vcpu_ioctl
  |  do_vfs_ioctl
  |  sys_ioctl
  |  system_call_fastpath
  |  ioctl
  |  |  
  |  |--54.91%-- 0x1010002
  |  |  
  |   --45.09%-- 0x1010006
   --0.01%-- [...]
 4.66% qemu-kvm  [kernel.kallsyms] [k] sub_preempt_count
 
   |
   --- sub_preempt_count
  |  
  |--99.77%-- _raw_spin_unlock_irqrestore
  |  |  
  |  |--99.99%-- compact_checklock_irqsave
  |  |  isolate_migratepages_range
  |  |  compact_zone
  |  |  compact_zone_order
  |  |  try_to_compact_pages
  |  |  __alloc_pages_direct_compact
  |  |  __alloc_pages_nodemask
  |  |  alloc_pages_vma
  |  |  do_huge_pmd_anonymous_page
  |  |  handle_mm_fault
  |  |  __get_user_pages
  |  |  get_user_page_nowait
  |  |  hva_to_pfn.isra.33
  |  |  __gfn_to_pfn
  |  |  gfn_to_pfn_async
  |  |  try_async_pf
  |  |  tdp_page_fault
  |  |  kvm_mmu_page_fault
  |  |  pf_interception
  |  |   

[Qemu-devel] How to edit qemu-kvm DMI strings?

[commiethebeastie]

I need to hide from programs that it runs in VM. How to edit DMI strings 
"Bochs"?

[Qemu-devel] [Bug 1022331] Re: -cpu ? causes confusion when directory has 1-character length filenames

[Aurelien Jarno]

** Changed in: qemu
   Status: Invalid => Fix Committed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1022331

Title:
  -cpu ? causes confusion when directory has 1-character length
  filenames

Status in QEMU:
  Fix Committed

Bug description:
  
  When user is in a directory with 1-character long filenames, parameter -cpu ? 
causes shell to expand ? into filename, which can cause a very confused user.

  One solution would be to replace/add alias to -cpu ?, for example
  -cpulist

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1022331/+subscriptions

[Qemu-devel] [PATCH for-1.2] tcg/arm: Fix broken CONFIG_TCG_PASS_AREG0 code

[Peter Maydell]

The CONFIG_TCG_PASS_AREG0 code for calling ld/st helpers was
broken in that it did not respect the ABI requirement that 64
bit values were passed in even-odd register pairs. The simplest
way to fix this is to implement some new utility functions
for marshalling function arguments into the correct registers
and stack, so that the code which sets up the address and
data arguments does not need to care whether there has been
a preceding env argument.

Signed-off-by: Peter Maydell 
---
The constraints changes here are slightly conservative to avoid
defining new constraint classes (which didn't seem worthwhile
and I also wanted to keep the change relatively small since
we're close to release).

 tcg/arm/tcg-target.c | 237 +++
 1 file changed, 144 insertions(+), 93 deletions(-)

diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index 4d59a63..cf0ca3d 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -176,6 +176,13 @@ static int target_parse_constraint(TCGArgConstraint *ct, 
const char **pct_str)
so don't use these. */
 tcg_regset_reset_reg(ct->u.regs, TCG_REG_R0);
 tcg_regset_reset_reg(ct->u.regs, TCG_REG_R1);
+#if defined(CONFIG_TCG_PASS_AREG0) && (TARGET_LONG_BITS == 64)
+/* If we're passing env to the helper as r0 and need a regpair
+ * for the address then r2 will be overwritten as we're setting
+ * up the args to the helper.
+ */
+tcg_regset_reset_reg(ct->u.regs, TCG_REG_R2);
+#endif
 #endif
 break;
 case 'L':
@@ -197,6 +204,12 @@ static int target_parse_constraint(TCGArgConstraint *ct, 
const char **pct_str)
use these. */
 tcg_regset_reset_reg(ct->u.regs, TCG_REG_R0);
 tcg_regset_reset_reg(ct->u.regs, TCG_REG_R1);
+#if defined(CONFIG_SOFTMMU) && \
+defined(CONFIG_TCG_PASS_AREG0) && (TARGET_LONG_BITS == 64)
+/* Avoid clashes with registers being used for helper args */
+tcg_regset_reset_reg(ct->u.regs, TCG_REG_R2);
+tcg_regset_reset_reg(ct->u.regs, TCG_REG_R3);
+#endif
 break;
 /* qemu_st64 data_reg2 */
 case 'S':
@@ -210,6 +223,10 @@ static int target_parse_constraint(TCGArgConstraint *ct, 
const char **pct_str)
 #ifdef CONFIG_SOFTMMU
 /* r2 is still needed to load data_reg, so don't use it. */
 tcg_regset_reset_reg(ct->u.regs, TCG_REG_R2);
+#if defined(CONFIG_TCG_PASS_AREG0) && (TARGET_LONG_BITS == 64)
+/* Avoid clashes with registers being used for helper args */
+tcg_regset_reset_reg(ct->u.regs, TCG_REG_R3);
+#endif
 #endif
 break;
 
@@ -388,6 +405,14 @@ static inline void tcg_out_dat_reg(TCGContext *s,
 (rn << 16) | (rd << 12) | shift | rm);
 }
 
+static inline void tcg_out_mov_reg(TCGContext *s, int cond, int rd, int rm)
+{
+/* Simple reg-reg move, optimising out the 'do nothing' case */
+if (rd != rm) {
+tcg_out_dat_reg(s, cond, ARITH_MOV, rd, 0, rm, SHIFT_IMM_LSL(0));
+}
+}
+
 static inline void tcg_out_dat_reg2(TCGContext *s,
 int cond, int opc0, int opc1, int rd0, int rd1,
 int rn0, int rn1, int rm0, int rm1, int shift)
@@ -966,6 +991,90 @@ static void *qemu_st_helpers[4] = {
 __stq_mmu,
 };
 #endif
+
+/* Helper routines for marshalling helper function arguments into
+ * the correct registers and stack.
+ * argreg is where we want to put this argument, arg is the argument itself.
+ * Return value is the updated argreg ready for the next call.
+ * Note that argreg 0..3 is real registers, 4+ on stack.
+ * When we reach the first stacked argument, we allocate space for it
+ * and the following stacked arguments using "str r8, [sp, #-0x10]!".
+ * Following arguments are filled in with "str r8, [sp, #0xNN]".
+ * For more than 4 stacked arguments we'd need to know how much
+ * space to allocate when we pushed the first stacked argument.
+ * We don't need this, so don't implement it (and will assert if you try it.)
+ *
+ * We provide routines for arguments which are: immediate, 32 bit
+ * value in register, 16 and 8 bit values in register (which must be zero
+ * extended before use) and 64 bit value in a lo:hi register pair.
+ */
+#define DEFINE_TCG_OUT_ARG(NAME, ARGPARAM) \
+static TCGReg NAME(TCGContext *s, TCGReg argreg, ARGPARAM) \
+{  \
+if (argreg < 4) {  \
+TCG_OUT_ARG_GET_ARG(argreg);   \
+} else if (argreg == 4) {  \
+TCG_OUT_ARG_GET_ARG(TCG_REG_R8);   \
+tcg_out32(s, (COND_AL << 28) | 0x052d8010);\
+} else {   \
+assert(argreg < 8);

[Qemu-devel] [PATCH for-1.2] target-ppc: fix altivec instructions

[Aurelien Jarno]

Altivec instructions are not working anymore in PowerPC emulation,
following commit d15f74fb, which inverted two registers in the call
to helper. Fix that.

Cc: Blue Swirl 
Cc: Alexander Graf 
Cc: Andreas Färber 
Signed-off-by: Aurelien Jarno 
---
 target-ppc/translate.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 91eb7a0..ac915cc 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx)   
  \
 ra = gen_avr_ptr(rA(ctx->opcode));  \
 rb = gen_avr_ptr(rB(ctx->opcode));  \
 rd = gen_avr_ptr(rD(ctx->opcode));  \
-gen_helper_##name(rd, cpu_env, ra, rb); \
+gen_helper_##name(cpu_env, rd, ra, rb); \
 tcg_temp_free_ptr(ra);  \
 tcg_temp_free_ptr(rb);  \
 tcg_temp_free_ptr(rd);  \
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] qdev: unplug request will propagate and release item bottom-up

[Paolo Bonzini]

Il 25/08/2012 09:26, Liu Ping Fan ha scritto:
> From: Liu Ping Fan 
> 
> To achieve uplug a sub tree, we propagate unplug event on the tree.
> 
> Signed-off-by: Liu Ping Fan 

You're improvising.

You need a clear plan of what you're setting to do, what
algorithms/techniques you will use, how you will test it, what changes
to other packages (kernel, BIOS) are needed, how you will handle the
transition while it is only partially done, etc.

Paolo

Re: [Qemu-devel] [PATCH for-1.2] target-ppc: fix altivec instructions

[Peter Maydell]

On 26 August 2012 15:14, Aurelien Jarno  wrote:
> Altivec instructions are not working anymore in PowerPC emulation,
> following commit d15f74fb, which inverted two registers in the call
> to helper. Fix that.
>
> Cc: Blue Swirl 
> Cc: Alexander Graf 
> Cc: Andreas Färber 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-ppc/translate.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 91eb7a0..ac915cc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx) 
> \
>  ra = gen_avr_ptr(rA(ctx->opcode));  \
>  rb = gen_avr_ptr(rB(ctx->opcode));  \
>  rd = gen_avr_ptr(rD(ctx->opcode));  \
> -gen_helper_##name(rd, cpu_env, ra, rb); \
> +gen_helper_##name(cpu_env, rd, ra, rb); \
>  tcg_temp_free_ptr(ra);  \
>  tcg_temp_free_ptr(rb);  \
>  tcg_temp_free_ptr(rd);  \

Reviewed-by: Peter Maydell 

(For these helpers, rd is an input to the helper, not an output,
which is why the cpu_env has to go first, unlike eg gen_helper_mulldo().)

-- PMM

Re: [Qemu-devel] [PATCH for-1.2] target-ppc: fix altivec instructions

[Andreas Färber]

Am 26.08.2012 16:14, schrieb Aurelien Jarno:
> Altivec instructions are not working anymore in PowerPC emulation,
> following commit d15f74fb, which inverted two registers in the call
> to helper. Fix that.
> 
> Cc: Blue Swirl 
> Cc: Alexander Graf 
> Cc: Andreas Färber 
> Signed-off-by: Aurelien Jarno 

Reviewed-by: Andreas Färber 

This looks right, but do you have a particular test case I can check?

Andreas

> ---
>  target-ppc/translate.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 91eb7a0..ac915cc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx) 
> \
>  ra = gen_avr_ptr(rA(ctx->opcode));  \
>  rb = gen_avr_ptr(rB(ctx->opcode));  \
>  rd = gen_avr_ptr(rD(ctx->opcode));  \
> -gen_helper_##name(rd, cpu_env, ra, rb); \
> +gen_helper_##name(cpu_env, rd, ra, rb); \
>  tcg_temp_free_ptr(ra);  \
>  tcg_temp_free_ptr(rb);  \
>  tcg_temp_free_ptr(rd);  \
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH for-1.2 0/2] migrate PV EOI MSR

[Michael S. Tsirkin]

It turns out PV EOI gets disabled after migration -
until next guest reset.
This is because we are missing code to actually migrate it.
This patch fixes it up: it does not do anything useful
without kvm irqchip but applies cleanly to qemu.git
as well as qemu-kvm.git, so I think it's cleaner
to apply it in qemu.git to keep diff to minimum.

Needed for 1.2.



Michael S. Tsirkin (2):
  linux-headers: update asm/kvm_para.h to 3.6
  kvm: get/set PV EOI MSR

 linux-headers/asm-x86/kvm_para.h |  7 +++
 target-i386/cpu.h|  1 +
 target-i386/kvm.c| 13 +
 target-i386/machine.c| 21 +
 4 files changed, 42 insertions(+)

-- 
MST

[Qemu-devel] [PATCH for-1.2 1/2] linux-headers: update asm/kvm_para.h to 3.6

[Michael S. Tsirkin]

Update asm-x96/kvm_para.h to version present in Linux 3.6.
This is needed for the new PV EOI feature.

Signed-off-by: Michael S. Tsirkin 
---
 linux-headers/asm-x86/kvm_para.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/linux-headers/asm-x86/kvm_para.h b/linux-headers/asm-x86/kvm_para.h
index f2ac46a..a1c3d72 100644
--- a/linux-headers/asm-x86/kvm_para.h
+++ b/linux-headers/asm-x86/kvm_para.h
@@ -22,6 +22,7 @@
 #define KVM_FEATURE_CLOCKSOURCE23
 #define KVM_FEATURE_ASYNC_PF   4
 #define KVM_FEATURE_STEAL_TIME 5
+#define KVM_FEATURE_PV_EOI 6
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -37,6 +38,7 @@
 #define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
+#define MSR_KVM_PV_EOI_EN  0x4b564d04
 
 struct kvm_steal_time {
__u64 steal;
@@ -89,5 +91,10 @@ struct kvm_vcpu_pv_apf_data {
__u32 enabled;
 };
 
+#define KVM_PV_EOI_BIT 0
+#define KVM_PV_EOI_MASK (0x1 << KVM_PV_EOI_BIT)
+#define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
+#define KVM_PV_EOI_DISABLED 0x0
+
 
 #endif /* _ASM_X86_KVM_PARA_H */
-- 
MST

[Qemu-devel] [PATCH for-1.2 2/2] kvm: get/set PV EOI MSR

[Michael S. Tsirkin]

Support get/set of new PV EOI MSR, for migration.
Add an optional section for MSR value - send it
out in case MSR was changed from the default value (0).

Signed-off-by: Michael S. Tsirkin 
---
 target-i386/cpu.h |  1 +
 target-i386/kvm.c | 13 +
 target-i386/machine.c | 21 +
 3 files changed, 35 insertions(+)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index aabf993..3c57d8b 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -699,6 +699,7 @@ typedef struct CPUX86State {
 uint64_t system_time_msr;
 uint64_t wall_clock_msr;
 uint64_t async_pf_en_msr;
+uint64_t pv_eoi_en_msr;
 
 uint64_t tsc;
 uint64_t tsc_deadline;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5e2d4f5..6790180 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -64,6 +64,7 @@ static bool has_msr_star;
 static bool has_msr_hsave_pa;
 static bool has_msr_tsc_deadline;
 static bool has_msr_async_pf_en;
+static bool has_msr_pv_eoi_en;
 static bool has_msr_misc_enable;
 static int lm_capable_kernel;
 
@@ -456,6 +457,8 @@ int kvm_arch_init_vcpu(CPUX86State *env)
 
 has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
 
+has_msr_pv_eoi_en = c->eax & (1 << KVM_FEATURE_PV_EOI);
+
 cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
 
 for (i = 0; i <= limit; i++) {
@@ -1018,6 +1021,10 @@ static int kvm_put_msrs(CPUX86State *env, int level)
 kvm_msr_entry_set(&msrs[n++], MSR_KVM_ASYNC_PF_EN,
   env->async_pf_en_msr);
 }
+if (has_msr_pv_eoi_en) {
+kvm_msr_entry_set(&msrs[n++], MSR_KVM_PV_EOI_EN,
+  env->pv_eoi_en_msr);
+}
 if (hyperv_hypercall_available()) {
 kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_GUEST_OS_ID, 0);
 kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_HYPERCALL, 0);
@@ -1260,6 +1267,9 @@ static int kvm_get_msrs(CPUX86State *env)
 if (has_msr_async_pf_en) {
 msrs[n++].index = MSR_KVM_ASYNC_PF_EN;
 }
+if (has_msr_pv_eoi_en) {
+msrs[n++].index = MSR_KVM_PV_EOI_EN;
+}
 
 if (env->mcg_cap) {
 msrs[n++].index = MSR_MCG_STATUS;
@@ -1339,6 +1349,9 @@ static int kvm_get_msrs(CPUX86State *env)
 case MSR_KVM_ASYNC_PF_EN:
 env->async_pf_en_msr = msrs[i].data;
 break;
+case MSR_KVM_PV_EOI_EN:
+env->pv_eoi_en_msr = msrs[i].data;
+break;
 }
 }
 
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a8be058..4771508 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -279,6 +279,13 @@ static bool async_pf_msr_needed(void *opaque)
 return cpu->async_pf_en_msr != 0;
 }
 
+static bool pv_eoi_msr_needed(void *opaque)
+{
+CPUX86State *cpu = opaque;
+
+return cpu->pv_eoi_en_msr != 0;
+}
+
 static const VMStateDescription vmstate_async_pf_msr = {
 .name = "cpu/async_pf_msr",
 .version_id = 1,
@@ -290,6 +297,17 @@ static const VMStateDescription vmstate_async_pf_msr = {
 }
 };
 
+static const VMStateDescription vmstate_pv_eoi_msr = {
+.name = "cpu/async_pv_eoi_msr",
+.version_id = 1,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField []) {
+VMSTATE_UINT64(pv_eoi_en_msr, CPUX86State),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static bool fpop_ip_dp_needed(void *opaque)
 {
 CPUX86State *env = opaque;
@@ -454,6 +472,9 @@ static const VMStateDescription vmstate_cpu = {
 .vmsd = &vmstate_async_pf_msr,
 .needed = async_pf_msr_needed,
 } , {
+.vmsd = &vmstate_pv_eoi_msr,
+.needed = pv_eoi_msr_needed,
+} , {
 .vmsd = &vmstate_fpop_ip_dp,
 .needed = fpop_ip_dp_needed,
 }, {
-- 
MST

Re: [Qemu-devel] [Qemu-ppc] [PATCH v9 1/1] Add USB option in machine options

[Blue Swirl]

On Sat, Aug 25, 2012 at 2:27 PM, Alexander Graf  wrote:
>
>
> On 25.08.2012, at 00:43, Blue Swirl  wrote:
>
>> On Wed, Aug 22, 2012 at 10:31 AM, Li Zhang  wrote:
>>> When -usb option is used, global varible usb_enabled is set.
>>> And all the plafrom will create one USB controller according
>>> to this variable. In fact, global varibles make code hard
>>> to read.
>>>
>>> So this patch is to remove global variable usb_enabled and
>>> add USB option in machine options. All the plaforms will get
>>> USB option value from machine options.
>>>
>>> USB option of machine options will be set either by:
>>>  * -usb
>>>  * -machine type=pseries,usb=on
>>>
>>> Both these ways can work now. They both set USB option in
>>> machine options. In the future, the first way will be removed.
>>>
>>> Signed-off-by: Li Zhang 
>>> ---
>>> v7->v8 :
>>>  * Declare usb_enabled() and set_usb_option() in sysemu.h
>>>  * Separate USB enablement on sPAPR platform.
>>>
>>> v8->v9:
>>>  * Fix usb_enable() default value on sPAPR and MAC99
>>>
>>> Signed-off-by: Li Zhang 
>>>
>>> diff --git a/hw/nseries.c b/hw/nseries.c
>>> index 4df2670..c67e95a 100644
>>> --- a/hw/nseries.c
>>> +++ b/hw/nseries.c
>>> @@ -1322,7 +1322,7 @@ static void n8x0_init(ram_addr_t ram_size, const char 
>>> *boot_device,
>>> n8x0_dss_setup(s);
>>> n8x0_cbus_setup(s);
>>> n8x0_uart_setup(s);
>>> -if (usb_enabled)
>>> +if (usb_enabled(false))
>>
>> Please add braces.
>>
>> I don't like this usb_enabled(false) way very much but I don't have
>> anything better to suggest.
>>
>>> n8x0_usb_setup(s);
>>>
>>> if (kernel_filename) {
>>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>>> index 0c0096f..b662192 100644
>>> --- a/hw/pc_piix.c
>>> +++ b/hw/pc_piix.c
>>> @@ -267,7 +267,7 @@ static void pc_init1(MemoryRegion *system_memory,
>>> pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
>>>  floppy, idebus[0], idebus[1], rtc_state);
>>>
>>> -if (pci_enabled && usb_enabled) {
>>> +if (pci_enabled && usb_enabled(false)) {
>>> pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>>> }
>>>
>>> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
>>> index e95cfe8..1d4f494 100644
>>> --- a/hw/ppc_newworld.c
>>> +++ b/hw/ppc_newworld.c
>>> @@ -348,10 +348,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>> ide_mem[1] = pmac_ide_init(hd, pic[0x0d], dbdma, 0x16, pic[0x02]);
>>> ide_mem[2] = pmac_ide_init(&hd[MAX_IDE_DEVS], pic[0x0e], dbdma, 0x1a, 
>>> pic[0x02]);
>>>
>>> -/* cuda also initialize ADB */
>>> -if (machine_arch == ARCH_MAC99_U3) {
>>> -usb_enabled = 1;
>>> -}
>>> cuda_init(&cuda_mem, pic[0x19]);
>>>
>>> adb_kbd_init(&adb_bus);
>>> @@ -360,15 +356,14 @@ static void ppc_core99_init (ram_addr_t ram_size,
>>> macio_init(pci_bus, PCI_DEVICE_ID_APPLE_UNI_N_KEYL, 0, pic_mem,
>>>dbdma_mem, cuda_mem, NULL, 3, ide_mem, escc_bar);
>>>
>>> -if (usb_enabled) {
>>> +if (usb_enabled(machine_arch == ARCH_MAC99_U3)) {
>>> pci_create_simple(pci_bus, -1, "pci-ohci");
>>> -}
>>> -
>>> -/* U3 needs to use USB for input because Linux doesn't support via-cuda
>>> -   on PPC64 */
>>> -if (machine_arch == ARCH_MAC99_U3) {
>>> -usbdevice_create("keyboard");
>>> -usbdevice_create("mouse");
>>> +/* U3 needs to use USB for input because Linux doesn't support 
>>> via-cuda
>>> +on PPC64 */
>>> +if (machine_arch == ARCH_MAC99_U3) {
>>> +usbdevice_create("keyboard");
>>> +usbdevice_create("mouse");
>>> +}
>>> }
>>>
>>> if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8)
>>> diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
>>> index 1dcd8a6..1468a32 100644
>>> --- a/hw/ppc_oldworld.c
>>> +++ b/hw/ppc_oldworld.c
>>> @@ -286,7 +286,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
>>> macio_init(pci_bus, PCI_DEVICE_ID_APPLE_343S1201, 1, pic_mem,
>>>dbdma_mem, cuda_mem, nvr, 2, ide_mem, escc_bar);
>>>
>>> -if (usb_enabled) {
>>> +if (usb_enabled(false)) {
>>> pci_create_simple(pci_bus, -1, "pci-ohci");
>>> }
>>>
>>> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
>>> index 7a87616..feeb903 100644
>>> --- a/hw/ppc_prep.c
>>> +++ b/hw/ppc_prep.c
>>> @@ -662,7 +662,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>>> memory_region_add_subregion(sysmem, 0xFEFF, xcsr);
>>> #endif
>>>
>>> -if (usb_enabled) {
>>> +if (usb_enabled(false)) {
>>> pci_create_simple(pci_bus, -1, "pci-ohci");
>>> }
>>>
>>> diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
>>> index d5f1420..4787279 100644
>>> --- a/hw/pxa2xx.c
>>> +++ b/hw/pxa2xx.c
>>> @@ -2108,7 +2108,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space,
>>> s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi");
>>> }
>>>
>>> -if (usb_enabled) {
>>> +if (usb_enabled(false)) {
>>> sysbus_create_

Re: [Qemu-devel] Get host virtual address corresponding to guest physical address?

[Blue Swirl]

On Sat, Aug 25, 2012 at 1:17 PM, 陳韋任 (Wei-Ren Chen)
 wrote:
> On Sat, Aug 25, 2012 at 11:56:13AM +0100, Peter Maydell wrote:
>> On 24 August 2012 04:14, 陳韋任 (Wei-Ren Chen)  wrote:
>> >   I would like to know if there is a function in QEMU which converts
>> > a guest physical address into corresponding host virtual address.
>>
>> So the question is, what do you want to do with the host virtual
>> address when you've got it? cpu_physical_memory_map() is really intended
>> (as Blue says) for the case where you have a bit of host code that wants
>> to write a chunk of data and doesn't want to do a sequence of
>> cpu_physical_memory_read()/_write() calls. Instead you _map() the memory,
>> write to it and then _unmap() it.
>
>   We want to let host MMU hardware to do what softmmu does. As a prototype
> (x86 guest on x86_64 host), we want to do the following:
>
>   1. Get guest page table entries (GVA -> GPA).
>
>   2. Get corresponding HVA.
>
>   3. Then we use /dev/mem (with host cr3) to find out HPA.
>
>   4. We insert GVA -> HPA mapping into host page table through /dev/mem,
>  we already move QEMU above 4G to make way for the guest.
>
> So we don't write data into the host virtual addr.

I don't think this GVA to HPA mapping function will help. I'd use the
memory API to construct the GPA-HVA mappings after board init. The
GVA-GPA mappings need to be gathered from guest MMU tables when MMU is
enabled. Then the page tables need to be tracked and any changes to
either guest MMU setup/tables or in guest physical memory space must
propagate to the host memory maps.

>
>> Note that not all guest physical addresses have a meaningful host
>> virtual address -- in particular memory mapped devices won't.
>
>   I guess in our case, we don't touch MMIO?
>
>> >   1. I am running x86 guest on a x86_64 host and using the cod below
>> >  to get the host virtual address, I am not sure what value of len
>> >  should be.
>>
>> The length should be the length of the area of memory you want to
>> either read or write from.
>
>   Actually I want to know where guest page are mapped to host virtual
> address. The GPA we get from step 1 points to guest page table, and
> we want to know its corresponding HVA.
>
>> > static inline void *gpa2hva(target_phys_addr_t addr)
>> > {
>> > target_phys_addr_t len = 4;
>> > return cpu_physical_memory_map(addr, &len, 0);
>> > }
>>
>> If you try this on a memory mapped device address then the first
>> time round it will give you back the address of a "bounce buffer",
>> ie a bit of temporary RAM you can read/write and which unmap will
>> then actually feed to the device's read/write functions. Since you
>> never call unmap, this means that anybody else who tries to use
>> cpu_physical_memory_map() on a device from now on will get back
>> NULL (meaning resource exhaustion, because the bouncebuffer is in
>> use).
>
>   You mean if I call cpu_physical_memory_map with a guest MMIO (physcial)
> address, the first time it'll return the address of a buffer that I can write
> data into. The second time it'll return NULL since I don't call
> cpu_physical_memory_umap to flush the buffer. Do I understand you correctly?
> Hmm, I think we don't not have such issue in our use case... What do you
> think?
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] [PATCH for-1.2] target-ppc: fix altivec instructions

[Blue Swirl]

On Sun, Aug 26, 2012 at 2:14 PM, Aurelien Jarno  wrote:
> Altivec instructions are not working anymore in PowerPC emulation,
> following commit d15f74fb, which inverted two registers in the call
> to helper. Fix that.
>
> Cc: Blue Swirl 

Acked-by: Blue Swirl 

I wonder why TCG debug did not catch this.

> Cc: Alexander Graf 
> Cc: Andreas Färber 
> Signed-off-by: Aurelien Jarno 
> ---
>  target-ppc/translate.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 91eb7a0..ac915cc 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6530,7 +6530,7 @@ static void glue(gen_, name)(DisasContext *ctx) 
> \
>  ra = gen_avr_ptr(rA(ctx->opcode));  \
>  rb = gen_avr_ptr(rB(ctx->opcode));  \
>  rd = gen_avr_ptr(rD(ctx->opcode));  \
> -gen_helper_##name(rd, cpu_env, ra, rb); \
> +gen_helper_##name(cpu_env, rd, ra, rb); \
>  tcg_temp_free_ptr(ra);  \
>  tcg_temp_free_ptr(rb);  \
>  tcg_temp_free_ptr(rd);  \
> --
> 1.7.10.4
>

Re: [Qemu-devel] [PATCH for-1.2] tcg/arm: Fix broken CONFIG_TCG_PASS_AREG0 code

[Blue Swirl]

Thanks, applied.

On Sun, Aug 26, 2012 at 1:40 PM, Peter Maydell  wrote:
> The CONFIG_TCG_PASS_AREG0 code for calling ld/st helpers was
> broken in that it did not respect the ABI requirement that 64
> bit values were passed in even-odd register pairs. The simplest
> way to fix this is to implement some new utility functions
> for marshalling function arguments into the correct registers
> and stack, so that the code which sets up the address and
> data arguments does not need to care whether there has been
> a preceding env argument.
>
> Signed-off-by: Peter Maydell 
> ---
> The constraints changes here are slightly conservative to avoid
> defining new constraint classes (which didn't seem worthwhile
> and I also wanted to keep the change relatively small since
> we're close to release).
>
>  tcg/arm/tcg-target.c | 237 
> +++
>  1 file changed, 144 insertions(+), 93 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index 4d59a63..cf0ca3d 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -176,6 +176,13 @@ static int target_parse_constraint(TCGArgConstraint *ct, 
> const char **pct_str)
> so don't use these. */
>  tcg_regset_reset_reg(ct->u.regs, TCG_REG_R0);
>  tcg_regset_reset_reg(ct->u.regs, TCG_REG_R1);
> +#if defined(CONFIG_TCG_PASS_AREG0) && (TARGET_LONG_BITS == 64)
> +/* If we're passing env to the helper as r0 and need a regpair
> + * for the address then r2 will be overwritten as we're setting
> + * up the args to the helper.
> + */
> +tcg_regset_reset_reg(ct->u.regs, TCG_REG_R2);
> +#endif
>  #endif
>  break;
>  case 'L':
> @@ -197,6 +204,12 @@ static int target_parse_constraint(TCGArgConstraint *ct, 
> const char **pct_str)
> use these. */
>  tcg_regset_reset_reg(ct->u.regs, TCG_REG_R0);
>  tcg_regset_reset_reg(ct->u.regs, TCG_REG_R1);
> +#if defined(CONFIG_SOFTMMU) && \
> +defined(CONFIG_TCG_PASS_AREG0) && (TARGET_LONG_BITS == 64)
> +/* Avoid clashes with registers being used for helper args */
> +tcg_regset_reset_reg(ct->u.regs, TCG_REG_R2);
> +tcg_regset_reset_reg(ct->u.regs, TCG_REG_R3);
> +#endif
>  break;
>  /* qemu_st64 data_reg2 */
>  case 'S':
> @@ -210,6 +223,10 @@ static int target_parse_constraint(TCGArgConstraint *ct, 
> const char **pct_str)
>  #ifdef CONFIG_SOFTMMU
>  /* r2 is still needed to load data_reg, so don't use it. */
>  tcg_regset_reset_reg(ct->u.regs, TCG_REG_R2);
> +#if defined(CONFIG_TCG_PASS_AREG0) && (TARGET_LONG_BITS == 64)
> +/* Avoid clashes with registers being used for helper args */
> +tcg_regset_reset_reg(ct->u.regs, TCG_REG_R3);
> +#endif
>  #endif
>  break;
>
> @@ -388,6 +405,14 @@ static inline void tcg_out_dat_reg(TCGContext *s,
>  (rn << 16) | (rd << 12) | shift | rm);
>  }
>
> +static inline void tcg_out_mov_reg(TCGContext *s, int cond, int rd, int rm)
> +{
> +/* Simple reg-reg move, optimising out the 'do nothing' case */
> +if (rd != rm) {
> +tcg_out_dat_reg(s, cond, ARITH_MOV, rd, 0, rm, SHIFT_IMM_LSL(0));
> +}
> +}
> +
>  static inline void tcg_out_dat_reg2(TCGContext *s,
>  int cond, int opc0, int opc1, int rd0, int rd1,
>  int rn0, int rn1, int rm0, int rm1, int shift)
> @@ -966,6 +991,90 @@ static void *qemu_st_helpers[4] = {
>  __stq_mmu,
>  };
>  #endif
> +
> +/* Helper routines for marshalling helper function arguments into
> + * the correct registers and stack.
> + * argreg is where we want to put this argument, arg is the argument itself.
> + * Return value is the updated argreg ready for the next call.
> + * Note that argreg 0..3 is real registers, 4+ on stack.
> + * When we reach the first stacked argument, we allocate space for it
> + * and the following stacked arguments using "str r8, [sp, #-0x10]!".
> + * Following arguments are filled in with "str r8, [sp, #0xNN]".
> + * For more than 4 stacked arguments we'd need to know how much
> + * space to allocate when we pushed the first stacked argument.
> + * We don't need this, so don't implement it (and will assert if you try it.)
> + *
> + * We provide routines for arguments which are: immediate, 32 bit
> + * value in register, 16 and 8 bit values in register (which must be zero
> + * extended before use) and 64 bit value in a lo:hi register pair.
> + */
> +#define DEFINE_TCG_OUT_ARG(NAME, ARGPARAM) \
> +static TCGReg NAME(TCGContext *s, TCGReg argreg, ARGPARAM) \
> +{  \
> +if (argreg < 4) {  \
> +TCG_OUT_ARG_GET_ARG(argreg);   \
> +} else if (argreg == 4) {  \
> +  

Re: [Qemu-devel] [PATCH for-1.2] target-ppc: fix altivec instructions

[Peter Maydell]

On 26 August 2012 18:56, Blue Swirl  wrote:
> On Sun, Aug 26, 2012 at 2:14 PM, Aurelien Jarno  wrote:
>> Altivec instructions are not working anymore in PowerPC emulation,
>> following commit d15f74fb, which inverted two registers in the call
>> to helper. Fix that.

> I wonder why TCG debug did not catch this.

Because all of ra, rb, rd and cpu_env are TCGv_ptr. Debug only
catches mismatches between _i32, _i64 and _ptr. It might be
possible to add support for enforcing that you pass a cpu_env
in where your DEF_HELPER_* had an 'env' parameter, but it would
be slightly different from the current checks because you want
to support passing a cpu_env TCGv in where a TCGv_ptr is OK
as well as the places which require exactly a TCGv_env.

-- PMM

[Qemu-devel] [PATCH] target-sparc: remove useless line in Makefile.objs

[Aurelien Jarno]

op_helper.c has been removed in commit 0184e266. Remove the corresponding
flags from Makefile.objs.

Cc: Blue Swirl 
Signed-off-by: Aurelien Jarno 
---
 target-sparc/Makefile.objs |2 --
 1 file changed, 2 deletions(-)

diff --git a/target-sparc/Makefile.objs b/target-sparc/Makefile.objs
index a93e07d..9fc42ea 100644
--- a/target-sparc/Makefile.objs
+++ b/target-sparc/Makefile.objs
@@ -4,5 +4,3 @@ obj-y += fop_helper.o cc_helper.o win_helper.o mmu_helper.o 
ldst_helper.o
 obj-$(TARGET_SPARC) += int32_helper.o
 obj-$(TARGET_SPARC64) += int64_helper.o
 obj-$(TARGET_SPARC64) += vis_helper.o
-
-$(obj)/op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
-- 
1.7.10.4

[Qemu-devel] [PATCH] hw/pl110: Fix spelling of 'palette'

[Peter Maydell]

Fix the spelling of 'palette' used in various local variables
and structure members.

Signed-off-by: Peter Maydell 
---
 hw/pl110.c  | 28 ++--
 hw/pl110_template.h | 22 +++---
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/pl110.c b/hw/pl110.c
index f94608c..11037d1 100644
--- a/hw/pl110.c
+++ b/hw/pl110.c
@@ -55,8 +55,8 @@ typedef struct {
 enum pl110_bppmode bpp;
 int invalidate;
 uint32_t mux_ctrl;
-uint32_t pallette[256];
-uint32_t raw_pallette[128];
+uint32_t palette[256];
+uint32_t raw_palette[128];
 qemu_irq irq;
 } pl110_state;
 
@@ -79,8 +79,8 @@ static const VMStateDescription vmstate_pl110 = {
 VMSTATE_INT32(rows, pl110_state),
 VMSTATE_UINT32(bpp, pl110_state),
 VMSTATE_INT32(invalidate, pl110_state),
-VMSTATE_UINT32_ARRAY(pallette, pl110_state, 256),
-VMSTATE_UINT32_ARRAY(raw_pallette, pl110_state, 128),
+VMSTATE_UINT32_ARRAY(palette, pl110_state, 256),
+VMSTATE_UINT32_ARRAY(raw_palette, pl110_state, 128),
 VMSTATE_UINT32_V(mux_ctrl, pl110_state, 2),
 VMSTATE_END_OF_LIST()
 }
@@ -236,7 +236,7 @@ static void pl110_update_display(void *opaque)
s->upbase, s->cols, s->rows,
src_width, dest_width, 0,
s->invalidate,
-   fn, s->pallette,
+   fn, s->palette,
&first, &last);
 if (first >= 0) {
 dpy_update(s->ds, 0, first, s->cols, last - first + 1);
@@ -253,13 +253,13 @@ static void pl110_invalidate_display(void * opaque)
 }
 }
 
-static void pl110_update_pallette(pl110_state *s, int n)
+static void pl110_update_palette(pl110_state *s, int n)
 {
 int i;
 uint32_t raw;
 unsigned int r, g, b;
 
-raw = s->raw_pallette[n];
+raw = s->raw_palette[n];
 n <<= 1;
 for (i = 0; i < 2; i++) {
 r = (raw & 0x1f) << 3;
@@ -271,17 +271,17 @@ static void pl110_update_pallette(pl110_state *s, int n)
 raw >>= 6;
 switch (ds_get_bits_per_pixel(s->ds)) {
 case 8:
-s->pallette[n] = rgb_to_pixel8(r, g, b);
+s->palette[n] = rgb_to_pixel8(r, g, b);
 break;
 case 15:
-s->pallette[n] = rgb_to_pixel15(r, g, b);
+s->palette[n] = rgb_to_pixel15(r, g, b);
 break;
 case 16:
-s->pallette[n] = rgb_to_pixel16(r, g, b);
+s->palette[n] = rgb_to_pixel16(r, g, b);
 break;
 case 24:
 case 32:
-s->pallette[n] = rgb_to_pixel32(r, g, b);
+s->palette[n] = rgb_to_pixel32(r, g, b);
 break;
 }
 n++;
@@ -314,7 +314,7 @@ static uint64_t pl110_read(void *opaque, target_phys_addr_t 
offset,
 return idregs[s->version][(offset - 0xfe0) >> 2];
 }
 if (offset >= 0x200 && offset < 0x400) {
-return s->raw_pallette[(offset - 0x200) >> 2];
+return s->raw_palette[(offset - 0x200) >> 2];
 }
 switch (offset >> 2) {
 case 0: /* LCDTiming0 */
@@ -366,8 +366,8 @@ static void pl110_write(void *opaque, target_phys_addr_t 
offset,
 if (offset >= 0x200 && offset < 0x400) {
 /* Pallette.  */
 n = (offset - 0x200) >> 2;
-s->raw_pallette[(offset - 0x200) >> 2] = val;
-pl110_update_pallette(s, n);
+s->raw_palette[(offset - 0x200) >> 2] = val;
+pl110_update_palette(s, n);
 return;
 }
 switch (offset >> 2) {
diff --git a/hw/pl110_template.h b/hw/pl110_template.h
index 1dce32a..e738e4a 100644
--- a/hw/pl110_template.h
+++ b/hw/pl110_template.h
@@ -129,14 +129,14 @@ static drawfn glue(pl110_draw_fn_,BITS)[48] =
 
 static void glue(pl110_draw_line1_,NAME)(void *opaque, uint8_t *d, const 
uint8_t *src, int width, int deststep)
 {
-uint32_t *pallette = opaque;
+uint32_t *palette = opaque;
 uint32_t data;
 while (width > 0) {
 data = *(uint32_t *)src;
 #ifdef SWAP_PIXELS
-#define FN(x, y) COPY_PIXEL(d, pallette[(data >> (y + 7 - (x))) & 1]);
+#define FN(x, y) COPY_PIXEL(d, palette[(data >> (y + 7 - (x))) & 1]);
 #else
-#define FN(x, y) COPY_PIXEL(d, pallette[(data >> ((x) + y)) & 1]);
+#define FN(x, y) COPY_PIXEL(d, palette[(data >> ((x) + y)) & 1]);
 #endif
 #ifdef SWAP_WORDS
 FN_8(24)
@@ -157,14 +157,14 @@ static void glue(pl110_draw_line1_,NAME)(void *opaque, 
uint8_t *d, const uint8_t
 
 static void glue(pl110_draw_line2_,NAME)(void *opaque, uint8_t *d, const 
uint8_t *src, int width, int deststep)
 {
-uint32_t *pallette = opaque;
+uint32_t *palette = opaque;
 uint32_t data;
 while (width > 0) {
 data = *(uint32_t *)src;
 #ifdef SWAP_PIXELS
-#define FN(x, y) COPY_PIXEL(d, pallette[(data >> (y + 6 - (x)*2)) & 3]);
+#define FN(x, y) COPY_PIXEL(d, palette[(data >> (y + 6 - (x)*2)) & 3]);
 #else
-#d

[Qemu-devel] [RFC PATCH 0/9] qom: improve reference counting and hotplug

[Anthony Liguori]

Right now, you need to pair up object_new with object_delete.  This is
impractical when using reference counting because we would like to ensure that
object_unref() also frees memory when needed.

The first few patches fix this problem by introducing a release callback so
that objects that need special release behavior (i.e. g_free) can do that.

Since link and child properties all hold references, in order to actually free
an object, we need to break those links.  User created devices end up as
children of a container.  But child properties cannot be removed which means
there's no obvious way to remove the reference and ultimately free the object.

We introduce the concept of "nullable child" properties to solve this.  This is
a child property that can be broken by writing NULL to the child link.  Today
we set all /peripheral* children to be nullable so that they can be deleted by
management tools.

In terms of modeling hotplug, we represent unplug by removing the object from
the parent bus.  We need to register a notifier for when this happens so that
we can also remove the parent's child property to ultimately release the object.

Putting it all together, we have:

1) qmp_device_del will issue a callback to a device.  The default callback will
   do a forced eject (which means writing NULL to the parent_bus link).

2) PCI hotplug is a bit more sophisticated in that it waits for the guest to
   do the ejection.

3) qmp_device_del will register an eject notifier such that the device gets
   completely removed.

There's a slightly change in behavior here.  A device is not automatically
destroyed based on a guest initiated eject.  A management tool must explicitly
break the parent's link to the child in order for the device to disappear
completely.  device_del behaves exactly as it does today though.

This is an RFC.  I've tested the series quite a lot (it was hard to get the
reference counting right) but not enough to apply.  I also don't think the
series is quite split right and may not bisect cleanly.

I also want to write up a document describing object life cycle since admittedly
the above is probably not that easy to follow.

I wanted to share this now though because it works and I think the concepts are
right.

[Qemu-devel] [PATCH 8/9] qdev: make qdev_set_parent_bus() just set a link property

[Anthony Liguori]

Also make setting the link to NULL break the bus link

Signed-off-by: Anthony Liguori 
---
 hw/qdev.c |   48 ++--
 1 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 86e1337..525a0cb 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -100,8 +100,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
 
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
 {
-dev->parent_bus = bus;
-bus_add_child(bus, dev);
+object_property_set_link(OBJECT(dev), OBJECT(bus), "parent_bus", NULL);
 }
 
 /* Create a new device.  This only initializes the device state structure
@@ -241,8 +240,8 @@ void qbus_reset_all_fn(void *opaque)
 /* can be used as ->unplug() callback for the simple cases */
 int qdev_simple_unplug_cb(DeviceState *dev)
 {
-/* just zap it */
-qdev_free(dev);
+/* Unplug from parent bus via a forced eject */
+qdev_set_parent_bus(dev, NULL);
 return 0;
 }
 
@@ -646,6 +645,40 @@ void qdev_property_add_static(DeviceState *dev, Property 
*prop,
 assert_no_error(local_err);
 }
 
+static void qdev_set_link_property(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+BusState *parent_bus = dev->parent_bus;
+
+object_set_link_property(obj, v, opaque, name, errp);
+
+if (parent_bus) {
+bus_remove_child(parent_bus, dev);
+}
+
+if (dev->parent_bus) {
+bus_add_child(dev->parent_bus, dev);
+}
+
+if (!dev->parent_bus) {
+notifier_list_notify(&dev->eject_notifier, dev);
+}
+}
+
+static void qdev_release_link_property(Object *obj, const char *name,
+   void *opaque)
+{
+DeviceState *dev = DEVICE(obj);
+
+if (dev->parent_bus) {
+bus_remove_child(dev->parent_bus, dev);
+object_unref(OBJECT(dev->parent_bus));
+}
+
+dev->parent_bus = NULL;
+}
+
 static void device_initfn(Object *obj)
 {
 DeviceState *dev = DEVICE(obj);
@@ -670,8 +703,11 @@ static void device_initfn(Object *obj)
 } while (class != object_class_by_name(TYPE_DEVICE));
 qdev_prop_set_globals(dev);
 
-object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
- (Object **)&dev->parent_bus, NULL);
+object_property_add(OBJECT(dev), "parent_bus", "link<" TYPE_BUS ">",
+object_get_link_property,
+qdev_set_link_property,
+qdev_release_link_property,
+&dev->parent_bus, NULL);
 
 notifier_list_init(&dev->eject_notifier);
 }
-- 
1.7.5.4

[Qemu-devel] [PATCH 9/9] hotplug: refactor hotplug to leverage new QOM functions

[Anthony Liguori]

1) DeviceState::unplug requests for an eject to happen
   - the default implementation is a forced eject

2) A bus can eject a device by setting the parent_bus to NULL
   - this detaches the device from the bus
   - this does *not* cause the device to disappear

3) The current implementation on unplug also registers an eject notifier
   - the eject notifier will detach the device the parent.  This will cause the
 device to disappear

4) A purely guest initiated unplug will not delete a device but will cause the
   device to appear detached from the guests PoV.

Signed-off-by: Anthony Liguori 
---
 hw/acpi_piix4.c   |3 ++-
 hw/pci.c  |   10 +-
 hw/pcie.c |2 +-
 hw/qdev.c |   22 ++
 hw/qdev.h |2 ++
 hw/shpc.c |2 +-
 hw/xen_platform.c |2 +-
 7 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 72d6e5c..eac53b3 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -305,7 +305,8 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned 
slots)
 if (pc->no_hotplug) {
 slot_free = false;
 } else {
-qdev_free(qdev);
+/* Force eject of device */
+qdev_set_parent_bus(qdev, NULL);
 }
 }
 }
diff --git a/hw/pci.c b/hw/pci.c
index 437af70..cc555c2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -46,6 +46,14 @@ static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
+static void pcibus_remove_child(BusState *bus, DeviceState *dev)
+{
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *pci_bus = PCI_BUS(bus);
+
+pci_bus->devices[pci_dev->devfn] = NULL;
+}
+
 static Property pci_props[] = {
 DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
 DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
@@ -65,6 +73,7 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
 k->get_dev_path = pcibus_get_dev_path;
 k->get_fw_dev_path = pcibus_get_fw_dev_path;
 k->reset = pcibus_reset;
+k->remove_child = pcibus_remove_child;
 }
 
 static const TypeInfo pci_bus_info = {
@@ -833,7 +842,6 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
 static void do_pci_unregister_device(PCIDevice *pci_dev)
 {
 qemu_free_irqs(pci_dev->irq);
-pci_dev->bus->devices[pci_dev->devfn] = NULL;
 pci_config_free(pci_dev);
 }
 
diff --git a/hw/pcie.c b/hw/pcie.c
index 7c92f19..d10ffea 100644
--- a/hw/pcie.c
+++ b/hw/pcie.c
@@ -235,7 +235,7 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
PCI_EXP_SLTSTA_PDS);
 pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
 } else {
-qdev_free(&pci_dev->qdev);
+qdev_set_parent_bus(DEVICE(pci_dev), NULL);
 pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
  PCI_EXP_SLTSTA_PDS);
 pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
diff --git a/hw/qdev.c b/hw/qdev.c
index 525a0cb..be41f00 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -62,6 +62,7 @@ static void qdev_property_add_legacy(DeviceState *dev, 
Property *prop,
 
 static void bus_remove_child(BusState *bus, DeviceState *child)
 {
+BusClass *bc = BUS_GET_CLASS(bus);
 BusChild *kid;
 
 QTAILQ_FOREACH(kid, &bus->children, sibling) {
@@ -71,6 +72,11 @@ static void bus_remove_child(BusState *bus, DeviceState 
*child)
 snprintf(name, sizeof(name), "child[%d]", kid->index);
 QTAILQ_REMOVE(&bus->children, kid, sibling);
 object_property_del(OBJECT(bus), name, NULL);
+
+if (bc->remove_child) {
+bc->remove_child(bus, kid->child);
+}
+
 g_free(kid);
 return;
 }
@@ -192,9 +198,20 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int 
alias_id,
 dev->alias_required_for_version = required_for_version;
 }
 
+static void qdev_finish_unplug(Notifier *notifier, void *data)
+{
+DeviceState *dev = DEVICE(data);
+
+/* unparent the object -- this should release the last reference to the
+   child*/
+object_unparent(OBJECT(dev));
+g_free(notifier);
+}
+
 void qdev_unplug(DeviceState *dev, Error **errp)
 {
 DeviceClass *dc = DEVICE_GET_CLASS(dev);
+Notifier *notifier;
 
 if (!dev->parent_bus->allow_hotplug) {
 error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
@@ -204,6 +221,11 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 
 qdev_hot_removed = true;
 
+notifier = g_malloc0(sizeof(*notifier));
+notifier->notify = qdev_finish_unplug;
+
+notifier_list_add(&dev->eject_notifier, notifier);
+
 if (dc->unplug(dev) < 0) {
 error_set(errp, QERR_UNDEFINED_ERROR);
 return;
diff --git a/hw/qdev.h b/hw/qdev.h
index 5009072..7ae8d5d 100644
--- a/hw/qdev.h
+++ b/hw/

[Qemu-devel] [PATCH 3/9] qbus: remove glib_allocated/qom_allocated and use release hook to free memory

[Anthony Liguori]

Signed-off-by: Anthony Liguori 
---
 hw/pci.c|7 ++-
 hw/qdev.c   |   15 ---
 hw/qdev.h   |7 ---
 hw/sysbus.c |7 ++-
 4 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 4d95984..437af70 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -292,6 +292,11 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
 vmstate_register(NULL, -1, &vmstate_pcibus, bus);
 }
 
+static void pci_bus_release(Object *obj)
+{
+g_free(obj);
+}
+
 PCIBus *pci_bus_new(DeviceState *parent, const char *name,
 MemoryRegion *address_space_mem,
 MemoryRegion *address_space_io,
@@ -300,9 +305,9 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
 PCIBus *bus;
 
 bus = g_malloc0(sizeof(*bus));
-bus->qbus.glib_allocated = true;
 pci_bus_new_inplace(bus, parent, name, address_space_mem,
 address_space_io, devfn_min);
+object_set_release_func(OBJECT(bus), pci_bus_release);
 return bus;
 }
 
diff --git a/hw/qdev.c b/hw/qdev.c
index b5a52ac..6b61daa 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -459,8 +459,6 @@ BusState *qbus_create(const char *typename, DeviceState 
*parent, const char *nam
 BusState *bus;
 
 bus = BUS(object_new(typename));
-bus->qom_allocated = true;
-
 bus->parent = parent;
 bus->name = name ? g_strdup(name) : NULL;
 qbus_realize(bus);
@@ -468,18 +466,6 @@ BusState *qbus_create(const char *typename, DeviceState 
*parent, const char *nam
 return bus;
 }
 
-void qbus_free(BusState *bus)
-{
-if (bus->qom_allocated) {
-object_delete(OBJECT(bus));
-} else {
-object_finalize(OBJECT(bus));
-if (bus->glib_allocated) {
-g_free(bus);
-}
-}
-}
-
 static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
 {
 BusClass *bc = BUS_GET_CLASS(bus);
@@ -698,7 +684,6 @@ static void device_finalize(Object *obj)
 if (dev->state == DEV_STATE_INITIALIZED) {
 while (dev->num_child_bus) {
 bus = QLIST_FIRST(&dev->child_bus);
-qbus_free(bus);
 }
 if (qdev_get_vmsd(dev)) {
 vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index d699194..3561e3a 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -106,17 +106,12 @@ typedef struct BusChild {
 
 /**
  * BusState:
- * @qom_allocated: Indicates whether the object was allocated by QOM.
- * @glib_allocated: Indicates whether the object was initialized in-place
- * yet is expected to be freed with g_free().
  */
 struct BusState {
 Object obj;
 DeviceState *parent;
 const char *name;
 int allow_hotplug;
-bool qom_allocated;
-bool glib_allocated;
 int max_index;
 QTAILQ_HEAD(ChildrenHead, BusChild) children;
 QLIST_ENTRY(BusState) sibling;
@@ -201,8 +196,6 @@ int qdev_walk_children(DeviceState *dev, qdev_walkerfn 
*devfn,
 void qdev_reset_all(DeviceState *dev);
 void qbus_reset_all_fn(void *opaque);
 
-void qbus_free(BusState *bus);
-
 #define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
 
 /* This should go away once we get rid of the NULL bus hack */
diff --git a/hw/sysbus.c b/hw/sysbus.c
index 9d8b1ea..8305337 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -267,6 +267,11 @@ static TypeInfo sysbus_device_type_info = {
 /* This is a nasty hack to allow passing a NULL bus to qdev_create.  */
 static BusState *main_system_bus;
 
+static void main_system_bus_release(Object *obj)
+{
+g_free(obj);
+}
+
 static void main_system_bus_create(void)
 {
 /* assign main_system_bus before qbus_create_inplace()
@@ -274,7 +279,7 @@ static void main_system_bus_create(void)
 main_system_bus = g_malloc0(system_bus_info.instance_size);
 qbus_create_inplace(main_system_bus, TYPE_SYSTEM_BUS, NULL,
 "main-system-bus");
-main_system_bus->glib_allocated = true;
+object_set_release_func(OBJECT(main_system_bus), main_system_bus_release);
 object_property_add_child(container_get(qdev_get_machine(),
 "/unattached"),
   "sysbus", OBJECT(main_system_bus), NULL);
-- 
1.7.5.4

[Qemu-devel] [PATCH 5/9] object: add support for nullable child properties

[Anthony Liguori]

A nullable child can be detached from its parent by setting the property to
NULL.  This provides a mechanism for a management tool to delete certain
types of objects.

Signed-off-by: Anthony Liguori 

object: link<>: release reference on finalize, externalize getter/setter

Reported-by: Ping Fan
Signed-off-by: Anthony Liguori 
---
 include/qemu/object.h |9 +
 qom/object.c  |   78 ++--
 2 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index 8bc9935..5bb62dd 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -907,6 +907,9 @@ Object *object_resolve_path_component(Object *parent, gchar 
*part);
 void object_property_add_child(Object *obj, const char *name,
Object *child, struct Error **errp);
 
+void object_property_add_nullable_child(Object *obj, const char *name,
+Object *child, struct Error **errp);
+
 /**
  * object_property_add_link:
  * @obj: the object to add a property to
@@ -925,6 +928,12 @@ void object_property_add_link(Object *obj, const char 
*name,
   const char *type, Object **child,
   struct Error **errp);
 
+void object_get_link_property(Object *obj, struct Visitor *v, void *opaque,
+  const char *name, struct Error **errp);
+
+void object_set_link_property(Object *obj, struct Visitor *v, void *opaque,
+  const char *name, struct Error **errp);
+
 /**
  * object_property_add_str:
  * @obj: the object to add a property to
diff --git a/qom/object.c b/qom/object.c
index 1144f79..61fa40f 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -744,8 +744,17 @@ char *object_property_get_str(Object *obj, const char 
*name,
 void object_property_set_link(Object *obj, Object *value,
   const char *name, Error **errp)
 {
-object_property_set_str(obj, object_get_canonical_path(value),
-name, errp);
+char *str;
+
+if (value == NULL) {
+str = g_strdup("");
+} else {
+str = object_get_canonical_path(value);
+}
+
+object_property_set_str(obj, str, name, errp);
+
+g_free(str);
 }
 
 Object *object_property_get_link(Object *obj, const char *name,
@@ -882,6 +891,27 @@ static void object_get_child_property(Object *obj, Visitor 
*v, void *opaque,
 g_free(path);
 }
 
+static void object_set_child_property(Object *obj, Visitor *v, void *opaque,
+  const char *name, Error **errp)
+{
+gchar *path = NULL;
+Error *local_err = NULL;
+
+visit_type_str(v, &path, name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+if (path && *path) {
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+  "Cannot set child properties to anything but empty string");
+return;
+}
+
+object_property_del(obj, name, errp);
+}
+
 static void object_finalize_child_property(Object *obj, const char *name,
void *opaque)
 {
@@ -890,15 +920,17 @@ static void object_finalize_child_property(Object *obj, 
const char *name,
 object_unref(child);
 }
 
-void object_property_add_child(Object *obj, const char *name,
-   Object *child, Error **errp)
+static void object_property_add_child_full(Object *obj, const char *name,
+   Object *child, bool writable,
+   Error **errp)
 {
 gchar *type;
 
 type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
 
 object_property_add(obj, name, type, object_get_child_property,
-NULL, object_finalize_child_property, child, errp);
+writable ? object_set_child_property : NULL,
+object_finalize_child_property, child, errp);
 
 object_ref(child);
 g_assert(child->parent == NULL);
@@ -907,8 +939,20 @@ void object_property_add_child(Object *obj, const char 
*name,
 g_free(type);
 }
 
-static void object_get_link_property(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
+void object_property_add_child(Object *obj, const char *name,
+   Object *child, Error **errp)
+{
+object_property_add_child_full(obj, name, child, false, errp);
+}
+
+void object_property_add_nullable_child(Object *obj, const char *name,
+Object *child, Error **errp)
+{
+object_property_add_child_full(obj, name, child, true, errp);
+}
+
+void object_get_link_property(Object *obj, Visitor *v, void *opaque,
+  const char *name, Error **errp)
 {
 Object **child = opaque;
 gchar *path;

[Qemu-devel] [PATCH 6/9] qdev: make devices created with device_add nullable so they can be deleted

[Anthony Liguori]

A management tool can destroy these devices by writing an empty string into
the child link property.

Signed-off-by: Anthony Liguori 
---
 hw/qdev-monitor.c |   15 ++-
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 018b386..3f08575 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -469,15 +469,20 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 return NULL;
 }
 if (qdev->id) {
-object_property_add_child(qdev_get_peripheral(), qdev->id,
-  OBJECT(qdev), NULL);
+object_property_add_nullable_child(qdev_get_peripheral(), qdev->id,
+   OBJECT(qdev), NULL);
 } else {
 static int anon_count;
 gchar *name = g_strdup_printf("device[%d]", anon_count++);
-object_property_add_child(qdev_get_peripheral_anon(), name,
-  OBJECT(qdev), NULL);
+object_property_add_nullable_child(qdev_get_peripheral_anon(), name,
+   OBJECT(qdev), NULL);
 g_free(name);
-}
+}
+
+/* Drop the allocation reference -- the container link will ensure the
+   object stays alive. */
+object_unref(OBJECT(qdev));
+
 if (qdev_init(qdev) < 0) {
 qerror_report(QERR_DEVICE_INIT_FAILED, driver);
 return NULL;
-- 
1.7.5.4

[Qemu-devel] [PATCH 4/9] object: remove object_finalize

[Anthony Liguori]

Callers should just use object_unref

Signed-off-by: Anthony Liguori 
---
 hw/qdev.c |4 
 include/qemu/object.h |9 -
 qom/object.c  |2 +-
 3 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 6b61daa..fdee91f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -678,13 +678,9 @@ static void device_initfn(Object *obj)
 static void device_finalize(Object *obj)
 {
 DeviceState *dev = DEVICE(obj);
-BusState *bus;
 DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
 if (dev->state == DEV_STATE_INITIALIZED) {
-while (dev->num_child_bus) {
-bus = QLIST_FIRST(&dev->child_bus);
-}
 if (qdev_get_vmsd(dev)) {
 vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
 }
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 487adcd..8bc9935 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -490,15 +490,6 @@ void object_initialize_with_type(void *data, Type type);
 void object_initialize(void *obj, const char *typename);
 
 /**
- * object_finalize:
- * @obj: The object to finalize.
- *
- * This function destroys and object without freeing the memory associated with
- * it.
- */
-void object_finalize(void *obj);
-
-/**
  * object_dynamic_cast:
  * @obj: The object to cast.
  * @typename: The @typename to cast to.
diff --git a/qom/object.c b/qom/object.c
index 44135c3..1144f79 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -375,7 +375,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
 }
 }
 
-void object_finalize(void *data)
+static void object_finalize(void *data)
 {
 Object *obj = data;
 TypeImpl *ti = obj->class->type;
-- 
1.7.5.4

[Qemu-devel] [PATCH 1/9] savevm: don't rely on paths if we can store a DeviceState object

[Anthony Liguori]

Paths break during tear down which can result in odd behavior since we do path
based lookup during unregister (which happens at tear down).

For devices, just store the DeviceState directly and use that.

Signed-off-by: Anthony Liguori 
---
 savevm.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/savevm.c b/savevm.c
index c7fe283..bb6494b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1177,6 +1177,7 @@ typedef struct SaveStateEntry {
 CompatEntry *compat;
 int no_migrate;
 int is_ram;
+DeviceState *dev;
 } SaveStateEntry;
 
 
@@ -1235,6 +1236,7 @@ int register_savevm_live(DeviceState *dev,
 se->opaque = opaque;
 se->vmsd = NULL;
 se->no_migrate = 0;
+se->dev = dev;
 /* if this is a live_savem then set is_ram */
 if (ops->save_live_setup != NULL) {
 se->is_ram = 1;
@@ -1298,7 +1300,8 @@ void unregister_savevm(DeviceState *dev, const char 
*idstr, void *opaque)
 pstrcat(id, sizeof(id), idstr);
 
 QTAILQ_FOREACH_SAFE(se, &savevm_handlers, entry, new_se) {
-if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
+if ((dev && se->dev == dev) ||
+(strcmp(se->idstr, id) == 0 && se->opaque == opaque)) {
 QTAILQ_REMOVE(&savevm_handlers, se, entry);
 if (se->compat) {
 g_free(se->compat);
-- 
1.7.5.4

[Qemu-devel] [PATCH 2/9] object: automatically free objects based on a release function

[Anthony Liguori]

Now object_delete() simply has the semantics of unref'ing an object and
unparenting it.

Signed-off-by: Anthony Liguori 
---
 include/qemu/object.h |5 +
 qom/object.c  |   16 +++-
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index cc75fee..487adcd 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -242,6 +242,8 @@ struct ObjectClass
 GSList *interfaces;
 };
 
+typedef void (ObjectReleaseFunc)(Object *obj);
+
 /**
  * Object:
  *
@@ -264,6 +266,7 @@ struct Object
 QTAILQ_HEAD(, ObjectProperty) properties;
 uint32_t ref;
 Object *parent;
+ObjectReleaseFunc *release;
 };
 
 /**
@@ -464,6 +467,8 @@ Object *object_new_with_type(Type type);
  */
 void object_delete(Object *obj);
 
+void object_set_release_func(Object *obj, ObjectReleaseFunc *func);
+
 /**
  * object_initialize_with_type:
  * @obj: A pointer to the memory to be used for the object.
diff --git a/qom/object.c b/qom/object.c
index e3e9242..44135c3 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -384,6 +384,20 @@ void object_finalize(void *data)
 object_property_del_all(obj);
 
 g_assert(obj->ref == 0);
+
+if (obj->release) {
+obj->release(obj);
+}
+}
+
+void object_set_release_func(Object *obj, ObjectReleaseFunc *func)
+{
+obj->release = func;
+}
+
+static void object_release_func(Object *obj)
+{
+g_free(obj);
 }
 
 Object *object_new_with_type(Type type)
@@ -395,6 +409,7 @@ Object *object_new_with_type(Type type)
 
 obj = g_malloc(type->instance_size);
 object_initialize_with_type(obj, type);
+object_set_release_func(obj, object_release_func);
 object_ref(obj);
 
 return obj;
@@ -412,7 +427,6 @@ void object_delete(Object *obj)
 object_unparent(obj);
 g_assert(obj->ref == 1);
 object_unref(obj);
-g_free(obj);
 }
 
 Object *object_dynamic_cast(Object *obj, const char *typename)
-- 
1.7.5.4

[Qemu-devel] [PATCH 7/9] qdev: add notifier for when the device loses its parent bus (eject)

[Anthony Liguori]

Signed-off-by: Anthony Liguori 
---
 hw/qdev.c |2 ++
 hw/qdev.h |3 +++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index fdee91f..86e1337 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -672,6 +672,8 @@ static void device_initfn(Object *obj)
 
 object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
  (Object **)&dev->parent_bus, NULL);
+
+notifier_list_init(&dev->eject_notifier);
 }
 
 /* Unlink device from bus and free the structure.  */
diff --git a/hw/qdev.h b/hw/qdev.h
index 3561e3a..5009072 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -8,6 +8,7 @@
 #include "qapi/qapi-visit-core.h"
 #include "qemu/object.h"
 #include "error.h"
+#include "notify.h"
 
 typedef struct Property Property;
 
@@ -76,6 +77,8 @@ struct DeviceState {
 int num_child_bus;
 int instance_id_alias;
 int alias_required_for_version;
+
+NotifierList eject_notifier;
 };
 
 #define TYPE_BUS "bus"
-- 
1.7.5.4

Re: [Qemu-devel] [Qemu-ppc] [PATCH v9 1/1] Add USB option in machine options

[Alexander Graf]

On 26.08.2012, at 10:34, Blue Swirl  wrote:

> On Sat, Aug 25, 2012 at 2:27 PM, Alexander Graf  wrote:
>> 
>> 
>> On 25.08.2012, at 00:43, Blue Swirl  wrote:
>> 
>>> On Wed, Aug 22, 2012 at 10:31 AM, Li Zhang  wrote:
 When -usb option is used, global varible usb_enabled is set.
 And all the plafrom will create one USB controller according
 to this variable. In fact, global varibles make code hard
 to read.
 
 So this patch is to remove global variable usb_enabled and
 add USB option in machine options. All the plaforms will get
 USB option value from machine options.
 
 USB option of machine options will be set either by:
 * -usb
 * -machine type=pseries,usb=on
 
 Both these ways can work now. They both set USB option in
 machine options. In the future, the first way will be removed.
 
 Signed-off-by: Li Zhang 
 ---
 v7->v8 :
 * Declare usb_enabled() and set_usb_option() in sysemu.h
 * Separate USB enablement on sPAPR platform.
 
 v8->v9:
 * Fix usb_enable() default value on sPAPR and MAC99
 
 Signed-off-by: Li Zhang 
 
 diff --git a/hw/nseries.c b/hw/nseries.c
 index 4df2670..c67e95a 100644
 --- a/hw/nseries.c
 +++ b/hw/nseries.c
 @@ -1322,7 +1322,7 @@ static void n8x0_init(ram_addr_t ram_size, const 
 char *boot_device,
n8x0_dss_setup(s);
n8x0_cbus_setup(s);
n8x0_uart_setup(s);
 -if (usb_enabled)
 +if (usb_enabled(false))
>>> 
>>> Please add braces.
>>> 
>>> I don't like this usb_enabled(false) way very much but I don't have
>>> anything better to suggest.
>>> 
n8x0_usb_setup(s);
 
if (kernel_filename) {
 diff --git a/hw/pc_piix.c b/hw/pc_piix.c
 index 0c0096f..b662192 100644
 --- a/hw/pc_piix.c
 +++ b/hw/pc_piix.c
 @@ -267,7 +267,7 @@ static void pc_init1(MemoryRegion *system_memory,
pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
 floppy, idebus[0], idebus[1], rtc_state);
 
 -if (pci_enabled && usb_enabled) {
 +if (pci_enabled && usb_enabled(false)) {
pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
}
 
 diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
 index e95cfe8..1d4f494 100644
 --- a/hw/ppc_newworld.c
 +++ b/hw/ppc_newworld.c
 @@ -348,10 +348,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
ide_mem[1] = pmac_ide_init(hd, pic[0x0d], dbdma, 0x16, pic[0x02]);
ide_mem[2] = pmac_ide_init(&hd[MAX_IDE_DEVS], pic[0x0e], dbdma, 0x1a, 
 pic[0x02]);
 
 -/* cuda also initialize ADB */
 -if (machine_arch == ARCH_MAC99_U3) {
 -usb_enabled = 1;
 -}
cuda_init(&cuda_mem, pic[0x19]);
 
adb_kbd_init(&adb_bus);
 @@ -360,15 +356,14 @@ static void ppc_core99_init (ram_addr_t ram_size,
macio_init(pci_bus, PCI_DEVICE_ID_APPLE_UNI_N_KEYL, 0, pic_mem,
   dbdma_mem, cuda_mem, NULL, 3, ide_mem, escc_bar);
 
 -if (usb_enabled) {
 +if (usb_enabled(machine_arch == ARCH_MAC99_U3)) {
pci_create_simple(pci_bus, -1, "pci-ohci");
 -}
 -
 -/* U3 needs to use USB for input because Linux doesn't support 
 via-cuda
 -   on PPC64 */
 -if (machine_arch == ARCH_MAC99_U3) {
 -usbdevice_create("keyboard");
 -usbdevice_create("mouse");
 +/* U3 needs to use USB for input because Linux doesn't support 
 via-cuda
 +on PPC64 */
 +if (machine_arch == ARCH_MAC99_U3) {
 +usbdevice_create("keyboard");
 +usbdevice_create("mouse");
 +}
}
 
if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8)
 diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
 index 1dcd8a6..1468a32 100644
 --- a/hw/ppc_oldworld.c
 +++ b/hw/ppc_oldworld.c
 @@ -286,7 +286,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
macio_init(pci_bus, PCI_DEVICE_ID_APPLE_343S1201, 1, pic_mem,
   dbdma_mem, cuda_mem, nvr, 2, ide_mem, escc_bar);
 
 -if (usb_enabled) {
 +if (usb_enabled(false)) {
pci_create_simple(pci_bus, -1, "pci-ohci");
}
 
 diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
 index 7a87616..feeb903 100644
 --- a/hw/ppc_prep.c
 +++ b/hw/ppc_prep.c
 @@ -662,7 +662,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
memory_region_add_subregion(sysmem, 0xFEFF, xcsr);
 #endif
 
 -if (usb_enabled) {
 +if (usb_enabled(false)) {
pci_create_simple(pci_bus, -1, "pci-ohci");
}
 
 diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
 index d5f1420..4787279 100644
 --- a/hw/pxa2xx.c
 +++ b/hw/pxa2xx.c
 @@ -2108,7 +2108,7 @@ PXA2xxState *pxa270_init(MemoryRegion 

Re: [Qemu-devel] [PATCH v2] register reset handler to write image into memory

[Yin Olivia-R63875]

Thanks to Dunrong and Andreas.

$ scripts/get_maintainer.pl -f hw/loader.c
Alexander Graf  (commit_signer:3/6=50%)
Anthony Liguori  (commit_signer:2/6=33%)
Stefan Weil  (commit_signer:1/6=17%)
Benjamin Herrenschmidt  (commit_signer:1/6=17%)
Avi Kivity  (commit_signer:1/6=17%)

Dear maintainers,
Could you please help review this patch?

So far I got feedback from Andreas and try to answer the question.

> This patch does not answer the question why you try to avoid the ROM blobs
> and what ROM blobs are still being used for after your patch. I don't 
> think it makes much sense to work around them for your use cases and to 
> leave them behind - if there's something fundamentally wrong with them 
> they should be ripped out completely or fixed. But maybe I'm misunderstanding 
> in the absence of explanations?

It's a general problem. 

For example, in my case, there're 3 different files loaded from host rootfs.
$ qemu-system-ppc -enable-kvm -m 256 -nographic -M mpc8544ds -kernel 
uImage.8572.agraf -initrd /media/ram/guest-8572.rootfs.ext2.gz -append 
"root=/dev/ram rw loglevel=7 console=ttyS0,115200" -serial tcp::4445,server 
-net nic

(qemu) info roms
 addr= size=0x782840 mem=ram name="uImage.8572.agraf"
 addr=00c0 size=0x01 mem=ram name="mpc8544ds.dtb"
 addr=0200 size=0x3f922f mem=ram 
name="/media/ram/guest-8572.rootfs.ext2.gz"

The problem is that rom_add_*() mallocs memory for the image, and then 
rom_reset()
copies those images into the guest's memory, but the QEMU memory does not get 
freed.
On a VM reset, the images get recopied from QEMU to guest.

Comparing the memory map of qemu process before and after starting up guest,
we can find that QEMU consumes much memory for those images.

$ diff -urN pmap.pre.log pmap.post.log
--- pmap.pre.log
+++ pmap.post.log
@@ -33,7 +33,14 @@
 0ffee000  8K rwx--  /lib/ld-2.13.so
 1000   3472K r-x--  qemu-system-ppc
 10374000112K rwx--  qemu-system-ppc
-1039   6524K rwx--[ anon ]
+1039   7100K rwx--[ anon ]
 48002000 16K rw---[ anon ]
+48006000  4K -[ anon ]
+48007000   8188K rw---[ anon ]
+48806000  8K rw-s-[ anon ]
+48808000  4K rw---[ anon ]
+48809000 262144K rw---[ anon ]
+58809000   5280K rw---[ anon ]
+5cb98000   7692K rw---[ anon ]
 bf93e000132K rw---[ stack ]
- total14456K
+ total   298352K

Exactly we can re-load them from disk on a reset instead of holding onto the 
images in QEMU's memory.

With this patch, the two big images (uImage and especially initrd) will not be 
loaded into QEMU's memory
(qemu) info roms
 addr=00c0 size=0x01 mem=ram name="mpc8544ds.dtb"

It will save much memory space according to memory map of QEMU process.
# diff -urN pmap.pre.log pmap.post.log
--- pmap.pre.log
+++ pmap.post.log
@@ -33,7 +33,14 @@
 0ffee000  8K rwx--  /lib/ld-2.13.so
 1000   3472K r-x--  qemu-system-ppc
 10374000112K rwx--  qemu-system-ppc
-1039   6524K rwx--[ anon ]
+1039   7036K rwx--[ anon ]
 48002000 16K rw---[ anon ]
+48006000  4K -[ anon ]
+48007000   8188K rw---[ anon ]
+48806000  8K rw-s-[ anon ]
+48808000  4K rw---[ anon ]
+48809000 262144K rw---[ anon ]
+58809000  4K rw---[ anon ]
+58c04000   1204K rw---[ anon ]
 bfb2a000132K rw---[ stack ]
- total14456K
+ total   286524K

This patch changes all the image load process called by load_uimage() and 
load_image_targphys() in platform initialization.

Best Regards,
Olivia

> -Original Message-
> From: Dunrong Huang [mailto:riegama...@gmail.com]
> Sent: Thursday, August 23, 2012 6:44 PM
> To: Yin Olivia-R63875
> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH v2] register reset handler to write
> image into memory
> 
> 2012/8/23 Yin Olivia-R63875 :
> > Dear All,
> >
> > I can't find MAINTAINER of hw/loader.c.
> > Who can help review and apply this patch?
> >
> Please use the script scripts/get_maintainer.pl, like:
> $ scripts/get_maintainer.pl your_patch_file.patch or
> $ scripts/get_maintainer.pl -f hw/loader.c
> > Best Regards,
> > Olivia Yin
> >
> 
> 
> --
> Best Regards,
> 
> Dunrong Huang

Re: [Qemu-devel] [PATCH] hw/pl110: Fix spelling of 'palette'

[Stefan Weil]

Am 26.08.2012 23:30, schrieb Peter Maydell:

Fix the spelling of 'palette' used in various local variables
and structure members.


... and in a comment. See below.



Signed-off-by: Peter Maydell 
---
  hw/pl110.c  | 28 ++--
  hw/pl110_template.h | 22 +++---
  2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/pl110.c b/hw/pl110.c
index f94608c..11037d1 100644
--- a/hw/pl110.c
+++ b/hw/pl110.c
@@ -55,8 +55,8 @@ typedef struct {
  enum pl110_bppmode bpp;
  int invalidate;
  uint32_t mux_ctrl;
-uint32_t pallette[256];
-uint32_t raw_pallette[128];
+uint32_t palette[256];
+uint32_t raw_palette[128];
  qemu_irq irq;
  } pl110_state;
  
@@ -79,8 +79,8 @@ static const VMStateDescription vmstate_pl110 = {

  VMSTATE_INT32(rows, pl110_state),
  VMSTATE_UINT32(bpp, pl110_state),
  VMSTATE_INT32(invalidate, pl110_state),
-VMSTATE_UINT32_ARRAY(pallette, pl110_state, 256),
-VMSTATE_UINT32_ARRAY(raw_pallette, pl110_state, 128),
+VMSTATE_UINT32_ARRAY(palette, pl110_state, 256),
+VMSTATE_UINT32_ARRAY(raw_palette, pl110_state, 128),
  VMSTATE_UINT32_V(mux_ctrl, pl110_state, 2),
  VMSTATE_END_OF_LIST()
  }
@@ -236,7 +236,7 @@ static void pl110_update_display(void *opaque)
 s->upbase, s->cols, s->rows,
 src_width, dest_width, 0,
 s->invalidate,
-   fn, s->pallette,
+   fn, s->palette,
 &first, &last);
  if (first >= 0) {
  dpy_update(s->ds, 0, first, s->cols, last - first + 1);
@@ -253,13 +253,13 @@ static void pl110_invalidate_display(void * opaque)
  }
  }
  
-static void pl110_update_pallette(pl110_state *s, int n)

+static void pl110_update_palette(pl110_state *s, int n)
  {
  int i;
  uint32_t raw;
  unsigned int r, g, b;
  
-raw = s->raw_pallette[n];

+raw = s->raw_palette[n];
  n <<= 1;
  for (i = 0; i < 2; i++) {
  r = (raw & 0x1f) << 3;
@@ -271,17 +271,17 @@ static void pl110_update_pallette(pl110_state *s, int n)
  raw >>= 6;
  switch (ds_get_bits_per_pixel(s->ds)) {
  case 8:
-s->pallette[n] = rgb_to_pixel8(r, g, b);
+s->palette[n] = rgb_to_pixel8(r, g, b);
  break;
  case 15:
-s->pallette[n] = rgb_to_pixel15(r, g, b);
+s->palette[n] = rgb_to_pixel15(r, g, b);
  break;
  case 16:
-s->pallette[n] = rgb_to_pixel16(r, g, b);
+s->palette[n] = rgb_to_pixel16(r, g, b);
  break;
  case 24:
  case 32:
-s->pallette[n] = rgb_to_pixel32(r, g, b);
+s->palette[n] = rgb_to_pixel32(r, g, b);
  break;
  }
  n++;
@@ -314,7 +314,7 @@ static uint64_t pl110_read(void *opaque, target_phys_addr_t 
offset,
  return idregs[s->version][(offset - 0xfe0) >> 2];
  }
  if (offset >= 0x200 && offset < 0x400) {
-return s->raw_pallette[(offset - 0x200) >> 2];
+return s->raw_palette[(offset - 0x200) >> 2];
  }
  switch (offset >> 2) {
  case 0: /* LCDTiming0 */
@@ -366,8 +366,8 @@ static void pl110_write(void *opaque, target_phys_addr_t 
offset,
  if (offset >= 0x200 && offset < 0x400) {
  /* Pallette.  */


What about this one? For V2 of your patch, you may add a

Reviewed-by: Stefan Weil 



  n = (offset - 0x200) >> 2;
-s->raw_pallette[(offset - 0x200) >> 2] = val;
-pl110_update_pallette(s, n);
+s->raw_palette[(offset - 0x200) >> 2] = val;
+pl110_update_palette(s, n);
  return;
  }
  switch (offset >> 2) {
diff --git a/hw/pl110_template.h b/hw/pl110_template.h
index 1dce32a..e738e4a 100644
--- a/hw/pl110_template.h
+++ b/hw/pl110_template.h
@@ -129,14 +129,14 @@ static drawfn glue(pl110_draw_fn_,BITS)[48] =
  
  static void glue(pl110_draw_line1_,NAME)(void *opaque, uint8_t *d, const uint8_t *src, int width, int deststep)

  {
-uint32_t *pallette = opaque;
+uint32_t *palette = opaque;
  uint32_t data;
  while (width > 0) {
  data = *(uint32_t *)src;
  #ifdef SWAP_PIXELS
-#define FN(x, y) COPY_PIXEL(d, pallette[(data >> (y + 7 - (x))) & 1]);
+#define FN(x, y) COPY_PIXEL(d, palette[(data >> (y + 7 - (x))) & 1]);
  #else
-#define FN(x, y) COPY_PIXEL(d, pallette[(data >> ((x) + y)) & 1]);
+#define FN(x, y) COPY_PIXEL(d, palette[(data >> ((x) + y)) & 1]);
  #endif
  #ifdef SWAP_WORDS
  FN_8(24)
@@ -157,14 +157,14 @@ static void glue(pl110_draw_line1_,NAME)(void *opaque, 
uint8_t *d, const uint8_t
  
  static void glue(pl110_draw_line2_,NAME)(void *opaque, uint8_t *d, const uint8_t *src, int width, int deststep)

  {
-uint32_t *pallette = opaque;
+uint32_t *palette = opaque;
  uint

Re: [Qemu-devel] [PATCH 1/4] qxl: create a qxl common struct!

[Gerd Hoffmann]

On 08/24/12 21:14, Erlon Cruz wrote:
> From: Fabiano Fidêncio 
> 
> This commit is creating a QXLDevice struct, entirely based on
> PCIQXLDevice struct, but separating parts that will be shared between
> PCIQXLDevice and VirtIOQXLDevice. All functions were changed to support
> the new QXLDevice struct.

> diff --git a/hw/qxl.h b/hw/qxl.h
> index 172baf6..f25e341 100644
> --- a/hw/qxl.h
> +++ b/hw/qxl.h
> @@ -25,19 +25,44 @@ enum qxl_mode {
>  #define QXL_NUM_DIRTY_RECTS 64
>  
>  typedef struct PCIQXLDevice {
> -PCIDevice  pci;
> -SimpleSpiceDisplay ssd;
> -intid;
> -uint32_t   debug;
> -uint32_t   guestdebug;
> -uint32_t   cmdlog;
> +PCIDevice pci;
> +int   generation;
> +uint32_t  revision;
> +uint32_t  guestdebug;
> +uint32_t  cmdlog;

Please avoid whitespace changes like this which make it harder to see
the actual changes.

> +typedef struct QXLDevice {
> +PCIQXLDevice   pci;

This is wrong, QXLDevice should only carry the shared parts.

Data structures should be this way:

struct QXLDevice {
  /* any shared fields go here */
};

struct PCIQXLDevice {
  PCIDevice pci;/* must be first because of qdev */
  QXLDevice qxl;/* common stuff */
  /* pci-specifiec fields (pci bars etc) go here */
};

struct VirtioQXLDevice {
  VirtIODevice vdev;
  QXLDevice qxl;
  /* virtio-specific fields go here */
};

If some function got a struct QXLDevice passed in and you need access to
PCIQXLDevice or VirtioQXLDevice you can use the container_of macro.

See hw/usb/hcd-ohci.c for an example, ohci emulation exists in pci and
sysbus variants and thus is structed in a simliar way.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/4] qxl: split qxl functions in common and pci files

[Gerd Hoffmann]

On 08/24/12 21:14, Erlon Cruz wrote:
> From: Fabiano Fidêncio 
> 
> This commit splits qxl functions into common functions (located in
> qxl.c) and pci-specific functions (located in qxl-pci.c).
> All prototypes are being kept in qxl.h, as common MACROS and inline
> functions. Moreover, this commit is exposing a lot of APIs, don't know
> if it's the correct way to do it, but it was the only way that we saw to
> do it.

Try enabling rename detection for this one (git format-patch -M).

> diff --git a/hw/qxl.h b/hw/qxl.h
> index f25e341..516e7da 100644
> --- a/hw/qxl.h
> +++ b/hw/qxl.h
> @@ -143,6 +143,44 @@ typedef struct QXLDevice {
>  }   \
>  } while (0)
>  
> +/*
> + * NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
> + * such can be changed by the guest, so to avoid a guest trigerrable
> + * abort we just qxl_set_guest_bug and set the return to NULL. Still
> + * it may happen as a result of emulator bug as well.
> + */

Why these are here and not in qxl-pci.c?

> +void init_qxl_rom(QXLDevice *d);
> +void init_qxl_ram(QXLDevice *d);

Same question.

> +void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info);
> +int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext);
> +int interface_req_cmd_notification(QXLInstance *sin);
> +void interface_release_resource(QXLInstance *sin, struct QXLReleaseInfoExt 
> ext);
> +int interface_get_cursor_command(QXLInstance *sin, struct QXLCommandExt 
> *ext);
> +int interface_req_cursor_notification(QXLInstance *sin);
> +void interface_notify_update(QXLInstance *sin, uint32_t update_id);
> +int interface_flush_resources(QXLInstance *sin);
> +void interface_update_area_complete(QXLInstance *sin, uint32_t surface_id, 
> QXLRect *dirty, uint32_t num_updated_rects);
> +void interface_async_complete(QXLInstance *sin, uint64_t cookie_token);
> +ram_addr_t qxl_rom_size(void);

Same question.

I'd expect at least some of these having a virtio-specific
implementation.  interface_get_command() for example, which gets a qxl
command from the ring ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/4] qxl: introducing virtio-qxl

[Gerd Hoffmann]

  Hi,

> To enable the VirtIOQXL device, use '-virtio-qxl'. Video output will be

Please don't add a new option.  'qemu -vga none -device virtio-qxl'
should work these days.  You could also make virtio-qxl a valid choice
for '-vga' for convenience.

cheers,
  Gerd

Re: [Qemu-devel] Implementing qxl-virtio on QEMU

[Gerd Hoffmann]

On 08/24/12 21:14, Erlon Cruz wrote:
> The following patches makes provides video support to non PCI architectures, 
> please review!

Can you give an overview on the virtio-qxl virtual hardware design?

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v2] register reset handler to write image into memory

[Alexander Graf]

On 26.08.2012, at 20:50, Yin Olivia-R63875  wrote:

> Thanks to Dunrong and Andreas.
> 
> $ scripts/get_maintainer.pl -f hw/loader.c
> Alexander Graf  (commit_signer:3/6=50%)
> Anthony Liguori  (commit_signer:2/6=33%)
> Stefan Weil  (commit_signer:1/6=17%)
> Benjamin Herrenschmidt  (commit_signer:1/6=17%)
> Avi Kivity  (commit_signer:1/6=17%)
> 
> Dear maintainers,
> Could you please help review this patch?
> 
> So far I got feedback from Andreas and try to answer the question.
> 
>> This patch does not answer the question why you try to avoid the ROM blobs
>> and what ROM blobs are still being used for after your patch. I don't 
>> think it makes much sense to work around them for your use cases and to 
>> leave them behind - if there's something fundamentally wrong with them 
>> they should be ripped out completely or fixed. But maybe I'm 
>> misunderstanding 
>> in the absence of explanations?
> 
> It's a general problem. 
> 
> For example, in my case, there're 3 different files loaded from host rootfs.
> $ qemu-system-ppc -enable-kvm -m 256 -nographic -M mpc8544ds -kernel 
> uImage.8572.agraf -initrd /media/ram/guest-8572.rootfs.ext2.gz -append 
> "root=/dev/ram rw loglevel=7 console=ttyS0,115200" -serial tcp::4445,server 
> -net nic
> 
> (qemu) info roms
> addr= size=0x782840 mem=ram name="uImage.8572.agraf"
> addr=00c0 size=0x01 mem=ram name="mpc8544ds.dtb"
> addr=0200 size=0x3f922f mem=ram 
> name="/media/ram/guest-8572.rootfs.ext2.gz"
> 
> The problem is that rom_add_*() mallocs memory for the image, and then 
> rom_reset()
> copies those images into the guest's memory, but the QEMU memory does not get 
> freed.
> On a VM reset, the images get recopied from QEMU to guest.
> 
> Comparing the memory map of qemu process before and after starting up guest,
> we can find that QEMU consumes much memory for those images.
> 
> $ diff -urN pmap.pre.log pmap.post.log
> --- pmap.pre.log
> +++ pmap.post.log
> @@ -33,7 +33,14 @@
> 0ffee000  8K rwx--  /lib/ld-2.13.so
> 1000   3472K r-x--  qemu-system-ppc
> 10374000112K rwx--  qemu-system-ppc
> -1039   6524K rwx--[ anon ]
> +1039   7100K rwx--[ anon ]
> 48002000 16K rw---[ anon ]
> +48006000  4K -[ anon ]
> +48007000   8188K rw---[ anon ]
> +48806000  8K rw-s-[ anon ]
> +48808000  4K rw---[ anon ]
> +48809000 262144K rw---[ anon ]
> +58809000   5280K rw---[ anon ]
> +5cb98000   7692K rw---[ anon ]
> bf93e000132K rw---[ stack ]
> - total14456K
> + total   298352K
> 
> Exactly we can re-load them from disk on a reset instead of holding onto the 
> images in QEMU's memory.
> 
> With this patch, the two big images (uImage and especially initrd) will not 
> be loaded into QEMU's memory
> (qemu) info roms
> addr=00c0 size=0x01 mem=ram name="mpc8544ds.dtb"
> 
> It will save much memory space according to memory map of QEMU process.
> # diff -urN pmap.pre.log pmap.post.log
> --- pmap.pre.log
> +++ pmap.post.log
> @@ -33,7 +33,14 @@
> 0ffee000  8K rwx--  /lib/ld-2.13.so
> 1000   3472K r-x--  qemu-system-ppc
> 10374000112K rwx--  qemu-system-ppc
> -1039   6524K rwx--[ anon ]
> +1039   7036K rwx--[ anon ]
> 48002000 16K rw---[ anon ]
> +48006000  4K -[ anon ]
> +48007000   8188K rw---[ anon ]
> +48806000  8K rw-s-[ anon ]
> +48808000  4K rw---[ anon ]
> +48809000 262144K rw---[ anon ]
> +58809000  4K rw---[ anon ]
> +58c04000   1204K rw---[ anon ]
> bfb2a000132K rw---[ stack ]
> - total14456K
> + total   286524K
> 
> This patch changes all the image load process called by load_uimage() and 
> load_image_targphys() in platform initialization.

This doesn't explain why you leave the old in-RAM code alive though. The only 
reason I can imagine would be to allow for reset to not reload new roms after 
an update.

Anthony, any opinion here? Do we need the keep-in-RAM rom code? Or could we 
just always load rom blobs on demand for everything?


Alex

> 
> Best Regards,
> Olivia
> 
>> -Original Message-
>> From: Dunrong Huang [mailto:riegama...@gmail.com]
>> Sent: Thursday, August 23, 2012 6:44 PM
>> To: Yin Olivia-R63875
>> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org
>> Subject: Re: [Qemu-devel] [PATCH v2] register reset handler to write
>> image into memory
>> 
>> 2012/8/23 Yin Olivia-R63875 :
>>> Dear All,
>>> 
>>> I can't find MAINTAINER of hw/loader.c.
>>> Who can help review and apply this patch?
>>> 
>> Please use the script scripts/get_maintainer.pl, like:
>> $ scripts/get_maintainer.pl your_patch_file.patch or
>> $ scripts/get_maintainer.pl -f hw/loader.c
>>> Best Regards,
>>> Olivia Yin
>>> 
>> 
>> 
>> --
>> Best Regards,
>> 
>> Dunrong Huang
> 
> 

Re: [Qemu-devel] [PATCH for-1.2 0/2] migrate PV EOI MSR

[Jan Kiszka]

On 2012-08-26 17:59, Michael S. Tsirkin wrote:
> It turns out PV EOI gets disabled after migration -
> until next guest reset.
> This is because we are missing code to actually migrate it.
> This patch fixes it up: it does not do anything useful
> without kvm irqchip but applies cleanly to qemu.git
> as well as qemu-kvm.git, so I think it's cleaner
> to apply it in qemu.git to keep diff to minimum.

There is nothing except pci-assign left in qemu-kvm (which will be
posted for upstream in a minute), so you are intuitively doing the right
thing.

Patch 2 looks good to me, see patch 1 for the clean procedure.

Jan




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-1.2 1/2] linux-headers: update asm/kvm_para.h to 3.6

[Jan Kiszka]

On 2012-08-26 17:59, Michael S. Tsirkin wrote:
> Update asm-x96/kvm_para.h to version present in Linux 3.6.

Nope, we have update-linux-headers.sh for this. Just run it again
3.6-rcX, grab the result, and mention the source (release version or
kvm.git hash).

Jan

> This is needed for the new PV EOI feature.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  linux-headers/asm-x86/kvm_para.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/linux-headers/asm-x86/kvm_para.h 
> b/linux-headers/asm-x86/kvm_para.h
> index f2ac46a..a1c3d72 100644
> --- a/linux-headers/asm-x86/kvm_para.h
> +++ b/linux-headers/asm-x86/kvm_para.h
> @@ -22,6 +22,7 @@
>  #define KVM_FEATURE_CLOCKSOURCE23
>  #define KVM_FEATURE_ASYNC_PF 4
>  #define KVM_FEATURE_STEAL_TIME   5
> +#define KVM_FEATURE_PV_EOI   6
>  
>  /* The last 8 bits are used to indicate how to interpret the flags field
>   * in pvclock structure. If no bits are set, all flags are ignored.
> @@ -37,6 +38,7 @@
>  #define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
>  #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
>  #define MSR_KVM_STEAL_TIME  0x4b564d03
> +#define MSR_KVM_PV_EOI_EN  0x4b564d04
>  
>  struct kvm_steal_time {
>   __u64 steal;
> @@ -89,5 +91,10 @@ struct kvm_vcpu_pv_apf_data {
>   __u32 enabled;
>  };
>  
> +#define KVM_PV_EOI_BIT 0
> +#define KVM_PV_EOI_MASK (0x1 << KVM_PV_EOI_BIT)
> +#define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
> +#define KVM_PV_EOI_DISABLED 0x0
> +
>  
>  #endif /* _ASM_X86_KVM_PARA_H */
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 0/4] uq/master: Add classic PCI device assignment

[Jan Kiszka]

I'm proud to present probably the last patch series to merge qemu-kvm
into upstream: This one adds PCI device assignment for x86 using the
classic interface that the KVM model provides. See the last patch for
reasons why we still want this while next-generation device assignment
via VFIO is approaching.

It's been a long journey, but once this is merged, I think we can close
the qemu-kvm chapter. I already did so, all work is based on QEMU now.

Jan Kiszka (4):
  kvm: Introduce kvm_irqchip_update_msi_route
  kvm: Introduce kvm_has_intx_set_mask
  kvm: i386: Add services required for PCI device assignment
  kvm: i386: Add classic PCI device assignment

 hw/kvm/Makefile.objs   |2 +-
 hw/kvm/pci-assign.c| 1929 
 kvm-all.c  |   50 ++
 kvm.h  |2 +
 target-i386/kvm.c  |  141 
 target-i386/kvm_i386.h |   22 +
 6 files changed, 2145 insertions(+), 1 deletions(-)
 create mode 100644 hw/kvm/pci-assign.c

-- 
1.7.3.4

[Qemu-devel] [PATCH 2/4] kvm: Introduce kvm_has_intx_set_mask

[Jan Kiszka]

From: Jan Kiszka 

Will be used by PCI device assignment code.

Signed-off-by: Jan Kiszka 
---
 kvm-all.c |8 
 kvm.h |1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index fd9d9b4..84d4f7f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -88,6 +88,7 @@ struct KVMState
 int pit_state2;
 int xsave, xcrs;
 int many_ioeventfds;
+int intx_set_mask;
 /* The man page (and posix) say ioctl numbers are signed int, but
  * they're not.  Linux, glibc and *BSD all treat ioctl numbers as
  * unsigned, and treating them as signed here can break things */
@@ -1387,6 +1388,8 @@ int kvm_init(void)
 s->irq_set_ioctl = KVM_IRQ_LINE_STATUS;
 }
 
+s->intx_set_mask = kvm_check_extension(s, KVM_CAP_PCI_2_3);
+
 ret = kvm_arch_init(s);
 if (ret < 0) {
 goto err;
@@ -1739,6 +1742,11 @@ int kvm_has_gsi_routing(void)
 #endif
 }
 
+int kvm_has_intx_set_mask(void)
+{
+return kvm_state->intx_set_mask;
+}
+
 void *kvm_vmalloc(ram_addr_t size)
 {
 #ifdef TARGET_S390X
diff --git a/kvm.h b/kvm.h
index 5cefe3a..dea2998 100644
--- a/kvm.h
+++ b/kvm.h
@@ -117,6 +117,7 @@ int kvm_has_xcrs(void);
 int kvm_has_pit_state2(void);
 int kvm_has_many_ioeventfds(void);
 int kvm_has_gsi_routing(void);
+int kvm_has_intx_set_mask(void);
 
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUArchState *env);
-- 
1.7.3.4

[Qemu-devel] [PATCH 1/4] kvm: Introduce kvm_irqchip_update_msi_route

[Jan Kiszka]

From: Jan Kiszka 

This service allows to update an MSI route without releasing/reacquiring
the associated VIRQ. Will be used by PCI device assignment, later on
likely also by virtio/vhost and VFIO.

Signed-off-by: Jan Kiszka 
---
 kvm-all.c |   42 ++
 kvm.h |1 +
 2 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index d4d8a1f..fd9d9b4 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -963,6 +963,30 @@ static void kvm_add_routing_entry(KVMState *s,
 kvm_irqchip_commit_routes(s);
 }
 
+static int kvm_update_routing_entry(KVMState *s,
+struct kvm_irq_routing_entry *new_entry)
+{
+struct kvm_irq_routing_entry *entry;
+int n;
+
+for (n = 0; n < s->irq_routes->nr; n++) {
+entry = &s->irq_routes->entries[n];
+if (entry->gsi != new_entry->gsi) {
+continue;
+}
+
+entry->type = new_entry->type;
+entry->flags = new_entry->flags;
+entry->u = new_entry->u;
+
+kvm_irqchip_commit_routes(s);
+
+return 0;
+}
+
+return -ESRCH;
+}
+
 void kvm_irqchip_add_irq_route(KVMState *s, int irq, int irqchip, int pin)
 {
 struct kvm_irq_routing_entry e;
@@ -1125,6 +1149,24 @@ int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage 
msg)
 return virq;
 }
 
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg)
+{
+struct kvm_irq_routing_entry kroute;
+
+if (!kvm_irqchip_in_kernel()) {
+return -ENOSYS;
+}
+
+kroute.gsi = virq;
+kroute.type = KVM_IRQ_ROUTING_MSI;
+kroute.flags = 0;
+kroute.u.msi.address_lo = (uint32_t)msg.address;
+kroute.u.msi.address_hi = msg.address >> 32;
+kroute.u.msi.data = msg.data;
+
+return kvm_update_routing_entry(s, &kroute);
+}
+
 static int kvm_irqchip_assign_irqfd(KVMState *s, int fd, int virq, bool assign)
 {
 struct kvm_irqfd irqfd = {
diff --git a/kvm.h b/kvm.h
index 37d1f81..5cefe3a 100644
--- a/kvm.h
+++ b/kvm.h
@@ -270,6 +270,7 @@ int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t 
val, bool assign,
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool 
assign);
 
 int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg);
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg);
 void kvm_irqchip_release_virq(KVMState *s, int virq);
 
 int kvm_irqchip_add_irqfd_notifier(KVMState *s, EventNotifier *n, int virq);
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/4] kvm: i386: Add services required for PCI device assignment

[Jan Kiszka]

From: Jan Kiszka 

These helpers abstract the interaction of upcoming pci-assign with the
KVM kernel services. Put them under i386 only as other archs will
implement device pass-through via VFIO and not this classic interface.

Signed-off-by: Jan Kiszka 
---
 target-i386/kvm.c  |  141 
 target-i386/kvm_i386.h |   22 
 2 files changed, 163 insertions(+), 0 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 696b14a..5e2d4f5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -31,6 +31,7 @@
 #include "hw/apic.h"
 #include "ioport.h"
 #include "hyperv.h"
+#include "hw/pci.h"
 
 //#define DEBUG_KVM
 
@@ -2055,3 +2056,143 @@ void kvm_arch_init_irq_routing(KVMState *s)
 kvm_msi_via_irqfd_allowed = true;
 kvm_gsi_routing_allowed = true;
 }
+
+/* Classic KVM device assignment interface. Will remain x86 only. */
+int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
+  uint32_t flags, uint32_t *dev_id)
+{
+struct kvm_assigned_pci_dev dev_data = {
+.segnr = dev_addr->domain,
+.busnr = dev_addr->bus,
+.devfn = PCI_DEVFN(dev_addr->slot, dev_addr->function),
+.flags = flags,
+};
+int ret;
+
+dev_data.assigned_dev_id =
+(dev_addr->domain << 16) | (dev_addr->bus << 8) | dev_data.devfn;
+
+ret = kvm_vm_ioctl(s, KVM_ASSIGN_PCI_DEVICE, &dev_data);
+if (ret < 0) {
+return ret;
+}
+
+*dev_id = dev_data.assigned_dev_id;
+
+return 0;
+}
+
+int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id)
+{
+struct kvm_assigned_pci_dev dev_data = {
+.assigned_dev_id = dev_id,
+};
+
+return kvm_vm_ioctl(s, KVM_DEASSIGN_PCI_DEVICE, &dev_data);
+}
+
+static int kvm_assign_irq_internal(KVMState *s, uint32_t dev_id,
+   uint32_t irq_type, uint32_t guest_irq)
+{
+struct kvm_assigned_irq assigned_irq = {
+.assigned_dev_id = dev_id,
+.guest_irq = guest_irq,
+.flags = irq_type,
+};
+
+if (kvm_check_extension(s, KVM_CAP_ASSIGN_DEV_IRQ)) {
+return kvm_vm_ioctl(s, KVM_ASSIGN_DEV_IRQ, &assigned_irq);
+} else {
+return kvm_vm_ioctl(s, KVM_ASSIGN_IRQ, &assigned_irq);
+}
+}
+
+int kvm_device_intx_assign(KVMState *s, uint32_t dev_id, bool use_host_msi,
+   uint32_t guest_irq)
+{
+uint32_t irq_type = KVM_DEV_IRQ_GUEST_INTX |
+(use_host_msi ? KVM_DEV_IRQ_HOST_MSI : KVM_DEV_IRQ_HOST_INTX);
+
+return kvm_assign_irq_internal(s, dev_id, irq_type, guest_irq);
+}
+
+int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked)
+{
+struct kvm_assigned_pci_dev dev_data = {
+.assigned_dev_id = dev_id,
+.flags = masked ? KVM_DEV_ASSIGN_MASK_INTX : 0,
+};
+
+return kvm_vm_ioctl(s, KVM_ASSIGN_SET_INTX_MASK, &dev_data);
+}
+
+static int kvm_deassign_irq_internal(KVMState *s, uint32_t dev_id,
+ uint32_t type)
+{
+struct kvm_assigned_irq assigned_irq = {
+.assigned_dev_id = dev_id,
+.flags = type,
+};
+
+return kvm_vm_ioctl(s, KVM_DEASSIGN_DEV_IRQ, &assigned_irq);
+}
+
+int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi)
+{
+return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_INTX |
+(use_host_msi ? KVM_DEV_IRQ_HOST_MSI : KVM_DEV_IRQ_HOST_INTX));
+}
+
+int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, int virq)
+{
+return kvm_assign_irq_internal(s, dev_id, KVM_DEV_IRQ_HOST_MSI |
+  KVM_DEV_IRQ_GUEST_MSI, virq);
+}
+
+int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id)
+{
+return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSI |
+KVM_DEV_IRQ_HOST_MSI);
+}
+
+bool kvm_device_msix_supported(KVMState *s)
+{
+/* The kernel lacks a corresponding KVM_CAP, so we probe by calling
+ * KVM_ASSIGN_SET_MSIX_NR with an invalid parameter. */
+return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, NULL) == -EFAULT;
+}
+
+int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
+ uint32_t nr_vectors)
+{
+struct kvm_assigned_msix_nr msix_nr = {
+.assigned_dev_id = dev_id,
+.entry_nr = nr_vectors,
+};
+
+return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
+}
+
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+   int virq)
+{
+struct kvm_assigned_msix_entry msix_entry = {
+.assigned_dev_id = dev_id,
+.gsi = virq,
+.entry = vector,
+};
+
+return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
+}
+
+int kvm_device_msix_assign(KVMState *s, uint32_t dev_id)
+{
+return kvm_assign_irq_internal(s, dev_id, KVM_DEV_IRQ_HOST_MSIX |
+  KVM_DEV_IR