Re: [Qemu-devel] spec, RFC: TLS support for NBD

2014-10-17 Thread Richard W.M. Jones
On Sat, Oct 18, 2014 at 12:03:23AM +0200, Wouter Verhelst wrote:
> Hi all,
> 
> (added rjones from nbdkit fame -- hi there)

[I'm happy to implement whatever you come up with, but I've added
Florian Weimer to CC who is part of Red Hat's product security group]

> So I think the following would make sense to allow TLS in NBD.
> 
> This would extend the newstyle negotiation by adding two options (i.e.,
> client requests), one server reply, and one server error as well as
> extend one existing reply, in the following manner:
> 
> - The two new commands are NBD_OPT_PEEK_EXPORT and NBD_OPT_STARTTLS. The
>   former would be used to verify if the server will do TLS for a given
>   export:
> 
>   C: NBD_OPT_PEEK_EXPORT
>   S: NBD_REP_SERVER, with an extra field after the export name
>  containing flags that describe the export (R/O vs R/W state,
>  whether TLS is allowed and/or required).
>   
>   If the server indicates that TLS is allowed, the client may now issue
>   NBD_OPT_STARTTLS:
> 
>   C: NBD_OPT_STARTTLS
>   S: NBD_REP_STARTTLS # or NBD_REP_ERR_POLICY, if unwilling
>   C: 
> 
>   Once the TLS handshake has completed, negotiation should continue over
>   the secure channel. The client should initiate that by sending an
>   NBD_OPT_* message.
> 
> - The server may reply to any and all negotiation request with
>   NBD_REP_ERR_TLS_REQD if it does not want to do anything without TLS.
>   However, if at least one export is supported without encryption, the
>   server must not in any case use this reply.
> 
> There is no command to "exit" TLS again. I don't think that makes sense,
> but I could be persuaded otherwise with sound technical arguments.
> 
> Thoughts?
> 
> (full spec (with numbers etc) exists as an (uncommitted) diff to
> doc/proto.txt on my laptop, ...)
> 
> -- 
> It is easy to love a country that is famous for chocolate and beer
> 
>   -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26



-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW



[Qemu-devel] [PATCH] vfio: fix adding memory listener to the right address space

2014-10-17 Thread Frank Blaschka
Depending on the device, container->space->as contains the valid AddressSpace.
Using address_space_memory breaks devices sitting behind an iommu (and using
a separate address space).

Signed-off-by: Frank Blaschka 
---
 hw/misc/vfio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index d66f3d2..fcc1958 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -3703,7 +3703,7 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as)
 container->iommu_data.release = vfio_listener_release;
 
 memory_listener_register(&container->iommu_data.type1.listener,
- &address_space_memory);
+ container->space->as);
 
 if (container->iommu_data.type1.error) {
 ret = container->iommu_data.type1.error;
-- 
1.8.5.5




Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Linhaifeng


On 2014/10/17 16:43, zhanghailiang wrote:
> On 2014/10/17 16:27, haifeng@huawei.com wrote:
>> From: linhaifeng 
>>
>> The VM start with share hugepage should close the hugefile fd
>> when exit.Because the hugepage fd may be send to other process
>> e.g vhost-user If qemu not close the fd the other process can
>> not free the hugepage otherwise exit process,this is ugly,so
>> qemu should close all shared fd when exit.
>>
>> Signed-off-by: linhaifeng 
>> ---
>>   exec.c | 12 
>>   vl.c   |  7 +++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/exec.c b/exec.c
>> index 759055d..d120b73 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1535,6 +1535,18 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t 
>> length)
>>   }
>>   }
>>   }
>> +
>> +void qemu_close_all_ram_fd(void)
>> +{
>> +RAMBlock *block;
>> +
>> +qemu_mutex_lock_ramlist();
>> +QTAILQ_FOREACH(block, &ram_list.blocks, next) {
>> +close(block->fd);
>> +}
>> +qemu_mutex_unlock_ramlist();
>> +}
>> +
>>   #endif /* !_WIN32 */
>>
>>   int qemu_get_ram_fd(ram_addr_t addr)
>> diff --git a/vl.c b/vl.c
>> index aee73e1..0b78f3f 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1658,6 +1658,7 @@ static int qemu_shutdown_requested(void)
>>   return r;
>>   }
>>
>> +extern void qemu_close_all_ram_fd(void);
>>   static void qemu_kill_report(void)
>>   {
>>   if (!qtest_driver() && shutdown_signal != -1) {
>> @@ -1671,6 +1672,12 @@ static void qemu_kill_report(void)
>>   fprintf(stderr, " from pid " FMT_pid "\n", shutdown_pid);
>>   }
>>   shutdown_signal = -1;
>> +
>> +/* Close all ram fd when exit. If the ram is shared by othter 
>> process
> 
> s/othter/other/
> 

OK.thank you.

>> + * e.g vhost-user, it can free the hugepage by close fd after qemu 
>> exit,
>> + * otherwise the process have to exit to free hugepage.
>> + */
>> +qemu_close_all_ram_fd();
>>   }
>>   }
>>
>>
> 
> 
> 
> .
> 

-- 
Regards,
Haifeng




Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Linhaifeng


On 2014/10/17 21:26, Daniel P. Berrange wrote:
> On Fri, Oct 17, 2014 at 04:57:27PM +0800, Linhaifeng wrote:
>>
>>
>> On 2014/10/17 16:33, Daniel P. Berrange wrote:
>>> On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
 From: linhaifeng 

 The VM start with share hugepage should close the hugefile fd
 when exit.Because the hugepage fd may be send to other process
 e.g vhost-user If qemu not close the fd the other process can
 not free the hugepage otherwise exit process,this is ugly,so
 qemu should close all shared fd when exit.

 Signed-off-by: linhaifeng 
>>>
>>> Err, all file descriptors are closed automatically when a process
>>> exits. So manually calling close(fd) before exit can't have any
>>> functional effect on a resource leak.
>>>
>>> If QEMU has sent the FD to another process, that process has a
>>> completely separate copy of the FD. Closing the FD in QEMU will
>>> not close the FD in the other process. You need the other process
>>> to exit for the copy to be closed.
>>>
>>> Regards,
>>> Daniel
>>>
>> Hi,daniel
>>
>> QEMU send the fd by unix domain socket.unix domain socket just install the 
>> fd to
>> other process and inc the f_count,if qemu not close the fd the f_count is 
>> not dec.
>> Then the other process even close the fd the hugepage would not freed whise 
>> the other process exit.
> 
> The kernel always closes all FDs when a process exits. So if this FD is
> not being correctly closed then it is a kernel bug. There should never
> be any reason for an application to do close(fd) before exiting.
> 
> Regards,
> Daniel
> 
Hi,daniel

I don't think this is kernel's bug.May be this a problem about usage.
If you open a file you should close it too.

This is <>about how to free resource of file.
http://linux.die.net/man/2/close


I'm trying to describe my problem.

For example, there are 2 VMs run with hugepage and the hugepage only for QEMU 
to use.

Before run VM the meminfo is :
HugePages_Total:4096
HugePages_Free: 4096
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

Run the two VMs.QEMU deal with hugepage as follow steps:
1.open
2.unlink
3.mmap
4.use memory of hugepage.After this step the meminfo is :
HugePages_Total:4096
HugePages_Free:0
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB
5.shutdown VM with signal 15 without close(fd).After this step the meminfo is :
HugePages_Total:4096
HugePages_Free: 4096
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

Yes,it works well,like you said the kernel recycle all resources.

For another example,there are 2 VMs run with hugepage and share the hugepage 
with vapp(a vhost-user application).

Before run VM the meminfo is :
HugePages_Total:4096
HugePages_Free: 4096
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

Run the first VM.QEMU deal with hugepage as follow steps:
1.open
2.unlink
3.mmap
4.use memory of hugepage and send the fd to vapp with unix domain socket.After 
this step the meminfo is:
HugePages_Total:4096
HugePages_Free: 2048
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

Run the second VM.After this step the meminfo is:
HugePages_Total:4096
HugePages_Free:0
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

Then I want to close the first VM and run another VM.After close the first VM 
and close the fd in vapp the meminfo is :
HugePages_Total:4096
HugePages_Free:0
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB

So failed to run the third VM because the first VM have not free the 
hugepage.After apply this patch the meminfo is:
HugePages_Total:4096
HugePages_Free: 2048
HugePages_Rsvd:0
HugePages_Surp:0
Hugepagesize:   2048 kB
So i can run the third VM success.

-- 
Regards,
Haifeng




Re: [Qemu-devel] [PATCH] hw/i386/pc_q35.c: Avoid g_assert_cmpint() as it is not in glib 2.12

2014-10-17 Thread Gonglei
> Subject: [Qemu-devel] [PATCH] hw/i386/pc_q35.c: Avoid g_assert_cmpint() as it
> is not in glib 2.12
> 
> The function g_assert_cmpint() is not in glib 2.12, which is our current
> minimum requirement. Rephrase the recently added assertion to avoid it.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/i386/pc_q35.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Reviewed-by: Gonglei 

Best regards,
-Gonglei





Re: [Qemu-devel] memory hotplug with 2.1.2

2014-10-17 Thread zhanghailiang

Hi,

Have you tried the latest qemu (you can git clone from 
http://git.qemu.org/qemu.git).

Actually, i have posted a patch fc50ff0666315be5120c70ad00cd0b0097484b84
pc-dimm: Don't check dimm->node when there is non-NUMA config

It should not break memory hotplug feature if there is non-NUMA option.

This patch would also allow to use pc-dimm as replacement for initial memory
for non-NUMA configs.

Note: After this patch, the memory hotplug can work normally for Linux 
guest OS
when there is non-NUMA option and NUMA option. But not support Windows 
guest OS
to hotplug memory with no-NUMA config, actully, it's Windows limitation.

Reviewed-By: Igor Mammedov 
Signed-off-by: zhanghailiang 
Acked-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 

I think this will fix your problem, it will support hotplug a pc-dimm without 
configure numa node.

Hope this can help you, Thanks

Best Regards,
zhanghailiang

On 2014/10/18 1:11, Mikhail Sennikovskii wrote:

Hi Andrey, thank you for your answer.

I know I can work around this by specifying a numa node.
My question is though is the current behaviour considered to be a bug, or
not. And if yes, when it is expected to get fixed.

Thanks,
Mikhail

On Fri, Oct 17, 2014 at 5:54 PM, Andrey Korolyov  wrote:


Please try to populate basic (single-node) NUMA topology to workaround
this (or specify numa node for dimm).









[Qemu-devel] spec, RFC: TLS support for NBD

2014-10-17 Thread Wouter Verhelst
Hi all,

(added rjones from nbdkit fame -- hi there)

So I think the following would make sense to allow TLS in NBD.

This would extend the newstyle negotiation by adding two options (i.e.,
client requests), one server reply, and one server error as well as
extend one existing reply, in the following manner:

- The two new commands are NBD_OPT_PEEK_EXPORT and NBD_OPT_STARTTLS. The
  former would be used to verify if the server will do TLS for a given
  export:

  C: NBD_OPT_PEEK_EXPORT
  S: NBD_REP_SERVER, with an extra field after the export name
 containing flags that describe the export (R/O vs R/W state,
 whether TLS is allowed and/or required).
  
  If the server indicates that TLS is allowed, the client may now issue
  NBD_OPT_STARTTLS:

  C: NBD_OPT_STARTTLS
  S: NBD_REP_STARTTLS # or NBD_REP_ERR_POLICY, if unwilling
  C: 

  Once the TLS handshake has completed, negotiation should continue over
  the secure channel. The client should initiate that by sending an
  NBD_OPT_* message.

- The server may reply to any and all negotiation request with
  NBD_REP_ERR_TLS_REQD if it does not want to do anything without TLS.
  However, if at least one export is supported without encryption, the
  server must not in any case use this reply.

There is no command to "exit" TLS again. I don't think that makes sense,
but I could be persuaded otherwise with sound technical arguments.

Thoughts?

(full spec (with numbers etc) exists as an (uncommitted) diff to
doc/proto.txt on my laptop, ...)

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26


signature.asc
Description: Digital signature


Re: [Qemu-devel] [PATCH v6 11/32] target-arm: add CPREG secure state support

2014-10-17 Thread Greg Bellows
I fixed the issue by adding naming and a couple of macros for short-cutting
the name.  Interestingly and somewhat baffling, the only fields that needed
the fix were the bank_fieldoffsets and fieldoffsets.  It appears to be
related to static initialization using these fields as all the CP15
anonymous unions/structs don't throw errors.

Greg

On 17 October 2014 10:20, Greg Bellows  wrote:

> So, I believe I reproduced the issue with gcc-4.4.  4.6 and 4.7 appear to
> work fine.  I tried adding the "-fms-extensions" flag through configure's
> "--extra-cflags" but it dow not appear to work.  Not sure if this is a
> configure/make issue or if anonymous unions/structs are still disallowed.
>
> I'll look into other options in case we deem it important to support older
> compilers.
>
> Greg
>
> On 17 October 2014 08:37, Greg Bellows  wrote:
>
>> Hmmm, I had not encountered this as I am using a new compiler which is
>> presumeably using -std=c11.   Here is what I am using:
>>
>> > gcc --version
>> gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1
>>
>> A quick search on gcc 4.4 shows that a different flag may be
>> needed (-fms-extensions). There is also a special flag for 4.6 apparently.
>>
>> One question I have is how old of a toolchain do we support?  Peter?
>>
>> In the meantime, I'll try and reproduce on my end and try the additional
>> flags.
>>
>> Thanks for pointing this out.
>>
>> Greg
>>
>> On 16 October 2014 20:32, Edgar E. Iglesias 
>> wrote:
>>
>>> On Fri, Oct 10, 2014 at 11:03:22AM -0500, Greg Bellows wrote:
>>> > From: Fabian Aggeler 
>>> >
>>> > Prepare ARMCPRegInfo to support specifying two fieldoffsets per
>>> > register definition. This will allow us to keep one register
>>> > definition for banked registers (different offsets for secure/
>>> > non-secure world).
>>>
>>> Hi Greg,
>>>
>>> I gave the series a try through my auto-tester and it fails on this
>>> patch with gcc-4.4:
>>> $ gcc-4.4 --version
>>> gcc-4.4 (Ubuntu/Linaro 4.4.7-8ubuntu1) 4.4.7
>>>
>>> We might need to pass additional options to gcc for the
>>> anonymous structs/unions or use a different approach.
>>>
>>> Cheers,
>>> Edgar
>>>
>>>
>>>
>>> >
>>> > Also added secure state tracking field and flags.  This allows for
>>> > identification of the register info secure state.
>>> >
>>> > Signed-off-by: Fabian Aggeler 
>>> > Signed-off-by: Greg Bellows 
>>> >
>>> > ==
>>> >
>>> > v5 -> v6
>>> > - Separate out secure CPREG flags
>>> > - Add convenience macro for testing flags
>>> > - Removed extraneous newline
>>> > - Move add_cpreg_to_hashtable() functionality to a later commit for
>>> which it is
>>> >   dependent on.
>>> > - Added comment explaining fieldoffset padding
>>> >
>>> > v4 -> v5
>>> > - Added ARM CP register secure and non-secure bank flags
>>> > - Added setting of secure and non-secure flags furing registration
>>> > ---
>>> >  target-arm/cpu.h | 42 +++---
>>> >  1 file changed, 39 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> > index 59414f3..4d8de9e 100644
>>> > --- a/target-arm/cpu.h
>>> > +++ b/target-arm/cpu.h
>>> > @@ -985,6 +985,24 @@ enum {
>>> >  ARM_CP_STATE_BOTH = 2,
>>> >  };
>>> >
>>> > +/* ARM CP register secure state flags.  These flags identify security
>>> state
>>> > + * attributes for a given CP register entry.
>>> > + * The existence of both or neither secure and non-secure flags
>>> indicates that
>>> > + * the register has both a secure and non-secure hash entry.  A
>>> single one of
>>> > + * these flags causes the register to only be hashed for the specified
>>> > + * security state.
>>> > + * Although definitions may have any combination of the S/NS bits,
>>> each
>>> > + * registered entry will only have one to identify whether the entry
>>> is secure
>>> > + * or non-secure.
>>> > + */
>>> > +enum {
>>> > +ARM_CP_SECSTATE_S =   (1 << 0), /* bit[0]: Secure state register
>>> */
>>> > +ARM_CP_SECSTATE_NS =  (1 << 1), /* bit[1]: Non-secure state
>>> register */
>>> > +};
>>> > +
>>> > +/* Convenience macro for checking for a specific bit */
>>> > +#define ARM_CP_SECSTATE_TEST(_ri, _flag) (((_ri)->secure & (_flag))
>>> == (_flag))
>>> > +
>>> >  /* Return true if cptype is a valid type field. This is used to try to
>>> >   * catch errors where the sentinel has been accidentally left off the
>>> end
>>> >   * of a list of registers.
>>> > @@ -1119,6 +1137,8 @@ struct ARMCPRegInfo {
>>> >  int type;
>>> >  /* Access rights: PL*_[RW] */
>>> >  int access;
>>> > +/* Security state: ARM_CP_SECSTATE_* bits/values */
>>> > +int secure;
>>> >  /* The opaque pointer passed to define_arm_cp_regs_with_opaque()
>>> when
>>> >   * this register was defined: can be used to hand data through to
>>> the
>>> >   * register read/write functions, since they are passed the
>>> ARMCPRegInfo*.
>>> > @@ -1128,12 +1148,28 @@ struct ARMCPRegInfo {
>>> >   * fieldoffset is non-zero, the reset 

Re: [Qemu-devel] Crashes of qemu-system-mips64 and qemu-system-mips64el

2014-10-17 Thread Aurelien Jarno
On Fri, Oct 17, 2014 at 08:57:38PM +0200, Torbjörn Granlund wrote:
> Aurelien Jarno  writes:
>   
>   I am using 2.1.2 under GNU/Linux.
>   
> Ah, so you're not trying to reproduce the problem!

I do. Well you talked about 2.1.0, the latest stable one is 2.1.2. Now
if you prefer, we can conclude the problem is solved in 2.1.2.

> Are you passing the -cpu 5Kc argument?

I used:

qemu-system-mips64 -M malta -cpu 5Kc -m 256 \
-drive file=disk.img,if=virtio,index=0 \
-net nic,macaddr=52:54:00:13:06:64 -net user,hostfwd=tcp::20008-:22 \
-kernel boot/vmlinux-3.2.0-4-5kc-malta \
-initrd boot/initrd.img-3.2.0-4-5kc-malta \
-append "root=/dev/vda1 console=ttyS0" \
-nographic -serial null -monitor null

And it's still running the testsuite in a loop.

>   I don't think it's irrelevant, that's why I asked. If you don't provide
>   this information, we won't be able to check which code paths are
>   involved
>   
>   Given you are the only one to reproduce the issue, please provide a
>   backtrace of a crash so that we can proceed further.
> 
> Really?  Has anybody tried to reproduce the issue?

At least me, and it doesn't crash here.

> My bug report contains all information for trivially reproducing the
> issue.  I kept the setup around for a long time, but now I have cleaned

Yes, but it doesn't mean it's reproducible.

> it up.  I am very busy in this period and cannot afford to set it up
> again, also considering that the effort on the developer part seems very
> very limited wrt reported problems.  (I am not complaining, I have no
> opinion on how volunteer hackers use their time!)

Ok fine, let's consider the issue fixed. 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v6 00/24] Split BlockBackend off BDS with an axe

2014-10-17 Thread Kevin Wolf
Am 07.10.2014 um 13:59 hat Markus Armbruster geschrieben:
> My last attempt got bogged down because I tried to do a reasonably
> complete job, and the complexity proved more than I could handle with
> the limited amount of uninterrupted time available.  This time, I'm
> cutting BlockBackend off with an axe, leaving most of the work for
> later.
> 
> Done in this series already:
> 
> * Introduce a BlockBackend type, and lift up BlockDriverState's
>   device_name, device_list, dev, dev_ops, dev_opaque.  Much more
>   remains to be lifted.
> 
> * Make BlockBackend own the DriveInfo.
> 
> * Wean hw/ off BlockDriverState, with two small exceptions.
> 
> * Fix blockdev-add not to create a bogus IDE drive (0,0).
> 
> * Take a few baby steps towards use of BlockBackend in monitor command
>   code where appropriate.
> 
> Coming soon, hopefully:
> 
> * QMP command blockdev-del
> 
> * blockdev-add accepts node-name without id at top level
> 
> * Lift up more stuff
> 
> * More BlockBackend use in monitor command code
> 
> I know the diffstat looks intimidating.  I tried very hard to split
> the patches so that the bigger ones do just one simple thing, and
> mostly mechanically.

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] Crashes of qemu-system-mips64 and qemu-system-mips64el

2014-10-17 Thread Torbjörn Granlund
Aurelien Jarno  writes:
  
  I am using 2.1.2 under GNU/Linux.
  
Ah, so you're not trying to reproduce the problem!

Are you passing the -cpu 5Kc argument?

  I don't think it's irrelevant, that's why I asked. If you don't provide
  this information, we won't be able to check which code paths are
  involved
  
  Given you are the only one to reproduce the issue, please provide a
  backtrace of a crash so that we can proceed further.

Really?  Has anybody tried to reproduce the issue?

My bug report contains all information for trivially reproducing the
issue.  I kept the setup around for a long time, but now I have cleaned
it up.  I am very busy in this period and cannot afford to set it up
again, also considering that the effort on the developer part seems very
very limited wrt reported problems.  (I am not complaining, I have no
opinion on how volunteer hackers use their time!)

-- 
Torbjörn
Please encrypt, key id 0xC8601622



[Qemu-devel] [PATCH] hw/i386/pc_q35.c: Avoid g_assert_cmpint() as it is not in glib 2.12

2014-10-17 Thread Peter Maydell
The function g_assert_cmpint() is not in glib 2.12, which is our current
minimum requirement. Rephrase the recently added assertion to avoid it.

Signed-off-by: Peter Maydell 
---
 hw/i386/pc_q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index bb0dc8e..e225c6d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -254,7 +254,7 @@ static void pc_q35_init(MachineState *machine)
true, "ich9-ahci");
 idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0");
 idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1");
-g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports);
+g_assert(MAX_SATA_PORTS == ICH_AHCI(ahci)->ahci.ports);
 ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports);
 ahci_ide_create_devs(ahci, hd);
 
-- 
1.9.1




Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Don Slutz

On 10/17/14 13:41, Stefano Stabellini wrote:

On Fri, 17 Oct 2014, Don Slutz wrote:

This adds synchronisation of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.

This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
fetch and put these 6 vcpu registers used by the code in vmport.c
and vmmouse.c

The registers are passed in the new shared page provided by
HVM_PARAM_VMPORT_IOREQ_PFN.

Add new array to XenIOState that allows selection of current_cpu by
ioreq_id.

Now pass XenIOState to handle_ioreq().

Add new routines regs_to_cpu(), regs_from_cpu(), and
handle_vmport_ioreq().

Signed-off-by: Don Slutz 
---
  include/hw/xen/xen_common.h |  15 ++
  xen-hvm.c   | 121 ++--
  2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 07731b9..9542756 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
  /* shutdown/destroy current domain because of an error */
  void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
  
+#ifdef HVM_PARAM_VMPORT_IOREQ_PFN

+static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
+   unsigned long *vmport_ioreq_pfn)
+{
+return xc_get_hvm_param(xc, dom, HVM_PARAM_VMPORT_IOREQ_PFN,
+vmport_ioreq_pfn);
+}
+#else
+static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
+   unsigned long *vmport_ioreq_pfn)
+{
+return -ENOSYS;
+}
+#endif
+
  #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/xen-hvm.c b/xen-hvm.c
index 05e522c..17b3cbd 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
  static bool xen_in_migration;
  
  /* Compatibility with older version */

+
+/* This allows QEMU to build on a system that has Xen 4.5 or earlier
+ * installed.  This here (not in hw/xen/xen_common.h) because xen/hvm/ioreq.h
+ * needs to be included before this block and hw/xen/xen_common.h needs to
+ * be included before xen/hvm/ioreq.h
+ */
+#ifndef IOREQ_TYPE_VMWARE_PORT
+#define IOREQ_TYPE_VMWARE_PORT  3
+struct vmware_ioreq {
+uint32_t esi;
+uint32_t edi;
+uint32_t ebx;
+uint32_t ecx;
+uint32_t edx;
+};
+typedef struct vmware_ioreq vmware_ioreq_t;
+
+struct shared_vmport_iopage {
+struct vmware_ioreq vcpu_vmport_ioreq[1];
+};
+typedef struct shared_vmport_iopage shared_vmport_iopage_t;
+#endif
+
  #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
  static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
  {
@@ -79,8 +102,10 @@ typedef struct XenPhysmap {
  
  typedef struct XenIOState {

  shared_iopage_t *shared_page;
+shared_vmport_iopage_t *shared_vmport_page;
  buffered_iopage_t *buffered_io_page;
  QEMUTimer *buffered_io_timer;
+CPUState **cpu_by_ioreq_id;
  /* the evtchn port for polling the notification, */
  evtchn_port_t *ioreq_local_port;
  /* evtchn local port for buffered io */
@@ -101,6 +126,8 @@ typedef struct XenIOState {
  Notifier wakeup;
  } XenIOState;
  
+static void handle_ioreq(XenIOState *state, ioreq_t *req);

+
  /* Xen specific function for piix pci */
  
  int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)

@@ -610,6 +637,20 @@ static ioreq_t 
*cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
  return req;
  }
  
+/* get the vmport ioreq packets from share mem */

+static vmware_ioreq_t *cpu_get_vmport_ioreq_from_shared_memory(
+XenIOState *state, int vcpu)
+{
+vmware_ioreq_t *vmport_req;
+
+assert(state->shared_vmport_page);
+vmport_req = &state->shared_vmport_page->vcpu_vmport_ioreq[vcpu];
+
+xen_rmb(); /* see IOREQ_READY /then/ read contents of ioreq */
+
+return vmport_req;
+}
+
  /* use poll to get the port notification */
  /* ioreq_vec--out,the */
  /* retval--the number of ioreq packet */
@@ -773,7 +814,50 @@ static void cpu_ioreq_move(ioreq_t *req)
  }
  }
  
-static void handle_ioreq(ioreq_t *req)

+static void regs_to_cpu(XenIOState *state, vmware_ioreq_t *vmport_req,
+ioreq_t *req)
+{
+X86CPU *cpu;
+CPUX86State *env;
+
+current_cpu = state->cpu_by_ioreq_id[state->send_vcpu];
+cpu = X86_CPU(current_cpu);
+env = &cpu->env;
+env->regs[R_EAX] = req->data;
+env->regs[R_EBX] = vmport_req->ebx;
+env->regs[R_ECX] = vmport_req->ecx;
+env->regs[R_EDX] = vmport_req->edx;
+env->regs[R_ESI] = vmport_req->esi;
+env->regs[R_EDI] = vmport_req->edi;
+}
+
+static void regs_from_cpu(XenIOState *state, vmware_ioreq_t *vmport_req,
+  ioreq_t *req)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+CPUX86State *env = &cpu->env;
+
+assert(sizeof(*vmport_req) <= sizeof(*req));
+
+vmport_req->ebx = env->regs[R_EBX];
+

Re: [Qemu-devel] [PATCH v6 16/24] hw: Convert from BlockDriverState to BlockBackend, mostly

2014-10-17 Thread Kevin Wolf
Am 07.10.2014 um 13:59 hat Markus Armbruster geschrieben:
> Device models should access their block backends only through the
> block-backend.h API.  Convert them, and drop direct includes of
> inappropriate headers.
> 
> Just four uses of BlockDriverState are left:
> 
> * The Xen paravirtual block device backend (xen_disk.c) opens images
>   itself when set up via xenbus, bypassing blockdev.c.  I figure it
>   should go through qmp_blockdev_add() instead.
> 
> * Device model "usb-storage" prompts for keys.  No other device model
>   does, and this one probably shouldn't do it, either.
> 
> * ide_issue_trim_cb() uses bdrv_aio_discard() instead of
>   blk_aio_discard() because it fishes its backend out of a BlockAIOCB,
>   which has only the BlockDriverState.
> 
> * PC87312State has an unused BlockDriverState[] member.
> 
> The next two commits take care of the latter two.
> 
> Signed-off-by: Markus Armbruster 

> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 198da2e..8b3f352 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -37,6 +37,7 @@ typedef struct HCIInfo HCIInfo;
>  typedef struct AudioState AudioState;
>  typedef struct BlockBackend BlockBackend;
>  typedef struct BlockDriverState BlockDriverState;
> +typedef struct BlockBackend BlockBackend;
>  typedef struct DriveInfo DriveInfo;
>  typedef struct DisplayState DisplayState;
>  typedef struct DisplayChangeListener DisplayChangeListener;

This is a duplicate typedef (the first definition is even in the context
of this hunk) and causes the build to fail on RHEL 6. I can drop the
hunk while applying if you don't object.

Kevin



Re: [Qemu-devel] Crashes of qemu-system-mips64 and qemu-system-mips64el

2014-10-17 Thread Aurelien Jarno
On Fri, Oct 17, 2014 at 03:53:35PM +0200, Torbjörn Granlund wrote:
> Aurelien Jarno  writes:
> 
>   I have installation running in qemu-system-mips with weeks of uptime
>   without any problem. I have however tried the above with QEMU 2.1, and
>   I have been unable to reproduce the issue.
>   
> I have never had these problems with qemu-system-mips. They only happen
> with a 64-bit kernel on qemu-system-mips64 or qemu-system-mips64el.

Sorry I meant qemu-system-mips64.

>   It's now running for more than 48 hours, and hasn't crashed yet.
>   
> Which version of qemu did you use for this experiment?  Please confirm
> that you used a qemu in the range I claim crashes quickly.

I am using 2.1.2 under GNU/Linux.

> This crashes for me with qemu 1.7.x through 2.1.0 under GNU/Linux and
> FreeBSD.  Since my reports about this issue have not until now sparked
> visible activity, I haven't tested any newer qemu release.  (I have
> tried every qemu release in that range but not all (qemu release) x (OS
> flavour) combinations.)
> 
> (I have a mips64 and a mips64el system running under qemu 1.6.2; the
> current uptime is 52 days.  These systems are used for large daily
> tests, thus are quite stable.)
> 
>   Could you give us more details about your host, especially if it is a
>   32-bit or a 64-bit one? Also a cat /proc/cpuinfo would be useful as some
>   instructions are enabled or not depending on the host support.
> 
> All my hosts are 64-bit x86.
> 
> I don't have the broken stuff set up any longer (2.5 month later) but
> since my many attempts over the years trying to get a qemu post 1.6.x to
> work I on many different pieces of hardware, the exact x86 hardware is
> almost certainly irrelevant.  My hardware platforms are quite diverse.

I don't think it's irrelevant, that's why I asked. If you don't provide
this information, we won't be able to check which code paths are
involved

Given you are the only one to reproduce the issue, please provide a
backtrace of a crash so that we can proceed further.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PULL 13/23] q35/ahci: Pick up -cdrom and -hda options

2014-10-17 Thread Peter Maydell
On 17 October 2014 19:02, Peter Maydell  wrote:
> On 17 October 2014 18:57, Michael Tokarev  wrote:
>> On 10/17/2014 08:53 PM, Peter Maydell wrote:
>>> g_assert_cmpint() was only added in glib 2.16, so this won't
>>> build on glib 2.12. A compat fudge in glib-compat.h should
>>> be easy, though.
>>
>> Or just change this to regular in-line comparison and g_assert().
>
> There are a ton of uses in the tests/ directory as well, though.
> It would be nice if we could "make check" RHEL5 as well as just
> building it...

...however, all the g_test_* APIs were only added in glib 2.12,
so we're never going to be able to get 'make check' running there.
Let's just fix this one instance so our build works (or will work
when the spapr hashtable issue is fixed too).

-- PMM



Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Stefano Stabellini
On Fri, 17 Oct 2014, Don Slutz wrote:
> This adds synchronisation of the 6 vcpu registers (only 32bits of
> them) that vmport.c needs between Xen and QEMU.
> 
> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> fetch and put these 6 vcpu registers used by the code in vmport.c
> and vmmouse.c
> 
> The registers are passed in the new shared page provided by
> HVM_PARAM_VMPORT_IOREQ_PFN.
> 
> Add new array to XenIOState that allows selection of current_cpu by
> ioreq_id.
> 
> Now pass XenIOState to handle_ioreq().
> 
> Add new routines regs_to_cpu(), regs_from_cpu(), and
> handle_vmport_ioreq().
> 
> Signed-off-by: Don Slutz 
> ---
>  include/hw/xen/xen_common.h |  15 ++
>  xen-hvm.c   | 121 
> ++--
>  2 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 07731b9..9542756 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
>  /* shutdown/destroy current domain because of an error */
>  void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  
> +#ifdef HVM_PARAM_VMPORT_IOREQ_PFN
> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> +   unsigned long *vmport_ioreq_pfn)
> +{
> +return xc_get_hvm_param(xc, dom, HVM_PARAM_VMPORT_IOREQ_PFN,
> +vmport_ioreq_pfn);
> +}
> +#else
> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> +   unsigned long *vmport_ioreq_pfn)
> +{
> +return -ENOSYS;
> +}
> +#endif
> +
>  #endif /* QEMU_HW_XEN_COMMON_H */
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 05e522c..17b3cbd 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
>  static bool xen_in_migration;
>  
>  /* Compatibility with older version */
> +
> +/* This allows QEMU to build on a system that has Xen 4.5 or earlier
> + * installed.  This here (not in hw/xen/xen_common.h) because xen/hvm/ioreq.h
> + * needs to be included before this block and hw/xen/xen_common.h needs to
> + * be included before xen/hvm/ioreq.h
> + */
> +#ifndef IOREQ_TYPE_VMWARE_PORT
> +#define IOREQ_TYPE_VMWARE_PORT  3
> +struct vmware_ioreq {
> +uint32_t esi;
> +uint32_t edi;
> +uint32_t ebx;
> +uint32_t ecx;
> +uint32_t edx;
> +};
> +typedef struct vmware_ioreq vmware_ioreq_t;
> +
> +struct shared_vmport_iopage {
> +struct vmware_ioreq vcpu_vmport_ioreq[1];
> +};
> +typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> +#endif
> +
>  #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
>  static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
>  {
> @@ -79,8 +102,10 @@ typedef struct XenPhysmap {
>  
>  typedef struct XenIOState {
>  shared_iopage_t *shared_page;
> +shared_vmport_iopage_t *shared_vmport_page;
>  buffered_iopage_t *buffered_io_page;
>  QEMUTimer *buffered_io_timer;
> +CPUState **cpu_by_ioreq_id;
>  /* the evtchn port for polling the notification, */
>  evtchn_port_t *ioreq_local_port;
>  /* evtchn local port for buffered io */
> @@ -101,6 +126,8 @@ typedef struct XenIOState {
>  Notifier wakeup;
>  } XenIOState;
>  
> +static void handle_ioreq(XenIOState *state, ioreq_t *req);
> +
>  /* Xen specific function for piix pci */
>  
>  int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> @@ -610,6 +637,20 @@ static ioreq_t 
> *cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
>  return req;
>  }
>  
> +/* get the vmport ioreq packets from share mem */
> +static vmware_ioreq_t *cpu_get_vmport_ioreq_from_shared_memory(
> +XenIOState *state, int vcpu)
> +{
> +vmware_ioreq_t *vmport_req;
> +
> +assert(state->shared_vmport_page);
> +vmport_req = &state->shared_vmport_page->vcpu_vmport_ioreq[vcpu];
> +
> +xen_rmb(); /* see IOREQ_READY /then/ read contents of ioreq */
> +
> +return vmport_req;
> +}
> +
>  /* use poll to get the port notification */
>  /* ioreq_vec--out,the */
>  /* retval--the number of ioreq packet */
> @@ -773,7 +814,50 @@ static void cpu_ioreq_move(ioreq_t *req)
>  }
>  }
>  
> -static void handle_ioreq(ioreq_t *req)
> +static void regs_to_cpu(XenIOState *state, vmware_ioreq_t *vmport_req,
> +ioreq_t *req)
> +{
> +X86CPU *cpu;
> +CPUX86State *env;
> +
> +current_cpu = state->cpu_by_ioreq_id[state->send_vcpu];
> +cpu = X86_CPU(current_cpu);
> +env = &cpu->env;
> +env->regs[R_EAX] = req->data;
> +env->regs[R_EBX] = vmport_req->ebx;
> +env->regs[R_ECX] = vmport_req->ecx;
> +env->regs[R_EDX] = vmport_req->edx;
> +env->regs[R_ESI] = vmport_req->esi;
> +env->regs[R_EDI] = vmport_req->edi;
> +}
> +
> +static void regs_from_cpu(XenIOState *state, vmware_ioreq_t *vmport_req,
> +   

Re: [Qemu-devel] [PATCH] glib: add compatibility interface for g_strcmp0()

2014-10-17 Thread Peter Maydell
On 16 October 2014 12:59,   wrote:
> From: Gonglei 
>
> This patch fixes compilation errors when building against glib < 2.16.0
> due to the missing g_strcmp0() function.
>
> Suggested-by: Peter Maydell 
> Signed-off-by: Gonglei 
> ---
>  Because g_strcmp0() was called in three places, I provide
>  a back-compat implementation.

Applied to master, thanks.

-- PMM



Re: [Qemu-devel] memory hotplug with 2.1.2

2014-10-17 Thread Mikhail Sennikovskii
Hi Andrey, thank you for your answer.

I know I can work around this by specifying a numa node.
My question is though is the current behaviour considered to be a bug, or
not. And if yes, when it is expected to get fixed.

Thanks,
Mikhail

On Fri, Oct 17, 2014 at 5:54 PM, Andrey Korolyov  wrote:

> Please try to populate basic (single-node) NUMA topology to workaround
> this (or specify numa node for dimm).
>


Re: [Qemu-devel] [PULL 31/32] spapr_pci: Use XICS interrupt allocator and do not cache interrupts in PHB

2014-10-17 Thread Peter Maydell
On 27 June 2014 13:52, Alexander Graf  wrote:
> From: Alexey Kardashevskiy 
>
> Currently SPAPR PHB keeps track of all allocated MSI (here and below
> MSI stands for both MSI and MSIX) interrupt because
> XICS used to be unable to reuse interrupts. This is a problem for
> dynamic MSI reconfiguration which happens when guest reloads a driver
> or performs PCI hotplug. Another problem is that the existing
> implementation can enable MSI on 32 devices maximum
> (SPAPR_MSIX_MAX_DEVS=32) and there is no good reason for that.
>
> This makes use of new XICS ability to reuse interrupts.
>
> This reorganizes MSI information storage in sPAPRPHBState. Instead of
> static array of 32 descriptors (one per a PCI function), this patch adds
> a GHashTable when @config_addr is a key and (first_irq, num) pair is
> a value. GHashTable can dynamically grow and shrink so the initial limit
> of 32 devices is gone.

> +static void spapr_pci_pre_save(void *opaque)
> +{
> +sPAPRPHBState *sphb = opaque;
> +GHashTableIter iter;
> +gpointer key, value;
> +int i;
> +
> +if (sphb->msi_devs) {
> +g_free(sphb->msi_devs);
> +sphb->msi_devs = NULL;
> +}
> +sphb->msi_devs_num = g_hash_table_size(sphb->msi);
> +if (!sphb->msi_devs_num) {
> +return;
> +}
> +sphb->msi_devs = g_malloc(sphb->msi_devs_num * 
> sizeof(spapr_pci_msi_mig));
> +
> +g_hash_table_iter_init(&iter, sphb->msi);
> +for (i = 0; g_hash_table_iter_next(&iter, &key, &value); ++i) {
> +sphb->msi_devs[i].key = *(uint32_t *) key;
> +sphb->msi_devs[i].value = *(spapr_pci_msi *) value;
> +}
> +}

Hi. I'm afraid this doesn't build under glib 2.12:

/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c: In function
'spapr_pci_pre_save':
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:710: error:
'GHashTableIter' undeclared (first use in this function)
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:710: error:
(Each undeclared identifier is reported only once
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:710: error:
for each function it appears in.)
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:710: error:
expected ';' before 'iter'
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:724: warning:
implicit declaration of function 'g_hash_table_iter_init'
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:724: warning:
nested extern declaration of 'g_hash_table_iter_init'
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:724: error:
'iter' undeclared (first use in this function)
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:725: warning:
implicit declaration of function 'g_hash_table_iter_next'
/home/petmay01/linaro/qemu-for-merges/hw/ppc/spapr_pci.c:725: warning:
nested extern declaration of 'g_hash_table_iter_next'

g_hash_table_iter_init was only added in glib 2.16;
the old style way to do this is using g_hash_table_foreach()
(and a helper function).

thanks
-- PMM



Re: [Qemu-devel] [PULL 13/23] q35/ahci: Pick up -cdrom and -hda options

2014-10-17 Thread Peter Maydell
On 17 October 2014 18:57, Michael Tokarev  wrote:
> On 10/17/2014 08:53 PM, Peter Maydell wrote:
>> g_assert_cmpint() was only added in glib 2.16, so this won't
>> build on glib 2.12. A compat fudge in glib-compat.h should
>> be easy, though.
>
> Or just change this to regular in-line comparison and g_assert().

There are a ton of uses in the tests/ directory as well, though.
It would be nice if we could "make check" RHEL5 as well as just
building it...

(I figured out that it was pretty easy to set up a centos5
schroot on an ubuntu box, so I'm adding this to the set of
things I build test pullreqs with.)

-- PMM



Re: [Qemu-devel] [PULL 13/23] q35/ahci: Pick up -cdrom and -hda options

2014-10-17 Thread Michael Tokarev
On 10/17/2014 08:53 PM, Peter Maydell wrote:
> On 4 October 2014 22:24, Stefan Hajnoczi  wrote:
>> From: John Snow 
>>
>> This patch implements the backend for the Q35 board
>> for us to be able to pick up and use drives defined
>> by the -cdrom, -hda, or -drive if=ide shorthand options.
>>
>> Signed-off-by: John Snow 
>> Reviewed-by: Markus Armbruster 
>> Reviewed-by: Michael S. Tsirkin 
>> Message-id: 1412187569-23452-7-git-send-email-js...@redhat.com
>> Signed-off-by: Stefan Hajnoczi 
> 
>> @@ -253,6 +254,9 @@ static void pc_q35_init(MachineState *machine)
>> true, "ich9-ahci");
>>  idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0");
>>  idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1");
>> +g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports);
> 
> g_assert_cmpint() was only added in glib 2.16, so this won't
> build on glib 2.12. A compat fudge in glib-compat.h should
> be easy, though.

Or just change this to regular in-line comparison and g_assert().

/mjt



Re: [Qemu-devel] [PULL 13/23] q35/ahci: Pick up -cdrom and -hda options

2014-10-17 Thread Peter Maydell
On 4 October 2014 22:24, Stefan Hajnoczi  wrote:
> From: John Snow 
>
> This patch implements the backend for the Q35 board
> for us to be able to pick up and use drives defined
> by the -cdrom, -hda, or -drive if=ide shorthand options.
>
> Signed-off-by: John Snow 
> Reviewed-by: Markus Armbruster 
> Reviewed-by: Michael S. Tsirkin 
> Message-id: 1412187569-23452-7-git-send-email-js...@redhat.com
> Signed-off-by: Stefan Hajnoczi 

> @@ -253,6 +254,9 @@ static void pc_q35_init(MachineState *machine)
> true, "ich9-ahci");
>  idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0");
>  idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1");
> +g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports);

g_assert_cmpint() was only added in glib 2.16, so this won't
build on glib 2.12. A compat fudge in glib-compat.h should
be easy, though.

-- PMM



[Qemu-devel] [PATCH qom v4 12/13] qdev: gpio: Define qdev_pass_gpios()

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Allows a container to take ownership of GPIOs in a contained
device and automatically connect them as GPIOs to the container.

This prepares for deprecation of the SYSBUS IRQ functionality, which
has this feature. We push it up to the device level instead of sysbus
level. There's nothing sysbus specific about passing GPIOs to
containers so its a legitimate device-level generic feature.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 26 ++
 include/hw/qdev-core.h |  3 +++
 2 files changed, 29 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c143cd0..0ff8039 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -446,6 +446,32 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, 
qemu_irq pin)
 qdev_connect_gpio_out_named(dev, NULL, n, pin);
 }
 
+void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
+ const char *name)
+{
+int i;
+NamedGPIOList *ngl = qdev_get_named_gpio_list(dev, name);
+
+for (i = 0; i < ngl->num_in; i++) {
+const char *nm = ngl->name ? ngl->name : "unnamed-gpio-in";
+char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+object_property_add_alias(OBJECT(container), propname,
+  OBJECT(dev), propname,
+  &error_abort);
+}
+for (i = 0; i < ngl->num_out; i++) {
+const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
+char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+object_property_add_alias(OBJECT(container), propname,
+  OBJECT(dev), propname,
+  &error_abort);
+}
+QLIST_REMOVE(ngl, node);
+QLIST_INSERT_HEAD(&container->gpios, ngl, node);
+}
+
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
 BusState *bus;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index eac603b..77b193b 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -289,6 +289,9 @@ void qdev_init_gpio_in_named(DeviceState *dev, 
qemu_irq_handler handler,
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
   const char *name, int n);
 
+void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
+ const char *name);
+
 BusState *qdev_get_parent_bus(DeviceState *dev);
 
 /*** BUS API. ***/
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 13/13] sysbus: Use TYPE_DEVICE GPIO functionality

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Re-implement the Sysbus GPIOs to use the existing TYPE_DEVICE
GPIO named framework. A constant string name is chosen to avoid
conflicts with existing unnamed GPIOs.

This unifies GPIOs are IRQs for sysbus devices and allows removal
of all Sysbus state for GPIOs.

Any existing and future-added functionality for GPIOs is now
also available for sysbus IRQs.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/sysbus.c| 20 +++-
 include/hw/sysbus.h |  7 +++
 2 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 414e2a1..e55c3c1 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -41,11 +41,7 @@ static const TypeInfo system_bus_info = {
 
 void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
 {
-assert(n >= 0 && n < dev->num_irq);
-dev->irqs[n] = NULL;
-if (dev->irqp[n]) {
-*dev->irqp[n] = irq;
-}
+qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
 }
 
 static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
@@ -89,22 +85,13 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, 
hwaddr addr,
 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
-int n;
-
-assert(dev->num_irq < QDEV_MAX_IRQ);
-n = dev->num_irq++;
-dev->irqp[n] = p;
+qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);
 }
 
 /* Pass IRQs from a target device.  */
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target)
 {
-int i;
-assert(dev->num_irq == 0);
-dev->num_irq = target->num_irq;
-for (i = 0; i < dev->num_irq; i++) {
-dev->irqp[i] = target->irqp[i];
-}
+qdev_pass_gpios(DEVICE(target), DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ);
 }
 
 void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
@@ -210,7 +197,6 @@ static void sysbus_dev_print(Monitor *mon, DeviceState 
*dev, int indent)
 hwaddr size;
 int i;
 
-monitor_printf(mon, "%*sirq %d\n", indent, "", s->num_irq);
 for (i = 0; i < s->num_mmio; i++) {
 size = memory_region_size(s->mmio[i].memory);
 monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index 0bb91a8..9fb1782 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -8,7 +8,6 @@
 
 #define QDEV_MAX_MMIO 32
 #define QDEV_MAX_PIO 32
-#define QDEV_MAX_IRQ 512
 
 #define TYPE_SYSTEM_BUS "System"
 #define SYSTEM_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
@@ -33,6 +32,9 @@ typedef struct SysBusDevice SysBusDevice;
  * SysBusDeviceClass is not overriding #DeviceClass.realize, so derived
  * classes overriding it are not required to invoke its implementation.
  */
+
+#define SYSBUS_DEVICE_GPIO_IRQ "sysbus-irq"
+
 typedef struct SysBusDeviceClass {
 /*< private >*/
 DeviceClass parent_class;
@@ -46,9 +48,6 @@ struct SysBusDevice {
 DeviceState parent_obj;
 /*< public >*/
 
-int num_irq;
-qemu_irq irqs[QDEV_MAX_IRQ];
-qemu_irq *irqp[QDEV_MAX_IRQ];
 int num_mmio;
 struct {
 hwaddr addr;
-- 
2.1.0




[Qemu-devel] [PATCH qom v4 11/13] qdev: gpio: Remove qdev_init_gpio_out x1 restriction

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Previously this was restricted to a single call per-dev/per-name. With
the conversion of the GPIO output state to QOM the implementation can
now handle repeated calls. Remove the restriction.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 8550486..c143cd0 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -367,8 +367,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq 
*pins,
 char *propname = g_strdup_printf("%s[*]", name ? name : 
"unnamed-gpio-out");
 
 assert(gpio_list->num_in == 0 || !name);
-assert(gpio_list->num_out == 0);
-gpio_list->num_out = n;
+gpio_list->num_out += n;
 
 for (i = 0; i < n; ++i) {
 memset(&pins[i], 0, sizeof(*pins));
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 10/13] qdev: gpio: delete NamedGPIOList::out

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

All users of GPIO outputs are fully QOMified, using QOM properties to
access the GPIO data. Delete.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 1 -
 include/hw/qdev-core.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2a88768..8550486 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -369,7 +369,6 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq 
*pins,
 assert(gpio_list->num_in == 0 || !name);
 assert(gpio_list->num_out == 0);
 gpio_list->num_out = n;
-gpio_list->out = pins;
 
 for (i = 0; i < n; ++i) {
 memset(&pins[i], 0, sizeof(*pins));
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 31301e5..eac603b 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -137,7 +137,6 @@ struct NamedGPIOList {
 char *name;
 qemu_irq *in;
 int num_in;
-qemu_irq *out;
 int num_out;
 QLIST_ENTRY(NamedGPIOList) node;
 };
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 09/13] irq: Remove qemu_irq_intercept_out

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

No more users left and obsoleted by qdev_intercept_gpio_out.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/irq.c| 6 --
 include/hw/irq.h | 1 -
 2 files changed, 7 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 4a580a2..8a62a36 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -144,12 +144,6 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, 
qemu_irq_handler handler, int n)
 }
 }
 
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int 
n)
-{
-qemu_irq *old_irqs = *gpio_out;
-*gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
-}
-
 static const TypeInfo irq_type_info = {
.name = TYPE_IRQ,
.parent = TYPE_OBJECT,
diff --git a/include/hw/irq.h b/include/hw/irq.h
index 6f874f5..4c4c2ea 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -61,6 +61,5 @@ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n);
 /* For internal use in qtest.  Similar to qemu_irq_split, but operating
on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int 
n);
 
 #endif
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 06/13] qdev: gpio: Re-implement qdev_connect_gpio QOM style

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Re-implement as a link setter. This should allow the QOM framework to
keep track of ref counts properly etc.

We need to add a default parent for the connecting input incase it's
coming from a non-qdev source. We simply parent the IRQ to the machine
in this case.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
v3->v4: add a comment
---
 hw/core/qdev.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2b42d5b..121a40b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -403,10 +403,19 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
  qemu_irq pin)
 {
-NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-assert(n >= 0 && n < gpio_list->num_out);
-gpio_list->out[n] = pin;
+char *propname = g_strdup_printf("%s[%d]",
+ name ? name : "unnamed-gpio-out", n);
+if (pin) {
+/* We need a name for object_property_set_link to work.  If the
+ * object has a parent, object_property_add_child will come back
+ * with an error without doing anything.  If it has none, it will
+ * never fail.  So we can just call it with a NULL Error pointer.
+ */
+object_property_add_child(qdev_get_machine(), "non-qdev-gpio[*]",
+  OBJECT(pin), NULL);
+}
+object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
+g_free(propname);
 }
 
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 07/13] qdev: gpio: Add API for intercepting a GPIO

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

To replace the old qemu_irq intercept API (which had users reaching
into qdev private state for GPIOs).

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 25 +
 include/hw/qdev-core.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 121a40b..2a88768 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -418,6 +418,31 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const 
char *name, int n,
 g_free(propname);
 }
 
+/* disconnect a GPIO ouput, returning the disconnected input (if any) */
+
+static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+   const char *name, int n)
+{
+char *propname = g_strdup_printf("%s[%d]",
+ name ? name : "unnamed-gpio-out", n);
+
+qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
+  NULL);
+if (ret) {
+object_property_set_link(OBJECT(dev), NULL, propname, NULL);
+}
+g_free(propname);
+return ret;
+}
+
+qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
+ const char *name, int n)
+{
+qemu_irq disconnected = qdev_disconnect_gpio_out_named(dev, name, n);
+qdev_connect_gpio_out_named(dev, name, n, icpt);
+return disconnected;
+}
+
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
 {
 qdev_connect_gpio_out_named(dev, NULL, n, pin);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 178fee2..31301e5 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -274,6 +274,8 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const 
char *name, int n);
 void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
  qemu_irq pin);
+qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
+ const char *name, int n);
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 08/13] qtest/irq: Rework IRQ interception

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Change the qtest intercept handler to accept just the individual IRQ
being intercepted as opaque. n is still expected to be correctly set
as for the original intercepted irq. qemu_intercept_irq_in is updated
accordingly.

Then covert the qemu_irq_intercept_out call to use qdev intercept
version. This stops qtest from having to mess with the raw IRQ pointers
(still has to mess with names and counts but a step in the right
direction).

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/irq.c |  2 +-
 qtest.c   | 15 +++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index cffced0..4a580a2 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -140,7 +140,7 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, 
qemu_irq_handler handler, int n)
 for (i = 0; i < n; i++) {
 *old_irqs[i] = *gpio_in[i];
 gpio_in[i]->handler = handler;
-gpio_in[i]->opaque = old_irqs;
+gpio_in[i]->opaque = &old_irqs[i];
 }
 }
 
diff --git a/qtest.c b/qtest.c
index 4b85995..946b560 100644
--- a/qtest.c
+++ b/qtest.c
@@ -201,8 +201,8 @@ static void GCC_FMT_ATTR(2, 3) qtest_send(CharDriverState 
*chr,
 
 static void qtest_irq_handler(void *opaque, int n, int level)
 {
-qemu_irq *old_irqs = opaque;
-qemu_set_irq(old_irqs[n], level);
+qemu_irq old_irq = *(qemu_irq *)opaque;
+qemu_set_irq(old_irq, level);
 
 if (irq_levels[n] != level) {
 CharDriverState *chr = qtest_chr;
@@ -264,8 +264,15 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 continue;
 }
 if (words[0][14] == 'o') {
-qemu_irq_intercept_out(&ngl->out, qtest_irq_handler,
-   ngl->num_out);
+int i;
+for (i = 0; i < ngl->num_out; ++i) {
+qemu_irq *disconnected = g_new0(qemu_irq, 1);
+qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,
+  disconnected, i);
+
+*disconnected = qdev_intercept_gpio_out(dev, icpt,
+ngl->name, i);
+}
 } else {
 qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
   ngl->num_in);
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 00/13] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

2014-10-17 Thread Paolo Bonzini
These are the QOM IRQ patches from Peter Crosthwaite.  I and Alex
made the small changes I requested, so here they are.

We tested them with v3 of the platform bus series.  "-device eTSEC"
works as expected and qom-test's property retrieval loop works fine with
an eTSEC platform device added to the machine.

Andreas, if you want I can send a pull request for this.

Paolo

Peter Crosthwaite (13):
  qdev: gpio: Don't allow name share between I and O
  qdev: gpio: Register GPIO inputs as child objects
  qdev: gpio: Register GPIO outputs as QOM links
  qom: Allow clearing of a Link property
  qom: Demote already-has-a-parent to a regular error
  qdev: gpio: Re-implement qdev_connect_gpio QOM style
  qdev: gpio: Add API for intercepting a GPIO
  qtest/irq: Rework IRQ interception
  irq: Remove qemu_irq_intercept_out
  qdev: gpio: delete NamedGPIOList::out
  qdev: gpio: Remove qdev_init_gpio_out x1 restriction
  qdev: gpio: Define qdev_pass_gpios()
  sysbus: Use TYPE_DEVICE GPIO functionality

 hw/core/irq.c  |  8 +
 hw/core/qdev.c | 95 ++
 hw/core/sysbus.c   | 20 ++-
 include/hw/irq.h   |  1 -
 include/hw/qdev-core.h |  6 +++-
 include/hw/sysbus.h|  7 ++--
 qom/object.c   | 16 ++---
 qtest.c| 15 +---
 8 files changed, 123 insertions(+), 45 deletions(-)
-- 
2.1.0




[Qemu-devel] [PATCH qom v4 03/13] qdev: gpio: Register GPIO outputs as QOM links

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Within the object that contains the GPIO output. This allows for
connecting GPIO outputs via setting of a Link property.

Also clear the link value to zero. This catch-alls the case
where a device improperly inits a gpio_out (malloc instead of
malloc0).

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index a140c79..2b42d5b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -362,12 +362,24 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler 
handler, int n)
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
   const char *name, int n)
 {
+int i;
 NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+char *propname = g_strdup_printf("%s[*]", name ? name : 
"unnamed-gpio-out");
 
 assert(gpio_list->num_in == 0 || !name);
 assert(gpio_list->num_out == 0);
 gpio_list->num_out = n;
 gpio_list->out = pins;
+
+for (i = 0; i < n; ++i) {
+memset(&pins[i], 0, sizeof(*pins));
+object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
+ (Object **)&pins[i],
+ object_property_allow_set_link,
+ OBJ_PROP_LINK_UNREF_ON_RELEASE,
+ &error_abort);
+}
+g_free(propname);
 }
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 05/13] qom: Demote already-has-a-parent to a regular error

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Rather than an abort(). This allows callers to decide whether parenting
an already-parented object is a fatal error condition.

Useful for providing a default value for an object's parent in the case
where you want to set one iff it doesn't already have one.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 qom/object.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 7d617da..04cb6fd 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1079,6 +1079,11 @@ void object_property_add_child(Object *obj, const char 
*name,
 gchar *type;
 ObjectProperty *op;
 
+if (child->parent != NULL) {
+error_setg(errp, "child object is already parented");
+return;
+}
+
 type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
 
 op = object_property_add(obj, name, type, object_get_child_property, NULL,
@@ -1090,7 +1095,6 @@ void object_property_add_child(Object *obj, const char 
*name,
 
 op->resolve = object_resolve_child_property;
 object_ref(child);
-g_assert(child->parent == NULL);
 child->parent = obj;
 
 out:
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 04/13] qom: Allow clearing of a Link property

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

By passing in "" to object_property_set_link.

The lead user of this is the QDEV GPIO framework which will implement
GPIO disconnects via an "unlink".  GPIO disconnection is used by
qtest's irq_intercept_out command.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
v3->v4: pass an empty string, not NULL
---
 qom/object.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index da0919a..7d617da 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -870,9 +870,13 @@ char *object_property_get_str(Object *obj, const char 
*name,
 void object_property_set_link(Object *obj, Object *value,
   const char *name, Error **errp)
 {
-gchar *path = object_get_canonical_path(value);
-object_property_set_str(obj, path, name, errp);
-g_free(path);
+if (value) {
+gchar *path = object_get_canonical_path(value);
+object_property_set_str(obj, path, name, errp);
+g_free(path);
+} else {
+object_property_set_str(obj, "", name, errp);
+}
 }
 
 Object *object_property_get_link(Object *obj, const char *name,
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 01/13] qdev: gpio: Don't allow name share between I and O

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

Only allow a GPIO name to be one or the other. Inputs and outputs are
functionally different and should be in different namespaces. Prepares
support for the QOMification of IRQs as Links or Child objects.

The alternative is to munge names .e.g. with "-in" or "-out" suffixes
when giving QOM names. But that reduces clarity and if there are cases
out there where users want I and O with same name they can manually add
their own suffixes.

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fcb1638..976e208 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -339,6 +339,7 @@ void qdev_init_gpio_in_named(DeviceState *dev, 
qemu_irq_handler handler,
 {
 NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
+assert(gpio_list->num_out == 0 || !name);
 gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
  dev, n);
 gpio_list->num_in += n;
@@ -354,6 +355,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq 
*pins,
 {
 NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
+assert(gpio_list->num_in == 0 || !name);
 assert(gpio_list->num_out == 0);
 gpio_list->num_out = n;
 gpio_list->out = pins;
-- 
2.1.0





[Qemu-devel] [PATCH qom v4 02/13] qdev: gpio: Register GPIO inputs as child objects

2014-10-17 Thread Paolo Bonzini
From: Peter Crosthwaite 

To the device that contains them. This will allow for referencing
a GPIO input from it's canonical path (exciting for dynamic machine
generation!)

Reviewed-by: Alexander Graf 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/core/qdev.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 976e208..a140c79 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -337,11 +337,20 @@ static NamedGPIOList 
*qdev_get_named_gpio_list(DeviceState *dev,
 void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
  const char *name, int n)
 {
+int i;
 NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+char *propname = g_strdup_printf("%s[*]", name ? name : "unnamed-gpio-in");
 
 assert(gpio_list->num_out == 0 || !name);
 gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
  dev, n);
+
+for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
+object_property_add_child(OBJECT(dev), propname,
+  OBJECT(gpio_list->in[i]), &error_abort);
+}
+g_free(propname);
+
 gpio_list->num_in += n;
 }
 
-- 
2.1.0





Re: [Qemu-devel] [PATCH v6 11/32] target-arm: add CPREG secure state support

2014-10-17 Thread Greg Bellows
Good to know.  Thanks Laurent.
On Oct 17, 2014 10:27 AM, "Laurent Desnogues" 
wrote:

> On Fri, Oct 17, 2014 at 5:20 PM, Greg Bellows 
> wrote:
> > So, I believe I reproduced the issue with gcc-4.4.  4.6 and 4.7 appear to
> > work fine.  I tried adding the "-fms-extensions" flag through configure's
> > "--extra-cflags" but it dow not appear to work.  Not sure if this is a
> > configure/make issue or if anonymous unions/structs are still disallowed.
> >
> > I'll look into other options in case we deem it important to support
> older
> > compilers.
>
> gcc 4.4 is what comes with RHEL6/CentOS 6 so IMHO it should be
> supported.
>
>
> Laurent
>
> > Greg
> >
> > On 17 October 2014 08:37, Greg Bellows  wrote:
> >>
> >> Hmmm, I had not encountered this as I am using a new compiler which is
> >> presumeably using -std=c11.   Here is what I am using:
> >>
> >> > gcc --version
> >> gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1
> >>
> >> A quick search on gcc 4.4 shows that a different flag may be needed
> >> (-fms-extensions). There is also a special flag for 4.6 apparently.
> >>
> >> One question I have is how old of a toolchain do we support?  Peter?
> >>
> >> In the meantime, I'll try and reproduce on my end and try the additional
> >> flags.
> >>
> >> Thanks for pointing this out.
> >>
> >> Greg
> >>
> >> On 16 October 2014 20:32, Edgar E. Iglesias 
> >> wrote:
> >>>
> >>> On Fri, Oct 10, 2014 at 11:03:22AM -0500, Greg Bellows wrote:
> >>> > From: Fabian Aggeler 
> >>> >
> >>> > Prepare ARMCPRegInfo to support specifying two fieldoffsets per
> >>> > register definition. This will allow us to keep one register
> >>> > definition for banked registers (different offsets for secure/
> >>> > non-secure world).
> >>>
> >>> Hi Greg,
> >>>
> >>> I gave the series a try through my auto-tester and it fails on this
> >>> patch with gcc-4.4:
> >>> $ gcc-4.4 --version
> >>> gcc-4.4 (Ubuntu/Linaro 4.4.7-8ubuntu1) 4.4.7
> >>>
> >>> We might need to pass additional options to gcc for the
> >>> anonymous structs/unions or use a different approach.
> >>>
> >>> Cheers,
> >>> Edgar
> >>>
> >>>
> >>>
> >>> >
> >>> > Also added secure state tracking field and flags.  This allows for
> >>> > identification of the register info secure state.
> >>> >
> >>> > Signed-off-by: Fabian Aggeler 
> >>> > Signed-off-by: Greg Bellows 
> >>> >
> >>> > ==
> >>> >
> >>> > v5 -> v6
> >>> > - Separate out secure CPREG flags
> >>> > - Add convenience macro for testing flags
> >>> > - Removed extraneous newline
> >>> > - Move add_cpreg_to_hashtable() functionality to a later commit for
> >>> > which it is
> >>> >   dependent on.
> >>> > - Added comment explaining fieldoffset padding
> >>> >
> >>> > v4 -> v5
> >>> > - Added ARM CP register secure and non-secure bank flags
> >>> > - Added setting of secure and non-secure flags furing registration
> >>> > ---
> >>> >  target-arm/cpu.h | 42 +++---
> >>> >  1 file changed, 39 insertions(+), 3 deletions(-)
> >>> >
> >>> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> >>> > index 59414f3..4d8de9e 100644
> >>> > --- a/target-arm/cpu.h
> >>> > +++ b/target-arm/cpu.h
> >>> > @@ -985,6 +985,24 @@ enum {
> >>> >  ARM_CP_STATE_BOTH = 2,
> >>> >  };
> >>> >
> >>> > +/* ARM CP register secure state flags.  These flags identify
> security
> >>> > state
> >>> > + * attributes for a given CP register entry.
> >>> > + * The existence of both or neither secure and non-secure flags
> >>> > indicates that
> >>> > + * the register has both a secure and non-secure hash entry.  A
> single
> >>> > one of
> >>> > + * these flags causes the register to only be hashed for the
> specified
> >>> > + * security state.
> >>> > + * Although definitions may have any combination of the S/NS bits,
> >>> > each
> >>> > + * registered entry will only have one to identify whether the entry
> >>> > is secure
> >>> > + * or non-secure.
> >>> > + */
> >>> > +enum {
> >>> > +ARM_CP_SECSTATE_S =   (1 << 0), /* bit[0]: Secure state register
> >>> > */
> >>> > +ARM_CP_SECSTATE_NS =  (1 << 1), /* bit[1]: Non-secure state
> >>> > register */
> >>> > +};
> >>> > +
> >>> > +/* Convenience macro for checking for a specific bit */
> >>> > +#define ARM_CP_SECSTATE_TEST(_ri, _flag) (((_ri)->secure & (_flag))
> ==
> >>> > (_flag))
> >>> > +
> >>> >  /* Return true if cptype is a valid type field. This is used to try
> to
> >>> >   * catch errors where the sentinel has been accidentally left off
> the
> >>> > end
> >>> >   * of a list of registers.
> >>> > @@ -1119,6 +1137,8 @@ struct ARMCPRegInfo {
> >>> >  int type;
> >>> >  /* Access rights: PL*_[RW] */
> >>> >  int access;
> >>> > +/* Security state: ARM_CP_SECSTATE_* bits/values */
> >>> > +int secure;
> >>> >  /* The opaque pointer passed to define_arm_cp_regs_with_opaque()
> >>> > when
> >>> >   * this register was defined: can be used to hand data through
> to
> >>> > the
> >>> >   * register read/write functions,

Re: [Qemu-devel] memory hotplug with 2.1.2

2014-10-17 Thread Mikhail Sennikovskii
Ah, just noticed there is a bug related to this: 
https://bugzilla.redhat.com/show_bug.cgi?id=1150930


Mikhail


On 17.10.2014 17:38, Mikhail Sennikovskii wrote:

Hi all,

Trying to hotplug RAM with qemu 2.1.2 using

object_add memory-backend-ram,id=ram1,size=128M
device_add pc-dimm,id=dimm1,memdev=ram1

gives the following error:
{{{
'DIMM property node has value 0' which exceeds the number of numa 
nodes: 0

Device 'pc-dimm' could not be initialized
}}}

which goes away if I add a "dummy" numa node via command line at VM 
creation  -numa node,nodeid=0


This behavior was introduced by the commit 
9a72433843d912a45046959b1953861211d1838d, and disabling the check 
makes the hotplug work again for me.


Is this a regression, or just some memory hotplug semantic change? And 
in the latter case, could you specify the proper semantic I should use?


Thanks,
Mikhail






[Qemu-devel] [PATCH v6] Support vhd type VHD_DIFFERENCING

2014-10-17 Thread Xiaodong Gong
Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu
can't read snapshot volume of vhd, and can't support other storage
features of vhd file.

This patch add read parent information in function "vpc_open", read
bitmap in "vpc_read", and change bitmap in "vpc_write".

Signed-off-by: Xiaodong Gong 
---
Changes since v5:
- Change malloc to g_malloc. (Gonglei)
- Fix the bug of free(null). (Gonglei)

Changes since v4:
- Parse the batmap only when the version of VHD > 1.2. (Lucian Petrut)
- Add support to parent location of W2RU. (Lucian Petrut) (Philipp Hahn)

Changes since v3:
- Remove the PARENT_MAX_LOC.

Changes since v2:
- Change MACX to PLATFAORM_MACX. (Kevin Wolf)
- Return with EINVAL to parent location is W2RU and W2KU. (Kevin Wolf)
- Change -1 == ret to a natrual order of ret == -1. (Kevin Wolf)
- Get rid of the get_sector_offset_diff, get_sector_offset
  supports VHD_DIFF. (Kevin Wolf)
- Return code of get_sector_offset is set to, -1 for error,
  -2 for not allocate, -3 for in parent. (Kevin Wolf)
- Fix un init ret of vpc_write, when nb_sector == 0. (Kevin Wolf)
- Change if (diff == ture) to if (diff) and so on. (Kevin Wolf)
- Add PARENT_MAX_LOC to more understand. (Kevin Wolf)
- Restore the boundary check to write on dynamic type in
  get_sector_offset. (Kevin Wolf)

Changes since v1:
- Add Boundary check to any input. (Stefan Hajnoczi)
- Clean the code no used after in vpc_open. (Stefan Hajnoczi)
- Change bdrv_co_readv() to bdrv_preadv in vpc_read. (Stefan Hajnoczi)
- Added some code to make it easy to understand. (Stefan Hajnoczi)
---
 block/vpc.c | 425 ++--
 1 file changed, 355 insertions(+), 70 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index e08144a..15e57c1 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -29,17 +29,27 @@
 #if defined(CONFIG_UUID)
 #include 
 #endif
+#include 
 
 /**/
 
 #define HEADER_SIZE 512
+#define DYNAMIC_HEADER_SIZE 1024
+#define PARENT_LOCATOR_NUM 8
+#define MACX_PREFIX_LEN 7 /* file:// */
+#define TBBATMAP_HEAD_SIZE 28
+
+#define PLATFORM_MACX 0x5863614d /* big endian */
+#define PLATFORM_W2RU 0x75723257
+
+#define VHD_VERSION(major, minor)  (((major) << 16) | ((minor) & 0x))
 
 //#define CACHE
 
 enum vhd_type {
 VHD_FIXED   = 2,
 VHD_DYNAMIC = 3,
-VHD_DIFFERENCING= 4,
+VHD_DIFF= 4,
 };
 
 // Seconds since Jan 1, 2000 0:00:00 (UTC)
@@ -138,6 +148,15 @@ typedef struct BDRVVPCState {
 Error *migration_blocker;
 } BDRVVPCState;
 
+typedef struct vhd_tdbatmap_header {
+char magic[8]; /* always "tdbatmap" */
+
+uint64_t batmap_offset;
+uint32_t batmap_size;
+uint32_t batmap_version;
+uint32_t checksum;
+} QEMU_PACKED VHDTdBatmapHeader;
+
 static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 {
 uint32_t res = 0;
@@ -157,6 +176,102 @@ static int vpc_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 0;
 }
 
+static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header,
+BlockDriverState *bs,
+Error **errp)
+{
+BDRVVPCState *s = bs->opaque;
+int64_t data_offset = 0;
+int data_length = 0;
+uint32_t platform;
+bool done = false;
+int parent_locator_offset = 0;
+int i;
+int ret = 0;
+
+for (i = 0; i < PARENT_LOCATOR_NUM; i++) {
+data_offset =
+be64_to_cpu(dyndisk_header->parent_locator[i].data_offset);
+data_length =
+be32_to_cpu(dyndisk_header->parent_locator[i].data_length);
+platform = dyndisk_header->parent_locator[i].platform;
+
+/* Extend the location offset */
+if (parent_locator_offset < data_offset) {
+parent_locator_offset = data_offset;
+}
+
+if (done) {
+continue;
+}
+
+/* Skip "file://" in MacX platform */
+if (platform == PLATFORM_MACX) {
+data_offset += MACX_PREFIX_LEN;
+data_length -= MACX_PREFIX_LEN;
+}
+
+/* Read location of backing file */
+if (platform == PLATFORM_MACX || platform == PLATFORM_W2RU) {
+if (data_offset > s->max_table_entries * s->block_size) {
+return -1;
+}
+if (data_length > BDRV_SECTOR_SIZE) {
+return -1;
+}
+ret = bdrv_pread(bs->file, data_offset, bs->backing_file,
+data_length);
+if (ret < 0) {
+return ret;
+}
+bs->backing_file[data_length] = '\0';
+}
+
+/* Convert location to ACSII string */
+if (platform == PLATFORM_MACX) {
+done = true;
+
+} else if (platform == PLATFORM_W2RU) {
+/* Must be UTF16-LE to ASCII */
+char *out, *optr;
+int j;
+
+optr = out = (char *) g_malloc(

[Qemu-devel] memory hotplug with 2.1.2

2014-10-17 Thread Mikhail Sennikovskii

Hi all,

Trying to hotplug RAM with qemu 2.1.2 using

object_add memory-backend-ram,id=ram1,size=128M
device_add pc-dimm,id=dimm1,memdev=ram1

gives the following error:
{{{
'DIMM property node has value 0' which exceeds the number of numa nodes: 0
Device 'pc-dimm' could not be initialized
}}}

which goes away if I add a "dummy" numa node via command line at VM 
creation  -numa node,nodeid=0


This behavior was introduced by the commit 
9a72433843d912a45046959b1953861211d1838d, and disabling the check makes 
the hotplug work again for me.


Is this a regression, or just some memory hotplug semantic change? And 
in the latter case, could you specify the proper semantic I should use?


Thanks,
Mikhail




Re: [Qemu-devel] [PATCH v6 11/32] target-arm: add CPREG secure state support

2014-10-17 Thread Laurent Desnogues
On Fri, Oct 17, 2014 at 5:20 PM, Greg Bellows  wrote:
> So, I believe I reproduced the issue with gcc-4.4.  4.6 and 4.7 appear to
> work fine.  I tried adding the "-fms-extensions" flag through configure's
> "--extra-cflags" but it dow not appear to work.  Not sure if this is a
> configure/make issue or if anonymous unions/structs are still disallowed.
>
> I'll look into other options in case we deem it important to support older
> compilers.

gcc 4.4 is what comes with RHEL6/CentOS 6 so IMHO it should be
supported.


Laurent

> Greg
>
> On 17 October 2014 08:37, Greg Bellows  wrote:
>>
>> Hmmm, I had not encountered this as I am using a new compiler which is
>> presumeably using -std=c11.   Here is what I am using:
>>
>> > gcc --version
>> gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1
>>
>> A quick search on gcc 4.4 shows that a different flag may be needed
>> (-fms-extensions). There is also a special flag for 4.6 apparently.
>>
>> One question I have is how old of a toolchain do we support?  Peter?
>>
>> In the meantime, I'll try and reproduce on my end and try the additional
>> flags.
>>
>> Thanks for pointing this out.
>>
>> Greg
>>
>> On 16 October 2014 20:32, Edgar E. Iglesias 
>> wrote:
>>>
>>> On Fri, Oct 10, 2014 at 11:03:22AM -0500, Greg Bellows wrote:
>>> > From: Fabian Aggeler 
>>> >
>>> > Prepare ARMCPRegInfo to support specifying two fieldoffsets per
>>> > register definition. This will allow us to keep one register
>>> > definition for banked registers (different offsets for secure/
>>> > non-secure world).
>>>
>>> Hi Greg,
>>>
>>> I gave the series a try through my auto-tester and it fails on this
>>> patch with gcc-4.4:
>>> $ gcc-4.4 --version
>>> gcc-4.4 (Ubuntu/Linaro 4.4.7-8ubuntu1) 4.4.7
>>>
>>> We might need to pass additional options to gcc for the
>>> anonymous structs/unions or use a different approach.
>>>
>>> Cheers,
>>> Edgar
>>>
>>>
>>>
>>> >
>>> > Also added secure state tracking field and flags.  This allows for
>>> > identification of the register info secure state.
>>> >
>>> > Signed-off-by: Fabian Aggeler 
>>> > Signed-off-by: Greg Bellows 
>>> >
>>> > ==
>>> >
>>> > v5 -> v6
>>> > - Separate out secure CPREG flags
>>> > - Add convenience macro for testing flags
>>> > - Removed extraneous newline
>>> > - Move add_cpreg_to_hashtable() functionality to a later commit for
>>> > which it is
>>> >   dependent on.
>>> > - Added comment explaining fieldoffset padding
>>> >
>>> > v4 -> v5
>>> > - Added ARM CP register secure and non-secure bank flags
>>> > - Added setting of secure and non-secure flags furing registration
>>> > ---
>>> >  target-arm/cpu.h | 42 +++---
>>> >  1 file changed, 39 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> > index 59414f3..4d8de9e 100644
>>> > --- a/target-arm/cpu.h
>>> > +++ b/target-arm/cpu.h
>>> > @@ -985,6 +985,24 @@ enum {
>>> >  ARM_CP_STATE_BOTH = 2,
>>> >  };
>>> >
>>> > +/* ARM CP register secure state flags.  These flags identify security
>>> > state
>>> > + * attributes for a given CP register entry.
>>> > + * The existence of both or neither secure and non-secure flags
>>> > indicates that
>>> > + * the register has both a secure and non-secure hash entry.  A single
>>> > one of
>>> > + * these flags causes the register to only be hashed for the specified
>>> > + * security state.
>>> > + * Although definitions may have any combination of the S/NS bits,
>>> > each
>>> > + * registered entry will only have one to identify whether the entry
>>> > is secure
>>> > + * or non-secure.
>>> > + */
>>> > +enum {
>>> > +ARM_CP_SECSTATE_S =   (1 << 0), /* bit[0]: Secure state register
>>> > */
>>> > +ARM_CP_SECSTATE_NS =  (1 << 1), /* bit[1]: Non-secure state
>>> > register */
>>> > +};
>>> > +
>>> > +/* Convenience macro for checking for a specific bit */
>>> > +#define ARM_CP_SECSTATE_TEST(_ri, _flag) (((_ri)->secure & (_flag)) ==
>>> > (_flag))
>>> > +
>>> >  /* Return true if cptype is a valid type field. This is used to try to
>>> >   * catch errors where the sentinel has been accidentally left off the
>>> > end
>>> >   * of a list of registers.
>>> > @@ -1119,6 +1137,8 @@ struct ARMCPRegInfo {
>>> >  int type;
>>> >  /* Access rights: PL*_[RW] */
>>> >  int access;
>>> > +/* Security state: ARM_CP_SECSTATE_* bits/values */
>>> > +int secure;
>>> >  /* The opaque pointer passed to define_arm_cp_regs_with_opaque()
>>> > when
>>> >   * this register was defined: can be used to hand data through to
>>> > the
>>> >   * register read/write functions, since they are passed the
>>> > ARMCPRegInfo*.
>>> > @@ -1128,12 +1148,28 @@ struct ARMCPRegInfo {
>>> >   * fieldoffset is non-zero, the reset value of the register.
>>> >   */
>>> >  uint64_t resetvalue;
>>> > -/* Offset of the field in CPUARMState for this register. This is
>>> > not
>>> > - * needed if either:
>>> > +/* Offsets of the fields (

Re: [Qemu-devel] [PATCH v6 11/32] target-arm: add CPREG secure state support

2014-10-17 Thread Greg Bellows
So, I believe I reproduced the issue with gcc-4.4.  4.6 and 4.7 appear to
work fine.  I tried adding the "-fms-extensions" flag through configure's
"--extra-cflags" but it dow not appear to work.  Not sure if this is a
configure/make issue or if anonymous unions/structs are still disallowed.

I'll look into other options in case we deem it important to support older
compilers.

Greg

On 17 October 2014 08:37, Greg Bellows  wrote:

> Hmmm, I had not encountered this as I am using a new compiler which is
> presumeably using -std=c11.   Here is what I am using:
>
> > gcc --version
> gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1
>
> A quick search on gcc 4.4 shows that a different flag may be
> needed (-fms-extensions). There is also a special flag for 4.6 apparently.
>
> One question I have is how old of a toolchain do we support?  Peter?
>
> In the meantime, I'll try and reproduce on my end and try the additional
> flags.
>
> Thanks for pointing this out.
>
> Greg
>
> On 16 October 2014 20:32, Edgar E. Iglesias 
> wrote:
>
>> On Fri, Oct 10, 2014 at 11:03:22AM -0500, Greg Bellows wrote:
>> > From: Fabian Aggeler 
>> >
>> > Prepare ARMCPRegInfo to support specifying two fieldoffsets per
>> > register definition. This will allow us to keep one register
>> > definition for banked registers (different offsets for secure/
>> > non-secure world).
>>
>> Hi Greg,
>>
>> I gave the series a try through my auto-tester and it fails on this
>> patch with gcc-4.4:
>> $ gcc-4.4 --version
>> gcc-4.4 (Ubuntu/Linaro 4.4.7-8ubuntu1) 4.4.7
>>
>> We might need to pass additional options to gcc for the
>> anonymous structs/unions or use a different approach.
>>
>> Cheers,
>> Edgar
>>
>>
>>
>> >
>> > Also added secure state tracking field and flags.  This allows for
>> > identification of the register info secure state.
>> >
>> > Signed-off-by: Fabian Aggeler 
>> > Signed-off-by: Greg Bellows 
>> >
>> > ==
>> >
>> > v5 -> v6
>> > - Separate out secure CPREG flags
>> > - Add convenience macro for testing flags
>> > - Removed extraneous newline
>> > - Move add_cpreg_to_hashtable() functionality to a later commit for
>> which it is
>> >   dependent on.
>> > - Added comment explaining fieldoffset padding
>> >
>> > v4 -> v5
>> > - Added ARM CP register secure and non-secure bank flags
>> > - Added setting of secure and non-secure flags furing registration
>> > ---
>> >  target-arm/cpu.h | 42 +++---
>> >  1 file changed, 39 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> > index 59414f3..4d8de9e 100644
>> > --- a/target-arm/cpu.h
>> > +++ b/target-arm/cpu.h
>> > @@ -985,6 +985,24 @@ enum {
>> >  ARM_CP_STATE_BOTH = 2,
>> >  };
>> >
>> > +/* ARM CP register secure state flags.  These flags identify security
>> state
>> > + * attributes for a given CP register entry.
>> > + * The existence of both or neither secure and non-secure flags
>> indicates that
>> > + * the register has both a secure and non-secure hash entry.  A single
>> one of
>> > + * these flags causes the register to only be hashed for the specified
>> > + * security state.
>> > + * Although definitions may have any combination of the S/NS bits, each
>> > + * registered entry will only have one to identify whether the entry
>> is secure
>> > + * or non-secure.
>> > + */
>> > +enum {
>> > +ARM_CP_SECSTATE_S =   (1 << 0), /* bit[0]: Secure state register */
>> > +ARM_CP_SECSTATE_NS =  (1 << 1), /* bit[1]: Non-secure state
>> register */
>> > +};
>> > +
>> > +/* Convenience macro for checking for a specific bit */
>> > +#define ARM_CP_SECSTATE_TEST(_ri, _flag) (((_ri)->secure & (_flag)) ==
>> (_flag))
>> > +
>> >  /* Return true if cptype is a valid type field. This is used to try to
>> >   * catch errors where the sentinel has been accidentally left off the
>> end
>> >   * of a list of registers.
>> > @@ -1119,6 +1137,8 @@ struct ARMCPRegInfo {
>> >  int type;
>> >  /* Access rights: PL*_[RW] */
>> >  int access;
>> > +/* Security state: ARM_CP_SECSTATE_* bits/values */
>> > +int secure;
>> >  /* The opaque pointer passed to define_arm_cp_regs_with_opaque()
>> when
>> >   * this register was defined: can be used to hand data through to
>> the
>> >   * register read/write functions, since they are passed the
>> ARMCPRegInfo*.
>> > @@ -1128,12 +1148,28 @@ struct ARMCPRegInfo {
>> >   * fieldoffset is non-zero, the reset value of the register.
>> >   */
>> >  uint64_t resetvalue;
>> > -/* Offset of the field in CPUARMState for this register. This is
>> not
>> > - * needed if either:
>> > +/* Offsets of the fields (secure/non-secure) in CPUARMState for
>> this
>> > + * register. The array will be accessed by the ns bit which means
>> the
>> > + * secure instance has to be at [0] while the non-secure instance
>> must be
>> > + * at [1]. If a register is not banked .fieldoffset can be used,
>> which maps
>> > + * to 

[Qemu-devel] writing a QEMU block driver

2014-10-17 Thread Sandeep Joshi
Hi there,

Do let me know if I am asking these questions on the wrong forum.  I'd like
to write a QEMU block driver which forwards IO requests to a custom-built
storage cluster.

I have seen Jeff Cody's presentation  and also
browsed the source code for sheepdog, nbd and gluster in the "block"
directory and had a few questions to confirm or correct my understanding.

1) What is the difference between bdrv_open and bdrv_file_open function
pointers in the BlockDriver ?

2) Is it possible to implement only a protocol driver without a format
driver (the distinction that Jeff made in his presentation above) ?  In
other words, can I only set the "protocol_name" and not "format_name" in
BlockDriver ?  I'd like to support all image formats (qemu, raw, etc)
without having to reimplement the logic for each.

3) The control flow for creating a file starts with the image format driver
and later invokes the protocol driver.

image_driver->bdrv_create()
--> bdrv_create_file
  --> bdrv_find_protocol(filename)
  --> bdrv_create
---> Protocol_driver->bdrv_create()

Is this the case for all functions?   Does the read/write first flow
through the image format driver before getting passed down to the protocol
driver (possibly via some coroutine invoked from the block layer or
virtio-blk ) ?  Can someone give me a hint as to how I can trace the
control flow ?

thx
-Sandeep


[Qemu-devel] qemu opengl status

2014-10-17 Thread Stéphane ANCELOT

Hi,
I would like to run OpenGL application in a pentium3 guest (host x86_64).

I would like to know if it is possible.
can you give me directions ?

I understand that altough I have smp host processors, only one cpu will 
emulate this kind of guest ?



Regards,
Steph





Re: [Qemu-devel] [PATCH v2] virtio-pci: fix migration for pci bus master

2014-10-17 Thread Michael S. Tsirkin
On Wed, Oct 15, 2014 at 05:34:25PM +0200, Greg Kurz wrote:
> On Tue, 14 Oct 2014 19:55:23 +0300
> "Michael S. Tsirkin"  wrote:
> > Current support for bus master (clearing OK bit) together with the need to
> > support guests which do not enable PCI bus mastering, leads to extra state 
> > in
> > VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust in case of 
> > cross-version
> > migration for the case when guests use the device before setting DRIVER_OK.
> > 
> > Rip out this code, and replace it:
> > -   Modern QEMU doesn't need VIRTIO_PCI_FLAG_BUS_MASTER_BUG
> > so just drop it for latest machine type.
> > -   For compat machine types, set PCI_COMMAND if DRIVER_OK
> > is set.
> > 
> > As this is needed for 2.1 for both pc and ppc, move PC_COMPAT macros from 
> > pc.h
> > to a new common header.
> > 
> > Cc: Greg Kurz 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> 
> Just a question, see below.
> 
> Apart from that, it looks good.
> 
> Reviewed-by: Greg Kurz 
> Tested-by: Greg Kurz 
> 
> >  hw/virtio/virtio-pci.h |  5 +
> >  include/hw/compat.h| 16 
> >  include/hw/i386/pc.h   | 10 ++
> >  hw/i386/pc_piix.c  |  2 +-
> >  hw/i386/pc_q35.c   |  2 +-
> >  hw/ppc/spapr.c |  9 -
> >  hw/virtio/virtio-pci.c | 29 +++--
> >  7 files changed, 44 insertions(+), 29 deletions(-)
> >  create mode 100644 include/hw/compat.h
> > 
> > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > index 1cea157..8873b6d 100644
> > --- a/hw/virtio/virtio-pci.h
> > +++ b/hw/virtio/virtio-pci.h
> > @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
> >  #define VIRTIO_PCI_BUS_CLASS(klass) \
> >  OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
> > 
> > +/* Need to activate work-arounds for buggy guests at vmstate load. */
> > +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
> > +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
> > +(1 << VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
> > +
> >  /* Performance improves when virtqueue kick processing is decoupled from 
> > the
> >   * vcpu thread using ioeventfd for some devices. */
> >  #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > new file mode 100644
> > index 000..47f6ff5
> > --- /dev/null
> > +++ b/include/hw/compat.h
> > @@ -0,0 +1,16 @@
> > +#ifndef HW_COMPAT_H
> > +#define HW_COMPAT_H
> > +
> > +#define HW_COMPAT_2_1 \
> > +{\
> > +.driver   = "intel-hda",\
> > +.property = "old_msi_addr",\
> > +.value= "on",\
> > +},\
> > +{\
> > +.driver   = "virtio-pci",\
> > +.property = "virtio-pci-bus-master-bug-migration",\
> > +.value= "on",\
> > +}
> > +
> > +#endif /* HW_COMPAT_H */
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index bae023a..82ad046 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -14,6 +14,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "hw/pci/pci.h"
> >  #include "hw/boards.h"
> > +#include "hw/compat.h"
> > 
> >  #define HPET_INTCAP "hpet-intcap"
> > 
> > @@ -307,15 +308,8 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >  int e820_get_num_entries(void);
> >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> > 
> > -#define PC_COMPAT_2_1 \
> > -{\
> > -.driver   = "intel-hda",\
> > -.property = "old_msi_addr",\
> > -.value= "on",\
> > -}
> > -
> >  #define PC_COMPAT_2_0 \
> > -PC_COMPAT_2_1, \
> > +HW_COMPAT_2_1, \
> >  {\
> >  .driver   = "virtio-scsi-pci",\
> >  .property = "any_layout",\
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 553afdd..a1634ab 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -502,7 +502,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = {
> >  .name = "pc-i440fx-2.1",
> >  .init = pc_init_pci,
> >  .compat_props = (GlobalProperty[]) {
> > -PC_COMPAT_2_1,
> > +HW_COMPAT_2_1,
> >  { /* end of list */ }
> >  },
> >  };
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index a199043..f330f7a 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -365,7 +365,7 @@ static QEMUMachine pc_q35_machine_v2_1 = {
> >  .name = "pc-q35-2.1",
> >  .init = pc_q35_init,
> >  .compat_props = (GlobalProperty[]) {
> > -PC_COMPAT_2_1,
> > +HW_COMPAT_2_1,
> >  { /* end of list */ }
> >  },
> >  };
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 2becc9f..8bc597f 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -57,6 +57,8 @@
> >  #include "trace.h"
> >  #include "hw/nmi.h"
> > 
> > +#include "hw/compat.h"
> > +
> >  #include 
> > 
> >  /* SLOF memory layout:
> > @@ -1689,10 +1691,15 @@ static const Ty

Re: [Qemu-devel] Crashes of qemu-system-mips64 and qemu-system-mips64el

2014-10-17 Thread Torbjörn Granlund
Aurelien Jarno  writes:

  I have installation running in qemu-system-mips with weeks of uptime
  without any problem. I have however tried the above with QEMU 2.1, and
  I have been unable to reproduce the issue.
  
I have never had these problems with qemu-system-mips. They only happen
with a 64-bit kernel on qemu-system-mips64 or qemu-system-mips64el.

  It's now running for more than 48 hours, and hasn't crashed yet.
  
Which version of qemu did you use for this experiment?  Please confirm
that you used a qemu in the range I claim crashes quickly.

This crashes for me with qemu 1.7.x through 2.1.0 under GNU/Linux and
FreeBSD.  Since my reports about this issue have not until now sparked
visible activity, I haven't tested any newer qemu release.  (I have
tried every qemu release in that range but not all (qemu release) x (OS
flavour) combinations.)

(I have a mips64 and a mips64el system running under qemu 1.6.2; the
current uptime is 52 days.  These systems are used for large daily
tests, thus are quite stable.)

  Could you give us more details about your host, especially if it is a
  32-bit or a 64-bit one? Also a cat /proc/cpuinfo would be useful as some
  instructions are enabled or not depending on the host support.

All my hosts are 64-bit x86.

I don't have the broken stuff set up any longer (2.5 month later) but
since my many attempts over the years trying to get a qemu post 1.6.x to
work I on many different pieces of hardware, the exact x86 hardware is
almost certainly irrelevant.  My hardware platforms are quite diverse.

-- 
Torbjörn
Please encrypt, key id 0xC8601622



Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Paul Durrant
> -Original Message-
> From: qemu-devel-bounces+paul.durrant=citrix@nongnu.org
> [mailto:qemu-devel-bounces+paul.durrant=citrix@nongnu.org] On
> Behalf Of Paul Durrant
> Sent: 17 October 2014 14:36
> To: Don Slutz; qemu-devel@nongnu.org
> Cc: xen-de...@lists.xensource.com; Marcel Apfelbaum; Michael S. Tsirkin;
> Markus Armbruster; Alexander Graf; Stefano Stabellini; Anthony Liguori;
> Andreas Färber
> Subject: Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen
> access to vmport
> 
> > -Original Message-
> > From: Don Slutz [mailto:dsl...@verizon.com]
> > Sent: 17 October 2014 14:31
> > To: Paul Durrant; Don Slutz; qemu-devel@nongnu.org
> > Cc: xen-de...@lists.xensource.com; Alexander Graf; Andreas Färber;
> > Anthony Liguori; Marcel Apfelbaum; Markus Armbruster; Michael S. Tsirkin;
> > Stefano Stabellini
> > Subject: Re: [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to
> > vmport
> >
> > On 10/17/14 08:34, Paul Durrant wrote:
> > >> -Original Message-
> > >> From: Don Slutz [mailto:dsl...@verizon.com]
> > >> Sent: 17 October 2014 13:14
> > >> To: qemu-devel@nongnu.org; Paul Durrant
> > >> Cc: xen-de...@lists.xensource.com; Alexander Graf; Andreas Färber;
> > >> Anthony Liguori; Don Slutz; Marcel Apfelbaum; Markus Armbruster;
> > Michael
> > >> S. Tsirkin; Stefano Stabellini
> > >> Subject: [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to
> vmport
> > >>
> > >> This adds synchronisation of the 6 vcpu registers (only 32bits of
> > >> them) that vmport.c needs between Xen and QEMU.
> > >>
> > >> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> > >> fetch and put these 6 vcpu registers used by the code in vmport.c
> > >> and vmmouse.c
> > >>
> > >> The registers are passed in the new shared page provided by
> > >> HVM_PARAM_VMPORT_IOREQ_PFN.
> > >>
> > >> Add new array to XenIOState that allows selection of current_cpu by
> > >> ioreq_id.
> > >>
> > >> Now pass XenIOState to handle_ioreq().
> > >>
> > >> Add new routines regs_to_cpu(), regs_from_cpu(), and
> > >> handle_vmport_ioreq().
> > >>
> > >> Signed-off-by: Don Slutz 
> > >> ---
> > >>   include/hw/xen/xen_common.h |  15 ++
> > >>   xen-hvm.c   | 121
> > >> ++--
> > >>   2 files changed, 131 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/include/hw/xen/xen_common.h
> > >> b/include/hw/xen/xen_common.h
> > >> index 07731b9..9542756 100644
> > >> --- a/include/hw/xen/xen_common.h
> > >> +++ b/include/hw/xen/xen_common.h
> > >> @@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
> > >>   /* shutdown/destroy current domain because of an error */
> > >>   void xen_shutdown_fatal_error(const char *fmt, ...)
> GCC_FMT_ATTR(1,
> > 2);
> > >>
> > >> +#ifdef HVM_PARAM_VMPORT_IOREQ_PFN
> > >> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> > >> +   unsigned long 
> > >> *vmport_ioreq_pfn)
> > >> +{
> > >> +return xc_get_hvm_param(xc, dom,
> > >> HVM_PARAM_VMPORT_IOREQ_PFN,
> > >> +vmport_ioreq_pfn);
> > >> +}
> > >> +#else
> > >> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> > >> +   unsigned long 
> > >> *vmport_ioreq_pfn)
> > >> +{
> > >> +return -ENOSYS;
> > >> +}
> > >> +#endif
> > >> +
> > >>   #endif /* QEMU_HW_XEN_COMMON_H */
> > >> diff --git a/xen-hvm.c b/xen-hvm.c
> > >> index 05e522c..17b3cbd 100644
> > >> --- a/xen-hvm.c
> > >> +++ b/xen-hvm.c
> > >> @@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
> > >>   static bool xen_in_migration;
> > >>
> > >>   /* Compatibility with older version */
> > >> +
> > >> +/* This allows QEMU to build on a system that has Xen 4.5 or earlier
> > >> + * installed.  This here (not in hw/xen/xen_common.h) because
> > >> xen/hvm/ioreq.h
> > >> + * needs to be included before this block and hw/xen/xen_common.h
> > >> needs to
> > >> + * be included before xen/hvm/ioreq.h
> > >> + */
> > >> +#ifndef IOREQ_TYPE_VMWARE_PORT
> > >> +#define IOREQ_TYPE_VMWARE_PORT  3
> > >> +struct vmware_ioreq {
> > >> +uint32_t esi;
> > >> +uint32_t edi;
> > >> +uint32_t ebx;
> > >> +uint32_t ecx;
> > >> +uint32_t edx;
> > >> +};
> > >> +typedef struct vmware_ioreq vmware_ioreq_t;
> > > This struct is not really a request any more. Maybe vmware_regs_t?
> >
> > Sure.
> >
> > >> +
> > >> +struct shared_vmport_iopage {
> > >> +struct vmware_ioreq vcpu_vmport_ioreq[1];
> > >> +};
> > >> +typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> > >> +#endif
> > >> +
> > >>   #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
> > >>   static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page,
> > int i)
> > >>   {
> > >> @@ -79,8 +102,10 @@ typedef struct XenPhysmap {
> > >>
> > >>   typedef struct XenIOState {
> > >>   shared_iopage_t *shared_page;
> > >> +shared_vmport_iopage_t *shared_vmport

Re: [Qemu-devel] [RFC 0/7] Optional toplevel sections

2014-10-17 Thread Paolo Bonzini
Il 15/10/2014 17:59, Juan Quintela ha scritto:
> My idea here is that, if you don't use libvirt, you just start without
> -S.

If you don't use libvirt or any other QEMU management layer, you're not
going to do migration except for debugging purposes.  There's just too
much state going on to be able to do it reliably.

> If you use libvirt, and you *don't* need to do anything special to run
> after migration, you shouldn't use -S.

Is this a real requirement, or just "it sounds nicer that way"?  How
much time really passes between the end of migration and the issuing of
the "-cont" command?

And the $1,000,000 questionL.aAre you _absolutely_ sure that an
automatic restart is entirely robust against a failure of the connection
between the two libvirtd instances?  Could you end up with the VM
running on two hosts?  Using -S gets QEMU completely out of the
equation, which is nice.

By the way, some of the states (I can think of io-error, guest-panicked,
watchdog) can be detected on the destination and restored.  Migrating a
machine with io-error state is definitely something that you want to do
no matter what versions of QEMU you have.  It may be the only way to
recover for a network partition like this:

   DISK
  /\
 |  \
 X   |
 |   |
SRC --- DEST

(not impossible: e.g. the SRC->DISK is fibre channel, but the SRC->DEST
link is Ethernet.  Or you have a replicated disk setup, some daemon
fails in SRC's replica but not DEST's).

> And I would emit an event saying
> "migration was finished".

The event should be emitted nevertheless. :)

Paolo



Re: [Qemu-devel] [PATCH v6 11/32] target-arm: add CPREG secure state support

2014-10-17 Thread Greg Bellows
Hmmm, I had not encountered this as I am using a new compiler which is
presumeably using -std=c11.   Here is what I am using:

> gcc --version
gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1

A quick search on gcc 4.4 shows that a different flag may be
needed (-fms-extensions). There is also a special flag for 4.6 apparently.

One question I have is how old of a toolchain do we support?  Peter?

In the meantime, I'll try and reproduce on my end and try the additional
flags.

Thanks for pointing this out.

Greg

On 16 October 2014 20:32, Edgar E. Iglesias 
wrote:

> On Fri, Oct 10, 2014 at 11:03:22AM -0500, Greg Bellows wrote:
> > From: Fabian Aggeler 
> >
> > Prepare ARMCPRegInfo to support specifying two fieldoffsets per
> > register definition. This will allow us to keep one register
> > definition for banked registers (different offsets for secure/
> > non-secure world).
>
> Hi Greg,
>
> I gave the series a try through my auto-tester and it fails on this
> patch with gcc-4.4:
> $ gcc-4.4 --version
> gcc-4.4 (Ubuntu/Linaro 4.4.7-8ubuntu1) 4.4.7
>
> We might need to pass additional options to gcc for the
> anonymous structs/unions or use a different approach.
>
> Cheers,
> Edgar
>
>
>
> >
> > Also added secure state tracking field and flags.  This allows for
> > identification of the register info secure state.
> >
> > Signed-off-by: Fabian Aggeler 
> > Signed-off-by: Greg Bellows 
> >
> > ==
> >
> > v5 -> v6
> > - Separate out secure CPREG flags
> > - Add convenience macro for testing flags
> > - Removed extraneous newline
> > - Move add_cpreg_to_hashtable() functionality to a later commit for
> which it is
> >   dependent on.
> > - Added comment explaining fieldoffset padding
> >
> > v4 -> v5
> > - Added ARM CP register secure and non-secure bank flags
> > - Added setting of secure and non-secure flags furing registration
> > ---
> >  target-arm/cpu.h | 42 +++---
> >  1 file changed, 39 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 59414f3..4d8de9e 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -985,6 +985,24 @@ enum {
> >  ARM_CP_STATE_BOTH = 2,
> >  };
> >
> > +/* ARM CP register secure state flags.  These flags identify security
> state
> > + * attributes for a given CP register entry.
> > + * The existence of both or neither secure and non-secure flags
> indicates that
> > + * the register has both a secure and non-secure hash entry.  A single
> one of
> > + * these flags causes the register to only be hashed for the specified
> > + * security state.
> > + * Although definitions may have any combination of the S/NS bits, each
> > + * registered entry will only have one to identify whether the entry is
> secure
> > + * or non-secure.
> > + */
> > +enum {
> > +ARM_CP_SECSTATE_S =   (1 << 0), /* bit[0]: Secure state register */
> > +ARM_CP_SECSTATE_NS =  (1 << 1), /* bit[1]: Non-secure state
> register */
> > +};
> > +
> > +/* Convenience macro for checking for a specific bit */
> > +#define ARM_CP_SECSTATE_TEST(_ri, _flag) (((_ri)->secure & (_flag)) ==
> (_flag))
> > +
> >  /* Return true if cptype is a valid type field. This is used to try to
> >   * catch errors where the sentinel has been accidentally left off the
> end
> >   * of a list of registers.
> > @@ -1119,6 +1137,8 @@ struct ARMCPRegInfo {
> >  int type;
> >  /* Access rights: PL*_[RW] */
> >  int access;
> > +/* Security state: ARM_CP_SECSTATE_* bits/values */
> > +int secure;
> >  /* The opaque pointer passed to define_arm_cp_regs_with_opaque()
> when
> >   * this register was defined: can be used to hand data through to
> the
> >   * register read/write functions, since they are passed the
> ARMCPRegInfo*.
> > @@ -1128,12 +1148,28 @@ struct ARMCPRegInfo {
> >   * fieldoffset is non-zero, the reset value of the register.
> >   */
> >  uint64_t resetvalue;
> > -/* Offset of the field in CPUARMState for this register. This is not
> > - * needed if either:
> > +/* Offsets of the fields (secure/non-secure) in CPUARMState for this
> > + * register. The array will be accessed by the ns bit which means
> the
> > + * secure instance has to be at [0] while the non-secure instance
> must be
> > + * at [1]. If a register is not banked .fieldoffset can be used,
> which maps
> > + * to the non-secure bank.
> > + *
> > + * Extra padding is added to align the default fieldoffset field
> with the
> > + * non-secure bank_fieldoffsets entry.  This is necessary for
> maintaining
> > + * the same storage offset when AArch64 and banked AArch32 are
> seperately
> > + * defined.
> > + *
> > + * This is not needed if either:
> >   *  1. type is ARM_CP_CONST or one of the ARM_CP_SPECIALs
> >   *  2. both readfn and writefn are specified
> >   */
> > -ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */
> > +union { /* o

Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Paul Durrant
> -Original Message-
> From: Don Slutz [mailto:dsl...@verizon.com]
> Sent: 17 October 2014 14:31
> To: Paul Durrant; Don Slutz; qemu-devel@nongnu.org
> Cc: xen-de...@lists.xensource.com; Alexander Graf; Andreas Färber;
> Anthony Liguori; Marcel Apfelbaum; Markus Armbruster; Michael S. Tsirkin;
> Stefano Stabellini
> Subject: Re: [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to
> vmport
> 
> On 10/17/14 08:34, Paul Durrant wrote:
> >> -Original Message-
> >> From: Don Slutz [mailto:dsl...@verizon.com]
> >> Sent: 17 October 2014 13:14
> >> To: qemu-devel@nongnu.org; Paul Durrant
> >> Cc: xen-de...@lists.xensource.com; Alexander Graf; Andreas Färber;
> >> Anthony Liguori; Don Slutz; Marcel Apfelbaum; Markus Armbruster;
> Michael
> >> S. Tsirkin; Stefano Stabellini
> >> Subject: [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport
> >>
> >> This adds synchronisation of the 6 vcpu registers (only 32bits of
> >> them) that vmport.c needs between Xen and QEMU.
> >>
> >> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> >> fetch and put these 6 vcpu registers used by the code in vmport.c
> >> and vmmouse.c
> >>
> >> The registers are passed in the new shared page provided by
> >> HVM_PARAM_VMPORT_IOREQ_PFN.
> >>
> >> Add new array to XenIOState that allows selection of current_cpu by
> >> ioreq_id.
> >>
> >> Now pass XenIOState to handle_ioreq().
> >>
> >> Add new routines regs_to_cpu(), regs_from_cpu(), and
> >> handle_vmport_ioreq().
> >>
> >> Signed-off-by: Don Slutz 
> >> ---
> >>   include/hw/xen/xen_common.h |  15 ++
> >>   xen-hvm.c   | 121
> >> ++--
> >>   2 files changed, 131 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/hw/xen/xen_common.h
> >> b/include/hw/xen/xen_common.h
> >> index 07731b9..9542756 100644
> >> --- a/include/hw/xen/xen_common.h
> >> +++ b/include/hw/xen/xen_common.h
> >> @@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
> >>   /* shutdown/destroy current domain because of an error */
> >>   void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1,
> 2);
> >>
> >> +#ifdef HVM_PARAM_VMPORT_IOREQ_PFN
> >> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> >> +   unsigned long 
> >> *vmport_ioreq_pfn)
> >> +{
> >> +return xc_get_hvm_param(xc, dom,
> >> HVM_PARAM_VMPORT_IOREQ_PFN,
> >> +vmport_ioreq_pfn);
> >> +}
> >> +#else
> >> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> >> +   unsigned long 
> >> *vmport_ioreq_pfn)
> >> +{
> >> +return -ENOSYS;
> >> +}
> >> +#endif
> >> +
> >>   #endif /* QEMU_HW_XEN_COMMON_H */
> >> diff --git a/xen-hvm.c b/xen-hvm.c
> >> index 05e522c..17b3cbd 100644
> >> --- a/xen-hvm.c
> >> +++ b/xen-hvm.c
> >> @@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
> >>   static bool xen_in_migration;
> >>
> >>   /* Compatibility with older version */
> >> +
> >> +/* This allows QEMU to build on a system that has Xen 4.5 or earlier
> >> + * installed.  This here (not in hw/xen/xen_common.h) because
> >> xen/hvm/ioreq.h
> >> + * needs to be included before this block and hw/xen/xen_common.h
> >> needs to
> >> + * be included before xen/hvm/ioreq.h
> >> + */
> >> +#ifndef IOREQ_TYPE_VMWARE_PORT
> >> +#define IOREQ_TYPE_VMWARE_PORT  3
> >> +struct vmware_ioreq {
> >> +uint32_t esi;
> >> +uint32_t edi;
> >> +uint32_t ebx;
> >> +uint32_t ecx;
> >> +uint32_t edx;
> >> +};
> >> +typedef struct vmware_ioreq vmware_ioreq_t;
> > This struct is not really a request any more. Maybe vmware_regs_t?
> 
> Sure.
> 
> >> +
> >> +struct shared_vmport_iopage {
> >> +struct vmware_ioreq vcpu_vmport_ioreq[1];
> >> +};
> >> +typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> >> +#endif
> >> +
> >>   #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
> >>   static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page,
> int i)
> >>   {
> >> @@ -79,8 +102,10 @@ typedef struct XenPhysmap {
> >>
> >>   typedef struct XenIOState {
> >>   shared_iopage_t *shared_page;
> >> +shared_vmport_iopage_t *shared_vmport_page;
> >>   buffered_iopage_t *buffered_io_page;
> >>   QEMUTimer *buffered_io_timer;
> >> +CPUState **cpu_by_ioreq_id;
> > This name implies the array is indexed by an id carries in the ioreq. That
> doesn't seem to be the case; it's just indexed by guest vcpu id isn't it?
> 
> Yes.  How about cpu_by_vcpu_id ?

Ok.

> 
> >>   /* the evtchn port for polling the notification, */
> >>   evtchn_port_t *ioreq_local_port;
> >>   /* evtchn local port for buffered io */
> >> @@ -101,6 +126,8 @@ typedef struct XenIOState {
> >>   Notifier wakeup;
> >>   } XenIOState;
> >>
> >> +static void handle_ioreq(XenIOState *state, ioreq_t *req);
> >> +
> >>   /* Xen specific function for piix pci */
> >>
> >>   int xen_pci_slot_get_pirq

Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Don Slutz

On 10/17/14 08:34, Paul Durrant wrote:

-Original Message-
From: Don Slutz [mailto:dsl...@verizon.com]
Sent: 17 October 2014 13:14
To: qemu-devel@nongnu.org; Paul Durrant
Cc: xen-de...@lists.xensource.com; Alexander Graf; Andreas Färber;
Anthony Liguori; Don Slutz; Marcel Apfelbaum; Markus Armbruster; Michael
S. Tsirkin; Stefano Stabellini
Subject: [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

This adds synchronisation of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.

This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
fetch and put these 6 vcpu registers used by the code in vmport.c
and vmmouse.c

The registers are passed in the new shared page provided by
HVM_PARAM_VMPORT_IOREQ_PFN.

Add new array to XenIOState that allows selection of current_cpu by
ioreq_id.

Now pass XenIOState to handle_ioreq().

Add new routines regs_to_cpu(), regs_from_cpu(), and
handle_vmport_ioreq().

Signed-off-by: Don Slutz 
---
  include/hw/xen/xen_common.h |  15 ++
  xen-hvm.c   | 121
++--
  2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/include/hw/xen/xen_common.h
b/include/hw/xen/xen_common.h
index 07731b9..9542756 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
  /* shutdown/destroy current domain because of an error */
  void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);

+#ifdef HVM_PARAM_VMPORT_IOREQ_PFN
+static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
+   unsigned long *vmport_ioreq_pfn)
+{
+return xc_get_hvm_param(xc, dom,
HVM_PARAM_VMPORT_IOREQ_PFN,
+vmport_ioreq_pfn);
+}
+#else
+static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
+   unsigned long *vmport_ioreq_pfn)
+{
+return -ENOSYS;
+}
+#endif
+
  #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/xen-hvm.c b/xen-hvm.c
index 05e522c..17b3cbd 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
  static bool xen_in_migration;

  /* Compatibility with older version */
+
+/* This allows QEMU to build on a system that has Xen 4.5 or earlier
+ * installed.  This here (not in hw/xen/xen_common.h) because
xen/hvm/ioreq.h
+ * needs to be included before this block and hw/xen/xen_common.h
needs to
+ * be included before xen/hvm/ioreq.h
+ */
+#ifndef IOREQ_TYPE_VMWARE_PORT
+#define IOREQ_TYPE_VMWARE_PORT  3
+struct vmware_ioreq {
+uint32_t esi;
+uint32_t edi;
+uint32_t ebx;
+uint32_t ecx;
+uint32_t edx;
+};
+typedef struct vmware_ioreq vmware_ioreq_t;

This struct is not really a request any more. Maybe vmware_regs_t?


Sure.


+
+struct shared_vmport_iopage {
+struct vmware_ioreq vcpu_vmport_ioreq[1];
+};
+typedef struct shared_vmport_iopage shared_vmport_iopage_t;
+#endif
+
  #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
  static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
  {
@@ -79,8 +102,10 @@ typedef struct XenPhysmap {

  typedef struct XenIOState {
  shared_iopage_t *shared_page;
+shared_vmport_iopage_t *shared_vmport_page;
  buffered_iopage_t *buffered_io_page;
  QEMUTimer *buffered_io_timer;
+CPUState **cpu_by_ioreq_id;

This name implies the array is indexed by an id carries in the ioreq. That 
doesn't seem to be the case; it's just indexed by guest vcpu id isn't it?


Yes.  How about cpu_by_vcpu_id ?


  /* the evtchn port for polling the notification, */
  evtchn_port_t *ioreq_local_port;
  /* evtchn local port for buffered io */
@@ -101,6 +126,8 @@ typedef struct XenIOState {
  Notifier wakeup;
  } XenIOState;

+static void handle_ioreq(XenIOState *state, ioreq_t *req);
+
  /* Xen specific function for piix pci */

  int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
@@ -610,6 +637,20 @@ static ioreq_t
*cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
  return req;
  }

+/* get the vmport ioreq packets from share mem */
+static vmware_ioreq_t *cpu_get_vmport_ioreq_from_shared_memory(

Is this worth its own function? I can only see one use of it in 
handle_vmport_ioreq below...


Will move it in-line.  I was just following
cpu_get_ioreq_from_shared_memory which also has 1 use.


+XenIOState *state, int vcpu)
+{
+vmware_ioreq_t *vmport_req;
+
+assert(state->shared_vmport_page);
+vmport_req = &state->shared_vmport_page-

vcpu_vmport_ioreq[vcpu];

+
+xen_rmb(); /* see IOREQ_READY /then/ read contents of ioreq */

I don't think that comment is applicable here, and I don't think you need the 
barrier anyway.


I was not sure either way, so went with the safe way.  Will drop.


+
+return vmport_req;
+}
+
  /* use poll to get the port notification */
  /* ioreq_vec--out,the */
  /* retval--the n

Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Daniel P. Berrange
On Fri, Oct 17, 2014 at 04:57:27PM +0800, Linhaifeng wrote:
> 
> 
> On 2014/10/17 16:33, Daniel P. Berrange wrote:
> > On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
> >> From: linhaifeng 
> >>
> >> The VM start with share hugepage should close the hugefile fd
> >> when exit.Because the hugepage fd may be send to other process
> >> e.g vhost-user If qemu not close the fd the other process can
> >> not free the hugepage otherwise exit process,this is ugly,so
> >> qemu should close all shared fd when exit.
> >>
> >> Signed-off-by: linhaifeng 
> > 
> > Err, all file descriptors are closed automatically when a process
> > exits. So manually calling close(fd) before exit can't have any
> > functional effect on a resource leak.
> > 
> > If QEMU has sent the FD to another process, that process has a
> > completely separate copy of the FD. Closing the FD in QEMU will
> > not close the FD in the other process. You need the other process
> > to exit for the copy to be closed.
> > 
> > Regards,
> > Daniel
> > 
> Hi,daniel
> 
> QEMU send the fd by unix domain socket.unix domain socket just install the fd 
> to
> other process and inc the f_count,if qemu not close the fd the f_count is not 
> dec.
> Then the other process even close the fd the hugepage would not freed whise 
> the other process exit.

The kernel always closes all FDs when a process exits. So if this FD is
not being correctly closed then it is a kernel bug. There should never
be any reason for an application to do close(fd) before exiting.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] The status about vhost-net on kvm-arm?

2014-10-17 Thread GAUGUEY Rémy 228890
Thanks for your feedback, 

>static irqreturn_t vm_interrupt(int irq, void *opaque) {
>   ..
>
>   /* Read and acknowledge interrupts */
>   /*status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
>   writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
>
>   if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
>   && vdrv && vdrv->config_changed) {
>   vdrv->config_changed(&vm_dev->vdev);
>   ret = IRQ_HANDLED;
>   }*/
>
>   //if (likely(status & VIRTIO_MMIO_INT_VRING)) {
>   spin_lock_irqsave(&vm_dev->lock, flags);
>   list_for_each_entry(info, &vm_dev->virtqueues, node)
>   ret |= vring_interrupt(irq, info->vq);
>   spin_unlock_irqrestore(&vm_dev->lock, flags);
>   //}
>
>   return ret;
>}
>
>This is very roughly :), and a lot of coding things need to be done.

I agree ;-)
Anyway, with this "workaround" you disable the control plane interrupt, which 
is needed to bring up/down the virtio link... unless VIRTIO_NET_F_STATUS 
feature is off.
I was thinking about connecting those 2 registers to an ioeventfd in order to 
emulate them in Vhost and bypass Qemu... but AFAIK ioeventfd can only work with 
"write" registers.
Any idea for a long term solution ?

best regards.
Rémy

-Message d'origine-
De : Li Liu [mailto:john.li...@huawei.com] 
Envoyé : vendredi 17 octobre 2014 14:27
À : GAUGUEY Rémy 228890; Yingshiuan Pan
Cc : kvm...@lists.cs.columbia.edu; k...@vger.kernel.org; qemu-devel
Objet : Re: [Qemu-devel] The status about vhost-net on kvm-arm?



On 2014/10/15 22:39, GAUGUEY Rémy 228890 wrote:
> Hello,
> 
> Using this Qemu patchset as well as recent irqfd work, I’ve tried to make 
> vhost-net working on Cortex-A15.
> Unfortunately, even if I can correctly generate irqs to the guest through 
> irqfd, it seems to me that some pieces are still missing….
> Indeed, virtio mmio interrupt status register (@ offset 0x60) is not 
> updated by vhost thread, and reading it or writing to the peer 
> interrupt ack register (offset 0x64) from the guest causes an VM exit 
> …
> 

Yeah, you are correct. But it's not far away from success if have injected irqs 
to the guest through irqfd. Do below things to let guest receive packets 
correctly without checking VIRTIO_MMIO_INTERRUPT_STATUS in guest virtio_mmio.c:

static irqreturn_t vm_interrupt(int irq, void *opaque) {
..

/* Read and acknowledge interrupts */
/*status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);

if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
&& vdrv && vdrv->config_changed) {
vdrv->config_changed(&vm_dev->vdev);
ret = IRQ_HANDLED;
}*/

//if (likely(status & VIRTIO_MMIO_INT_VRING)) {
spin_lock_irqsave(&vm_dev->lock, flags);
list_for_each_entry(info, &vm_dev->virtqueues, node)
ret |= vring_interrupt(irq, info->vq);
spin_unlock_irqrestore(&vm_dev->lock, flags);
//}

return ret;
}

This is very roughly :), and a lot of coding things need to be done.

Li.

> After reading older posts, I understand that vhost-net with irqfd support 
> could only work with MSI-X support :
> 
> On 01/20/2011 09:35 AM, Michael S. Tsirkin wrote:
> “When MSI is off, each interrupt needs to be bounced through the io 
> thread when it's set/cleared, so vhost-net causes more context switches and 
> higher CPU utilization than userspace virtio which handles networking in the 
> same thread.
> “
> Indeed, in case of MSI-X support, Virtio spec indicates that the ISR 
> Status field is unused…
> 
> I understand that Vhost does not emulate a complete virtio PCI adapter but 
> only manage virtqueue operations.
> However I don’t have a clear view of what is performed by Qemu and 
> what is performed by vhost-thread… Could someone highlight me on this point, 
> and maybe give some clues for an implementation of Vhost with irqfd and 
> without MSI support ???
> 
> Thanks a lot in advance.
> Best regards.
> Rémy
> 
> 
> 
> De : kvmarm-boun...@lists.cs.columbia.edu 
> [mailto:kvmarm-boun...@lists.cs.columbia.edu] De la part de Yingshiuan 
> Pan Envoyé : vendredi 15 août 2014 09:25 À : Li Liu Cc : 
> kvm...@lists.cs.columbia.edu; k...@vger.kernel.org; qemu-devel Objet : 
> Re: [Qemu-devel] The status about vhost-net on kvm-arm?
> 
> Hi, Li,
> 
> It's ok, I did get those mails from mailing list. I guess it was because I 
> did not subscribe some of mailing lists.
> 
> Currently, I think I will not have any plan to renew my patcheset since I 
> have resigned from my previous company, I do not have Cortex-A15 platform to 
> test/verify.
> 
> I'm fine with that, it would be great if you or someone can take it and 
> improve it.
> Thanks.
> 
> 
> Best Regards,
> Yingshiuan Pan
> 
> 

Re: [Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Paul Durrant
> -Original Message-
> From: Don Slutz [mailto:dsl...@verizon.com]
> Sent: 17 October 2014 13:14
> To: qemu-devel@nongnu.org; Paul Durrant
> Cc: xen-de...@lists.xensource.com; Alexander Graf; Andreas Färber;
> Anthony Liguori; Don Slutz; Marcel Apfelbaum; Markus Armbruster; Michael
> S. Tsirkin; Stefano Stabellini
> Subject: [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport
> 
> This adds synchronisation of the 6 vcpu registers (only 32bits of
> them) that vmport.c needs between Xen and QEMU.
> 
> This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
> fetch and put these 6 vcpu registers used by the code in vmport.c
> and vmmouse.c
> 
> The registers are passed in the new shared page provided by
> HVM_PARAM_VMPORT_IOREQ_PFN.
> 
> Add new array to XenIOState that allows selection of current_cpu by
> ioreq_id.
> 
> Now pass XenIOState to handle_ioreq().
> 
> Add new routines regs_to_cpu(), regs_from_cpu(), and
> handle_vmport_ioreq().
> 
> Signed-off-by: Don Slutz 
> ---
>  include/hw/xen/xen_common.h |  15 ++
>  xen-hvm.c   | 121
> ++--
>  2 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/xen/xen_common.h
> b/include/hw/xen/xen_common.h
> index 07731b9..9542756 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
>  /* shutdown/destroy current domain because of an error */
>  void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> 
> +#ifdef HVM_PARAM_VMPORT_IOREQ_PFN
> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> +   unsigned long *vmport_ioreq_pfn)
> +{
> +return xc_get_hvm_param(xc, dom,
> HVM_PARAM_VMPORT_IOREQ_PFN,
> +vmport_ioreq_pfn);
> +}
> +#else
> +static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
> +   unsigned long *vmport_ioreq_pfn)
> +{
> +return -ENOSYS;
> +}
> +#endif
> +
>  #endif /* QEMU_HW_XEN_COMMON_H */
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 05e522c..17b3cbd 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
>  static bool xen_in_migration;
> 
>  /* Compatibility with older version */
> +
> +/* This allows QEMU to build on a system that has Xen 4.5 or earlier
> + * installed.  This here (not in hw/xen/xen_common.h) because
> xen/hvm/ioreq.h
> + * needs to be included before this block and hw/xen/xen_common.h
> needs to
> + * be included before xen/hvm/ioreq.h
> + */
> +#ifndef IOREQ_TYPE_VMWARE_PORT
> +#define IOREQ_TYPE_VMWARE_PORT  3
> +struct vmware_ioreq {
> +uint32_t esi;
> +uint32_t edi;
> +uint32_t ebx;
> +uint32_t ecx;
> +uint32_t edx;
> +};
> +typedef struct vmware_ioreq vmware_ioreq_t;

This struct is not really a request any more. Maybe vmware_regs_t?

> +
> +struct shared_vmport_iopage {
> +struct vmware_ioreq vcpu_vmport_ioreq[1];
> +};
> +typedef struct shared_vmport_iopage shared_vmport_iopage_t;
> +#endif
> +
>  #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
>  static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
>  {
> @@ -79,8 +102,10 @@ typedef struct XenPhysmap {
> 
>  typedef struct XenIOState {
>  shared_iopage_t *shared_page;
> +shared_vmport_iopage_t *shared_vmport_page;
>  buffered_iopage_t *buffered_io_page;
>  QEMUTimer *buffered_io_timer;
> +CPUState **cpu_by_ioreq_id;

This name implies the array is indexed by an id carries in the ioreq. That 
doesn't seem to be the case; it's just indexed by guest vcpu id isn't it?

>  /* the evtchn port for polling the notification, */
>  evtchn_port_t *ioreq_local_port;
>  /* evtchn local port for buffered io */
> @@ -101,6 +126,8 @@ typedef struct XenIOState {
>  Notifier wakeup;
>  } XenIOState;
> 
> +static void handle_ioreq(XenIOState *state, ioreq_t *req);
> +
>  /* Xen specific function for piix pci */
> 
>  int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> @@ -610,6 +637,20 @@ static ioreq_t
> *cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
>  return req;
>  }
> 
> +/* get the vmport ioreq packets from share mem */
> +static vmware_ioreq_t *cpu_get_vmport_ioreq_from_shared_memory(

Is this worth its own function? I can only see one use of it in 
handle_vmport_ioreq below...

> +XenIOState *state, int vcpu)
> +{
> +vmware_ioreq_t *vmport_req;
> +
> +assert(state->shared_vmport_page);
> +vmport_req = &state->shared_vmport_page-
> >vcpu_vmport_ioreq[vcpu];
> +
> +xen_rmb(); /* see IOREQ_READY /then/ read contents of ioreq */

I don't think that comment is applicable here, and I don't think you need the 
barrier anyway.

> +
> +return vmport_req;
> +}
> +
>  /* use poll to get the port notification */
>  /* ioreq_vec--out,the */
>  /* retval--the number o

Re: [Qemu-devel] The status about vhost-net on kvm-arm?

2014-10-17 Thread Li Liu


On 2014/10/15 22:39, GAUGUEY Rémy 228890 wrote:
> Hello,
> 
> Using this Qemu patchset as well as recent irqfd work, I’ve tried to make 
> vhost-net working on Cortex-A15.
> Unfortunately, even if I can correctly generate irqs to the guest through 
> irqfd, it seems to me that some pieces are still missing….
> Indeed, virtio mmio interrupt status register (@ offset 0x60) is not updated 
> by vhost thread, and reading it or writing to the peer interrupt ack register 
> (offset 0x64) from the guest causes an VM exit …
> 

Yeah, you are correct. But it's not far away from success if have injected irqs
to the guest through irqfd. Do below things to let guest receive packets
correctly without checking VIRTIO_MMIO_INTERRUPT_STATUS in guest virtio_mmio.c:

static irqreturn_t vm_interrupt(int irq, void *opaque)
{
..

/* Read and acknowledge interrupts */
/*status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);

if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
&& vdrv && vdrv->config_changed) {
vdrv->config_changed(&vm_dev->vdev);
ret = IRQ_HANDLED;
}*/

//if (likely(status & VIRTIO_MMIO_INT_VRING)) {
spin_lock_irqsave(&vm_dev->lock, flags);
list_for_each_entry(info, &vm_dev->virtqueues, node)
ret |= vring_interrupt(irq, info->vq);
spin_unlock_irqrestore(&vm_dev->lock, flags);
//}

return ret;
}

This is very roughly :), and a lot of coding things need to be done.

Li.

> After reading older posts, I understand that vhost-net with irqfd support 
> could only work with MSI-X support :
> 
> On 01/20/2011 09:35 AM, Michael S. Tsirkin wrote:
> “When MSI is off, each interrupt needs to be bounced through the io thread 
> when it's set/cleared, so vhost-net causes more context switches and
> higher CPU utilization than userspace virtio which handles networking in the 
> same thread.
> “
> Indeed, in case of MSI-X support, Virtio spec indicates that the ISR Status 
> field is unused…
> 
> I understand that Vhost does not emulate a complete virtio PCI adapter but 
> only manage virtqueue operations.
> However I don’t have a clear view of what is performed by Qemu and what is 
> performed by vhost-thread…
> Could someone highlight me on this point, and maybe give some clues for an 
> implementation of Vhost with irqfd and without MSI support ???
> 
> Thanks a lot in advance.
> Best regards.
> Rémy
> 
> 
> 
> De : kvmarm-boun...@lists.cs.columbia.edu 
> [mailto:kvmarm-boun...@lists.cs.columbia.edu] De la part de Yingshiuan Pan
> Envoyé : vendredi 15 août 2014 09:25
> À : Li Liu
> Cc : kvm...@lists.cs.columbia.edu; k...@vger.kernel.org; qemu-devel
> Objet : Re: [Qemu-devel] The status about vhost-net on kvm-arm?
> 
> Hi, Li,
> 
> It's ok, I did get those mails from mailing list. I guess it was because I 
> did not subscribe some of mailing lists.
> 
> Currently, I think I will not have any plan to renew my patcheset since I 
> have resigned from my previous company, I do not have Cortex-A15 platform to 
> test/verify.
> 
> I'm fine with that, it would be great if you or someone can take it and 
> improve it.
> Thanks.
> 
> 
> Best Regards,
> Yingshiuan Pan
> 
> 2014-08-15 11:04 GMT+08:00 Li Liu 
> mailto:john.li...@huawei.com>>:
> Hi Ying-Shiuan Pan,
> 
> I don't know why for missing your mail in mailbox. Sorry about that.
> The results of vhost-net performance have been attached in another mail.
> 
> Do you have a plan to renew your patchset to support irqfd. If not,
> we will try to finish it based on yours.
> 
> On 2014/8/14 11:50, Li Liu wrote:
>>
>>
>> On 2014/8/13 19:25, Nikolay Nikolaev wrote:
>>> On Wed, Aug 13, 2014 at 12:10 PM, Nikolay Nikolaev
>>> mailto:n.nikol...@virtualopensystems.com>>
>>>  wrote:
 On Tue, Aug 12, 2014 at 6:47 PM, Nikolay Nikolaev
 mailto:n.nikol...@virtualopensystems.com>>
  wrote:
>
> Hello,
>
>
> On Tue, Aug 12, 2014 at 5:41 AM, Li Liu 
> mailto:john.li...@huawei.com>> wrote:
>>
>> Hi all,
>>
>> Is anyone there can tell the current status of vhost-net on kvm-arm?
>>
>> Half a year has passed from Isa Ansharullah asked this question:
>> http://www.spinics.net/lists/kvm-arm/msg08152.html
>>
>> I have found two patches which have provided the kvm-arm support of
>> eventfd and irqfd:
>>
>> 1) [RFC PATCH 0/4] ARM: KVM: Enable the ioeventfd capability of KVM on 
>> ARM
>> http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg01770.html
>>
>> 2) [RFC,v3] ARM: KVM: add irqfd and irq routing support
>> https://patches.linaro.org/32261/
>>
>> And there's a rough patch for qemu to support eventfd from Ying-Shiuan 
>> Pan:
>>
>> [Qemu-devel] [PATCH 0/4] ioeventfd support for virtio-mmio
>> https://lists.gnu.or

[Qemu-devel] [PATCH v4 1/1] xen-hvm.c: Add support for Xen access to vmport

2014-10-17 Thread Don Slutz
This adds synchronisation of the 6 vcpu registers (only 32bits of
them) that vmport.c needs between Xen and QEMU.

This is to avoid a 2nd and 3rd exchange between QEMU and Xen to
fetch and put these 6 vcpu registers used by the code in vmport.c
and vmmouse.c

The registers are passed in the new shared page provided by
HVM_PARAM_VMPORT_IOREQ_PFN.

Add new array to XenIOState that allows selection of current_cpu by
ioreq_id.

Now pass XenIOState to handle_ioreq().

Add new routines regs_to_cpu(), regs_from_cpu(), and
handle_vmport_ioreq().

Signed-off-by: Don Slutz 
---
 include/hw/xen/xen_common.h |  15 ++
 xen-hvm.c   | 121 ++--
 2 files changed, 131 insertions(+), 5 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 07731b9..9542756 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -164,4 +164,19 @@ void destroy_hvm_domain(bool reboot);
 /* shutdown/destroy current domain because of an error */
 void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
+#ifdef HVM_PARAM_VMPORT_IOREQ_PFN
+static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
+   unsigned long *vmport_ioreq_pfn)
+{
+return xc_get_hvm_param(xc, dom, HVM_PARAM_VMPORT_IOREQ_PFN,
+vmport_ioreq_pfn);
+}
+#else
+static inline int xen_get_vmport_ioreq_pfn(XenXC xc, domid_t dom,
+   unsigned long *vmport_ioreq_pfn)
+{
+return -ENOSYS;
+}
+#endif
+
 #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/xen-hvm.c b/xen-hvm.c
index 05e522c..17b3cbd 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -41,6 +41,29 @@ static MemoryRegion *framebuffer;
 static bool xen_in_migration;
 
 /* Compatibility with older version */
+
+/* This allows QEMU to build on a system that has Xen 4.5 or earlier
+ * installed.  This here (not in hw/xen/xen_common.h) because xen/hvm/ioreq.h
+ * needs to be included before this block and hw/xen/xen_common.h needs to
+ * be included before xen/hvm/ioreq.h
+ */
+#ifndef IOREQ_TYPE_VMWARE_PORT
+#define IOREQ_TYPE_VMWARE_PORT  3
+struct vmware_ioreq {
+uint32_t esi;
+uint32_t edi;
+uint32_t ebx;
+uint32_t ecx;
+uint32_t edx;
+};
+typedef struct vmware_ioreq vmware_ioreq_t;
+
+struct shared_vmport_iopage {
+struct vmware_ioreq vcpu_vmport_ioreq[1];
+};
+typedef struct shared_vmport_iopage shared_vmport_iopage_t;
+#endif
+
 #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
 static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
 {
@@ -79,8 +102,10 @@ typedef struct XenPhysmap {
 
 typedef struct XenIOState {
 shared_iopage_t *shared_page;
+shared_vmport_iopage_t *shared_vmport_page;
 buffered_iopage_t *buffered_io_page;
 QEMUTimer *buffered_io_timer;
+CPUState **cpu_by_ioreq_id;
 /* the evtchn port for polling the notification, */
 evtchn_port_t *ioreq_local_port;
 /* evtchn local port for buffered io */
@@ -101,6 +126,8 @@ typedef struct XenIOState {
 Notifier wakeup;
 } XenIOState;
 
+static void handle_ioreq(XenIOState *state, ioreq_t *req);
+
 /* Xen specific function for piix pci */
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
@@ -610,6 +637,20 @@ static ioreq_t 
*cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
 return req;
 }
 
+/* get the vmport ioreq packets from share mem */
+static vmware_ioreq_t *cpu_get_vmport_ioreq_from_shared_memory(
+XenIOState *state, int vcpu)
+{
+vmware_ioreq_t *vmport_req;
+
+assert(state->shared_vmport_page);
+vmport_req = &state->shared_vmport_page->vcpu_vmport_ioreq[vcpu];
+
+xen_rmb(); /* see IOREQ_READY /then/ read contents of ioreq */
+
+return vmport_req;
+}
+
 /* use poll to get the port notification */
 /* ioreq_vec--out,the */
 /* retval--the number of ioreq packet */
@@ -773,7 +814,50 @@ static void cpu_ioreq_move(ioreq_t *req)
 }
 }
 
-static void handle_ioreq(ioreq_t *req)
+static void regs_to_cpu(XenIOState *state, vmware_ioreq_t *vmport_req,
+ioreq_t *req)
+{
+X86CPU *cpu;
+CPUX86State *env;
+
+current_cpu = state->cpu_by_ioreq_id[state->send_vcpu];
+cpu = X86_CPU(current_cpu);
+env = &cpu->env;
+env->regs[R_EAX] = req->data;
+env->regs[R_EBX] = vmport_req->ebx;
+env->regs[R_ECX] = vmport_req->ecx;
+env->regs[R_EDX] = vmport_req->edx;
+env->regs[R_ESI] = vmport_req->esi;
+env->regs[R_EDI] = vmport_req->edi;
+}
+
+static void regs_from_cpu(XenIOState *state, vmware_ioreq_t *vmport_req,
+  ioreq_t *req)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+CPUX86State *env = &cpu->env;
+
+assert(sizeof(*vmport_req) <= sizeof(*req));
+
+vmport_req->ebx = env->regs[R_EBX];
+vmport_req->ecx = env->regs[R_ECX];
+vmport_req->edx = env->regs[R_EDX];
+vmport_req->esi = env->regs[R_ESI];
+vm

[Qemu-devel] [PATCH v4 0/1] Add support for Xen access to vmport

2014-10-17 Thread Don Slutz
Changes RFC-v2x to v4:
  Stefano Stabellini
Please try to get rid of the #ifdefs.
  Moved 2 #ifdefs into hw/xen/xen_common.h

Changes v2 to RFC-v2x:
  Paul Durrant
Use a 2nd shared page.
  Added HVM_PARAM_VMPORT_IOREQ_PFN usage.

Changes v1 to v2:
   More info in commit message.

  Stefano Stabellini
the registers being passes explicitely by Xen rather than
"hiding" them into other ioreq fields.
   Added vmware_ioreq_t
  Paolo Bonzini & Alexander Graf
Fixup env access
  Added cpu_by_ioreq_id.
  Set current_cpu in regs_to_cpu(), clear in regs_from_cpu().
  Drop all changes to vmport.c

Note: to use this with Xen either a version of:

[Qemu-devel] [PATCH] -machine vmport=off: Allow disabling of VMWare ioport 
emulation

or

>From f70663d9fb86914144ba340b6186cb1e67ac6eec Mon Sep 17 00:00:00 2001
From: Don Slutz 
Date: Fri, 26 Sep 2014 08:11:39 -0400
Subject: [PATCH 1/2] hack: force enable vmport

Signed-off-by: Don Slutz 
---
 hw/i386/pc_piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 103d756..b76dfbc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -234,7 +234,7 @@ static void pc_init1(MachineState *machine,
 pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
 
 /* init basic PC hardware */
-pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled(),
+pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, false,
 0x4);
 
 pc_nic_init(isa_bus, pci_bus);
-- 
1.8.4

needs to be done to QEMU.

And the Xen RFC patch:

[RFC][PATCH v2 1/1] Add IOREQ_TYPE_VMWARE_PORT

needs to be done to Xen.

Don Slutz (1):
  xen-hvm.c: Add support for Xen access to vmport

 include/hw/xen/xen_common.h |  15 ++
 xen-hvm.c   | 121 ++--
 2 files changed, 131 insertions(+), 5 deletions(-)

-- 
1.8.4




Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory

2014-10-17 Thread Paolo Bonzini
Il 14/10/2014 14:48, Peter Maydell ha scritto:
> Why don't we just do that automatically for all RAM regions
> the machine creates? We're already splitting up a contiguous
> chunk of RAM to parcel out to RAM regions, that's what the
> indirection through ramaddrs is all about.

I can think of one reason to not do this.  If we just mmap-ed RAM
regions consecutively from a file, the offset in the file should be
aligned to the huge page size for "main" RAM region(s).  For example,
assume gigabyte-sized huge pages; the offset for main RAM must be
gigabyte aligned.  If the board creates 16MiB of VRAM first, and 2 GiB
of main RAM second, you will end up wasting 1024-16 MiB of RAM for VRAM.
 This same waste happened before the introduction of
memory_region_allocate_system_memory, in fact.

Of course this could in principle be fixed by having boards create main
RAM first, but it would be an even more complicated change.  You could
come up with a way of reserving ram_addr_t ranges, which would also help
hotplug; however, I am not sure if all architectures require the maximum
hotplugged memory size to be specified upfront.

Ultimately, if a RAM region other than main RAM has benefits from huge
pages, the device that creates it probably should have a memdev
property.  This is not the case for any current device, except perhaps
ivshmem.

Paolo



Re: [Qemu-devel] Counting barrier instructions in ARM

2014-10-17 Thread Alex Bennée

Pranith Kumar  writes:

> Hi Peter,
>
> On Thu, Oct 16, 2014 at 4:05 AM, Peter Maydell  
> wrote:
>> On 16 October 2014 03:45, Pranith Kumar  wrote:
>>> The problem I am facing is that this seems to be crashing when run with a
>>> multi-threaded executable.
>>
>> This is nothing to do with your changes -- user-mode QEMU does not
>> support multi-threaded guest executables. QEMU may crash, hang,
>> or stop with an assertion failure, fairly randomly. Don't try
>> to run multithreaded guests :-)
>
> OK, I will try to gather the statistics in system mode. Is there any
> way to indicate from within the system to  qemu to start collecting the stats?
>
> I dont want to collect the stats for bootup and other unrelated code
> paths.

You could enable CONTEXTIDR in the kernel and then make your counts
against an array indexed by it. I've used that technique before to
profile guest hot-blocks and work out which guest PID was responsible.

-- 
Alex Bennée



[Qemu-devel] [Bug 1382477] [NEW] hw/i386/intel_iommu.c:902: wrong logical operator ?

2014-10-17 Thread dcb
Public bug reported:

/home/dcb/qemu/trunk/qemu/hw/i386/intel_iommu.c:902:5: error: logical ‘and’ 
applied to non-boolean constant [-Werror=logical-op]
 pvtd_as = s->address_spaces[VTD_SID_TO_BUS(source_id)];
 ^

$ fgrep VTD_SID_TO_BUS `find . -name \*.h -print`
./include/hw/i386/intel_iommu.h:#define VTD_SID_TO_BUS(sid) (((sid) >> 
8) && 0xff)
$ 

Sounds to me like

#define VTD_SID_TO_BUS(sid) (((sid) >> 8) & 0xff)

would be better.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  hw/i386/intel_iommu.c:902: wrong logical operator ?

Status in QEMU:
  New

Bug description:
  /home/dcb/qemu/trunk/qemu/hw/i386/intel_iommu.c:902:5: error: logical ‘and’ 
applied to non-boolean constant [-Werror=logical-op]
   pvtd_as = s->address_spaces[VTD_SID_TO_BUS(source_id)];
   ^

  $ fgrep VTD_SID_TO_BUS `find . -name \*.h -print`
  ./include/hw/i386/intel_iommu.h:#define VTD_SID_TO_BUS(sid) (((sid) 
>> 8) && 0xff)
  $ 

  Sounds to me like

  #define VTD_SID_TO_BUS(sid) (((sid) >> 8) & 0xff)

  would be better.

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



Re: [Qemu-devel] [PATCH 3/6] target-mips: CP0_Status.CU0 no longer allows the user to access CP0

2014-10-17 Thread Yongbok Kim

Reviewed-by: Yongbok Kim 

Regards,
Yongbok

On 14/07/2014 17:19, Leon Alrae wrote:

Signed-off-by: Leon Alrae 
---
  target-mips/cpu.h |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index a35ab9d..b981ec7 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -789,7 +789,8 @@ static inline void compute_hflags(CPUMIPSState *env)
  }
  }
  #endif
-if ((env->CP0_Status & (1 << CP0St_CU0)) ||
+if (((env->CP0_Status & (1 << CP0St_CU0)) &&
+ !(env->insn_flags & ISA_MIPS32R6)) ||
  !(env->hflags & MIPS_HFLAG_KSU)) {
  env->hflags |= MIPS_HFLAG_CP0;
  }





[Qemu-devel] [PATCH v4 3/3] monitor: delete device_del_bus_completion

2014-10-17 Thread Zhu Guihua
device_del_bus_completion() that gathers devices from buses is unused; delete
it.

Signed-off-by: Zhu Guihua 
---
 monitor.c | 20 
 1 file changed, 20 deletions(-)

diff --git a/monitor.c b/monitor.c
index 9c3fa01..42affb7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4340,25 +4340,6 @@ void object_add_completion(ReadLineState *rs, int 
nb_args, const char *str)
 g_slist_free(list);
 }
 
-static void device_del_bus_completion(ReadLineState *rs,  BusState *bus,
-  const char *str, size_t len)
-{
-BusChild *kid;
-
-QTAILQ_FOREACH(kid, &bus->children, sibling) {
-DeviceState *dev = kid->child;
-BusState *dev_child;
-
-if (dev->id && !strncmp(str, dev->id, len)) {
-readline_add_completion(rs, dev->id);
-}
-
-QLIST_FOREACH(dev_child, &dev->child_bus, sibling) {
-device_del_bus_completion(rs, dev_child, str, len);
-}
-}
-}
-
 static void peripheral_device_del_completion(ReadLineState *rs,
  const char *str, size_t len)
 {
@@ -4454,7 +4435,6 @@ void device_del_completion(ReadLineState *rs, int 
nb_args, const char *str)
 
 len = strlen(str);
 readline_set_completion_index(rs, len);
-device_del_bus_completion(rs, sysbus_get_default(), str, len);
 peripheral_device_del_completion(rs, str, len);
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH v4 0/3] monitor: add peripheral device del completion support

2014-10-17 Thread Zhu Guihua
After inputting device_del command in monitor, we expect to list all
hotpluggable devices automatically by pressing tab key. This patchset provides
the function to list all peripheral devices such as memory devices.

v4:
- delete unused device_del_bus_completion (Igor)
- modify the way to get value of hotpluggable property (Igor)

v3:
- commit message changes (Igor)
- rename function in patch 1 (Igor)
- use 'hotpluggable' property to discard non-hotpluggable devices (Igor)

v2:
- use object_child_foreach() to simplify the implementation (Andreas)

Zhu Guihua (3):
  qdev: add qdev_build_hotpluggable_device_list helper
  monitor: add del completion for peripheral device
  monitor: delete device_del_bus_completion

 hw/core/qdev.c | 13 +
 include/hw/qdev-core.h |  2 ++
 monitor.c  | 26 +++---
 3 files changed, 30 insertions(+), 11 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH v4 1/3] qdev: add qdev_build_hotpluggable_device_list helper

2014-10-17 Thread Zhu Guihua
For peripheral device del completion, add a function to build a list for
hotpluggable devices.

Signed-off-by: Zhu Guihua 
---
 hw/core/qdev.c | 13 +
 include/hw/qdev-core.h |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index a1e9247..9357aba 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -866,6 +866,19 @@ void qdev_alias_all_properties(DeviceState *target, Object 
*source)
 } while (class != object_class_by_name(TYPE_DEVICE));
 }
 
+int qdev_build_hotpluggable_device_list(Object *obj, void *opaque)
+{
+GSList **list = opaque;
+DeviceState *dev = DEVICE(obj);
+
+if (dev->realized && object_property_get_bool(obj, "hotpluggable", NULL)) {
+*list = g_slist_append(*list, dev);
+}
+
+object_child_foreach(obj, qdev_build_hotpluggable_device_list, opaque);
+return 0;
+}
+
 static bool device_get_realized(Object *obj, Error **errp)
 {
 DeviceState *dev = DEVICE(obj);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1fca75c..22820fe 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -360,6 +360,8 @@ extern int qdev_hotplug;
 
 char *qdev_get_dev_path(DeviceState *dev);
 
+int qdev_build_hotpluggable_device_list(Object *obj, void *opaque);
+
 void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
   Error **errp);
 
-- 
1.9.3




[Qemu-devel] [PATCH v4 2/3] monitor: add del completion for peripheral device

2014-10-17 Thread Zhu Guihua
Add peripheral_device_del_completion() to let peripheral device del completion
be possible.

Signed-off-by: Zhu Guihua 
---
 monitor.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/monitor.c b/monitor.c
index 2d14f39..9c3fa01 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4359,6 +4359,29 @@ static void device_del_bus_completion(ReadLineState *rs, 
 BusState *bus,
 }
 }
 
+static void peripheral_device_del_completion(ReadLineState *rs,
+ const char *str, size_t len)
+{
+Object *peripheral;
+GSList *list = NULL, *item;
+
+peripheral = object_resolve_path("/machine/peripheral/", NULL);
+if (peripheral == NULL) {
+return;
+}
+
+object_child_foreach(peripheral, qdev_build_hotpluggable_device_list,
+ &list);
+
+for (item = list; item; item = g_slist_next(item)) {
+DeviceState *dev = item->data;
+
+if (dev->id && !strncmp(str, dev->id, len)) {
+readline_add_completion(rs, dev->id);
+}
+}
+}
+
 void chardev_remove_completion(ReadLineState *rs, int nb_args, const char *str)
 {
 size_t len;
@@ -4432,6 +4455,7 @@ void device_del_completion(ReadLineState *rs, int 
nb_args, const char *str)
 len = strlen(str);
 readline_set_completion_index(rs, len);
 device_del_bus_completion(rs, sysbus_get_default(), str, len);
+peripheral_device_del_completion(rs, str, len);
 }
 
 void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
-- 
1.9.3




Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Linhaifeng


On 2014/10/17 16:57, Linhaifeng wrote:
> 
> 
> On 2014/10/17 16:33, Daniel P. Berrange wrote:
>> On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
>>> From: linhaifeng 
>>>
>>> The VM start with share hugepage should close the hugefile fd
>>> when exit.Because the hugepage fd may be send to other process
>>> e.g vhost-user If qemu not close the fd the other process can
>>> not free the hugepage otherwise exit process,this is ugly,so
>>> qemu should close all shared fd when exit.
>>>
>>> Signed-off-by: linhaifeng 
>>
>> Err, all file descriptors are closed automatically when a process
>> exits. So manually calling close(fd) before exit can't have any
>> functional effect on a resource leak.
>>
>> If QEMU has sent the FD to another process, that process has a
>> completely separate copy of the FD. Closing the FD in QEMU will
>> not close the FD in the other process. You need the other process
>> to exit for the copy to be closed.
>>
>> Regards,
>> Daniel
>>
> Hi,daniel
> 
> QEMU send the fd by unix domain socket.unix domain socket just install the fd 
> to
> other process and inc the f_count,if qemu not close the fd the f_count is not 
> dec.
> Then the other process even close the fd the hugepage would not freed whise 
> the other process exit.
> 
> I have test it for many times.
> 

The point is I want to free the hugepage when close port in vhost-user process 
and not exit.
-- 
Regards,
Haifeng




Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Linhaifeng


On 2014/10/17 16:56, Gonglei wrote:
> On 2014/10/17 16:33, Daniel P. Berrange wrote:
> 
>> On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
>>> From: linhaifeng 
>>>
>>> The VM start with share hugepage should close the hugefile fd
>>> when exit.Because the hugepage fd may be send to other process
>>> e.g vhost-user If qemu not close the fd the other process can
>>> not free the hugepage otherwise exit process,this is ugly,so
>>> qemu should close all shared fd when exit.
>>>
>>> Signed-off-by: linhaifeng 
>>
>> Err, all file descriptors are closed automatically when a process
>> exits. So manually calling close(fd) before exit can't have any
>> functional effect on a resource leak.
>>
>> If QEMU has sent the FD to another process, that process has a
>> completely separate copy of the FD. Closing the FD in QEMU will
>> not close the FD in the other process. You need the other process
>> to exit for the copy to be closed.
>>
> 
> Actually, when vhost-user close the FD manually, the hugepage leak too
> unless the vhost-user process exit. So, maybe the FD is not a separate
> copy IMHO, but simply add the ref-count of FD. When QEMU exit,
> because the ref is not zero, the operate system will not free the FD
> automatically, and when vhost-user close the FD, because of the same
> reason, OS will not free FD resource.
> 
> BTW, I don't think this patch is good. When Qemu exit exceptionally,
> sush as 'by kill -9', this problem of memory leak still exist.
> 

So,we should close qemu by 'kill -15' or close with virsh.

> Best Regards,
> -Gonglei
> 
> 
> 
> 
> .
> 



-- 
Regards,
Haifeng




Re: [Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory

2014-10-17 Thread Gonglei
On 2014/10/17 16:50, zhanghailiang wrote:

> When do memory hotplug, if there is numa node, we should add
> the memory size to the corresponding node memory size.
> 
> For now, it mainly affects the result of hmp command "info numa".
> 
> Reviewed-by: Igor Mammedov 
> Signed-off-by: zhanghailiang 
> ---
>  v6:
> - remove unnecessary 'di' variable (GongLei)
>  v5:
> - reword the subject (Igor Mammedov)
> - turn query_numa_node_mem to void (Igor Mammedov)
>  v4:
> - s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
> - rewrite numa_stat_memory_devices and this will also fix compile error for 
>   targets that don't support memory hotplug
>  v3:
> - cold-plugged memory should not be excluded (Igor Mammedov)
>  v2:
> - Don't modify the numa_info.node_mem directly when treating hotplug memory,
>   fix the "info numa" instead (Igor Mammedov)
> 
> Thanks for review!;)
> ---
> 
>  include/sysemu/sysemu.h |  1 +
>  monitor.c   |  6 +-
>  numa.c  | 38 ++
>  3 files changed, 44 insertions(+), 1 deletion(-)
> 

Reviewed-by: Gonglei 

Best regards,
-Gonglei




Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Gonglei
On 2014/10/17 16:33, Daniel P. Berrange wrote:

> On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
>> From: linhaifeng 
>>
>> The VM start with share hugepage should close the hugefile fd
>> when exit.Because the hugepage fd may be send to other process
>> e.g vhost-user If qemu not close the fd the other process can
>> not free the hugepage otherwise exit process,this is ugly,so
>> qemu should close all shared fd when exit.
>>
>> Signed-off-by: linhaifeng 
> 
> Err, all file descriptors are closed automatically when a process
> exits. So manually calling close(fd) before exit can't have any
> functional effect on a resource leak.
> 
> If QEMU has sent the FD to another process, that process has a
> completely separate copy of the FD. Closing the FD in QEMU will
> not close the FD in the other process. You need the other process
> to exit for the copy to be closed.
> 

Actually, when vhost-user close the FD manually, the hugepage leak too
unless the vhost-user process exit. So, maybe the FD is not a separate
copy IMHO, but simply add the ref-count of FD. When QEMU exit,
because the ref is not zero, the operate system will not free the FD
automatically, and when vhost-user close the FD, because of the same
reason, OS will not free FD resource.

BTW, I don't think this patch is good. When Qemu exit exceptionally,
sush as 'by kill -9', this problem of memory leak still exist.

Best Regards,
-Gonglei






Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Linhaifeng


On 2014/10/17 16:33, Daniel P. Berrange wrote:
> On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
>> From: linhaifeng 
>>
>> The VM start with share hugepage should close the hugefile fd
>> when exit.Because the hugepage fd may be send to other process
>> e.g vhost-user If qemu not close the fd the other process can
>> not free the hugepage otherwise exit process,this is ugly,so
>> qemu should close all shared fd when exit.
>>
>> Signed-off-by: linhaifeng 
> 
> Err, all file descriptors are closed automatically when a process
> exits. So manually calling close(fd) before exit can't have any
> functional effect on a resource leak.
> 
> If QEMU has sent the FD to another process, that process has a
> completely separate copy of the FD. Closing the FD in QEMU will
> not close the FD in the other process. You need the other process
> to exit for the copy to be closed.
> 
> Regards,
> Daniel
> 
Hi,daniel

QEMU send the fd by unix domain socket.unix domain socket just install the fd to
other process and inc the f_count,if qemu not close the fd the f_count is not 
dec.
Then the other process even close the fd the hugepage would not freed whise the 
other process exit.

I have test it for many times.
-- 
Regards,
Haifeng




[Qemu-devel] [PATCH v6] numa: make 'info numa' take into account hotplugged memory

2014-10-17 Thread zhanghailiang
When do memory hotplug, if there is numa node, we should add
the memory size to the corresponding node memory size.

For now, it mainly affects the result of hmp command "info numa".

Reviewed-by: Igor Mammedov 
Signed-off-by: zhanghailiang 
---
 v6:
- remove unnecessary 'di' variable (GongLei)
 v5:
- reword the subject (Igor Mammedov)
- turn query_numa_node_mem to void (Igor Mammedov)
 v4:
- s/pc_dimm_stat_node_mem/numa_stat_memory_devices/ (Igor Mammedov)
- rewrite numa_stat_memory_devices and this will also fix compile error for 
  targets that don't support memory hotplug
 v3:
- cold-plugged memory should not be excluded (Igor Mammedov)
 v2:
- Don't modify the numa_info.node_mem directly when treating hotplug memory,
  fix the "info numa" instead (Igor Mammedov)

Thanks for review!;)
---

 include/sysemu/sysemu.h |  1 +
 monitor.c   |  6 +-
 numa.c  | 38 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 0037a69..ef5eaf4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -161,6 +161,7 @@ typedef struct node_info {
 extern NodeInfo numa_info[MAX_NODES];
 void set_numa_nodes(void);
 void set_numa_modes(void);
+void query_numa_node_mem(uint64_t *node_mem);
 extern QemuOptsList qemu_numa_opts;
 int numa_init_func(QemuOpts *opts, void *opaque);
 
diff --git a/monitor.c b/monitor.c
index 2d14f39..d45b0a3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1949,7 +1949,10 @@ static void do_info_numa(Monitor *mon, const QDict 
*qdict)
 {
 int i;
 CPUState *cpu;
+uint64_t *node_mem;
 
+node_mem = g_new0(uint64_t, nb_numa_nodes);
+query_numa_node_mem(node_mem);
 monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
 for (i = 0; i < nb_numa_nodes; i++) {
 monitor_printf(mon, "node %d cpus:", i);
@@ -1960,8 +1963,9 @@ static void do_info_numa(Monitor *mon, const QDict *qdict)
 }
 monitor_printf(mon, "\n");
 monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
-numa_info[i].node_mem >> 20);
+   node_mem[i] >> 20);
 }
+g_free(node_mem);
 }
 
 #ifdef CONFIG_PROFILER
diff --git a/numa.c b/numa.c
index 3b98135..ae1c7f4 100644
--- a/numa.c
+++ b/numa.c
@@ -35,6 +35,7 @@
 #include "hw/boards.h"
 #include "sysemu/hostmem.h"
 #include "qmp-commands.h"
+#include "hw/mem/pc-dimm.h"
 
 QemuOptsList qemu_numa_opts = {
 .name = "numa",
@@ -315,6 +316,43 @@ void memory_region_allocate_system_memory(MemoryRegion 
*mr, Object *owner,
 }
 }
 
+static void numa_stat_memory_devices(uint64_t *node_mem)
+{
+MemoryDeviceInfoList *info_list = NULL;
+MemoryDeviceInfoList **prev = &info_list;
+MemoryDeviceInfoList *info;
+
+qmp_pc_dimm_device_list(qdev_get_machine(), &prev);
+for (info = info_list; info; info = info->next) {
+MemoryDeviceInfo *value = info->value;
+
+if (value) {
+switch (value->kind) {
+case MEMORY_DEVICE_INFO_KIND_DIMM:
+node_mem[value->dimm->node] += value->dimm->size;
+break;
+default:
+break;
+}
+}
+}
+qapi_free_MemoryDeviceInfoList(info_list);
+}
+
+void query_numa_node_mem(uint64_t *node_mem)
+{
+int i;
+
+if (nb_numa_nodes <= 0) {
+return;
+}
+
+numa_stat_memory_devices(node_mem);
+for (i = 0; i < nb_numa_nodes; i++) {
+node_mem[i] += numa_info[i].node_mem;
+}
+}
+
 static int query_memdev(Object *obj, void *opaque)
 {
 MemdevList **list = opaque;
-- 
1.7.12.4





Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread zhanghailiang

On 2014/10/17 16:27, haifeng@huawei.com wrote:

From: linhaifeng 

The VM start with share hugepage should close the hugefile fd
when exit.Because the hugepage fd may be send to other process
e.g vhost-user If qemu not close the fd the other process can
not free the hugepage otherwise exit process,this is ugly,so
qemu should close all shared fd when exit.

Signed-off-by: linhaifeng 
---
  exec.c | 12 
  vl.c   |  7 +++
  2 files changed, 19 insertions(+)

diff --git a/exec.c b/exec.c
index 759055d..d120b73 100644
--- a/exec.c
+++ b/exec.c
@@ -1535,6 +1535,18 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
  }
  }
  }
+
+void qemu_close_all_ram_fd(void)
+{
+RAMBlock *block;
+
+qemu_mutex_lock_ramlist();
+QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+close(block->fd);
+}
+qemu_mutex_unlock_ramlist();
+}
+
  #endif /* !_WIN32 */

  int qemu_get_ram_fd(ram_addr_t addr)
diff --git a/vl.c b/vl.c
index aee73e1..0b78f3f 100644
--- a/vl.c
+++ b/vl.c
@@ -1658,6 +1658,7 @@ static int qemu_shutdown_requested(void)
  return r;
  }

+extern void qemu_close_all_ram_fd(void);
  static void qemu_kill_report(void)
  {
  if (!qtest_driver() && shutdown_signal != -1) {
@@ -1671,6 +1672,12 @@ static void qemu_kill_report(void)
  fprintf(stderr, " from pid " FMT_pid "\n", shutdown_pid);
  }
  shutdown_signal = -1;
+
+/* Close all ram fd when exit. If the ram is shared by othter process


s/othter/other/


+ * e.g vhost-user, it can free the hugepage by close fd after qemu 
exit,
+ * otherwise the process have to exit to free hugepage.
+ */
+qemu_close_all_ram_fd();
  }
  }








Re: [Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread Daniel P. Berrange
On Fri, Oct 17, 2014 at 04:27:17PM +0800, haifeng@huawei.com wrote:
> From: linhaifeng 
> 
> The VM start with share hugepage should close the hugefile fd
> when exit.Because the hugepage fd may be send to other process
> e.g vhost-user If qemu not close the fd the other process can
> not free the hugepage otherwise exit process,this is ugly,so
> qemu should close all shared fd when exit.
> 
> Signed-off-by: linhaifeng 

Err, all file descriptors are closed automatically when a process
exits. So manually calling close(fd) before exit can't have any
functional effect on a resource leak.

If QEMU has sent the FD to another process, that process has a
completely separate copy of the FD. Closing the FD in QEMU will
not close the FD in the other process. You need the other process
to exit for the copy to be closed.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



[Qemu-devel] [PATCH] fix the memory leak for share hugepage

2014-10-17 Thread haifeng.lin
From: linhaifeng 

The VM start with share hugepage should close the hugefile fd
when exit.Because the hugepage fd may be send to other process
e.g vhost-user If qemu not close the fd the other process can
not free the hugepage otherwise exit process,this is ugly,so
qemu should close all shared fd when exit.

Signed-off-by: linhaifeng 
---
 exec.c | 12 
 vl.c   |  7 +++
 2 files changed, 19 insertions(+)

diff --git a/exec.c b/exec.c
index 759055d..d120b73 100644
--- a/exec.c
+++ b/exec.c
@@ -1535,6 +1535,18 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 }
 }
 }
+
+void qemu_close_all_ram_fd(void)
+{
+RAMBlock *block;
+
+qemu_mutex_lock_ramlist();
+QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+close(block->fd);
+}
+qemu_mutex_unlock_ramlist();
+}
+
 #endif /* !_WIN32 */
 
 int qemu_get_ram_fd(ram_addr_t addr)
diff --git a/vl.c b/vl.c
index aee73e1..0b78f3f 100644
--- a/vl.c
+++ b/vl.c
@@ -1658,6 +1658,7 @@ static int qemu_shutdown_requested(void)
 return r;
 }
 
+extern void qemu_close_all_ram_fd(void);
 static void qemu_kill_report(void)
 {
 if (!qtest_driver() && shutdown_signal != -1) {
@@ -1671,6 +1672,12 @@ static void qemu_kill_report(void)
 fprintf(stderr, " from pid " FMT_pid "\n", shutdown_pid);
 }
 shutdown_signal = -1;
+
+/* Close all ram fd when exit. If the ram is shared by othter process
+ * e.g vhost-user, it can free the hugepage by close fd after qemu 
exit,
+ * otherwise the process have to exit to free hugepage.
+ */
+qemu_close_all_ram_fd();
 }
 }
 
-- 
1.9.0





Re: [Qemu-devel] Crashes of qemu-system-mips64 and qemu-system-mips64el

2014-10-17 Thread Aurelien Jarno
On Sun, Aug 03, 2014 at 02:11:30AM +0200, Torbjörn Granlund wrote:
> I forgot to mention one of the popular crashes:
> 
> Assertion failed: (len <= 64), function tcg_gen_deposit_i64, file 
> /var/tmp/pkg/usr/ports/emulators/qemu-devel/work/qemu-2.0.0/tcg/tcg-op.h, 
> line 2206.
> 
> (This corresponds to qemu 2.1.0)

Hmm it looks quite strange, looking quickly at the code, the only place
were we have a variable and unsafe length passed to tcg_gen_deposit_i64
is when using MIPS64R2 instruction, which is not your case according to
the previous mail as you said you pass -cpu 5Kc to qemu. I remember
there was a patch to fix these instrucions on the mailing list

The best would be to get a backtrace using gdb so that we can now what
calls tcg_gen_deposit_i64 with this too big len.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] Crashes of qemu-system-mips64 and qemu-system-mips64el

2014-10-17 Thread Aurelien Jarno
On Sat, Aug 02, 2014 at 10:49:22PM +0200, Torbjörn Granlund wrote:
> Qemu versions 1.7.0, 1.7.1, 2.0.0, 2.1.0 do not run Debian mips64 BE or
> LE stably.  While install in 32-bit mode typically works, running the
> 64-bit kernel causes qemu to segfault or abort after a while.
> 
> How to reproduce:
> 
> Grab installation kernel and initrd:
> 
> ftp://ftp.debian.org/debian/dists/wheezy/main/installer-mips/current/images/malta/netboot/vmlinux-3.2.0-4-4kc-malta
> ftp://ftp.debian.org/debian/dists/wheezy/main/installer-mips/current/images/malta/netboot/initrd.gz
> 
> Start qemu-system-mips64 and perform an installation.  Choose defaults,
> i.e., "Standard system utilities" and "SSH server".
> 
>   qemu-system-mips64 -M malta -m 256 -nographic \
> -drive file=disk.img,if=virtio,index=0 \
> -net nic,macaddr=52:54:00:13:06:64 -net user,hostfwd=tcp::20008-:22 \
> -kernel vmlinux-3.2.0-4-4kc-malta \
> -initrd initrd.gz \
> -append "console=ttyS0"
> 
> Copy out /boot somehow.  I usually do "Execute a shell" just before the
> installation is about to finish, and there do
> 
>   mount /dev/vda2 /target
>   mount /dev/vda1 /target/boot
>   mount -t proc proc /target/proc
>   mount --rbind /sys /target/sys
>   mount --rbind /dev /target/dev
>   chroot /target bash
>   /etc/init.d/ssh start
> 
> (assuming "put all files in one partition" was chosen; mount commands
> might need adjustment for other partitioning schemes).
> 
> Then from the host system I do
> 
>   scp -pr -P 20008 localhost:/boot .
> 
> and then quit the shell and finish up the installation.  (Alternatively
> use qemu-nbd or guestfish.)
> 
> Then boot the installed system:
> 
>   qemu-system-mips64 -M malta -cpu 5Kc -m 256 \
> -drive file=disk.img,if=virtio,index=0 \
> -net nic,macaddr=52:54:00:13:06:64 -net user,hostfwd=tcp::20008-:22 \
> -kernel boot/vmlinux-3.2.0-4-5kc-malta \
> -initrd boot/initrd.img-3.2.0-4-5kc-malta \
> -append "root=/dev/vda1 console=ttyS0" \
> -nographic -serial null -monitor null
> 
> Then, log in to the system ("ssh -p 20008 root@localhost) and try the
> system.  It will crash within hours, but usually within minutes.

I have installation running in qemu-system-mips with weeks of uptime
without any problem. I have however tried the above with QEMU 2.1, and
I have been unable to reproduce the issue.

> Safe crash strategy:
> 
>   package_list="gcc g++ gdb make emacs23-nox postfix sharutils zsh rsync ntp"
>   apt-get -y install $package_list gcc-multilib g++-multilib
> 
> Not crashed yet?  Let's provoke it somewhat more:

It didn't crash for me.

>   wget https://ftp.gnu.org/gnu/gmp/gmp-6.0.0a.tar.bz2
>   tar xf gmp-6.0.0a.tar.bz2
>   cd gmp-6.0.0
>   (configure && make && while true; do make check; done) >&/dev/null
> 
> Note that I have reproduced this problem on several host machines.  Host
> system stability is not the root cause.  The host system has been either
> GNU/Linux or FreeBSD.  I have not found a stable setup since before qemu
> 1.7.0.  I know it was stable around 1.5.x or 1.6.x.  (Even older qemus
> executed a few instructions incorrectly, making them inadequate for my
> use.)
> 

It's now running for more than 48 hours, and hasn't crashed yet.

Could you give us more details about your host, especially if it is a
32-bit or a 64-bit one? Also a cat /proc/cpuinfo would be useful as some
instructions are enabled or not depending on the host support.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net