Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Michael Tokarev]

15.02.2013 07:43, Kevin O'Connor wrote:
> On Fri, Feb 15, 2013 at 04:10:59AM +0100, Laszlo Ersek wrote:
>> On 02/15/13 02:22, Kevin O'Connor wrote:
>>> On Thu, Feb 14, 2013 at 08:16:02PM -0500, Kevin O'Connor wrote:
>>> By chance, are you using an older version of kvm?  There was a bug in
>>> kvm that caused changes to memory mapped at 0xe-0xf to also be
>>> reflected in the "rom" image at 0xfffe-0x.  It was my
>>> understand that this bug was fixed though.
>>
>> You are great! Disabling KVM for the guest (/domain/@type='qemu') made
>> the reboot work on both the RHEL-6 devel version of qemu and on upstream
>> 1.3.1.
>>
>> (I didn't try suspend/resume yet.)
>>
>> Do you recall the precise commit that fixed the "reflection"? I've been
>> eyeballing kvm commit messages for a few ten minutes now, but of course
>> in vain. (CC'ing Gleb and Marcelo.)
> 
> I found this email thread:
> 
> http://kerneltrap.org/mailarchive/linux-kvm/2010/9/21/6267744
> 
> and: http://marc.info/?l=kvm-commits&m=128576215909532

This patch is more than 2 years old and is applied to all more or
less recent qemu versions.  This does not tell us why disabling
kvm (with this patch applied!) makes a difference.  So there must
be another (maybe similar) bug somewhere...

/mjt

[Qemu-devel] [PATCH v1 3/5] cadence_gem: fix interrupt events

[Peter Crosthwaite]

Bits in the ISR were continually mirroring their corresponding TX/RX SR bits.
This is incorrect. The ISR bits are only every set at the time their
corresponding event occurs.

Signed-off-by: Peter Crosthwaite 
---

 hw/cadence_gem.c |   27 ---
 1 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
index 966ab4f..a1ac069 100644
--- a/hw/cadence_gem.c
+++ b/hw/cadence_gem.c
@@ -427,32 +427,9 @@ static int gem_can_receive(NetClientState *nc)
  */
 static void gem_update_int_status(GemState *s)
 {
-uint32_t new_interrupts = 0;
-/* Packet transmitted ? */
-if (s->regs[GEM_TXSTATUS] & GEM_TXSTATUS_TXCMPL) {
-new_interrupts |= GEM_INT_TXCMPL;
-}
-/* End of TX ring ? */
-if (s->regs[GEM_TXSTATUS] & GEM_TXSTATUS_USED) {
-new_interrupts |= GEM_INT_TXUSED;
-}
-
-/* Frame received ? */
-if (s->regs[GEM_RXSTATUS] & GEM_RXSTATUS_FRMRCVD) {
-new_interrupts |= GEM_INT_RXCMPL;
-}
-/* RX ring full ? */
-if (s->regs[GEM_RXSTATUS] & GEM_RXSTATUS_NOBUF) {
-new_interrupts |= GEM_INT_RXUSED;
-}
-
-s->regs[GEM_ISR] |= new_interrupts & ~(s->regs[GEM_IMR]);
-
 if (s->regs[GEM_ISR]) {
 DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
 qemu_set_irq(s->irq, 1);
-} else {
-qemu_set_irq(s->irq, 0);
 }
 }
 
@@ -697,6 +674,7 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 DB_PRINT("descriptor 0x%x owned by sw.\n",
  (unsigned)packet_desc_addr);
 s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
+s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
 /* Handle interrupt consequences */
 gem_update_int_status(s);
 return -1;
@@ -765,6 +743,7 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
   (uint8_t *)&desc[0], sizeof(desc));
 
 s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_FRMRCVD;
+s->regs[GEM_ISR] |= GEM_INT_RXCMPL & ~(s->regs[GEM_IMR]);
 
 /* Handle interrupt consequences */
 gem_update_int_status(s);
@@ -894,6 +873,7 @@ static void gem_transmit(GemState *s)
 DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr);
 
 s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
+s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
 
 /* Handle interrupt consequences */
 gem_update_int_status(s);
@@ -931,6 +911,7 @@ static void gem_transmit(GemState *s)
 
 if (tx_desc_get_used(desc)) {
 s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_USED;
+s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
 gem_update_int_status(s);
 }
 }
-- 
1.7.0.4

[Qemu-devel] [PATCH v1 4/5] cadence_gem: Dont reset rx desc pointer on rx_en

[Peter Crosthwaite]

This doesnt happen in the real hardware. The Zynq TRM explicitly states that
this bit has no effect on the rx descriptor pointer ("The receive queue
pointer register is unaffected").

Signed-off-by: Peter Crosthwaite 
---

 hw/cadence_gem.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
index a1ac069..61f1801 100644
--- a/hw/cadence_gem.c
+++ b/hw/cadence_gem.c
@@ -1083,10 +1083,6 @@ static void gem_write(void *opaque, hwaddr offset, 
uint64_t val,
 /* Reset to start of Q when transmit disabled. */
 s->tx_desc_addr = s->regs[GEM_TXQBASE];
 }
-if (!(val & GEM_NWCTRL_RXENA)) {
-/* Reset to start of Q when receive disabled. */
-s->rx_desc_addr = s->regs[GEM_RXQBASE];
-}
 if (val & GEM_NWCTRL_RXENA) {
 qemu_flush_queued_packets(qemu_get_queue(s->nic));
 }
-- 
1.7.0.4

[Qemu-devel] [PATCH v1 5/5] cadence_gem: Add debug msgs for rx desc movement

[Peter Crosthwaite]

Add some helpful messages that show the rx descriptor pointer moving as packets
are rxed.

Signed-off-by: Peter Crosthwaite 
---

 hw/cadence_gem.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
index 61f1801..de7d15a 100644
--- a/hw/cadence_gem.c
+++ b/hw/cadence_gem.c
@@ -724,7 +724,9 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 s->rx_desc_addr = last_desc_addr;
 if (rx_desc_get_wrap(desc)) {
 s->rx_desc_addr = s->regs[GEM_RXQBASE];
+DB_PRINT("wrapping RX descriptor list\n");
 } else {
+DB_PRINT("incrementing RX descriptor list\n");
 s->rx_desc_addr += 8;
 }
 
-- 
1.7.0.4

[Qemu-devel] [PATCH v1 2/5] cadence_gem: factor out can_rx() logic replication

[Peter Crosthwaite]

The gem_receive() function replicates the logic for whether or not the device
can rx. Just call the actual gem_can_receive() function in place.

Signed-off-by: Peter Crosthwaite 
---

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

diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
index e6032ea..966ab4f 100644
--- a/hw/cadence_gem.c
+++ b/hw/cadence_gem.c
@@ -615,7 +615,7 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 s = qemu_get_nic_opaque(nc);
 
 /* Do nothing if receive is not enabled. */
-if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_RXENA)) {
+if (!gem_can_receive(nc)) {
 return -1;
 }
 
-- 
1.7.0.4

[Qemu-devel] [PATCH v1 1/5] cadence_gem: Flush queued packets

[Peter Crosthwaite]

The device needs to check for queued RX packets when the RX path is re-enabled.

Signed-off-by: Peter Crosthwaite 
---

 hw/cadence_gem.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/cadence_gem.c b/hw/cadence_gem.c
index ab86c17..e6032ea 100644
--- a/hw/cadence_gem.c
+++ b/hw/cadence_gem.c
@@ -1106,6 +1106,9 @@ static void gem_write(void *opaque, hwaddr offset, 
uint64_t val,
 /* Reset to start of Q when receive disabled. */
 s->rx_desc_addr = s->regs[GEM_RXQBASE];
 }
+if (val & GEM_NWCTRL_RXENA) {
+qemu_flush_queued_packets(qemu_get_queue(s->nic));
+}
 break;
 
 case GEM_TXSTATUS:
-- 
1.7.0.4

[Qemu-devel] [PATCH v1 0/5] Zynq GEM updates

[Peter Crosthwaite]

Misc fixes and cleanups to the Zynq GEM device model. Patch 1 fixes a flow 
control issue talking to the network. Patches 2 and 5 are trivial. Patches 3 & 
4 correct inaccuracies in the device model.

Peter Crosthwaite (5):
  cadence_gem: Flush queued packets
  cadence_gem: factor out can_rx() logic replication
  cadence_gem: fix interrupt events
  cadence_gem: Dont reset rx desc pointer on rx_en
  cadence_gem: Add debug msgs for rx desc movement

 hw/cadence_gem.c |   36 +---
 1 files changed, 9 insertions(+), 27 deletions(-)

Re: [Qemu-devel] [RFC PATCH 02/10] Fix errors and warnings while compiling with c++ compilier

[Tomoki Sekiyama]

On 2013/02/14 21:27, Luiz Capitulino wrote:
> On Thu, 14 Feb 2013 15:10:36 +0900
> Tomoki Sekiyama  wrote:
> 
>> Rename 'class' member in class_info of PciDeviceInfo to 'dev_class', and
>> add some casts to avoid errors from c++ compiler.
> 
> [...]
> 
>>  #
>>  # @class_info.desc: #optional a string description of the device's class
>>  #
>> -# @class_info.class: the class code of the device
>> +# @class_info.dev_class: the class code of the device
>>  #
>>  # @id.device: the PCI device id
>>  #
>> @@ -1171,7 +1171,7 @@
>>  ##
>>  { 'type': 'PciDeviceInfo',
>>'data': {'bus': 'int', 'slot': 'int', 'function': 'int',
>> -   'class_info': {'*desc': 'str', 'class': 'int'},
>> +   'class_info': {'*desc': 'str', 'dev_class': 'int'},
>> 'id': {'device': 'int', 'vendor': 'int'},
>> '*irq': 'int', 'qdev_id': 'str', '*pci_bridge': 'PciBridgeInfo',
>> 'regions': ['PciMemoryRegion']} }
> 
> The right way of doing this is to add 'class' to the set of reserved
> words in scripts/qapi.py:c_var(). Then you'll have to adapt the code to
> use the 'q_' prefix.

Thank you for the information, I will try that.

> Now, is using C++ required? Why can't you use plain C?

It is because Windows COM+ framework (which VSS uses) is designed based
on C++ objective programming interface. Implementing this with plain C
is theoretically possible, but that will require parsing C++ objects'
vtables manually so the code would be much complex.
(However, It might be possible to push Windows-specific C++ stuff into
 a DLL to and avoid involving qemu related headers.)

Thanks,
-- 
Tomoki Sekiyama

Re: [Qemu-devel] [RFC PATCH 03/10] qemu-ga: Add an configure option to specify path to Windows VSS SDK

[Tomoki Sekiyama]

Hi Paolo,

On 2013/02/14 23:36, Paolo Bonzini wrote:
> Il 14/02/2013 07:10, Tomoki Sekiyama ha scritto:
>> To enable VSS support in qemu-ga for Windows, header files included in
>> VSS SDK is required.
>> The VSS support is enabled when the option like below:
>>   ./configure --with-vss-sdk="/pass/to/VSS SDK"
>>
>> VSS SDK is available from:
>>   http://www.microsoft.com/en-us/download/details.aspx?id=23490
>>
>> To cross-compilie using mingw32 for Linux, you need to setup the SDK on
>> Windows environments to extract headers. You can also use wine to run the
>> setup of SDK on Linux etc.
> 
> You can also use msitools (https://live.gnome.org/msitools; right now
> they are not packaged for any distro, but will be in Fedora soon):
> 
> -
> #! /bin/bash
> 
> # extract-vsssdk-headers
> # Author: Paolo Bonzini 
> 
> set -e
> if test $# = 0 || ! test -f "$1"; then
>   echo 'Usage: extract-vsssdk-headers /path/to/setup.exe
>   exit 1
> fi
> 
> # Extract .MSI file in the .exe, looking for the OLE compound
> # document signature.  Extra data at the end does not matter.
> export LC_ALL=C
> MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1'
> offset=`grep -abom1 "$MAGIC" setup.exe | sed -n 's/:/\n/; P' `
> (dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi
> 
> # Now extract the files.
> tmpdir=tmp$$
> mkdir $tmpdir
> msiextract -C $tmpdir vsssdk.msi
> mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc
> rm -rf $tmpdir vsssdk.msi
> exit 0
> -
> 
> Can you add this in scripts/extract-vsssdk-headers please?

Thank you for the code (tricky!), I will add this.

>>  ##
>> +# check if we have VSS SDK headers for win
>> +
>> +if test "$mingw32" = "yes" -a "$guest_agent" = "yes" ; then
>> +  case "$vss_win32_sdk" in
>> +"")   vss_win32_include="" ;;
>> +*\ *) # The SDK is installed in "Program Files" by default, but we 
>> cannot
>> +  # handle path with spaces. So we copy the headers into ".sdk/sdk".
>> +  vss_win32_include="-I$source_path/.sdk/vss"
>> +  symlink "$vss_win32_sdk/inc" "$source_path/.sdk/vss/inc"
>> +  ;;
>> +*)vss_win32_include="-I$vss_win32_sdk"
>> +  esac
> 
> Please also add support for these:
> 
> --with-vss-sdk=no and --without-vss-sdk to disable VSS
> 
> --with-vss-sdk (with no path) is the same as "--with-vss-sdk=", but
> should fail if the program does not compile.
> 
> The default should be what you have now, i.e. test and proceed according
> to the result.

I see.

>> +  cat > $TMPC << EOF
>> +#define __MIDL_user_allocate_free_DEFINED__
>> +#include 
>> +int main(void) { return VSS_CTX_BACKUP; }
>> +EOF
>> +  if compile_prog "$vss_win32_include" "" ; then
>> +guest_agent_with_vss="yes"
>> +QEMU_CFLAGS="$QEMU_CFLAGS $vss_win32_include"
>> +libs_qga="-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ 
>> -Wl,--enable-stdcall-fixup $libs_qga"
>> +  else
>> +if test "$vss_win32_sdk" != "" ; then
>> +  echo "ERROR: Please download and install Microsoft VSS SDK from"
>> +  echo "ERROR: 
>> http://www.microsoft.com/en-us/download/details.aspx?id=23490";
> 
> Please add a note here detailing how to extract the headers on POSIX
> systems.

OK, thanks again.

> Paolo
> 
>> +  feature_not_found "VSS support"
>> +fi
>> +guest_agent_with_vss="no"
>> +  fi
>> +fi
>> +
>> +##
>>  
>>  ##
>>  # check if we have fdatasync
>> @@ -3343,6 +3380,7 @@ echo "usb net redir $usb_redir"
>>  echo "OpenGL support$opengl"
>>  echo "libiscsi support  $libiscsi"
>>  echo "build guest agent $guest_agent"
>> +echo "QGA VSS support   $guest_agent_with_vss"
>>  echo "seccomp support   $seccomp"
>>  echo "coroutine backend $coroutine_backend"
>>  echo "GlusterFS support $glusterfs"
>> @@ -3404,6 +3442,9 @@ if test "$mingw32" = "yes" ; then
>>version_micro=0
>>echo 
>> "CONFIG_FILEVERSION=$version_major,$version_minor,$version_subminor,$version_micro"
>>  >> $config_host_mak
>>echo 
>> "CONFIG_PRODUCTVERSION=$version_major,$version_minor,$version_subminor,$version_micro"
>>  >> $config_host_mak
>> +  if test "$guest_agent_with_vss" = "yes" ; then
>> +echo "CONFIG_QGA_VSS=y" >> $config_host_mak
>> +  fi
>>  else
>>echo "CONFIG_POSIX=y" >> $config_host_mak
>>  fi
>>
>>
>>

-- 
Tomoki Sekiyama

Re: [Qemu-devel] [RFC PATCH 03/10] qemu-ga: Add an configure option to specify path to Windows VSS SDK

[Tomoki Sekiyama]

On 2013/02/15 9:47, mdroth wrote:
[...]
>
> Hi Tomoki,

Hi,

> Did you happen to run into this issue compiling the test program?
>
> [mdroth@vm qemu-build]$ ls ~/w/vsssdk/inc/win2003/
> vdslun.hvsbackup.hvscoordint.idl  vsmgmt.idl  vsprov.idl
> vss.idlvsswprv.idl
> vdslun.idl  vscoordint.h  vsmgmt.hvsprov.hvss.h
> vsswprv.h  vswriter.h
> [mdroth@vm qemu-build]$ cat test.c
> #define __MIDL_user_allocate_free_DEFINED__
> #include 
> int main(void) { return VSS_CTX_BACKUP; }
> [mdroth@vm qemu-build]$ gcc -I ~/w/vsssdk/ -o test test.c
> In file included from test.c:2:0:
> /home/mdroth/w/vsssdk/inc/win2003/vss.h:25:17: fatal error: rpc.h: No
> such file or directory
> compilation terminated.
>
> I can't seem to locate any mingw or microsoft package that provides that.
> It's also not present anywhere on the Wine filesystem I installed the
> VSS SDK to.
>

Hmm, I haven't seen this yet.
I am testing this on Fedora 18 x86_64 host, and it provides package
"mingw32-headers.noarch" or "mingw64-headers.noarch" which contains
the rpc.h
(filepath is /usr/x86_64-w64-mingw32/sys-root/mingw/include/rpc.h )

-- 
Tomoki Sekiyama 
Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory

Re: [Qemu-devel] [RFC PATCH 04/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze

[Tomoki Sekiyama]

Hi Paolo, thanks for your review.

On 2013/02/14 22:22, Paolo Bonzini wrote:> Il 14/02/2013 07:10, Tomoki
Sekiyama ha scritto:
>> diff --git a/qga/vss-win32-provider/Makefile
b/qga/vss-win32-provider/Makefile
>> new file mode 100644
>> index 000..1f213f2
>> --- /dev/null
>> +++ b/qga/vss-win32-provider/Makefile
>> @@ -0,0 +1,30 @@
>> +-include ../../config-host.mak
>> +-include ../../rules.mak
>
> Please try to use a non-recursive makefile style.  See
> libcacard/Makefile for an example.

OK.

>> +# To build .tlb from .idl, WindowsSDK and C++ must be installed
>> +MIDL=midl
>> +WINSDK="C:\\Program Files\\Microsoft SDKs\\Windows\\v7.1\\Include"
>
> When cross-compiling, does it work to use Wine's widl implementation?
> widl needs a "-t" flag when creating a .tlb, but otherwise should be
> compatible.  And unlike some other wine tools, it has no dependency on
> the Wine runtime.
>
> It would be great to choose between midl or widl in configure.  It's
> okay to require a specific option (like --midl=FOO) when compiling under
> Windows, and to require installation of Wine tools to build the VSS
> stuff under non-Windows.

I see. I am not sure whether this works without importlib line,
but if it does, I will add the configuration for use widl.

Thanks,
-- 
Tomoki Sekiyama

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Kevin O'Connor]

On Fri, Feb 15, 2013 at 04:10:59AM +0100, Laszlo Ersek wrote:
> On 02/15/13 02:22, Kevin O'Connor wrote:
> > On Thu, Feb 14, 2013 at 08:16:02PM -0500, Kevin O'Connor wrote:
> > By chance, are you using an older version of kvm?  There was a bug in
> > kvm that caused changes to memory mapped at 0xe-0xf to also be
> > reflected in the "rom" image at 0xfffe-0x.  It was my
> > understand that this bug was fixed though.
> 
> You are great! Disabling KVM for the guest (/domain/@type='qemu') made
> the reboot work on both the RHEL-6 devel version of qemu and on upstream
> 1.3.1.
> 
> (I didn't try suspend/resume yet.)
> 
> Do you recall the precise commit that fixed the "reflection"? I've been
> eyeballing kvm commit messages for a few ten minutes now, but of course
> in vain. (CC'ing Gleb and Marcelo.)

I found this email thread:

http://kerneltrap.org/mailarchive/linux-kvm/2010/9/21/6267744

and: http://marc.info/?l=kvm-commits&m=128576215909532

-Kevin

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Laszlo Ersek]

On 02/15/13 02:22, Kevin O'Connor wrote:
> On Thu, Feb 14, 2013 at 08:16:02PM -0500, Kevin O'Connor wrote:
>> On Fri, Feb 15, 2013 at 12:01:38AM +0100, Laszlo Ersek wrote:
>>> On 02/14/13 21:55, David Woodhouse wrote:
>>>
 Thanks for testing this, btw. Are you looking at suspend/resume too? :)
>>>
>>> Entering S3 seemed OK (except the screen was not cleared; using Cirrus).
>>> I woke up the guest with
>>>
>>> # virsh qemu-monitor-command fw-ovmf.g-f18xfce2012121716.e-rhel63 \
>>>   --hmp --cmd system_wakeup
>>>
>>> Trailing portion of the log:
>>>
>>>   In resume (status=254)
>>>   In 32bit resume
>>>   rsdp=0x
>>>   No resume vector set!
>>
>> That is strange.  As noted elsewhere, on a resume or reboot the cpu
>> should have started execution at 0xfff0 which is OVMF and not
>> SeaBIOS.  I don't understand why/how SeaBIOS would be involved in the
>> resume code path at all.
> 
> By chance, are you using an older version of kvm?  There was a bug in
> kvm that caused changes to memory mapped at 0xe-0xf to also be
> reflected in the "rom" image at 0xfffe-0x.  It was my
> understand that this bug was fixed though.

You are great! Disabling KVM for the guest (/domain/@type='qemu') made
the reboot work on both the RHEL-6 devel version of qemu and on upstream
1.3.1.

(I didn't try suspend/resume yet.)

Do you recall the precise commit that fixed the "reflection"? I've been
eyeballing kvm commit messages for a few ten minutes now, but of course
in vain. (CC'ing Gleb and Marcelo.)

Thank you,
Laszlo

Re: [Qemu-devel] [RFC PATCH 03/10] qemu-ga: Add an configure option to specify path to Windows VSS SDK

[mdroth]

On Thu, Feb 14, 2013 at 03:10:39PM +0900, Tomoki Sekiyama wrote:
> To enable VSS support in qemu-ga for Windows, header files included in
> VSS SDK is required.
> The VSS support is enabled when the option like below:
>   ./configure --with-vss-sdk="/pass/to/VSS SDK"
> 
> VSS SDK is available from:
>   http://www.microsoft.com/en-us/download/details.aspx?id=23490
> 
> To cross-compilie using mingw32 for Linux, you need to setup the SDK on
> Windows environments to extract headers. You can also use wine to run the
> setup of SDK on Linux etc.
> 
> Signed-off-by: Tomoki Sekiyama 
> ---
>  .gitignore |1 +
>  Makefile   |1 +
>  configure  |   41 +
>  3 files changed, 43 insertions(+)
> 
> diff --git a/.gitignore b/.gitignore
> index 53fe9c3..3f450e8 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -77,6 +77,7 @@ fsdev/virtfs-proxy-helper.pod
>  *.la
>  *.pc
>  .libs
> +.sdk
>  *.swp
>  *.orig
>  .pc
> diff --git a/Makefile b/Makefile
> index 0d9099a..fab664f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -252,6 +252,7 @@ distclean: clean
>   for d in $(TARGET_DIRS); do \
>   rm -rf $$d || exit 1 ; \
>  done
> + rm -Rf .sdk
>   if test -f pixman/config.log; then make -C pixman distclean; fi
> 
>  KEYMAPS=da en-gb  et  fr fr-ch  is  lt  modifiers  no  pt-br  sv \
> diff --git a/configure b/configure
> index e279263..da49c52 100755
> --- a/configure
> +++ b/configure
> @@ -220,6 +220,8 @@ usb_redir=""
>  opengl=""
>  zlib="yes"
>  guest_agent="yes"
> +guest_agent_with_vss="no"
> +vss_win32_sdk=""
>  want_tools="yes"
>  libiscsi=""
>  coroutine=""
> @@ -884,6 +886,8 @@ for opt do
>;;
>--disable-guest-agent) guest_agent="no"
>;;
> +  --with-vss-sdk=*) vss_win32_sdk="$optarg"
> +  ;;
>--enable-tools) want_tools="yes"
>;;
>--disable-tools) want_tools="no"
> @@ -1142,6 +1146,7 @@ echo "  --disable-usb-redir  disable usb network 
> redirection support"
>  echo "  --enable-usb-redir   enable usb network redirection support"
>  echo "  --disable-guest-agentdisable building of the QEMU Guest Agent"
>  echo "  --enable-guest-agent enable building of the QEMU Guest Agent"
> +echo "  --with-vss-sdk=SDK-path  enable Windows VSS support in QEMU Guest 
> Agent"
>  echo "  --disable-seccompdisable seccomp support"
>  echo "  --enable-seccomp enables seccomp support"
>  echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
> @@ -2897,6 +2902,38 @@ if test "$usb_redir" != "no" ; then
>  fi
> 
>  ##
> +# check if we have VSS SDK headers for win
> +
> +if test "$mingw32" = "yes" -a "$guest_agent" = "yes" ; then
> +  case "$vss_win32_sdk" in
> +"")   vss_win32_include="" ;;
> +*\ *) # The SDK is installed in "Program Files" by default, but we cannot
> +  # handle path with spaces. So we copy the headers into ".sdk/sdk".
> +  vss_win32_include="-I$source_path/.sdk/vss"
> +  symlink "$vss_win32_sdk/inc" "$source_path/.sdk/vss/inc"
> +   ;;
> +*)vss_win32_include="-I$vss_win32_sdk"
> +  esac
> +  cat > $TMPC << EOF
> +#define __MIDL_user_allocate_free_DEFINED__
> +#include 
> +int main(void) { return VSS_CTX_BACKUP; }
> +EOF

Hi Tomoki,

Did you happen to run into this issue compiling the test program?

[mdroth@vm qemu-build]$ ls ~/w/vsssdk/inc/win2003/
vdslun.hvsbackup.hvscoordint.idl  vsmgmt.idl  vsprov.idl
vss.idlvsswprv.idl
vdslun.idl  vscoordint.h  vsmgmt.hvsprov.hvss.h
vsswprv.h  vswriter.h
[mdroth@vm qemu-build]$ cat test.c 
#define __MIDL_user_allocate_free_DEFINED__
#include 
int main(void) { return VSS_CTX_BACKUP; }
[mdroth@vm qemu-build]$ gcc -I ~/w/vsssdk/ -o test test.c
In file included from test.c:2:0:
/home/mdroth/w/vsssdk/inc/win2003/vss.h:25:17: fatal error: rpc.h: No
such file or directory
compilation terminated.

I can't seem to locate any mingw or microsoft package that provides that.
It's also not present anywhere on the Wine filesystem I installed the
VSS SDK to.

> +  if compile_prog "$vss_win32_include" "" ; then
> +guest_agent_with_vss="yes"
> +QEMU_CFLAGS="$QEMU_CFLAGS $vss_win32_include"
> +libs_qga="-lole32 -loleaut32 -lshlwapi -luuid -lstdc++ 
> -Wl,--enable-stdcall-fixup $libs_qga"
> +  else
> +if test "$vss_win32_sdk" != "" ; then
> +  echo "ERROR: Please download and install Microsoft VSS SDK from"
> +  echo "ERROR: 
> http://www.microsoft.com/en-us/download/details.aspx?id=23490";
> +  feature_not_found "VSS support"
> +fi
> +guest_agent_with_vss="no"
> +  fi
> +fi
> +
> +##
> 
>  ##
>  # check if we have fdatasync
> @@ -3343,6 +3380,7 @@ echo "usb net redir $usb_redir"
>  echo "OpenGL support$opengl"
>  echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
> +echo "QGA VSS support   $guest_agent_with_vs

Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.

[Antoine Mathys]

First, the ds1338 code was in a poor state and never handled the 12 hour 
clock correctly. My first patch failed to fully fix the problem so I had 
to write a second one, but at no point did Peter or I introduce a 
regression, quite the opposite.


Second, I don't know where you got the idea that I refuse to write test 
cases. I just didn't have one ready or in the works at the time.


Third, bug 1090558 in mc146818rtc is a good example of a bug which was 
not due to insufficient testing, but to poorly structured code.


There is no point worrying about unit testing if you accept code of such 
low quality. This goes for the tests too. For instance 
cmos_get_date_time() in tests/rtc-test.c doesn't work correctly in 12 
hour mode.


Fourth, I am not interested in the PC architecture, I only wrote a fix 
for bug 1090558 because Paolo asked me to. It is nice to see that fixing 
your crappy code makes me "not a nice guy" who is making things worse. 
But don't worry, I'll focus on ARM from now on.

Re: [Qemu-devel] [edk2] [SeaBIOS] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Andrew Fish]

On Feb 14, 2013, at 2:09 PM, "H. Peter Anvin"  wrote:

> On 02/14/2013 01:27 PM, David Woodhouse wrote:
>> 
>> So it *is* jumping to 0xfff0 but the memory at that location isn't
>> what we expect? Do the PAM registers affect *that* too, or only the
>> region from 0xc-0xf? Surely the contents at 4GiB-δ should be
>> unchanged by *anything* we do with the PAM registers?
>> 
>> Or maybe not... after also downloading the i440fx data sheet, I'm even
>> more confused. There's some aliasing with... not the region at 1MiB-δ
>> but the region at 16MiB-δ:
>> 

I don't remember the specific registers for the 440BX

 The i486 moved the reset vector to 0xFFF0, but it is in real mode. The 
processor CS register has some magic internal value that lets you run real mode 
code  up high, but the 1st long jmp you do sends you down low. Thus the chipset 
needs to alias 0xF000:0xFFF0 to the high address. If you BIOS is written in 
protected mode then it will turn on the HIgh BIOS Area and jump back into the 
just under the 4GB region and now it has access to a ROM that can be up to 2MB 
in size after it turns on the high BIOS area. 

If you hardware reset the PAM registers should get set back to defaults, and 
CPU goes into the reset state.
If you soft (also called warm) reset, jump to 0xF000:0xFFF0 then, you are not 
running the reset code in ROM (called SEC in the PI lingo) you are running the 
shadowed copy from memory provided by the SeaBIOS for  compatibility. 

Thanks,

Andrew

>> (From §4.1 System Address Map):
>> 
>> 2. High BIOS Area (FFE0_h−− _h)
>>   The top 2 Mbytes of the Extended Memory Region is reserved for System
>>   BIOS (High BIOS), extended BIOS for PCI devices, and the A20 alias of
>>   the system BIOS. The CPU begins execution from the High BIOS after
>>   reset. This region is mapped to the PCI so that the upper subset of
>>   this region is aliased to 16 Mbytes minus 256-Kbyte range.
>> 
> 
> That is presumably a 286 compatibility hack -- the 286 had 24 address 
> lines.  I doubt anyone gives a hoot about it, and neither EDK2 nor 
> SeaBIOS should care.
> 
>   -hpa
> 
> -- 
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
> 

Re: [Qemu-devel] [seabios][PATCH 1/2] move all acpi-table related definitions to acpi.h

[li guang]

在 2013-02-06三的 23:15 -0500，Kevin O'Connor写道：
> On Mon, Feb 04, 2013 at 10:27:59AM +0800, liguang wrote:
> > Signed-off-by: liguang 
> 
> Thanks.  Some comments.
> 
> [...]
> > --- a/src/acpi.h
> > +++ b/src/acpi.h
> [...]
> > +#include "acpi-dsdt.hex"
> 
> Moving the acpi structure defines to the header is fine, but moving
> the DSDT code into the header is definitely not right.

OK, will remove this include.

> 
> -Kevin
> 

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Kevin O'Connor]

On Thu, Feb 14, 2013 at 08:16:02PM -0500, Kevin O'Connor wrote:
> On Fri, Feb 15, 2013 at 12:01:38AM +0100, Laszlo Ersek wrote:
> > On 02/14/13 21:55, David Woodhouse wrote:
> > 
> > > Thanks for testing this, btw. Are you looking at suspend/resume too? :)
> > 
> > Entering S3 seemed OK (except the screen was not cleared; using Cirrus).
> > I woke up the guest with
> > 
> > # virsh qemu-monitor-command fw-ovmf.g-f18xfce2012121716.e-rhel63 \
> >   --hmp --cmd system_wakeup
> > 
> > Trailing portion of the log:
> > 
> >   In resume (status=254)
> >   In 32bit resume
> >   rsdp=0x
> >   No resume vector set!
> 
> That is strange.  As noted elsewhere, on a resume or reboot the cpu
> should have started execution at 0xfff0 which is OVMF and not
> SeaBIOS.  I don't understand why/how SeaBIOS would be involved in the
> resume code path at all.

By chance, are you using an older version of kvm?  There was a bug in
kvm that caused changes to memory mapped at 0xe-0xf to also be
reflected in the "rom" image at 0xfffe-0x.  It was my
understand that this bug was fixed though.

-Kevin

Re: [Qemu-devel] [seabios][PATCH 2/2] acpi: change numa data format from fw_cfg interface

[li guang]

Sorry for late reply

在 2013-02-06三的 23:18 -0500，Kevin O'Connor写道：
> On Mon, Feb 04, 2013 at 10:28:00AM +0800, liguang wrote:
> > the old numa format got form fw_cfg is:
> > number of nodes
> > node id of cpu (array)
> > node memory size (array)
> > 
> > now, format it like array of:
> > apci_map,
> > memory_size,
> > 
> > it has the advantage of simple and clear.
> 
> With this change, will old versions of seabios work with new versions
> of qemu, and old versions of qemu work with new versions of seabios?

may be not.

> 
> Also, can you provide a high-level summary of why this change is
> useful to an end-user?

This change have nothing to do with end-user,
just for clear design.

> 
> -Kevin
> 

Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently

[Laszlo Ersek]

On 02/15/13 01:20, Laszlo Ersek wrote:
> On 02/14/13 17:36, Luiz Capitulino wrote:
>> On Thu, 14 Feb 2013 14:31:50 +0100
>> Markus Armbruster  wrote:

>>> chardev-add: the schema defines an object type for each backend
>>> (ChardevFile, ChardevSocket, ...), and collects them together in
>>> discriminated union ChardevBackend.  chardev_add takes that union.
>>> Thus, the schema accurately describes chardev-add's arguments, and the
>>> generated marshaller takes care of parsing chardev-add arguments into
>>> the appropriate objects.
>>
>> Yes, it's a nice solution. I don't know why we didn't have that idea
>> when discussing netdev_add. Maybe we were biased by the old
>> implementation.
>>
>>> netdev_add: the schema defines an object type for each backend
>>> (NetdevNoneOptions, NetdevUserOptions, ...).  netdev_add doesn't use
>>> them, but takes arbitrary parameters instead.  The connection is made in
>>> code, which stuffs the parameters in a QemuOpts (losing all JSON type
>>> information), then takes them out again to stuff them into the
>>> appropriate object type.  A job the marshaller should be able to do for
>>> us.
>>>
>>> For me, the way chardev-add works makes a whole lot more sense, and I
>>> think we should clean up netdev_add to work the same way.

So, regarding netdev_add again,

--[schema dict]--> qmp_netdev_add()  ---\
--[QemuOpts]-->net_client_init() == opts-visitor ---\
--[C struct]-->specific logic

(a) I agree that the intermediary QemuOpts representation is
superfluous, and that we could directly expose the schema over QMP, ie.
go from schema dict right to C struct. However,

(b) opts-visitor's primary responsibility remains mangling one QemuOpts
instance into a C struct. This effectively hamstrings any affected
schema. You *can* choose to expose/reuse that schema over the wire, but
you won't have any freedom massaging the schema later on just for the
QMP caller's sake.

--[schema dict]--> qmp_netdev_add() --[C struct]--> specific logic
--[QemuOpts]-->opts-visitor --[C struct]--> specific logic

Obviously, you want to share the [C struct] and the "specific logic"
between the two cases. However the C struct (the schema) is hamstrung by
QemuOpts, thus ultimately the QMP wire format is dictated by the
QemuOpts format that you accept on the command line.

At first sight this might come through as a "semantic match" (same stuff
available on the command line as over QMP), but:
- you can't compose the underlying schema just any way you like,
- what you can't express as a single QemuOpts object (think dictionaries
in dictionaries), you can't allow over QMP.

With chardev_add, IIUC, first you create a logical schema, and expose it
over QMP (all dandy), then hand-craft qemu_opt_get_() code, with
properties encoded as C string literals, in order to shoehorn the
logical schema into the command line (QemuOpts). I couldn't call this
approach "bad" with a straight face (it clearly works in practice!), but
as I perceived it, opts-visitor had been invented precisely to eliminate
this.

Sorry for ranting...

Laszlo

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Kevin O'Connor]

On Fri, Feb 15, 2013 at 12:01:38AM +0100, Laszlo Ersek wrote:
> On 02/14/13 21:55, David Woodhouse wrote:
> 
> > Thanks for testing this, btw. Are you looking at suspend/resume too? :)
> 
> Entering S3 seemed OK (except the screen was not cleared; using Cirrus).
> I woke up the guest with
> 
> # virsh qemu-monitor-command fw-ovmf.g-f18xfce2012121716.e-rhel63 \
>   --hmp --cmd system_wakeup
> 
> Trailing portion of the log:
> 
>   In resume (status=254)
>   In 32bit resume
>   rsdp=0x
>   No resume vector set!

That is strange.  As noted elsewhere, on a resume or reboot the cpu
should have started execution at 0xfff0 which is OVMF and not
SeaBIOS.  I don't understand why/how SeaBIOS would be involved in the
resume code path at all.

-Kevin

Re: [Qemu-devel] [PATCH moxie 3/5] Moxie target code

[Richard Henderson]

On 02/14/2013 03:19 PM, Richard Henderson wrote:
>> > +tcg_gen_brcond_i32(TCG_COND_EQ, REG(a), REG(b), label_equal);
>> > +
>> > +#define CMPTEST(t,CODE) \
>> > +{   \
>> > +  int lyes = gen_new_label();   \
>> > +  int lno = gen_new_label();\
>> > +  TCGv t1 = new_tmp();  \
>> > +  tcg_gen_brcond_i32(t, REG(a), REG(b), lyes);  \
>> > +  tcg_gen_br(lno);  \
>> > +  gen_set_label(lyes);  \
>> > +  tcg_gen_ori_i32(t1, cc, CODE);\
>> > +  tcg_gen_mov_i32(cc, t1);  \
>> > +  gen_set_label(lno);   \
>> > +  dead_tmp(t1); \
> Consider making use of tcg_setcond_i32 here.  It's probably new since
> you wrote all this in the first place.
> 

That said, I wonder if the amount of code generated here even with
setcond would be too big.

The quick and easy option is to use a helper function.  If we mark it
as-if gcc's pure, aka TCG_CALL_NO_RWG_SE, it should be relatively
efficient, as far as TCG goes:

target_long helper_cmp(target_long x, target_long y)
{
target_ulong ux = x, uy = y;
if (x == y) {
return CC_EQ;
}
return (x < y ? CC_LT : CC_GT) | (ux < uy ? CC_LTU : CC_GTU);
}

Optimizes pretty well inside gcc:

cmpl%esi, %edi
movl$4, %eax
je  .L11
setl%dl
movzbl  %dl, %edx
addl$1, %edx
cmpl%esi, %edi
sbbl%eax, %eax
andl$8, %eax
addl$8, %eax
orl %edx, %eax
.L11:
rep
ret

Now, given that CC is only settable via CMP, and apparently only
readable via branches. means that we could optimize things a bit.

What if the CMP insn merely copies its inputs to TCG globals CMP1 and
CMP2?  Then the actual implementation of BEQ (et al) is

tcg_gen_brcond_i32(TCG_COND_EQ, cpu_cmp1, cpu_cmp2, lab_true);

Since these are globals, a subsequent BGT in a different TB will still
read the same values and perform the appropriate comparison in
performing its branch.

What I don't know is how this affects the moxie exception model, of
which I can find no information.  There doesn't seem to be any
implementation of such a model in the patches sent here.

When saving/restoring the CC value, one could presumably translate the
two variables into a canonical CC code as above, and for the reverse
transformation chose two values that create any valid comparison.

cmp1cmp2
EQ  0   0
LT & LTU0   1
LT & GTU-1  1
GT & LTU1   -1
GT & GTU1   0

as everything else is logically impossible.


r~

Re: [Qemu-devel] [PATCH for-1.4 07/19] target-sparc: Fix debug output for DEBUG_MMU

[Andreas Färber]

Am 14.02.2013 22:42, schrieb Blue Swirl:
> On Thu, Feb 14, 2013 at 3:46 PM, Andreas Färber  wrote:
>> Am 27.01.2013 14:32, schrieb Andreas Färber:
>>> Signed-off-by: Andreas Färber 
>>> ---
>>>  target-sparc/ldst_helper.c |2 +-
>>>  1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
>>>
>>> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
>>> index cf1bddf..7decd66 100644
>>> --- a/target-sparc/ldst_helper.c
>>> +++ b/target-sparc/ldst_helper.c
>>> @@ -1850,7 +1850,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
>>> addr, target_ulong val,
>>>  DPRINTF_MMU("LSU change: 0x%" PRIx64 " -> 0x%" PRIx64 "\n",
>>>  oldreg, env->lsu);
>>>  #ifdef DEBUG_MMU
>>> -dump_mmu(stdout, fprintf, env1);
>>> +dump_mmu(stdout, fprintf, env);
>>>  #endif
>>>  tlb_flush(env, 1);
>>>  }
>>
>> This patch was not applied to master. Was that an oversight or is
>> something wrong with it? (Preparing a v2 series for 1.5.)
> 
> Wasn't this a part of RFC series? What happened with the rest of the 19?

This is a standalone bugfix, originally intended for 1.4 as indicated by
the subject.

cris, ppc and s390x bugfixes were applied through the maintainers during
Soft Freeze.

The cris Coding Style cleanup is still pending, no feedback from Edgar
on whether it is okay or not yet (already 3x on the list).

The actual RFC patches based on those are being redone similar to the
tmp105 v3 patch I ping'ed, based on suggestions from Anthony and Alex.
I inserted a patch to add qemu_log_mask_vprintf() as prerequisite.

Andreas

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

Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently

[Laszlo Ersek]

Hi,

sorry for the late answer. I can only address the netdev_add /
opts-visitor stuff now.

On 02/14/13 17:36, Luiz Capitulino wrote:
> On Thu, 14 Feb 2013 14:31:50 +0100
> Markus Armbruster  wrote:
>> Luiz Capitulino  writes:
>>> On Thu, 14 Feb 2013 10:45:22 +0100
>>> Markus Armbruster  wrote:

 netdev_add() uses QAPI wizardry to create the appropriate object from
 the QemuOpts.  The parsing is done by visitors.  Things get foggy for me
 from there on, but it looks like one of them, opts_type_size(), can
 parse size suffixes, which is inappropriate for QMP.  A quick test
 confirms that this is indeed possible:

 {"execute": "netdev_add","arguments": {"type":"tap","id":"net.1",
 "sndbuf": "8k" }}

 Sets NetdevTapOptions member sndbuf to 8192.
>>>
>>> Well, I've just tested this with commit 783e9b4, which is before
>>> netdev_add conversion to the qapi, and the command above just works.
>>>
>>> Didn't check if sndbuf is actually set to 8192, but this shows that
>>> we've always accepted such a command.
>>
>> Plausible.  Nevertheless, we really shouldn't.
> 
> Agreed. I would be willing to break compatibility to fix the suffix
> part of the problem, but there's another issue: sndbuf shouldn't be
> a string in QMP, and that's unfixable.

The main purpose / requirement / guideline of the netdev_add &
opts-visitor conversion was that the exact same command lines had to
keep working. The primary source of input was the command line, ie.
QemuOpts. The qapi-schema was absolutely bastardized for the task, with
the single goal that the QemuOpts stuff *already gotten from the user*
would be auto-parsed into C structs -- structs that were generated from
the json. So, no QMP callers had been in anyone's mind as a priority;
the qapi/visitor etc. scaffolding was retrofitted to QemuOpts.

Please read the message on the main commit of the series:

  http://git.qemu.org/?p=qemu.git;a=commitdiff;h=eb7ee2cb

plus here's the blurb:

  http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02102.html


 Netdev could be done without this key=value business in the schema.  We
 have just a few backends, and they're well-known, so we could just
 collect them all in a union, like Gerd did for chardev backends.

The -netdev option centers on [type=]discriminator, and it had to work
as transparently as possible. I can't recall all the details any longer
(luckily!), but I remember sweating bullets wrapping the visitor API
around QemuOpts.

>>> I honestly don't know if this is a good idea, but should be possible
>>> to change it if you're willing to.
>>
>> chardev-add: the schema defines an object type for each backend
>> (ChardevFile, ChardevSocket, ...), and collects them together in
>> discriminated union ChardevBackend.  chardev_add takes that union.
>> Thus, the schema accurately describes chardev-add's arguments, and the
>> generated marshaller takes care of parsing chardev-add arguments into
>> the appropriate objects.
> 
> Yes, it's a nice solution. I don't know why we didn't have that idea
> when discussing netdev_add. Maybe we were biased by the old
> implementation.
> 
>> netdev_add: the schema defines an object type for each backend
>> (NetdevNoneOptions, NetdevUserOptions, ...).  netdev_add doesn't use
>> them, but takes arbitrary parameters instead.  The connection is made in
>> code, which stuffs the parameters in a QemuOpts (losing all JSON type
>> information), then takes them out again to stuff them into the
>> appropriate object type.  A job the marshaller should be able to do for
>> us.
>>
>> For me, the way chardev-add works makes a whole lot more sense, and I
>> think we should clean up netdev_add to work the same way.
>> Unfortunately, I can't commit to this cleanup job right now.
> 
> Agreed, and I think we won't break compatibility by doing this
> improvement.

The most important things to compare are qemu_chr_new_from_opts() and
net_client_init(), if my reading is correct. Each is the corresponding
iteration callback for the list of chardev/netdev list of QemuOpts.

(a) qemu_chr_new_from_opts() dives in, fishes out the discriminator
manually -- "backend" is encoded as a C string literal, and there's a
similar access to "mux" --, and looks up the appropriate open function
based on backend (with linear search & strcmp()).

No matter which open function we choose, each one is chock-full of
qemu_opt_get_() calls with property names hard-coded as C string
literals. *Killing this* was the exact purpose of opts-visitor. Cf.:

(b) net_client_init() parses the QemuOpts object into a C struct, based
on the schema, validating basic syntax simultaneously. Then
net_client_init1(), rather than a linear, string-based search, uses the
enum value ("kind") as the controlling expression of a switch statement,
and as immediate offset into the array of function pointers.

None of those init functions makes one qemu_opt_get_() call with a
hard-coded property na

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall

[Alexander Graf]

On 13.02.2013, at 06:21, David Gibson wrote:

> On Tue, Feb 12, 2013 at 11:07:10PM +0100, Alexander Graf wrote:
>> 
>> On 07.02.2013, at 12:28, Erlon Cruz wrote:
>> 
>>> From: Erlon Cruz 
>>> 
>>> This h_call is useful for DLPAR in future amongst other things. Given an 
>>> index
>>> it fetches the corresponding PTE stored in the htab.
>>> 
>>> Signed-off-by: Erlon Cruz 
>>> ---
>>> hw/spapr_hcall.c |   58 
>>> ++
>>> 1 file changed, 58 insertions(+)
>>> 
>>> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
>>> index 2889742..5ba07e5 100644
>>> --- a/hw/spapr_hcall.c
>>> +++ b/hw/spapr_hcall.c
>>> @@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu, 
>>> sPAPREnvironment *spapr,
>>>return H_SUCCESS;
>>> }
>>> 
>>> +static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>>> +target_ulong opcode, target_ulong *args)
>>> +{
>>> +CPUPPCState *env = &cpu->env;
>>> +target_ulong flags = args[0];
>>> +target_ulong pte_index = args[1];
>>> +uint8_t *hpte;
>>> +
>>> +if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
>>> +return H_PARAMETER;
>>> +}
>>> +
>>> +if (!(flags & H_READ_4)) {
>>> +target_ulong v, r;
>>> +target_ulong *pteh = &args[0];
>>> +target_ulong *ptel = &args[1];
>>> +
>>> +hpte = env->external_htab + (pte_index * HASH_PTE_SIZE_64);
>> 
>> You are not guaranteed that there is an external htab.
> 
> Actually in the case of spapr, you are - the existing hash table
> management calls all assume the existence of an external htab.

Ok, just leave the code using external_htab and we'll make it use the helpers 
once they're there.


Alex

> 
>> In fact, looking at the external_htab users, we should probably
>> introduce a few helper read functions for the htab that abstract the
>> glorious external_htab/htab_base details away from you.
> 
> That said, I actually wrote such helpers about 15 minutes ago as part
> of my MMU cleanup series.
> 
> -- 
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
> _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall

[David Gibson]

On Thu, Feb 14, 2013 at 10:44:34AM -0200, Erlon Cruz wrote:
> On Sun, Feb 10, 2013 at 10:10 PM, David Gibson
>  wrote:
> >
> > On Thu, Feb 07, 2013 at 09:28:20AM -0200, Erlon Cruz wrote:
> > > From: Erlon Cruz 
> > >
> > > This h_call is useful for DLPAR in future amongst other things. Given an 
> > > index
> > > it fetches the corresponding PTE stored in the htab.
> >
> > Nice.  It would be good to add in this little bit of PAPR compliance.
> >
> > Couple of small nits in the implementation:
> >
> > >
> > > Signed-off-by: Erlon Cruz 
> > > ---
> > >  hw/spapr_hcall.c |   58 
> > > ++
> > >  1 file changed, 58 insertions(+)
> > >
> > > diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
> > > index 2889742..5ba07e5 100644
> > > --- a/hw/spapr_hcall.c
> > > +++ b/hw/spapr_hcall.c
> > > @@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu, 
> > > sPAPREnvironment *spapr,
> > >  return H_SUCCESS;
> > >  }
> > >
> > > +static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> > > +target_ulong opcode, target_ulong *args)
> > > +{
> > > +CPUPPCState *env = &cpu->env;
> > > +target_ulong flags = args[0];
> > > +target_ulong pte_index = args[1];
> > > +uint8_t *hpte;
> > > +
> > > +if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> > > +return H_PARAMETER;
> > > +}
> > > +
> > > +if (!(flags & H_READ_4)) {
> >
> > It would be nice to combine the H_READ_4 and !H_READ_4 paths together,
> > since except for the masking and stopping sooner the !H_READ_4 path is
> > just like the H_READ_4 path.
> 
> Ok.
> 
> >
> >
> > > +target_ulong v, r;
> > > +target_ulong *pteh = &args[0];
> > > +target_ulong *ptel = &args[1];
> > > +
> > > +hpte = env->external_htab + (pte_index * HASH_PTE_SIZE_64);
> > > +
> > > +v = ldq_p(hpte);
> > > +r = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
> > > +
> > > +if (flags & H_R_XLATE) {
> > > +/* FIXME:  include a valid logical page num in the pte */
> >
> > This comment is misleading.  Since you do copy out both words of the
> > hpte, and qemu stores the external_htab in terms of guest physical (==
> > logical) addresses, in fact you're *always* supplying a valid logical
> > page num.  So you've already correctly implemented the flag as a
> > no-op.
> >
> > I believe that flag is included for the benefit of a true hypervisor,
> > where the native htab would be stored as true physical addresses, and
> > it might be expensive for the hypervisor to recompute the logical
> > address.
> 
> Ok, then I think we can just skip this checking.

Yes.

> > That said, I actually wrote such helpers about 15 minutes ago as part
> > of my MMU cleanup series.
> 
>  Should I wait for the helpers to send It again?

Probably not, my series will be a little while because it will
probably need to wait for the big cpu qomification series.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: Digital signature

Re: [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[David Woodhouse]

On Fri, 2013-02-15 at 00:01 +0100, Laszlo Ersek wrote:
> 
> Entering S3 seemed OK (except the screen was not cleared; using
> Cirrus).
> I woke up the guest with
> 
> # virsh qemu-monitor-command fw-ovmf.g-f18xfce2012121716.e-rhel63 \
>   --hmp --cmd system_wakeup
> 
> Trailing portion of the log:
> 
>   In resume (status=254)
>   In 32bit resume
>   rsdp=0x
>   No resume vector set!
>   Attempting a hard reboot
>   i8042_wait_write
>   In resume (status=0)
>   In 32bit resume
>   Attempting a hard reboot
>   [...]
> 
> I can see the following CSM calls earlier:
> - Legacy16InitializeYourself
> - Legacy16GetTableAddress
> - Legacy16DispatchOprom
> - Legacy16UpdateBbs
> 
> No calls to PrepareToBoot (which could set RsdpAddr); this is an UEFI
> guest. (The CSM is used for the GOP only.)

So you have the same problem as with reset — you're ending up back in
the CSM in RAM, when you ought to be in the OVMF "ROM". 

I wonder if a *Legacy* guest might actually fare a little better? At
least find_resume_vector() would have a chance of working if the CSM has
actually been told where the ACPI tables are...



-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH moxie 3/5] Moxie target code

[Richard Henderson]

On 02/13/2013 02:26 PM, Anthony Green wrote:
> +typedef struct CPUMoxieState {
> +
> +
> +  uint32_t flags;   /* general execution flags */
> +  uint32_t gregs[16];   /* general registers */
> +  uint32_t sregs[256];  /* special registers */
> +  uint32_t pc;
> +  uint32_t cc;/* condition codes */
> +
> +  uint32_t t0;
> +  uint32_t t1;
> +  uint32_t t2;

Are these t[0-2] used anywhere?  I couldn't immediately see.
Such named temporaries were legacy on a couple of targets,
so depending on when you copied this from, e.g. sparc, you
might have gotten these unnecessarily.

> +/* This is the state at translation time.  */
> +typedef struct DisasContext {
> +  struct TranslationBlock *tb;
> +  target_ulong pc, saved_pc;
> +  uint32_t opcode;
> +  uint32_t fp_status;
> +  /* Routine used to access memory */
> +  int memidx;
> +  uint32_t hflags, saved_hflags;
> +  int bstate;
> +  target_ulong btarget;
> +  void *last_T0_store;
> +  int last_T0_gpr;
> +  int singlestep_enabled;
> +} DisasContext;
> +
> +enum {
> +  BS_NONE = 0, /* We go out of the TB without reaching a branch or an
> +* exception condition */
> +  BS_STOP = 1, /* We want to stop translation for any reason */
> +  BS_BRANCH   = 2, /* We reached a branch condition */
> +  BS_EXCP = 3, /* We reached an exception condition */
> +};
> +
> +static TCGv cpu_pc;
> +static TCGv cpu_gregs[16];
> +static TCGv cpu_sregs[256];

You're slowing down the compiler by allocating registers for all of
these SREGS, where they're only used in move to/from insns, and in the
SWI insn.  You'll be better off issuing explicit tcg_gen_ld/st_i32 in
those cases.

> +/* The code generator doesn't like lots of temporaries, so maintain our own
> +   cache for reuse within a function.  */
> +#define MAX_TEMPS 8
> +static int num_temps;
> +static TCGv temps[MAX_TEMPS];

The code generator does fine with quite a few temporaries.  The biggest
problem being that you're never freeing them.  You don't need a local
cache, just use tcg_temp_free_i32 in dead_tmp.

FYI, there are some debugging hooks that might help:
tcg_clear_temp_count, tcg_check_temp_count.  Placing these around e.g.
each individual insn can make sure that you're not leaking.

> +/* Create a new temporary and set it to the value of a CPU register.  */
> +static inline TCGv load_reg(DisasContext *s, int reg)
> +{
> +  TCGv tmp = new_tmp();
> +  tcg_gen_ld_i32(tmp, cpu_env, offsetof(CPUMoxieState, gregs[reg]));
> +  return tmp;
> +}

This function won't work, since there's no synchronization between
cpu_env memory and tcg global registers.  On the good side, it's
actually unused.

Please delete all the dead code.  E.g. by not marking any functions
inline, and letting the compiler work out what should be inlined, and
warning about unused.

> +case 0x00: /* beq */
> +  {
> +int l1 = gen_new_label();
> +tcg_gen_brcondi_i32 (TCG_COND_EQ, cc, CC_EQ, l1);
> +gen_goto_tb(env, ctx, 1, ctx->pc+2);
> +gen_set_label(l1);
> +gen_goto_tb(env, ctx, 0, INST2OFFSET(opcode)+ctx->pc+2);
> +ctx->bstate = BS_BRANCH;
> +  }
> +  break;
> +case 0x01: /* bne */
> +  {
> +int l1 = gen_new_label();
> +tcg_gen_brcondi_i32 (TCG_COND_NE, cc, CC_EQ, l1);
> +gen_goto_tb(env, ctx, 1, ctx->pc+2);
> +gen_set_label(l1);
> +gen_goto_tb(env, ctx, 0, INST2OFFSET(opcode)+ctx->pc+2);
> +ctx->bstate = BS_BRANCH;
> +  }
> +  break;
> +case 0x02: /* blt */
> +  {
> +int l1 = gen_new_label();
> +TCGv t1 = new_tmp();
> +tcg_gen_andi_i32(t1, cc, CC_LT);
> +tcg_gen_brcondi_i32 (TCG_COND_EQ, t1, CC_LT, l1);
> +dead_tmp(t1);
> +gen_goto_tb(env, ctx, 1, ctx->pc+2);
> +gen_set_label(l1);
> +gen_goto_tb(env, ctx, 0, INST2OFFSET(opcode)+ctx->pc+2);
> +ctx->bstate = BS_BRANCH;
> +  }
> +  break;
> +case 0x03: /* bgt */
> +  {
> +int l1 = gen_new_label();
> +TCGv t1 = new_tmp();
> +tcg_gen_andi_i32(t1, cc, CC_GT);
> +tcg_gen_brcondi_i32 (TCG_COND_EQ, t1, CC_GT, l1);
> +dead_tmp(t1);
> +gen_goto_tb(env, ctx, 1, ctx->pc+2);
> +gen_set_label(l1);
> +gen_goto_tb(env, ctx, 0, INST2OFFSET(opcode)+ctx->pc+2);
> +ctx->bstate = BS_BRANCH;
> +  }
> +  break;
> +case 0x04: /* bltu */
> +  {
> +int l1 = gen_new_label();
> +TCGv t1 = new_tmp();
> +tcg_gen_andi_i32(t1, cc, CC_LTU

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Laszlo Ersek]

On 02/14/13 21:55, David Woodhouse wrote:

> Thanks for testing this, btw. Are you looking at suspend/resume too? :)

Entering S3 seemed OK (except the screen was not cleared; using Cirrus).
I woke up the guest with

# virsh qemu-monitor-command fw-ovmf.g-f18xfce2012121716.e-rhel63 \
  --hmp --cmd system_wakeup

Trailing portion of the log:

  In resume (status=254)
  In 32bit resume
  rsdp=0x
  No resume vector set!
  Attempting a hard reboot
  i8042_wait_write
  In resume (status=0)
  In 32bit resume
  Attempting a hard reboot
  [...]

I can see the following CSM calls earlier:
- Legacy16InitializeYourself
- Legacy16GetTableAddress
- Legacy16DispatchOprom
- Legacy16UpdateBbs

No calls to PrepareToBoot (which could set RsdpAddr); this is an UEFI
guest. (The CSM is used for the GOP only.)

Laszlo

Re: [Qemu-devel] [PATCH v2] Move File operations to qemu-file.c

[Anthony Liguori]

Blue Swirl  writes:

> On Thu, Feb 14, 2013 at 2:34 AM, Anthony Liguori  
> wrote:
>> Joel Schopp  writes:
>>
> +if(popen_file == NULL) {

 Please make a preparatory patch which adds missing spaces between 'if'
 statements and '('.
>>>
>>> I'll do a preparatory style cleanup patch of existing code if it is
>>> deemed necessary by the maintainers, but I don't think it's a good
>>> idea.
>>
>> I basically hate checkpatch :-)  There's no need to do a style cleanup,
>> it's just going to confuse gits move detection and screw up merging.  In
>> this case, it's such a trivial thing too.
>
> Either we do code cleanups when possible or we forget about
> CODING_STYLE and checkpatch.pl mess.

We should never force people to clean up coding style in code they
aren't touching.

> The whole point of having a separate patch to do the cleanup is to
> keep git move detection happy.
>
>>
>> I disabled the automated checkpatch bot because it got too annoying.  It
>> throws way too many false positives or annoying nits that shouldn't keep
>> us from merging useful code.
>
> Those 'nits' improve the code base.

A change that only does coding style makes it harder to trouble shoot
problems because there's an extra step of walking past the formating
change to find the real source of the problem.

I spend a lot of time chasing problems and having lots of little
"improvements" just makes that more difficult.

I value debuggability a lot more than whether a line of code has 'if('
or 'if ('.

> It means that a patch to fix one
> thing must also improve the CODING_STYLE while at it. The alternative
> to enforcing this is to do codebase cleanups separately, for example
> in form of global reformatting and flag days.
>
> We don't have many written rules and nobody seems to want to follow
> them.

I'm okay with enforcing Coding Style on *new* code but moving code from
one file to another is *not* new code.

Regards,

Anthony Liguori

Re: [Qemu-devel] [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[H. Peter Anvin]

On 02/14/2013 01:27 PM, David Woodhouse wrote:


So it *is* jumping to 0xfff0 but the memory at that location isn't
what we expect? Do the PAM registers affect *that* too, or only the
region from 0xc-0xf? Surely the contents at 4GiB-δ should be
unchanged by *anything* we do with the PAM registers?

Or maybe not... after also downloading the i440fx data sheet, I'm even
more confused. There's some aliasing with... not the region at 1MiB-δ
but the region at 16MiB-δ:

(From §4.1 System Address Map):

2. High BIOS Area (FFE0_h−− _h)
   The top 2 Mbytes of the Extended Memory Region is reserved for System
   BIOS (High BIOS), extended BIOS for PCI devices, and the A20 alias of
   the system BIOS. The CPU begins execution from the High BIOS after
   reset. This region is mapped to the PCI so that the upper subset of
   this region is aliased to 16 Mbytes minus 256-Kbyte range.



That is presumably a 286 compatibility hack -- the 286 had 24 address 
lines.  I doubt anyone gives a hoot about it, and neither EDK2 nor 
SeaBIOS should care.


-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [Qemu-devel] [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Laszlo Ersek]

On 02/14/13 22:27, David Woodhouse wrote:
> On Thu, 2013-02-14 at 22:14 +0100, Laszlo Ersek wrote:
>> On 02/14/13 21:54, H. Peter Anvin wrote:
>>> On 02/14/2013 12:41 PM, Laszlo Ersek wrote:

 ). cpu_reset() [target-i386/helper.c] sets CS:IP to f000:fff0, which is
 the exact address of... reset_vector() in SeaBIOS.

>>>
>>> This would be a bug, but it isn't quite true.
>>>
>>> If you look at x86_cpu_reset() you will note that it sets the code
>>> segment base to 0x, not 0xf as one could expect from the
>>> above.  This is also true of a physical x86.
>>>
>>> As such, the *real* reset vector is at 0xfff0 as opposed to the
>>> SeaBIOS vector at 0x0 -- this is a backwards compatibility vector
>>> which typically just issues a real reset.
>>>
>>> Now, if Qemu doesn't handle the distinction here correctly, that is a bug.
>>
>> I think I was simply wrong :)
> 
> So it *is* jumping to 0xfff0 but the memory at that location isn't
> what we expect? Do the PAM registers affect *that* too, or only the
> region from 0xc-0xf? Surely the contents at 4GiB-δ should be
> unchanged by *anything* we do with the PAM registers?

I meant that my reading of what x86_cpu_reset() [nee cpu_reset()] was
wrong, because the constant that it passes as argument in fact conforms
to what Peter says.

(
Also check the rom_add_file_fixed() call in pc_init1() / qemu:

ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);

("bios_size" is an "int"). I referred to this in my thread starter as

When qemu-kvm starts, "OVMF.fd" is installed as ROM, such that the
last address it occupies is "all-bits-one", independently of its
size [...]

Namely, the value of (uint32_t)(-bios_size) is

UINT32_MAX + 1 - bios_size

(the above is meant as a math formula, not as a C expression), hence the
last byte occupied is at UINT32_MAX. In my first post I silently thought
that this value would be truncated to fewer bits somewhere, but
apparently that's not the case.
)

> 
> Or maybe not... after also downloading the i440fx data sheet, I'm even
> more confused. There's some aliasing with... not the region at 1MiB-δ
> but the region at 16MiB-δ:
> 
> (From §4.1 System Address Map):
> 
> 2. High BIOS Area (FFE0_h−− _h)
>   The top 2 Mbytes of the Extended Memory Region is reserved for System
>   BIOS (High BIOS), extended BIOS for PCI devices, and the A20 alias of
>   the system BIOS. The CPU begins execution from the High BIOS after
>   reset. This region is mapped to the PCI so that the upper subset of
>   this region is aliased to 16 Mbytes minus 256-Kbyte range.

After Peter emphasized that the code segment base was 0x, I went
back to wikipedia , and
finally managed to parse

The reset vector for the 80386DX and later x86 processors is
0x0, although the value of the CS register at reset is 0xF000
and the value of the IP register at reset is 0xFFF0. In actuality,
current x86 processors fetch from the physical address 0xFFF0.
This is due to a hidden base address portion of the CS register in
real mode which defaults to 0x after reset.

This is again consistent with the 0xfff0 vector pointed out by Peter
(= 0x + 0xFFF0), but I don't know how to match it to the data
sheet language...

Thanks
Laszlo

Re: [Qemu-devel] [PATCH moxie 4/5] Add sample moxie system

[Anthony Green]

Thanks to you and everybody else for the detailed feedback.  I really
appreciate the time you put into this.

I've cleaned up the formatting and licensing, and will address the
other comments when I repost my patches in the new few days.

Thanks,

AG

On Thu, Feb 14, 2013 at 7:08 AM, Andreas Färber  wrote:
> Am 13.02.2013 23:27, schrieb Anthony Green:
>> Add a simple moxie target, similar to what we have in the gdb simulator 
>> today.
>>
>>
>> Signed-off-by: Anthony Green 
>> ---
>>  hw/moxie/Makefile.objs |   5 ++
>>  hw/moxiesim.c  | 200 
>> +
>>  include/sysemu/arch_init.h |   1 +
>>  3 files changed, 206 insertions(+)
>>  create mode 100644 hw/moxie/Makefile.objs
>>  create mode 100644 hw/moxiesim.c
>
> Patch has Coding Style issues and is line-wrapped, i.e. broken.
>
> Please place your moxie-specific board in hw/moxie/. In Makefile.objs
> just place obj-y = moxiesim.o below the $(addprefix ...) line.
>
> Also respect C99 by not using underscore at the beginning of identifiers
> (e.g., struct _loaderparams -> struct LoaderParams).
>
> Are you aware that your loaderparams truncate ram_size from ram_addr_t
> to int? If you don't want to use ram_addr_t there you might want to use
> [u]int64_t.
>
> Again some overuses of CPUMoxieState, please fix.
>
> Please make your QEMUMachine static, it is not needed elsewhere.
> Please drop semicolon after machine_init() macro, it's not a statement.
>
> Please add a comma after QEMU_ARCH_MOXIE enum entry so that further
> architectures can more easily be added.
>
> Are pic_info and irq_info really needed? If so, they would be candidates
> to place in stubs/ directory instead.
>
> There's #if 0'ed code. If there's a question about that, please label as
> RFC rather than PATCH. Otherwise please drop.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[David Woodhouse]

On Thu, 2013-02-14 at 21:41 +0100, Laszlo Ersek wrote:
> I noticed that under OVMF + SeaBIOS CSM + your related patches for both,
> reset requested by the guest doesn't work as expected. The behavior is
> an infinite loop, with the following debug fragment repeated by the
> CSM-ized SeaBIOS:
> 
>   In resume (status=0)
>   In 32bit resume
>   Attempting a hard reboot
>   i8042_wait_write

Hmm. My build from http://david.woodhou.se/OVMF.fd works fine. I did a
legacy boot into (Ubuntu Oneiric's) Grub, then issued the 'reboot'
command...

This appears to be the case for qemu 1.2.0 and 1.3.0, both with and
without KVM.

enter handle_13:
   a=4200  b=0801  c=003f  d=0080 ds=6000 es= ss=
  si=fe00 di= bp=1ff0 sp=1ff2 cs= ip=9157  f=0202
disk_op d=0xdb20 lba=9269505 buf=0x00068000 count=63 cmd=2
pmtimer: 2:15494096
pmtimer: 2:15494211
In resume (status=0)
In 32bit resume
Attempting a hard reboot
i8042_wait_write
pmtimer: 2:15501497
pmtimer: 2:15501593
pmtimer: 2:15501750
SecCoreStartupWithStack(0xFFFE6000, 0x8)
File->Type: 0xB
Section->Type: 0x2
Section->Type: 0x19
Section->Type (0x19) != SectionType (0x17)

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [RFC] Disassembler options going forward

[Anthony Liguori]

Peter Maydell  writes:

> On 14 February 2013 21:10, Richard Henderson  wrote:
>> The only options I can see going forward are
>>
>>  1) Provide a configure time option to link to the "system" libopcodes.
>>  2) Use someone else's (bsd licensed?) disassember.
>>  3) Rearrange relevant translators so that they can disassemble and
>> not translate.
>>
>> The complete solution could be a combination of all three.
>
> 4) Since we only do disassembly for debug logging, have our debug
> logging just log hex, with postprocessing by a script that runs
> objdump or something

Ack.  A simple binary format would be nice too since you probably want
the traces to be as small a possible.

>
>> To me, option (1) means that qemu the project is not "infecting" the
>> binary with GPLv3, but requiring the user to make that choice.  Which
>> seems fine; those that have moral objections to v3 can simply not use
>> that configure option.  It's a bit awkward that most distributions don't
>> package up libopcodes for install, but if you manually build binutils
>> you'll have it done.
>
> I'm not hugely convinced by the idea of "here's a configure switch
> to produce binaries you can't legally distribute".
>
>> I hope we'll all agree that option (3) is not ideal, since having a
>> separate disassembler works as a check against the translator.  However,
>> for odd parts that will never be a host it's not a totally unreasonable
>> solution, as it at least provides *some* disassembly.
>>
>> As for option (2), I'm not even sure what to suggest.  I suppose there's
>> some part of LLVM that does textual disassembly.  Whether we want to
>> drag that in as a submodule or just require it to be installed and
>> notice it at configure time, I have no opinion.  But because of the odd
>> parts, (2) can't be the only option.
>
> I had a look at the LLVM disassembler the other day. From a quick
> glance it seems like LLVM drives the disassembler off a generalised
> machine description language (which it also uses for various other
> things), so getting the disassemblers would also require us to
> pull in quite a bit of LLVM infrastructure for parsing the machine
> descriptions. It didn't look particularly easy, but this is just
> from 15 minutes browsing a source tree, so if anybody more LLVM
> aware here has an opinion do say.

LLVM is a beast.  I played around with using cfront to make a QAPI front
end and it took 4-5G worth of dispatch just to get it building.

Regards,

Anthony Liguori

>
>> But most of all I think we should have a plan.
>
> Agreed.
>
> -- PMM

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Laszlo Ersek]

On 02/14/13 21:55, David Woodhouse wrote:

> So, if real hardware would reset the PAMs on reset and avoid the need
> for SeaBIOS to do so, I think we should be doing the same in qemu too.

That's what I couldn't figure out from the i440FX spec, but I believe
one could argue that "reset" should in fact re-set the state that was
observable at VM startup.

> Thanks for testing this, btw. Are you looking at suspend/resume too? :)

I'm either not looking, or not admitting it! :)

In earnest: I "approached" Platform Initialization S3 resume cautiously,
then fled in a panic. See Chapter 8 in Volume 5 -- Jordan convinced me
that this scripting language was in fact reasonable, but the work needed
to follow through OVMF PI, make it S3 resume compliant, and record
everything into a script at cold boot looks very threatening.

Regarding S3 in SeaBIOS CSM: I haven't tried it yet, but I can press the
button in guests if you want me to.

Laszlo

[Qemu-devel] [PATCH V22 0/7] QEMU Trusted Platform Module (TPM) integration

[Stefan Berger]

From: root 

The following series of patches adds TPM (Trusted Platform Module) support
to QEMU. An emulator for the TIS (TPM Interface Spec) interface is
added that provides the basis for accessing a 'backend' implementing the actual
TPM functionality. The TIS emulator serves as a 'frontend' enabling for
example Linux's TPM TIS (tpm_tis) driver.

In this series I am posting a backend implementation that makes use of the
host's TPM through a passthrough driver, which on Linux is accessed
using /dev/tpm0.

v22:
 - applies to checkout of 571f65ec2 (Feb. 14)
 - addressed comments from Corey Bryant and Eric Blake on v21

v21:
 - applies to checkout of 70ef6a5b7 (Feb. 7)
 - addressed comments from Corey Bryant and Luiz Capitulino on v20
 - adapted code to new directory structure:
   - split tpm.h into public part in include/tpm/tpm.h
 and private part in tpm/tpm_int.h
   - all TPM code is now in tpm/ directory

v20:
 - applies to checkout of v1.3.0 (6d6c9f59, Dec. 3)
 - addressed comments from Corey Bryant on v19
 - introduced support for canceling commands

v19:
 - applies to checkout of 8cc9b43 (Jun 4)

v18:
 - applies to checkout of 563987d (May 1)
 - removed some dead variable in 7/7

v17:
 - applies to checkout of 6507470 (Apr 30)
 - split up path and fd into two optional parameters

v16:
 - applied to checkout of 42fe1c2 (Apr 27)
 - followed Anthony's suggestions for v15
 - changed qemu-options.hx and vl.c to not show anything TPM-related if
   --enable-tpm-passthrough was not used on configure line

v15:
 - applies to checkout of 8a22565 (Mar 27)
 - replacing g_malloc's with g_new; no more checks for NULL after allocs
 - introducing usage of bottom half in TIS frontend to deliver responses
 - get rid of locks since global lock is held by all threads entering TIS
   code
 - cleanups

v14:
 - applies to checkout of da5361c (Dec 12)
 - implemented Anthony Liguori's suggestions
 - dropping the version log on individual patches

v13:
 - applies to checkout of 61a5872 (Dec 12)
 - only allowing character devices as fd parameter
 - fixing error path in tpm_tis_init

v12:
 - applies to checkout of ebffe2a (Oct 11)
 - added documentation for fd parameter
 - nits

v11:
 - applies to checkout of 46f3069 (Sep 28)
 - some filing on the documentation
 - small nits fixed

v10:
 - applies to checkout of 1ce9ce6 (Sep 27)
 - addressed Michael Tsirkin's comments on v9

v9:
 - addressed Michael Tsirkin's and other reviewers' comments
 - only posting Andreas Niederl's passthrough driver as the backend driver

v8:
 - applies to checkout of f0fb8b7 (Aug 30)
 - fixing compilation error pointed out by Andreas Niederl
 - adding patch that allows to feed an initial state into the libtpms TPM
 - following memory API changes (glib) where necessary

v7:
 - applies to checkout of b9c6cbf (Aug 9)
 - measuring the modules if multiboot is used
 - coding style fixes

v6:
 - applies to checkout of 75ef849 (July 2nd)
 - some fixes and improvements to existing patches; see individual patches
 - added a patch with a null driver responding to all TPM requests with
   a response indicating failure; this backend has no dependencies and
   can alwayy be built;
 - added a patch to support the hashing of kernel, ramfs and command line
   if those were passed to Qemu using -kernel, -initrd and -append
   respectively. Measurements are taken, logged, and passed to SeaBIOS using
   the firmware interface.
 - libtpms revision 7 now requires 83kb of block storage due to having more
   NVRAM space

v5:
 - applies to checkout of 1fddfba1
 - adding support for split command line using the -tpmdev ... -device ...
   options while keeping the -tpm option
 - support for querying the device models using -tpm model=?
 - support for monitor 'info tpm'
 - adding documentation of command line options for man page and web page
 - increasing room for ACPI tables that qemu reserves to 128kb (from 64kb)
 - adding (experimental) support for block migration
 - adding (experimental) support for taking measurements when kernel,
   initrd and kernel command line are directly passed to Qemu

v4:
 - applies to checkout of d2d979c6
 - more coding style fixes
 - adding patch for supporting blob encryption (in addition to the existing
   QCoW2-level encryption)
   - this allows for graceful termination of a migration if the target
 is detected to have a wrong key
   - tested with big and little endian hosts
 - main thread releases mutex while checking for work to do on behalf of
   backend
 - introducing file locking (fcntl) on the block layer for serializing access
   to shared (QCoW2) files (used during migration)

v3:
 - Building a null driver at patch 5/8 that responds to all requests
   with an error response; subsequently this driver is transformed to the
   libtpms-based driver for real TPM functionality
 - Reworked the threading; dropped the patch for qemu_thread_join; the
   main thread synchronizing with the TPM thread termination may need
   to write data to t

Re: [Qemu-devel] [RFC] Disassembler options going forward

[Blue Swirl]

On Thu, Feb 14, 2013 at 9:10 PM, Richard Henderson  wrote:
> On 02/13/2013 06:28 PM, Anthony Liguori wrote:
>> QEMU is GPLv2 only so we can't take GPLv3 code.  We're stuck on binutils
>> code that predates the v3 relicense.
>
> Ok, this is something that's going to bite us more and more.
>
> We need *some* solution that allows us to disassemble current cpus.
> What we've got in disas/ is, except for really old cpus, completely out
> of date:
>
>  * x86 missing all opcodes from sse4 and beyond,
>  * s390 missing tons of opcodes (I filled in some, but not all)
>  * ppc missing tons of opcodes (2.06 and later?)

Sparc64 is probably missing a few opcodes too.

>
> The only options I can see going forward are
>
>  1) Provide a configure time option to link to the "system" libopcodes.
>  2) Use someone else's (bsd licensed?) disassember.
>  3) Rearrange relevant translators so that they can disassemble and
> not translate.

4) Ask binutils' authors of x86, s390 and ppc disassembly code (or
FSF) to dual license their code as GPLv2+ and GPLv3. May be difficult.

5) Relicense QEMU as GPLv3. Need to reimplement some parts.

>
> The complete solution could be a combination of all three.
>
> To me, option (1) means that qemu the project is not "infecting" the
> binary with GPLv3, but requiring the user to make that choice.  Which
> seems fine; those that have moral objections to v3 can simply not use
> that configure option.  It's a bit awkward that most distributions don't
> package up libopcodes for install, but if you manually build binutils
> you'll have it done.
>
> I hope we'll all agree that option (3) is not ideal, since having a
> separate disassembler works as a check against the translator.  However,
> for odd parts that will never be a host it's not a totally unreasonable
> solution, as it at least provides *some* disassembly.
>
> As for option (2), I'm not even sure what to suggest.  I suppose there's
> some part of LLVM that does textual disassembly.  Whether we want to
> drag that in as a submodule or just require it to be installed and
> notice it at configure time, I have no opinion.  But because of the odd
> parts, (2) can't be the only option.
>
> But most of all I think we should have a plan.
>
>
> r~
>

[Qemu-devel] [PATCH V22 1/7] Support for TPM command line options

[Stefan Berger]

This patch adds support for TPM command line options.
The command line options supported here are

./qemu-... -tpmdev passthrough,path=,id=
   -device tpm-tis,tpmdev=

and

./qemu-... -tpmdev help

where the latter works similar to -soundhw ? and shows a list of
available TPM backends (for example 'passthrough').

Using the type parameter, the backend is chosen, i.e., 'passthrough' for the
passthrough driver. The interpretation of the other parameters along
with determining whether enough parameters were provided is pushed into
the backend driver, which needs to implement the interface function
'create' and return a TPMDriver structure if the VM can be started or 'NULL'
if not enough or bad parameters were provided.

Monitor support for 'info tpm' has been added. It for example prints the
following:

(qemu) info tpm
TPM devices:
 tpm0: model=tpm-tis
  \ tpm0: type=passthrough,path=/dev/tpm0

Signed-off-by: Stefan Berger 
---
 Makefile.objs |   1 +
 hmp-commands.hx   |   2 +
 hmp.c |  44 +
 hmp.h |   1 +
 include/tpm/tpm.h |  21 +
 monitor.c |   8 ++
 qapi-schema.json  |  83 +
 qemu-options.hx   |  33 +++
 qmp-commands.hx   |  18 
 tpm/Makefile.objs |   1 +
 tpm/tpm.c | 272 ++
 tpm/tpm_int.h |  79 
 tpm/tpm_tis.h |  78 
 vl.c  |  37 
 14 files changed, 678 insertions(+)
 create mode 100644 include/tpm/tpm.h
 create mode 100644 tpm/Makefile.objs
 create mode 100644 tpm/tpm.c
 create mode 100644 tpm/tpm_int.h
 create mode 100644 tpm/tpm_tis.h

diff --git a/Makefile.objs b/Makefile.objs
index 21e9c91..d52ea49 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -74,6 +74,7 @@ common-obj-y += bt-host.o bt-vhci.o
 common-obj-y += dma-helpers.o
 common-obj-y += qtest.o
 common-obj-y += vl.o
+common-obj-y += tpm/
 
 common-obj-$(CONFIG_SLIRP) += slirp/
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 64008a9..a952fd1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1642,6 +1642,8 @@ show device tree
 show qdev device model list
 @item info roms
 show roms
+@item info tpm
+show the TPM device
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index 2f47a8a..b0a861c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -607,6 +607,50 @@ void hmp_info_block_jobs(Monitor *mon, const QDict *qdict)
 }
 }
 
+void hmp_info_tpm(Monitor *mon, const QDict *qdict)
+{
+TPMInfoList *info_list, *info;
+Error *err = NULL;
+unsigned int c = 0;
+TPMPassthroughOptions *tpo;
+
+info_list = qmp_query_tpm(&err);
+if (err) {
+monitor_printf(mon, "TPM device not supported\n");
+error_free(err);
+return;
+}
+
+if (info_list) {
+monitor_printf(mon, "TPM device:\n");
+}
+
+for (info = info_list; info; info = info->next) {
+TPMInfo *ti = info->value;
+monitor_printf(mon, " tpm%d: model=%s\n",
+   c, TpmModel_lookup[ti->model]);
+
+monitor_printf(mon, "  \\ %s: type=%s",
+   ti->id, TpmType_lookup[ti->type]);
+
+switch (ti->tpm_options->kind) {
+case TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS:
+tpo = ti->tpm_options->tpm_passthrough_options;
+monitor_printf(mon, "%s%s%s%s",
+   tpo->has_path ? ",path=" : "",
+   tpo->has_path ? tpo->path : "",
+   tpo->has_cancel_path ? ",cancel-path=" : "",
+   tpo->has_cancel_path ? tpo->cancel_path : "");
+break;
+case TPM_TYPE_OPTIONS_KIND_MAX:
+break;
+}
+monitor_printf(mon, "\n");
+c++;
+}
+qapi_free_TPMInfoList(info_list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
 monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 30b3c20..95fe76e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -36,6 +36,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict);
 void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
+void hmp_info_tpm(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/include/tpm/tpm.h b/include/tpm/tpm.h
new file mode 100644
index 000..cc8f20e
--- /dev/null
+++ b/include/tpm/tpm.h
@@ -0,0 +1,21 @@
+/*
+ * Public TPM functions
+ *
+ * Copyright (C) 2011-2013 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_TPM_H
+#define QEMU_TPM_H
+
+#include "qemu/option.h"
+
+int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
+int tpm_init(v

[Qemu-devel] [PATCH V22 3/7] Add a debug register

[Stefan Berger]

This patch uses the possibility to add a vendor-specific register and
adds a debug register useful for dumping the TIS's internal state. This
register is only active in a debug build (#define DEBUG_TIS).

Signed-off-by: Stefan Berger 
Reviewed-by: Corey Bryant 
---
 tpm/tpm_tis.c | 70 +++
 1 file changed, 70 insertions(+)

diff --git a/tpm/tpm_tis.c b/tpm/tpm_tis.c
index 565e28d..ff5cd6a 100644
--- a/tpm/tpm_tis.c
+++ b/tpm/tpm_tis.c
@@ -52,6 +52,9 @@
 #define TPM_TIS_REG_DID_VID   0xf00
 #define TPM_TIS_REG_RID   0xf04
 
+/* vendor-specific registers */
+#define TPM_TIS_REG_DEBUG 0xf90
+
 #define TPM_TIS_STS_VALID (1 << 7)
 #define TPM_TIS_STS_COMMAND_READY (1 << 6)
 #define TPM_TIS_STS_TPM_GO(1 << 5)
@@ -105,6 +108,11 @@
 
 #define TPM_TIS_NO_DATA_BYTE  0xff
 
+/* local prototypes */
+
+static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
+  unsigned size);
+
 /* utility functions */
 
 static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
@@ -346,6 +354,63 @@ static uint32_t tpm_tis_data_read(TPMState *s, uint8_t 
locty)
 return ret;
 }
 
+#ifdef DEBUG_TIS
+static void tpm_tis_dump_state(void *opaque, hwaddr addr)
+{
+static const unsigned regs[] = {
+TPM_TIS_REG_ACCESS,
+TPM_TIS_REG_INT_ENABLE,
+TPM_TIS_REG_INT_VECTOR,
+TPM_TIS_REG_INT_STATUS,
+TPM_TIS_REG_INTF_CAPABILITY,
+TPM_TIS_REG_STS,
+TPM_TIS_REG_DID_VID,
+TPM_TIS_REG_RID,
+0xfff};
+int idx;
+uint8_t locty = tpm_tis_locality_from_addr(addr);
+hwaddr base = addr & ~0xfff;
+TPMState *s = opaque;
+TPMTISEmuState *tis = &s->s.tis;
+
+DPRINTF("tpm_tis: active locality  : %d\n"
+"tpm_tis: state of locality %d : %d\n"
+"tpm_tis: register dump:\n",
+tis->active_locty,
+locty, tis->loc[locty].state);
+
+for (idx = 0; regs[idx] != 0xfff; idx++) {
+DPRINTF("tpm_tis: 0x%04x : 0x%08x\n", regs[idx],
+(uint32_t)tpm_tis_mmio_read(opaque, base + regs[idx], 4));
+}
+
+DPRINTF("tpm_tis: read offset   : %d\n"
+"tpm_tis: result buffer : ",
+tis->loc[locty].r_offset);
+for (idx = 0;
+ idx < tpm_tis_get_size_from_buffer(&tis->loc[locty].r_buffer);
+ idx++) {
+DPRINTF("%c%02x%s",
+tis->loc[locty].r_offset == idx ? '>' : ' ',
+tis->loc[locty].r_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+DPRINTF("\n"
+"tpm_tis: write offset  : %d\n"
+"tpm_tis: request buffer: ",
+tis->loc[locty].w_offset);
+for (idx = 0;
+ idx < tpm_tis_get_size_from_buffer(&tis->loc[locty].w_buffer);
+ idx++) {
+DPRINTF("%c%02x%s",
+tis->loc[locty].w_offset == idx ? '>' : ' ',
+tis->loc[locty].w_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+DPRINTF("\n");
+}
+#endif
+
 /*
  * Read a register of the TIS interface
  * See specs pages 33-63 for description of the registers
@@ -425,6 +490,11 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
addr,
 case TPM_TIS_REG_RID:
 val = TPM_TIS_TPM_RID;
 break;
+#ifdef DEBUG_TIS
+case TPM_TIS_REG_DEBUG:
+tpm_tis_dump_state(opaque, addr);
+break;
+#endif
 }
 
 if (shift) {
-- 
1.7.11.7

[Qemu-devel] [PATCH V22 5/7] Add a TPM Passthrough backend driver implementation

[Stefan Berger]

>From Andreas Niederl's original posting with adaptations where necessary:

This patch is based of off version 9 of Stefan Berger's patch series
  "QEMU Trusted Platform Module (TPM) integration"
and adds a new backend driver for it.

This patch adds a passthrough backend driver for passing commands sent to the
emulated TPM device directly to a TPM device opened on the host machine.
Thus it is possible to use a hardware TPM device in a system running on QEMU,
providing the ability to access a TPM in a special state (e.g. after a Trusted
Boot).

This functionality is being used in the acTvSM Trusted Virtualization Platform
which is available on [1].

Usage example:
  qemu-system-x86_64 -tpmdev passthrough,id=tpm0,path=/dev/tpm0 \
 -device tpm-tis,tpmdev=tpm0 \
 -cdrom test.iso -boot d

Some notes about the host TPM:
The TPM needs to be enabled and activated. If that's not the case one
has to go through the BIOS/UEFI and enable and activate that TPM for TPM
commands to work as expected.
It may be necessary to boot the kernel using tpm_tis.force=1 in the boot
command line or 'modprobe tpm_tis force=1' in case of using it as a module.

Regards,
Andreas Niederl, Stefan Berger

[1] http://trustedjava.sourceforge.net/

Signed-off-by: Andreas Niederl 
Signed-off-by: Stefan Berger 
Reviewed-by: Corey Bryant 
---
 include/qemu/sockets.h |   1 +
 qemu-char.c|  24 
 qemu-options.hx|  38 -
 tpm/Makefile.objs  |   3 +-
 tpm/tpm.c  |  17 +++
 tpm/tpm_backend.c  |  58 
 tpm/tpm_backend.h  |  45 ++
 tpm/tpm_int.h  |  33 +
 tpm/tpm_passthrough.c  | 378 +
 vl.c   |   2 +
 10 files changed, 597 insertions(+), 2 deletions(-)
 create mode 100644 tpm/tpm_backend.c
 create mode 100644 tpm/tpm_backend.h
 create mode 100644 tpm/tpm_passthrough.c

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 803ae17..16569e2 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -37,6 +37,7 @@ int socket_set_cork(int fd, int v);
 void socket_set_block(int fd);
 void socket_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
+int recv_all(int fd, void *buf, int len1, bool single_read);
 
 /* callback function for nonblocking connect
  * valid fd on success, negative error code on failure
diff --git a/qemu-char.c b/qemu-char.c
index e4b0f53..05f8cff 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -537,6 +537,30 @@ int send_all(int fd, const void *_buf, int len1)
 }
 return len1 - len;
 }
+
+int recv_all(int fd, void *_buf, int len1, bool single_read)
+{
+int ret, len;
+uint8_t *buf = _buf;
+
+len = len1;
+while ((len > 0) && (ret = read(fd, buf, len)) != 0) {
+if (ret < 0) {
+if (errno != EINTR && errno != EAGAIN) {
+return -1;
+}
+continue;
+} else {
+if (single_read) {
+return ret;
+}
+buf += ret;
+len -= ret;
+}
+}
+return len1 - len;
+}
+
 #endif /* !_WIN32 */
 
 #define STDIO_MAX_CLIENTS 1
diff --git a/qemu-options.hx b/qemu-options.hx
index 48c9888..ede6d94 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2189,10 +2189,12 @@ ETEXI
 DEFHEADING()
 
 #ifdef CONFIG_TPM
+# ifdef CONFIG_TPM_PASSTHROUGH
 DEFHEADING(TPM device options:)
 
 DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
-"-tpmdev [],id=str[,option][,option][,...]\n",
+"-tpmdev passthrough,id=id[,path=path]\n"
+"use path to provide path to a character device; default 
is /dev/tpm0\n",
 QEMU_ARCH_ALL)
 STEXI
 
@@ -2202,6 +2204,7 @@ The general form of a TPM device option is:
 @item -tpmdev @var{backend} ,id=@var{id} [,@var{options}]
 @findex -tpmdev
 Backend type must be:
+@option{passthrough}.
 
 The specific backend type will determine the applicable options.
 The @code{-tpmdev} options requires a @code{-device} option.
@@ -2213,12 +2216,45 @@ Use 'help' to print all available TPM backend types.
 qemu -tpmdev help
 @end example
 
+@item -tpmdev passthrough, id=@var{id}, path=@var{path}
+
+(Linux-host only) Enable access to the host's TPM using the passthrough
+driver.
+
+@option{path} specifies the path to the host's TPM device, i.e., on
+a Linux host this would be @code{/dev/tpm0}.
+@option{path} is optional and by default @code{/dev/tpm0} is used.
+
+Some notes about using the host's TPM with the passthrough driver:
+
+The TPM device accessed by the passthrough driver must not be
+used by any other application on the host.
+
+Since the host's firmware (BIOS/UEFI) has already initialized the TPM,
+the VM's firmware (BIOS/UEFI) will not be able to initialize the
+TPM again and may therefore not show a TPM-specific menu that would
+otherwise allow the user to configure the TPM, e.g., allow the user to
+enable/disable or activate/deactivate the TPM.
+Furthe

[Qemu-devel] [PATCH V22 7/7] Introduce --enable-tpm-passthrough configure option

[Stefan Berger]

Signed-off-by: Stefan Berger 
Reviewed-by: Corey Bryant 
---
 configure | 17 +
 1 file changed, 17 insertions(+)

diff --git a/configure b/configure
index b7359aa..b4f0a82 100755
--- a/configure
+++ b/configure
@@ -227,6 +227,7 @@ seccomp=""
 glusterfs=""
 virtio_blk_data_plane=""
 tpm="no"
+tpm_passthrough="no"
 
 # parse CC options first
 for opt do
@@ -900,11 +901,20 @@ for opt do
   ;;
   --enable-tpm) tpm="yes"
   ;;
+  --enable-tpm-passthrough) tpm_passthrough="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
 done
 
+if test "$tpm" = "no" ; then
+if test "$tpm_passthrough" = "yes"; then
+echo "ERROR: --enable-tpm-passthrough requires --enable-tpm"
+exit 1
+fi
+fi
+
 case "$cpu" in
 sparc)
LDFLAGS="-m32 $LDFLAGS"
@@ -1150,6 +1160,7 @@ echo "  --disable-glusterfs  disable GlusterFS 
backend"
 echo "  --enable-gcovenable test coverage analysis with gcov"
 echo "  --gcov=GCOV  use specified gcov [$gcov_tool]"
 echo "  --enable-tpm enable TPM support"
+echo "  --enable-tpm-passthrough enable TPM passthrough driver"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -3349,6 +3360,7 @@ echo "virtio-blk-data-plane $virtio_blk_data_plane"
 echo "gcov  $gcov_tool"
 echo "gcov enabled  $gcov"
 echo "TPM support   $tpm"
+echo "TPM passthrough   $tpm_passthrough"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -4258,6 +4270,11 @@ fi
 
 if test "$tpm" = "yes"; then
   if test "$target_softmmu" = "yes" ; then
+if test "$linux" = "yes" ; then
+  if test "$tpm_passthrough" = "yes" ; then
+echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
+  fi
+fi
 echo "CONFIG_TPM=y" >> $config_host_mak
   fi
 fi
-- 
1.7.11.7

[Qemu-devel] [PATCH V22 6/7] Add support for cancelling of a TPM command

[Stefan Berger]

This patch adds support for cancelling an executing TPM command.
In Linux for example a user can cancel a command through the TPM's
sysfs 'cancel' entry using

echo "1" > /sysfs/class/misc/tpm0/device/cancel

This patch propagates the cancellation of a command inside a VM
to the host TPM's sysfs entry.
It also uses the possibility to cancel the command before QEMU VM
shutdown or reboot, which helps in preventing QEMU from hanging while
waiting for the completion of the command.
To relieve higher layers or users from having to determine the TPM's
cancel sysfs entry, the driver searches for the entry in well known
locations.

Signed-off-by: Stefan Berger 
---
 qemu-options.hx   |  13 +++-
 tpm/tpm_passthrough.c | 166 ++
 vl.c  |   5 ++
 3 files changed, 168 insertions(+), 16 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index ede6d94..410b7fa 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2193,8 +2193,10 @@ DEFHEADING()
 DEFHEADING(TPM device options:)
 
 DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
-"-tpmdev passthrough,id=id[,path=path]\n"
-"use path to provide path to a character device; default 
is /dev/tpm0\n",
+"-tpmdev passthrough,id=id[,path=path][,cancel-path=path]\n"
+"use path to provide path to a character device; default 
is /dev/tpm0\n"
+"use cancel-path to provide path to TPM's cancel sysfs 
entry; if\n"
+"not provided it will be searched for in 
/sys/class/misc/tpm?/device\n",
 QEMU_ARCH_ALL)
 STEXI
 
@@ -2216,7 +2218,7 @@ Use 'help' to print all available TPM backend types.
 qemu -tpmdev help
 @end example
 
-@item -tpmdev passthrough, id=@var{id}, path=@var{path}
+@item -tpmdev passthrough, id=@var{id}, path=@var{path}, path=@var{cancel-path}
 
 (Linux-host only) Enable access to the host's TPM using the passthrough
 driver.
@@ -2225,6 +2227,11 @@ driver.
 a Linux host this would be @code{/dev/tpm0}.
 @option{path} is optional and by default @code{/dev/tpm0} is used.
 
+@option{cancel-path} specifies the path to the host TPM device's sysfs
+entry allowing for cancellation of an ongoing TPM command.
+@option{cancel-path} is optional and by default QEMU will search for the
+sysfs entry to use.
+
 Some notes about using the host's TPM with the passthrough driver:
 
 The TPM device accessed by the passthrough driver must not be
diff --git a/tpm/tpm_passthrough.c b/tpm/tpm_passthrough.c
index 7d5de8e..300575b 100644
--- a/tpm/tpm_passthrough.c
+++ b/tpm/tpm_passthrough.c
@@ -22,6 +22,8 @@
  * License along with this library; if not, see 
  */
 
+#include 
+
 #include "qemu-common.h"
 #include "qapi/error.h"
 #include "qemu/sockets.h"
@@ -57,11 +59,18 @@ struct TPMPassthruState {
 
 char *tpm_dev;
 int tpm_fd;
+bool tpm_executing;
+bool tpm_op_canceled;
+int cancel_fd;
 bool had_startup_error;
 };
 
 #define TPM_PASSTHROUGH_DEFAULT_DEVICE "/dev/tpm0"
 
+/* functions */
+
+static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
+
 static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
 {
 return send_all(fd, buf, len);
@@ -79,25 +88,36 @@ static uint32_t tpm_passthrough_get_size_from_buffer(const 
uint8_t *buf)
 return be32_to_cpu(resp->len);
 }
 
-static int tpm_passthrough_unix_tx_bufs(int tpm_fd,
+static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
 const uint8_t *in, uint32_t in_len,
 uint8_t *out, uint32_t out_len)
 {
 int ret;
 
-ret = tpm_passthrough_unix_write(tpm_fd, in, in_len);
+tpm_pt->tpm_op_canceled = false;
+tpm_pt->tpm_executing = true;
+
+ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len);
 if (ret != in_len) {
-error_report("tpm_passthrough: error while transmitting data "
- "to TPM: %s (%i)\n",
- strerror(errno), errno);
+if (!tpm_pt->tpm_op_canceled ||
+(tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
+error_report("tpm_passthrough: error while transmitting data "
+ "to TPM: %s (%i)\n",
+ strerror(errno), errno);
+}
 goto err_exit;
 }
 
-ret = tpm_passthrough_unix_read(tpm_fd, out, out_len);
+tpm_pt->tpm_executing = false;
+
+ret = tpm_passthrough_unix_read(tpm_pt->tpm_fd, out, out_len);
 if (ret < 0) {
-error_report("tpm_passthrough: error while reading data from "
- "TPM: %s (%i)\n",
- strerror(errno), errno);
+if (!tpm_pt->tpm_op_canceled ||
+(tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
+error_report("tpm_passthrough: error while reading data from "
+ "TPM: %s (%i)\n",
+ strerror(errno), er

Re: [Qemu-devel] [RFC PATCH v2 13/23] qcow2: handle_copied(): Implement non-zero host_offset

[Blue Swirl]

On Thu, Feb 14, 2013 at 9:40 AM, Kevin Wolf  wrote:
> Am 13.02.2013 22:17, schrieb Blue Swirl:
>> On Wed, Feb 13, 2013 at 1:22 PM, Kevin Wolf  wrote:
>>> Look only for clusters that start at a given physical offset.
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  block/qcow2-cluster.c |   26 ++
>>>  1 files changed, 18 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
>>> index 5ce2c88..90fe36c 100644
>>> --- a/block/qcow2-cluster.c
>>> +++ b/block/qcow2-cluster.c
>>> @@ -827,8 +827,6 @@ static int handle_dependencies(BlockDriverState *bs, 
>>> uint64_t guest_offset,
>>>   *  the length of the area that can be written to.
>>>   *
>>>   *  -errno: in error cases
>>> - *
>>> - * TODO Make non-zero host_offset behave like describe above
>>>   */
>>>  static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
>>>  uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m)
>>> @@ -843,7 +841,6 @@ static int handle_copied(BlockDriverState *bs, uint64_t 
>>> guest_offset,
>>>
>>>  trace_qcow2_handle_copied(qemu_coroutine_self(), guest_offset, 
>>> *host_offset,
>>>*bytes);
>>> -assert(*host_offset == 0);
>>>
>>>  /*
>>>   * Calculate the number of clusters to look for. We stop at L2 table
>>> @@ -867,6 +864,15 @@ static int handle_copied(BlockDriverState *bs, 
>>> uint64_t guest_offset,
>>>  if (qcow2_get_cluster_type(cluster_offset) == QCOW2_CLUSTER_NORMAL
>>>  && (cluster_offset & QCOW_OFLAG_COPIED))
>>>  {
>>> +/* If a specific host_offset is required, check it */
>>> +if (*host_offset != 0
>>> +&& (cluster_offset & L2E_OFFSET_MASK) != *host_offset)
>>> +{
>>
>> Braces should cuddle with the previous line.
>
> Can we get rid of this rule for multiline ifs? Having the
> second/third/... line of the condition and the first line of the then
> branch with no clear separation severely hurts readability in my opinion.

Perhaps the problem is that the condition is long, it could be
rewritten in this style:
bool has_host_offset = *host_offset != 0;
bool offset_matches = (cluster_offset & L2E_OFFSET_MASK) != *host_offset;
if (has_host_offset && offset_matches) {

>
>>
>>> +*bytes = 0;
>>> +ret = 0;
>>> +goto out;
>>> +}
>>> +
>>>  /* We keep all QCOW_OFLAG_COPIED clusters */
>>>  keep_clusters =
>>>  count_contiguous_clusters(nb_clusters, s->cluster_size,
>
> Kevin

[Qemu-devel] [PATCH V22 2/7] Add TPM (frontend) hardware interface (TPM TIS) to QEMU

[Stefan Berger]

This patch adds the main code of the TPM frontend driver, the TPM TIS
interface, to QEMU. The code is largely based on the previous implementation
for Xen but has been significantly extended to meet the standard's
requirements, such as the support for changing of localities and all the
functionality of the available flags.

Communication with the backend (i.e., for Xen or the libtpms-based one)
is cleanly separated through an interface which the backend driver needs
to implement.

Whenever the frontend has collected a complete packet, it will submit
a task to the backend, which then starts processing the command. Once
the result has been returned, the backend invokes a callback function
(tpm_tis_receive_cb()).

Testing the proper functioning of the different flags and localities
cannot be done from user space when running in Linux for example, since
access to the address space of the TPM TIS interface is not possible. Also
the Linux driver itself does not exercise all functionality. So, for
testing there is a fairly extensive test suite as part of the SeaBIOS patches
since from within the BIOS one can have full access to all the TPM's registers.


Signed-off-by: Stefan Berger 
Reviewed-by: Corey Bryant 
---
 tpm/tpm_tis.c | 857 ++
 1 file changed, 857 insertions(+)
 create mode 100644 tpm/tpm_tis.c

diff --git a/tpm/tpm_tis.c b/tpm/tpm_tis.c
new file mode 100644
index 000..565e28d
--- /dev/null
+++ b/tpm/tpm_tis.c
@@ -0,0 +1,857 @@
+/*
+ * tpm_tis.c - QEMU's TPM TIS interface emulator
+ *
+ * Copyright (C) 2006,2010-2013 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger 
+ *  David Safford 
+ *
+ * Xen 4 support: Andrease Niederl 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Implementation of the TIS interface according to specs found at
+ * http://www.trustedcomputiggroup.org. This implementation currently
+ * supports version 1.21, revision 1.0.
+ * In the developers menu choose the PC Client section then find the TIS
+ * specification.
+ */
+
+#include "tpm_int.h"
+#include "block/block.h"
+#include "exec/address-spaces.h"
+#include "hw/hw.h"
+#include "hw/pc.h"
+#include "hw/pci/pci_ids.h"
+#include "tpm/tpm_tis.h"
+#include "qemu-common.h"
+
+/*#define DEBUG_TIS */
+
+#ifdef DEBUG_TIS
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do { } while (0)
+#endif
+
+/* whether the STS interrupt is supported */
+#define RAISE_STS_IRQ
+
+/* tis registers */
+#define TPM_TIS_REG_ACCESS0x00
+#define TPM_TIS_REG_INT_ENABLE0x08
+#define TPM_TIS_REG_INT_VECTOR0x0c
+#define TPM_TIS_REG_INT_STATUS0x10
+#define TPM_TIS_REG_INTF_CAPABILITY   0x14
+#define TPM_TIS_REG_STS   0x18
+#define TPM_TIS_REG_DATA_FIFO 0x24
+#define TPM_TIS_REG_DID_VID   0xf00
+#define TPM_TIS_REG_RID   0xf04
+
+#define TPM_TIS_STS_VALID (1 << 7)
+#define TPM_TIS_STS_COMMAND_READY (1 << 6)
+#define TPM_TIS_STS_TPM_GO(1 << 5)
+#define TPM_TIS_STS_DATA_AVAILABLE(1 << 4)
+#define TPM_TIS_STS_EXPECT(1 << 3)
+#define TPM_TIS_STS_RESPONSE_RETRY(1 << 1)
+
+#define TPM_TIS_BURST_COUNT_SHIFT 8
+#define TPM_TIS_BURST_COUNT(X) \
+((X) << TPM_TIS_BURST_COUNT_SHIFT)
+
+#define TPM_TIS_ACCESS_TPM_REG_VALID_STS  (1 << 7)
+#define TPM_TIS_ACCESS_ACTIVE_LOCALITY(1 << 5)
+#define TPM_TIS_ACCESS_BEEN_SEIZED(1 << 4)
+#define TPM_TIS_ACCESS_SEIZE  (1 << 3)
+#define TPM_TIS_ACCESS_PENDING_REQUEST(1 << 2)
+#define TPM_TIS_ACCESS_REQUEST_USE(1 << 1)
+#define TPM_TIS_ACCESS_TPM_ESTABLISHMENT  (1 << 0)
+
+#define TPM_TIS_INT_ENABLED   (1 << 31)
+#define TPM_TIS_INT_DATA_AVAILABLE(1 << 0)
+#define TPM_TIS_INT_STS_VALID (1 << 1)
+#define TPM_TIS_INT_LOCALITY_CHANGED  (1 << 2)
+#define TPM_TIS_INT_COMMAND_READY (1 << 7)
+
+#define TPM_TIS_INT_POLARITY_MASK (3 << 3)
+#define TPM_TIS_INT_POLARITY_LOW_LEVEL(1 << 3)
+
+#ifndef RAISE_STS_IRQ
+
+#define TPM_TIS_INTERRUPTS_SUPPORTED (TPM_TIS_INT_LOCALITY_CHANGED | \
+  TPM_TIS_INT_DATA_AVAILABLE   | \
+  TPM_TIS_INT_COMMAND_READY)
+
+#else
+
+#define TPM_TIS_INTERRUPTS_SUPPORTED (TPM_TIS_INT_LOCALITY_CHANGED | \
+  TPM_TIS_INT_DATA_AVAILABLE   | \
+  TPM_TIS_INT_STS_VALID | \
+  TPM_TIS_INT_COMMAND_READY)
+
+#endif
+
+#define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is mandatory */
+#define TPM_TIS_CAPABILITIES_SUPPORTED   (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
+  TPM_TIS_INTERRUPTS_SUP

[Qemu-devel] [PATCH V22 4/7] Build the TPM frontend code

[Stefan Berger]

Build the TPM frontend code that has been added so far.

Signed-off-by: Stefan Berger 
Reviewed-by: Corey Bryant 
---
 configure | 11 +++
 tpm/Makefile.objs |  1 +
 2 files changed, 12 insertions(+)

diff --git a/configure b/configure
index 8789324..b7359aa 100755
--- a/configure
+++ b/configure
@@ -226,6 +226,7 @@ coroutine=""
 seccomp=""
 glusterfs=""
 virtio_blk_data_plane=""
+tpm="no"
 
 # parse CC options first
 for opt do
@@ -897,6 +898,8 @@ for opt do
   ;;
   --enable-virtio-blk-data-plane) virtio_blk_data_plane="yes"
   ;;
+  --enable-tpm) tpm="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1146,6 +1149,7 @@ echo "  --enable-glusterfs   enable GlusterFS backend"
 echo "  --disable-glusterfs  disable GlusterFS backend"
 echo "  --enable-gcovenable test coverage analysis with gcov"
 echo "  --gcov=GCOV  use specified gcov [$gcov_tool]"
+echo "  --enable-tpm enable TPM support"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -3344,6 +3348,7 @@ echo "GlusterFS support $glusterfs"
 echo "virtio-blk-data-plane $virtio_blk_data_plane"
 echo "gcov  $gcov_tool"
 echo "gcov enabled  $gcov"
+echo "TPM support   $tpm"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -4251,6 +4256,12 @@ if test "$gprof" = "yes" ; then
   fi
 fi
 
+if test "$tpm" = "yes"; then
+  if test "$target_softmmu" = "yes" ; then
+echo "CONFIG_TPM=y" >> $config_host_mak
+  fi
+fi
+
 if test "$ARCH" = "tci"; then
   linker_script=""
 else
diff --git a/tpm/Makefile.objs b/tpm/Makefile.objs
index dffb567..63bfcea 100644
--- a/tpm/Makefile.objs
+++ b/tpm/Makefile.objs
@@ -1 +1,2 @@
 common-obj-y = tpm.o
+common-obj-$(CONFIG_TPM) += tpm_tis.o
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH for-1.4 07/19] target-sparc: Fix debug output for DEBUG_MMU

[Blue Swirl]

On Thu, Feb 14, 2013 at 3:46 PM, Andreas Färber  wrote:
> Blue,
>
> Am 27.01.2013 14:32, schrieb Andreas Färber:
>> Signed-off-by: Andreas Färber 
>> ---
>>  target-sparc/ldst_helper.c |2 +-
>>  1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
>>
>> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
>> index cf1bddf..7decd66 100644
>> --- a/target-sparc/ldst_helper.c
>> +++ b/target-sparc/ldst_helper.c
>> @@ -1850,7 +1850,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
>> addr, target_ulong val,
>>  DPRINTF_MMU("LSU change: 0x%" PRIx64 " -> 0x%" PRIx64 "\n",
>>  oldreg, env->lsu);
>>  #ifdef DEBUG_MMU
>> -dump_mmu(stdout, fprintf, env1);
>> +dump_mmu(stdout, fprintf, env);
>>  #endif
>>  tlb_flush(env, 1);
>>  }
>
> This patch was not applied to master. Was that an oversight or is
> something wrong with it? (Preparing a v2 series for 1.5.)

Wasn't this a part of RFC series? What happened with the rest of the 19?

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

Re: [Qemu-devel] [RFC] Disassembler options going forward

[Anthony Liguori]

Richard Henderson  writes:

> On 02/13/2013 06:28 PM, Anthony Liguori wrote:
>> QEMU is GPLv2 only so we can't take GPLv3 code.  We're stuck on binutils
>> code that predates the v3 relicense.
>
> Ok, this is something that's going to bite us more and more.
>
> We need *some* solution that allows us to disassemble current cpus.
> What we've got in disas/ is, except for really old cpus, completely out
> of date:

disas/ is just used for tracing, right?

Could we not write out a binary log and then have another tool that was
GPLv3 and linked against libopcodes to display the logs in a text
format?

Regards,

Anthony Liguori

>
>  * x86 missing all opcodes from sse4 and beyond,
>  * s390 missing tons of opcodes (I filled in some, but not all)
>  * ppc missing tons of opcodes (2.06 and later?)
>
> The only options I can see going forward are
>
>  1) Provide a configure time option to link to the "system" libopcodes.
>  2) Use someone else's (bsd licensed?) disassember.
>  3) Rearrange relevant translators so that they can disassemble and
> not translate.
>
> The complete solution could be a combination of all three.
>
> To me, option (1) means that qemu the project is not "infecting" the
> binary with GPLv3, but requiring the user to make that choice.  Which
> seems fine; those that have moral objections to v3 can simply not use
> that configure option.  It's a bit awkward that most distributions don't
> package up libopcodes for install, but if you manually build binutils
> you'll have it done.
>
> I hope we'll all agree that option (3) is not ideal, since having a
> separate disassembler works as a check against the translator.  However,
> for odd parts that will never be a host it's not a totally unreasonable
> solution, as it at least provides *some* disassembly.
>
> As for option (2), I'm not even sure what to suggest.  I suppose there's
> some part of LLVM that does textual disassembly.  Whether we want to
> drag that in as a submodule or just require it to be installed and
> notice it at configure time, I have no opinion.  But because of the odd
> parts, (2) can't be the only option.
>
> But most of all I think we should have a plan.
>
>
> r~

Re: [Qemu-devel] [RFC] Disassembler options going forward

[Peter Maydell]

On 14 February 2013 21:10, Richard Henderson  wrote:
> The only options I can see going forward are
>
>  1) Provide a configure time option to link to the "system" libopcodes.
>  2) Use someone else's (bsd licensed?) disassember.
>  3) Rearrange relevant translators so that they can disassemble and
> not translate.
>
> The complete solution could be a combination of all three.

4) Since we only do disassembly for debug logging, have our debug
logging just log hex, with postprocessing by a script that runs
objdump or something

> To me, option (1) means that qemu the project is not "infecting" the
> binary with GPLv3, but requiring the user to make that choice.  Which
> seems fine; those that have moral objections to v3 can simply not use
> that configure option.  It's a bit awkward that most distributions don't
> package up libopcodes for install, but if you manually build binutils
> you'll have it done.

I'm not hugely convinced by the idea of "here's a configure switch
to produce binaries you can't legally distribute".

> I hope we'll all agree that option (3) is not ideal, since having a
> separate disassembler works as a check against the translator.  However,
> for odd parts that will never be a host it's not a totally unreasonable
> solution, as it at least provides *some* disassembly.
>
> As for option (2), I'm not even sure what to suggest.  I suppose there's
> some part of LLVM that does textual disassembly.  Whether we want to
> drag that in as a submodule or just require it to be installed and
> notice it at configure time, I have no opinion.  But because of the odd
> parts, (2) can't be the only option.

I had a look at the LLVM disassembler the other day. From a quick
glance it seems like LLVM drives the disassembler off a generalised
machine description language (which it also uses for various other
things), so getting the disassemblers would also require us to
pull in quite a bit of LLVM infrastructure for parsing the machine
descriptions. It didn't look particularly easy, but this is just
from 15 minutes browsing a source tree, so if anybody more LLVM
aware here has an opinion do say.

> But most of all I think we should have a plan.

Agreed.

-- PMM

Re: [Qemu-devel] [PATCH v2] Move File operations to qemu-file.c

[Blue Swirl]

On Thu, Feb 14, 2013 at 2:34 AM, Anthony Liguori  wrote:
> Joel Schopp  writes:
>
 +if(popen_file == NULL) {
>>>
>>> Please make a preparatory patch which adds missing spaces between 'if'
>>> statements and '('.
>>
>> I'll do a preparatory style cleanup patch of existing code if it is
>> deemed necessary by the maintainers, but I don't think it's a good
>> idea.
>
> I basically hate checkpatch :-)  There's no need to do a style cleanup,
> it's just going to confuse gits move detection and screw up merging.  In
> this case, it's such a trivial thing too.

Either we do code cleanups when possible or we forget about
CODING_STYLE and checkpatch.pl mess.

The whole point of having a separate patch to do the cleanup is to
keep git move detection happy.

>
> I disabled the automated checkpatch bot because it got too annoying.  It
> throws way too many false positives or annoying nits that shouldn't keep
> us from merging useful code.

Those 'nits' improve the code base. It means that a patch to fix one
thing must also improve the CODING_STYLE while at it. The alternative
to enforcing this is to do codebase cleanups separately, for example
in form of global reformatting and flag days.

We don't have many written rules and nobody seems to want to follow them.

>
> I haven't applied this patch because we're in freeze for the 1.4
> release.
>
> Regards,
>
> Anthony Liguori
>
>>   The patch as it stands now simply moves existing code to another file
>> and thus is pretty safe.  Adding a preparatory patch to reformat the
>> code is easy to mess up and raises the chances of introducing a regression.
>>
>> Why not just submit patches to clean up coding style for the entire code
>> base independent of any refactoring?
>>
>> When I originally wrote checkpatch.pl it was with the intention of
>> avoiding arguments over coding style.  I see that I missed a corner case
>> by having it not notice code moves as a special case to ignore.
>>
>> -Joel

Re: [Qemu-devel] [SeaBIOS] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[David Woodhouse]

On Thu, 2013-02-14 at 22:14 +0100, Laszlo Ersek wrote:
> On 02/14/13 21:54, H. Peter Anvin wrote:
> > On 02/14/2013 12:41 PM, Laszlo Ersek wrote:
> >>
> >> ). cpu_reset() [target-i386/helper.c] sets CS:IP to f000:fff0, which is
> >> the exact address of... reset_vector() in SeaBIOS.
> >>
> > 
> > This would be a bug, but it isn't quite true.
> > 
> > If you look at x86_cpu_reset() you will note that it sets the code
> > segment base to 0x, not 0xf as one could expect from the
> > above.  This is also true of a physical x86.
> > 
> > As such, the *real* reset vector is at 0xfff0 as opposed to the
> > SeaBIOS vector at 0x0 -- this is a backwards compatibility vector
> > which typically just issues a real reset.
> > 
> > Now, if Qemu doesn't handle the distinction here correctly, that is a bug.
> 
> I think I was simply wrong :)

So it *is* jumping to 0xfff0 but the memory at that location isn't
what we expect? Do the PAM registers affect *that* too, or only the
region from 0xc-0xf? Surely the contents at 4GiB-δ should be
unchanged by *anything* we do with the PAM registers?

Or maybe not... after also downloading the i440fx data sheet, I'm even
more confused. There's some aliasing with... not the region at 1MiB-δ
but the region at 16MiB-δ:

(From §4.1 System Address Map):

2. High BIOS Area (FFE0_h−− _h)
  The top 2 Mbytes of the Extended Memory Region is reserved for System
  BIOS (High BIOS), extended BIOS for PCI devices, and the A20 alias of
  the system BIOS. The CPU begins execution from the High BIOS after
  reset. This region is mapped to the PCI so that the upper subset of
  this region is aliased to 16 Mbytes minus 256-Kbyte range.


-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Lucas Meneghel Rodrigues]

On 02/14/2013 05:57 PM, Eduardo Habkost wrote:

On Thu, Feb 14, 2013 at 08:28:03PM +0100, Igor Mammedov wrote:

On Thu, 14 Feb 2013 13:13:18 +0100
Paolo Bonzini  wrote:


Il 14/02/2013 12:18, Eduardo Habkost ha scritto:

qemu boots from disk image 3 times faster than direct kernel load.

That's surprising. Do you have any idea why that happens?


Because kernel load uses MMIO (from fw_cfg), while booting from disk
uses at worst PCI DMA and at best virtio.

Paolo

I've tried Paolo's suggestion to run in TCG mode with instruction logging,
and it spends too much time reading 11K kernel from qemu compared to
kvm-unit-test kernel. I'll need debug it further to find out why.
But this patch looks unnecessary if I could fix qemu/seabios or test kernel
to reach the same load time as kvm-unit-test kernel.


Agreed.

Lucas, please don't apply this patch yet, until we find the actual cause
of the slow booting.


Okey dokey.

[Qemu-devel] [Bug 1079080] Re: ARM instruction "srs" wrong behaviour

[vcesson]

** Attachment added: "test case image (similar image)"
   
https://bugs.launchpad.net/qemu/+bug/1079080/+attachment/3528447/+files/serial2

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

Title:
  ARM instruction "srs" wrong behaviour

Status in QEMU:
  Confirmed

Bug description:
  Quote from ARM Architecture Reference Manual ARMv7-A and ARMv7-R :
  "Store Return State stores the LR and SPSR of the current mode to the stack 
of a specified mode"

  Problem:
  When executing this instruction, the register stored is CPSR instead of SPSR.

  Context:
  Using QEMU 1.2.0 to simulate a Zynq application (processor Cortex-a9 mpcore) 
with the following command line:
  qemu-system-arm -M xilinx-zynq-a9 -m 512 -serial null -serial mon:stdio -dtb 
/home/vcesson/workspace/xilinx_zynq.dtb -kernel 
install/tests/io/serial/current/tests/serial2 -S -s -nographic

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

[Qemu-devel] [Bug 1079080] Re: ARM instruction "srs" wrong behaviour

[vcesson]

** Attachment added: "test case dtb"
   
https://bugs.launchpad.net/qemu/+bug/1079080/+attachment/3528448/+files/xilinx_zynq.dtb

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

Title:
  ARM instruction "srs" wrong behaviour

Status in QEMU:
  Confirmed

Bug description:
  Quote from ARM Architecture Reference Manual ARMv7-A and ARMv7-R :
  "Store Return State stores the LR and SPSR of the current mode to the stack 
of a specified mode"

  Problem:
  When executing this instruction, the register stored is CPSR instead of SPSR.

  Context:
  Using QEMU 1.2.0 to simulate a Zynq application (processor Cortex-a9 mpcore) 
with the following command line:
  qemu-system-arm -M xilinx-zynq-a9 -m 512 -serial null -serial mon:stdio -dtb 
/home/vcesson/workspace/xilinx_zynq.dtb -kernel 
install/tests/io/serial/current/tests/serial2 -S -s -nographic

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

Re: [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Laszlo Ersek]

On 02/14/13 21:54, H. Peter Anvin wrote:
> On 02/14/2013 12:41 PM, Laszlo Ersek wrote:
>>
>> ). cpu_reset() [target-i386/helper.c] sets CS:IP to f000:fff0, which is
>> the exact address of... reset_vector() in SeaBIOS.
>>
> 
> This would be a bug, but it isn't quite true.
> 
> If you look at x86_cpu_reset() you will note that it sets the code
> segment base to 0x, not 0xf as one could expect from the
> above.  This is also true of a physical x86.
> 
> As such, the *real* reset vector is at 0xfff0 as opposed to the
> SeaBIOS vector at 0x0 -- this is a backwards compatibility vector
> which typically just issues a real reset.
> 
> Now, if Qemu doesn't handle the distinction here correctly, that is a bug.

I think I was simply wrong :)

Thanks
Laszlo

[Qemu-devel] [RFC] Disassembler options going forward

[Richard Henderson]

On 02/13/2013 06:28 PM, Anthony Liguori wrote:
> QEMU is GPLv2 only so we can't take GPLv3 code.  We're stuck on binutils
> code that predates the v3 relicense.

Ok, this is something that's going to bite us more and more.

We need *some* solution that allows us to disassemble current cpus.
What we've got in disas/ is, except for really old cpus, completely out
of date:

 * x86 missing all opcodes from sse4 and beyond,
 * s390 missing tons of opcodes (I filled in some, but not all)
 * ppc missing tons of opcodes (2.06 and later?)

The only options I can see going forward are

 1) Provide a configure time option to link to the "system" libopcodes.
 2) Use someone else's (bsd licensed?) disassember.
 3) Rearrange relevant translators so that they can disassemble and
not translate.

The complete solution could be a combination of all three.

To me, option (1) means that qemu the project is not "infecting" the
binary with GPLv3, but requiring the user to make that choice.  Which
seems fine; those that have moral objections to v3 can simply not use
that configure option.  It's a bit awkward that most distributions don't
package up libopcodes for install, but if you manually build binutils
you'll have it done.

I hope we'll all agree that option (3) is not ideal, since having a
separate disassembler works as a check against the translator.  However,
for odd parts that will never be a host it's not a totally unreasonable
solution, as it at least provides *some* disassembly.

As for option (2), I'm not even sure what to suggest.  I suppose there's
some part of LLVM that does textual disassembly.  Whether we want to
drag that in as a submodule or just require it to be installed and
notice it at configure time, I have no opinion.  But because of the odd
parts, (2) can't be the only option.

But most of all I think we should have a plan.


r~

Re: [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[David Woodhouse]

On Thu, 2013-02-14 at 12:54 -0800, H. Peter Anvin wrote:
> This would be a bug, but it isn't quite true.
> 
> If you look at x86_cpu_reset() you will note that it sets the code
> segment base to 0x, not 0xf as one could expect from the
> above.  This is also true of a physical x86.
> 
> As such, the *real* reset vector is at 0xfff0 as opposed to the
> SeaBIOS vector at 0x0 -- this is a backwards compatibility vector
> which typically just issues a real reset.

In SeaBIOS it doesn't. It jumps to entry_post(). Which is fine for
native SeaBIOS, but I suppose I need to fix it to do a *real* reset in
the CSM case, for those operating systems which will switch back to
16-bit mode and jump to f000:fff0 to reboot.

Of course, if said "real reset" is only going to get straight back to
the same 0x0 reset vector, that's not going to help. But at least
then none of it will be *my* fault :)

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH v2] Add option to mlock qemu and guest memory

[Satoru Moriya]

On 02/14/2013 03:39 PM, Michael Tokarev wrote:
> 15.02.2013 00:21, Satoru Moriya wrote:
>> We have some plans to migrate legacy enterprise systems which require 
>> low latency (10 msec order) to kvm virtualized environment. In our 
>> usecase, the system runs with other untrusted guests and so locking 
>> memory which is used by the system is needed to avoid latency impacts 
>> from other guests' memory activity.
> 
> Not a patch review, but maybe using (explicit) huge pages will solve 
> this issue already?  Huge pages aren't swappable...

For some scenarios, you're right.

But, I think, -mem-prealloc/-mem-path uses huge pages only for
guest RAM and it doesn't for qemu itself. It may cause latency impact.

Also using huge page reduces flexibility/usability because if
we do live migration, we have to allocate huge pages before
live migration (basically we don't use live migration for the
guest which needs low latency but sometimes we need to do it).

Regards,
Satoru

[Qemu-devel] [ANNOUNCE] QEMU 1.4.0-rc2 is now available

[Anthony Liguori]

Hi,

On behalf of the QEMU Team, I'd like to announce the availability of the
third release candidate for the QEMU 1.4 release.  This release is meant
for testing purposes and should not be used in a production environment.

http://wiki.qemu.org/download/qemu-1.4.0-rc2.tar.bz2

You can help improve the quality of the QEMU 1.4 release by testing this
release and reporting bugs on Launchpad:

https://bugs.launchpad.net/qemu/

The release plan for the 1.4 release is available at:

http://wiki.qemu.org/Planning/1.4

Please add entries to the ChangeLog for the 1.4 release below:

http://wiki.qemu.org/ChangeLog/Next

 - Revert "Update OpenBIOS images" (Alexander Graf)
 - cadance_uart: Accept input after rx FIFO pop (Peter Crosthwaite)
 - block/curl: only restrict protocols with libcurl>=7.19.4 (Stefan Hajnoczi)
 - qapi: Flatten away ChardevPort (Markus Armbruster)
 - chardev: Fix manual page and qemu-doc for -chardev tty (Markus Armbruster)
 - net: Avoid NULL function pointer dereference on cleanup (Andreas Färber)
 - s390: Fix handling of iscs. (Cornelia Huck)
 - s390: Keep I/O interrupts enabled for all iscs. (Cornelia Huck)
 - s390/sclpconsole: prevent char layer callback during initialization 
(Christian Borntraeger)
 - xilinx.h: s/xilinx_axiethernetdma()/xilinx_axidma() (Peter Crosthwaite)
 - xilinx.h: Dont qdev_create from ethernet_create() (Peter Crosthwaite)
 - block-migration: fix pending() and iterate() return values (Stefan Hajnoczi)
 - migration: make qemu_ftell() public and support writable files (Stefan 
Hajnoczi)
 - trace: deal with deprecated glib thread functions (Stefan Hajnoczi)
 - trace: use glib atomic int types (Stefan Hajnoczi)
 - Revert "block/vpc: Fix size calculation" (Stefan Hajnoczi)
 - block/raw-posix: detect readonly Linux block devices using BLKROGET (Stefan 
Hajnoczi)
 - hw/m25p80.c: add WRSR(0x01) support (Kuo-Jung Su)
 - qapi: Improve chardev-add documentation (Markus Armbruster)
 - migration: restrict scope of incoming fd read handler (Stefan Hajnoczi)
 - libi2c-omap: Fix endianness dependency (Andreas Färber)
 - qtest: Use strtoull() for uint64_t (Andreas Färber)
 - libqtest: Fix documentation copy&paste errors (Andreas Färber)
 - block/vpc: Fix size calculation (Stefan Weil)
 - block-migration: fix block_save_iterate() return value (Stefan Hajnoczi)
 - block-migration: fix blk_mig_save_dirty_block() return value checking 
(Stefan Hajnoczi)
 - block-migration: improve "Unknown flags" error message (Stefan Hajnoczi)
 - vl: Exit unsuccessfully on option argument syntax error (Markus Armbruster)
 - vl: Drop redundant "parse error" reports (Markus Armbruster)
 - qemu-option: Disable two helpful messages that got broken recently (Markus 
Armbruster)
 - error: Strip trailing '\n' from error string arguments (again) (Markus 
Armbruster)
 - error: Clean up abuse of error_report() for help (Markus Armbruster)
 - error: Clean up error strings with embedded newlines (Markus Armbruster)
 - Update OpenBIOS images (Blue Swirl)
 - xilinx_zynq: Fix wrong IRQ number of the second EHCI controller (Liming Wang)
 - block/curl: disable extra protocols to prevent CVE-2013-0249 (Stefan 
Hajnoczi)
 - qemu-nbd: document --cache and --aio options (Paolo Bonzini)
 - hw/virtio-net: disable multiqueue by default (Jesse Larrew)
 - hw/virtio-net.c: set config size using host features (Jesse Larrew)
 - virtio-net: pass host features to virtio_net_init (Anthony Liguori)
 - net: fix infinite loop on exit (Michael Roth)
 - tests/test-string-input-visitor: Handle errors provoked by fuzz test (Peter 
Maydell)

Regards,

Anthony Liguori

Re: [Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[David Woodhouse]

On Thu, 2013-02-14 at 21:41 +0100, Laszlo Ersek wrote:
> Can we dumb down ^W^W generalize this code? :) Or maybe should qemu
> introduce a reset handler for PAMs?

In the UEFI+CSM model, I believe the handling of PAM stuff is left
*entirely* to the UEFI side and the CSM is supposed to be
hardware-agnostic. So actually bashing on the PAM registers from the CSM
side would be my last resort. It's why it's important to fix the
UmbStart/UmbEnd thing correctly, too.

Other people might have been happy to hack up something
machine-specific, given that they control both UEFI and CSM sides of it
and they're shipping their own proprietary version where nobody can see
what they're doing. But when the CSM spec is the interface between two
entirely *separate* projects (OVMF and SeaBIOS), I think it's important
that we *follow* the spec and don't have nasty hacks.

So, if real hardware would reset the PAMs on reset and avoid the need
for SeaBIOS to do so, I think we should be doing the same in qemu too.

Thanks for testing this, btw. Are you looking at suspend/resume too? :)

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [edk2] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[H. Peter Anvin]

On 02/14/2013 12:41 PM, Laszlo Ersek wrote:
> 
> ). cpu_reset() [target-i386/helper.c] sets CS:IP to f000:fff0, which is
> the exact address of... reset_vector() in SeaBIOS.
> 

This would be a bug, but it isn't quite true.

If you look at x86_cpu_reset() you will note that it sets the code
segment base to 0x, not 0xf as one could expect from the
above.  This is also true of a physical x86.

As such, the *real* reset vector is at 0xfff0 as opposed to the
SeaBIOS vector at 0x0 -- this is a backwards compatibility vector
which typically just issues a real reset.

Now, if Qemu doesn't handle the distinction here correctly, that is a bug.

-hpa

Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

[Michael S. Tsirkin]

On Thu, Feb 14, 2013 at 01:50:04PM -0600, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
> > QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
> > 1.3 machine during link auto negotiation, the guest link will be set to 
> > down.
> > Fix this by just disabling auto negotiation for 1.3.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  hw/e1000.c   | 25 +
> >  hw/pc_piix.c |  4 
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/hw/e1000.c b/hw/e1000.c
> > index d6fe815..263f2f4 100644
> > --- a/hw/e1000.c
> > +++ b/hw/e1000.c
> > @@ -131,6 +131,11 @@ typedef struct E1000State_st {
> >  } eecd_state;
> >  
> >  QEMUTimer *autoneg_timer;
> > +
> > +/* Enabled compatibility for migration to/from rhel6.3.0 and older */
> 
> I presume you mean QEMU 1.3...

Yack.

> > +#define E1000_FLAG_QEMU_13_BIT 0
> > +#define E1000_FLAG_QEMU_13 (1 << E1000_FLAG_QEMU_13_BIT)
> > +uint32_t compat_flags;
> >  } E1000State;
> >  
> >  #definedefreg(x)   x = (E1000_##x>>2)
> > @@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
> >  static void
> >  set_phy_ctrl(E1000State *s, int index, uint16_t val)
> >  {
> > +/*
> > + * QEMU 1.3 does not support link auto-negotiation emulation, so if we
> > + * migrate during auto negotiation, after migration the link will be
> > + * down.
> > + */
> > +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> > +return;
> > +}
> >  if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
> >  e1000_link_down(s);
> >  s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
> > @@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
> >  {
> >  E1000State *s = opaque;
> >  NetClientState *nc = qemu_get_queue(s->nic);
> > +
> > +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> > +return;
> > +}
> > +
> >  /*
> >   * If link is down and auto-negotiation is ongoing, complete
> >   * auto-negotiation immediately.  This allows is to look at
> > @@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int 
> > version_id)
> >   * to link status bit in mac_reg[STATUS].
> >   * Alternatively, restart link negotiation if it was in progress. */
> >  nc->link_down = (s->mac_reg[STATUS] & E1000_STATUS_LU) == 0;
> > +
> > +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> > +return 0;
> > +}
> > +
> >  if (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN &&
> >  s->phy_reg[PHY_CTRL] & MII_CR_RESTART_AUTO_NEG &&
> >  !(s->phy_reg[PHY_STATUS] & MII_SR_AUTONEG_COMPLETE)) {
> > @@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
> >  
> >  static Property e1000_properties[] = {
> >  DEFINE_NIC_PROPERTIES(E1000State, conf),
> > +DEFINE_PROP_BIT("x-qemu_13_compat", E1000State,
> > +compat_flags, E1000_FLAG_QEMU_13, false),
> >  DEFINE_PROP_END_OF_LIST(),
> 
> Would it make more sense to simply call this auto-negotiation?

Okay.

> Regards,
> 
> Anthony Liguori
> 
> >  };
> >  
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index ba09714..89a3f1f 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
> >  .driver   = "vmware-svga",
> >  .property = "rombar",
> >  .value= stringify(0),
> > +},{
> > +.driver   = "e1000",
> > +.property = "x-qemu_13_compat",
> > +.value= "on",
> >  },
> >  { /* end of list */ }
> >  },
> > -- 
> > MST

Re: [Qemu-devel] [PATCH v2] Add option to mlock qemu and guest memory

[Michael Tokarev]

15.02.2013 00:21, Satoru Moriya wrote:
> We have some plans to migrate legacy enterprise systems which require
> low latency (10 msec order) to kvm virtualized environment. In our
> usecase, the system runs with other untrusted guests and so locking
> memory which is used by the system is needed to avoid latency
> impacts from other guests' memory activity.

Not a patch review, but maybe using (explicit) huge pages will solve
this issue already?  Huge pages aren't swappable...

Thanks,

/mjt

[Qemu-devel] (PAM stuff) reset doesn't work on OVMF + SeaBIOS CSM

[Laszlo Ersek]

Hi,

I noticed that under OVMF + SeaBIOS CSM + your related patches for both,
reset requested by the guest doesn't work as expected. The behavior is
an infinite loop, with the following debug fragment repeated by the
CSM-ized SeaBIOS:

  In resume (status=0)
  In 32bit resume
  Attempting a hard reboot
  i8042_wait_write

The corresponding call chain seems to be:

  reset_vector() [src/romlayout.S]
entry_post()
  entry_resume()
handle_resume() [src/resume.c]
  prints "In resume"
  handle_resume32()
prints "In 32bit resume"
tryReboot()
  prints "Attempting a hard reboot"
  i8042_reboot() [src/ps2port.c]
i8042_wait_write()
  prints "i8042_wait_write"
outb(0xfe, PORT_PS2_STATUS)

(The entry_post -> entry_resume jump occurs because HaveRunPost has been
set to 1 by csm_maininit() --> interface_init() --> ivt_init().)

At this point kbd_write_command() in qemu-kvm's "hw/pckbd.c", case
KBD_CCMD_RESET, requests a system reset. Soon the reset handlers run,
among them cpu_reset() (which was registered by

  pc_init1() [hw/pc.c]
pc_new_cpu()

). cpu_reset() [target-i386/helper.c] sets CS:IP to f000:fff0, which is
the exact address of... reset_vector() in SeaBIOS.

Of course OVMF should be re-run instead of SeaBIOS. When qemu-kvm
starts, "OVMF.fd" is installed as ROM, such that the last address it
occupies is "all-bits-one", independently of its size (below a limit of
course):

  pc_init1() [hw/pc.c]
rom_add_file_fixed() []
  open() / read() /close()
  rom_insert()
some calls to inform KVM


I think when OVMF runs SeaBIOS as CSM, OVMF shadows the original ROM
(containing the OVMF binary itself) with the SeaBIOS code + static data
(I'm peeking at
...). This should
render SeaBIOS visible / executable / writeable in the top 16-bit
segment, and leave OVMF in a permanently unusable state (in RAM at least).

My guess at the relevant edk2 function is ShadowAndStartLegacy16()
[IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c]. The
LegacyRegion->UnLock() call should be instrumental (implemented in
"OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c" with PAM (Programmable
Attribute Map) registers?)

Hence I *presume* qemu-kvm should un-shadow the ROM at reset time (ie.
make OVMF visible again as ROM) not later than allowing the VCPU to
continue at f000:fff0 again. Normally that address should be occupied by
OVMF code from (I guess) "UefiCpuPkg/ResetVector/Vtf0".

Does this make any sense? Is qemu-kvm forgetting to reset the PAMs? Or
would that be the responsibility of tryReboot() in SeaBIOS?

... Aah! qemu_prep_reset() in SeaBIOS [src/shadow.c] goes like:

void
qemu_prep_reset(void)
{
if (!CONFIG_QEMU)
return;
// QEMU doesn't map 0xc-0xf back to the original rom on a
// reset, so do that manually before invoking a hard reset.
make_bios_writable();
extern u8 code32flat_start[], code32flat_end[];
memcpy(code32flat_start, code32flat_start + BIOS_SRC_OFFSET
   , code32flat_end - code32flat_start);

if (HaveRunPost)
// Memory copy failed to work - try to halt the machine.
apm_shutdown();
}

and this function is actually called inside the infinite loop (I ignored
it before):

tryReboot()
  prints "Attempting a hard reboot"
  qemu_prep_reset() [src/shadow.c] <-- here
  i8042_reboot() [src/ps2port.c]
i8042_wait_write()
  prints "i8042_wait_write"
outb(0xfe, PORT_PS2_STATUS)

but of course it doesn't do anything with CONFIG_CSM (since that implies
!CONFIG_QEMU). What's more, qemu_prep_reset() and
make_bios_writable_intel() seem to exploit SeaBIOS characteristics
(code32flat_*, HaveRunPost etc.) that probably make no sense when the
data being restored is a different (= OVMF) image.

Can we dumb down ^W^W generalize this code? :) Or maybe should qemu
introduce a reset handler for PAMs?

(I realize I've been reading all the time about PAMs, in the "Writeable
files in fw_cfg" thread, in the discussion about unlocking the 0xE
segment for stack purposes... Didn't understand a single word before,
sorry. Downloaded my copy of the i440FX spec just today; I finally have
a remote idea how shadowing / write-protecting works.)

Thanks!
Laszlo

[Qemu-devel] [PATCH v2] Add option to mlock qemu and guest memory

[Satoru Moriya]

We have some plans to migrate legacy enterprise systems which require
low latency (10 msec order) to kvm virtualized environment. In our
usecase, the system runs with other untrusted guests and so locking
memory which is used by the system is needed to avoid latency
impacts from other guests' memory activity.

ChangeLog:
v2
 - Change the option name from -mlock to -realtime mlock=on|off
 - Rebase qemu version 1.3.91
 - Update patch description

---
In certain scenario, latency induced by paging is significant and
memory locking is needed. Also, in the scenario with untrusted
guests, latency improvement due to mlock is desired.

This patch introduces a following new option to mlock guest and
qemu memory:

-realtime mlock=on|off

Signed-off-by: Satoru Moriya 
---
 include/sysemu/os-posix.h |  1 +
 include/sysemu/os-win32.h |  1 +
 os-posix.c|  8 
 qemu-options.hx   | 13 +
 vl.c  | 31 +++
 5 files changed, 54 insertions(+)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 7f198e4..2f2ead6 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -31,6 +31,7 @@ void os_set_proc_name(const char *s);
 void os_setup_signal_handling(void);
 void os_daemonize(void);
 void os_setup_post(void);
+void os_mlock(void);
 
 typedef struct timeval qemu_timeval;
 #define qemu_gettimeofday(tp) gettimeofday(tp, NULL)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index bf9edeb..a74ca13 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -80,6 +80,7 @@ static inline void os_daemonize(void) {}
 static inline void os_setup_post(void) {}
 void os_set_line_buffering(void);
 static inline void os_set_proc_name(const char *dummy) {}
+static inline void os_mlock(void) {}
 
 #if !defined(EPROTONOSUPPORT)
 # define EPROTONOSUPPORT EINVAL
diff --git a/os-posix.c b/os-posix.c
index 5c64518..1304b0e 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -363,3 +363,11 @@ bool is_daemonized(void)
 {
 return daemonize;
 }
+
+void os_mlock(void)
+{
+if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
+perror("mlockall");
+exit(1);
+}
+}
diff --git a/qemu-options.hx b/qemu-options.hx
index 9d7131a..843fcb4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2464,6 +2464,19 @@ STEXI
 Do not start CPU at startup (you must type 'c' in the monitor).
 ETEXI
 
+DEF("realtime", HAS_ARG, QEMU_OPTION_realtime,
+"-realtime [mlock=on|off]\n"
+"run qemu with realtime features\n"
+"mlock=on|off controls mlock support (default: on)\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -realtime mlock=on|off
+@findex -realtime
+Run qemu with realtime features.
+mlocking qemu and guest memory can be enabled via @option{mlock=on}
+(enabled by default).
+ETEXI
+
 DEF("gdb", HAS_ARG, QEMU_OPTION_gdb, \
 "-gdb devwait for gdb connection on 'dev'\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/vl.c b/vl.c
index 1355f69..c16c8ad 100644
--- a/vl.c
+++ b/vl.c
@@ -491,6 +491,18 @@ static QemuOptsList qemu_object_opts = {
 },
 };
 
+static QemuOptsList qemu_realtime_opts = {
+.name = "realtime",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_realtime_opts.head),
+.desc = {
+{
+.name = "mlock",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
 const char *qemu_get_vm_name(void)
 {
 return qemu_name;
@@ -1384,6 +1396,17 @@ static void smp_parse(const char *optarg)
 max_cpus = smp_cpus;
 }
 
+static void configure_realtime(QemuOpts *opts)
+{
+bool is_mlock;
+
+is_mlock = qemu_opt_get_bool(opts, "mlock", true);
+
+if (is_mlock) {
+os_mlock();
+}
+}
+
 /***/
 /* USB devices */
 
@@ -2860,6 +2883,7 @@ int main(int argc, char **argv, char **envp)
 qemu_add_opts(&qemu_sandbox_opts);
 qemu_add_opts(&qemu_add_fd_opts);
 qemu_add_opts(&qemu_object_opts);
+qemu_add_opts(&qemu_realtime_opts);
 
 runstate_init();
 
@@ -3806,6 +3830,13 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 break;
+case QEMU_OPTION_realtime:
+opts = qemu_opts_parse(qemu_find_opts("realtime"), optarg, 0);
+if (!opts) {
+exit(1);
+}
+configure_realtime(opts);
+break;
 default:
 os_parse_cmd_args(popt->index, optarg);
 }
-- 
1.7.11.7

Re: [Qemu-devel] [RFC PATCH RDMA support v2: 5/6] connection-setup code between client/server

[Michael R. Hines]

Orit (and anthony if you're not busy),

I forgot to respond to this very important comment:

On 02/13/2013 03:46 AM, Orit Wasserman wrote:
Are you still using the tcp for transferring device state? If so you 
can call the tcp functions from the migration rdma code as a first 
step but I would prefer it to use RDMA too.


This is the crux of the problem of using RDMA for migration: Currently 
all of the QEMU migration control logic and device state goes through 
the the QEMUFile implementation. RDMA, however is by nature a zero-copy 
solution and is incompatible with QEMUFile.


Using RDMA for transferring device state is not recommended: Setuping an 
RDMA requires registering the memory locations on both sides with the 
RDMA hardware. This is not ideal because this would require pinning the 
memory holding the device state and then issuing the RDMA transfer for 
*each* type of device - which would require changing the control path of 
every type of migrated device in QEMU.


Currently the Patch we submitted bypasses QEMUFile. It does just issues 
the RDMA transfer for the memory that was dirtied and then continues 
along with the rest of the migration call path normally.


In an ideal world, we would prefer a hyrbid approach, something like:

*Begin Migration Iteration Round:*
1. stop VCPU
2. start iterative pass over memory
3. send control signals (if any) / device state to QEMUFile
4. When a dirty memory page is found, do:
 a) Instruct the QEMUFile to block
 b) Issue the RDMA transfer
 c) Instruct the QEMUFile to unblock
5. resume VCPU

This would require a "smarter" QEMUFile implementation that understands 
when to block and for how long.


Comments?

- Michael

Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Eduardo Habkost]

On Thu, Feb 14, 2013 at 08:28:03PM +0100, Igor Mammedov wrote:
> On Thu, 14 Feb 2013 13:13:18 +0100
> Paolo Bonzini  wrote:
> 
> > Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> > >> > qemu boots from disk image 3 times faster than direct kernel load.
> > > That's surprising. Do you have any idea why that happens?
> > 
> > Because kernel load uses MMIO (from fw_cfg), while booting from disk
> > uses at worst PCI DMA and at best virtio.
> > 
> > Paolo
> I've tried Paolo's suggestion to run in TCG mode with instruction logging,
> and it spends too much time reading 11K kernel from qemu compared to
> kvm-unit-test kernel. I'll need debug it further to find out why.
> But this patch looks unnecessary if I could fix qemu/seabios or test kernel
> to reach the same load time as kvm-unit-test kernel.

Agreed.

Lucas, please don't apply this patch yet, until we find the actual cause
of the slow booting.

-- 
Eduardo

Re: [Qemu-devel] [RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM subclasses

[Eduardo Habkost]

On Thu, Feb 14, 2013 at 08:16:32PM +0100, Igor Mammedov wrote:
> > [...]
> > > > > +
> > > > > +}
> > > > > +
> > > > > +static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
> > > > > +{
> > > > > +X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > > > +x86_def_t *def = data;
> > > > > +int i;
> > > > > +static const char *versioned_models[] = { "qemu32", "qemu64",
> > > > > "athlon" }; +
> > > > > +memcpy(&xcc->info, def, sizeof(x86_def_t));
> > > > > +xcc->info.ext_features |= CPUID_EXT_HYPERVISOR;
> > > > 
> > > > If this is TCG-specific now, we could make the class match the reality
> > > > and mask TCG_FEATURES/TCG_EXT_FEATURES/etc here. That's what happens in
> > > > practice, e.g.: "-cpu SandyBridge" in TCG mode is a completely different
> > > > CPU, today.
> > > well, this function is shared between TCG and KVM, I mean, it's common
> > > code for both. Which asks for one more TCG class_init function for TCG
> > > specific behavior.
> > 
> > Having both TCG-specific and KVM-specific subclasses instead of making
> > the KVM class inherit from the TCG class would make sense to me, as we
> > may have TCG-specific behavior in other places too.
> > 
> > Or we could do memcpy() again on the KVM class_init function. Or we
> > could set a different void* pointer on the TCG class, with
> > already-filtered x86_def_t object. There are multiple possible
> > solutions.
> > 
> > 
> > > But could it be a separate patch (i.e. fixing TCG filtering), I think
> > > just moving tcg filtering might cause regression. And need to worked on
> > > separately.
> > 
> > I'm OK with doing that in a separate patch, as it is not a bug fix or
> > behavior change. But it would be nice to do that before we introduce the
> > feature properties, to make the reported defaults right since the
> > beginning.
> It's behavior change, if we move filtering from realizefn to class_init, user
> would be able to add features not visible now to guest.
> 
> ordering now:
> class_init, initfn, setting defautls, custom features, realizefn(filtering)
> 
> would be ordering with static properties:
> clas_init, static props defaults, global properties(custom features),
> x86_cpu_initfn, realizefn
> 
> in would be scenario filtering comes before custom features, which is not
> necessarily bad and may be nice to consciously add/test features before
> enabling them in filter for everyone, but it's behavior change.
> 

We should keep the filtering on realize because of custom -cpu strings,
but if we filter on class_init too, we will make the class introspection
reflect reality more closely. Wasn't this one the main reasons you
argued (and convinced me) for having separate subclasses?

Also, if we do this we will be able to make "enforce" work for TCG too.
For example: if "enforce" worked for TCG today, people who want to work
around the existing n270 MOVBE bug could use "-cpu n270,+movbe,enforce"
to make sure the QEMU version they are using really accepts movbe.


> > [...]
> > > > > +#ifdef CONFIG_KVM
> > > > > +static void x86_cpu_kvm_def_class_init(ObjectClass *oc, void *data)
> > > > > +{
> > > > > +uint32_t eax, ebx, ecx, edx;
> > > > > +X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > > > +
> > > > > +x86_cpu_def_class_init(oc, data);
> > > > > +/* sysenter isn't supported in compatibility mode on AMD,
> > > > > + * syscall isn't supported in compatibility mode on Intel.
> > > > > + * Normally we advertise the actual CPU vendor, but you can
> > > > > + * override this using the 'vendor' property if you want to use
> > > > > + * KVM's sysenter/syscall emulation in compatibility mode and
> > > > > + * when doing cross vendor migration
> > > > > + */
> > > > > +host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
> > > > 
> > > > Cool, this is exactly what I was going to suggest, to avoid depending on
> > > > the "host" CPU class and KVM initialization.
> > > > 
> > > > I see two options when making the "vendor" property static, and I don't
> > > > know which one is preferable:
> > > > 
> > > > One solution is the one in this patch: to call host_cpuid() here in
> > > > class_init, and have a different property default depending on the host
> > > > CPU.
> > > I prefer this one ^^^.
> > > 
> > > > Another solution is to have default to vendor="host", and have
> > > > instance_init understand vendor="host" as "use the host CPU vendor".
> > > > This would make the property default really static (not depending on the
> > > > host CPU).
> > > > 
> > > 
> > > 
> > > > I am inclined for the latter, because I am assuming that the QOM design
> > > > expects class_init results to be completely static. This is not as bad
> > > > as depending on KVM initialization, but it may be an unexpected
> > > > surprise, or even something not allowed by QOM.
> > > That's where I have to disagree :)
> > > 
> > > You might have a point with 'static' if our goal is for introspection for
> > > source level to work. But w

Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

[Anthony Liguori]

"Michael S. Tsirkin"  writes:

> QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
> 1.3 machine during link auto negotiation, the guest link will be set to down.
> Fix this by just disabling auto negotiation for 1.3.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/e1000.c   | 25 +
>  hw/pc_piix.c |  4 
>  2 files changed, 29 insertions(+)
>
> diff --git a/hw/e1000.c b/hw/e1000.c
> index d6fe815..263f2f4 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -131,6 +131,11 @@ typedef struct E1000State_st {
>  } eecd_state;
>  
>  QEMUTimer *autoneg_timer;
> +
> +/* Enabled compatibility for migration to/from rhel6.3.0 and older */

I presume you mean QEMU 1.3...

> +#define E1000_FLAG_QEMU_13_BIT 0
> +#define E1000_FLAG_QEMU_13 (1 << E1000_FLAG_QEMU_13_BIT)
> +uint32_t compat_flags;
>  } E1000State;
>  
>  #define  defreg(x)   x = (E1000_##x>>2)
> @@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
>  static void
>  set_phy_ctrl(E1000State *s, int index, uint16_t val)
>  {
> +/*
> + * QEMU 1.3 does not support link auto-negotiation emulation, so if we
> + * migrate during auto negotiation, after migration the link will be
> + * down.
> + */
> +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> +return;
> +}
>  if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
>  e1000_link_down(s);
>  s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
> @@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
>  {
>  E1000State *s = opaque;
>  NetClientState *nc = qemu_get_queue(s->nic);
> +
> +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> +return;
> +}
> +
>  /*
>   * If link is down and auto-negotiation is ongoing, complete
>   * auto-negotiation immediately.  This allows is to look at
> @@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int 
> version_id)
>   * to link status bit in mac_reg[STATUS].
>   * Alternatively, restart link negotiation if it was in progress. */
>  nc->link_down = (s->mac_reg[STATUS] & E1000_STATUS_LU) == 0;
> +
> +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> +return 0;
> +}
> +
>  if (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN &&
>  s->phy_reg[PHY_CTRL] & MII_CR_RESTART_AUTO_NEG &&
>  !(s->phy_reg[PHY_STATUS] & MII_SR_AUTONEG_COMPLETE)) {
> @@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
>  
>  static Property e1000_properties[] = {
>  DEFINE_NIC_PROPERTIES(E1000State, conf),
> +DEFINE_PROP_BIT("x-qemu_13_compat", E1000State,
> +compat_flags, E1000_FLAG_QEMU_13, false),
>  DEFINE_PROP_END_OF_LIST(),

Would it make more sense to simply call this auto-negotiation?

Regards,

Anthony Liguori

>  };
>  
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index ba09714..89a3f1f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
>  .driver   = "vmware-svga",
>  .property = "rombar",
>  .value= stringify(0),
> +},{
> +.driver   = "e1000",
> +.property = "x-qemu_13_compat",
> +.value= "on",
>  },
>  { /* end of list */ }
>  },
> -- 
> MST

Re: [Qemu-devel] test images for musicpal and milkymist?

[Michael Walle]

Am Donnerstag 14 Februar 2013, 17:48:09 schrieb Peter Maydell:
> Hi; I'm looking for test images (and command lines) for the musicpal
> and milkymist targets, because I have a patchset which makes some
> changes to them [I'm trying to get rid of sysbus_add_memory()].
> Does anybody have any to hand?
> 
> thanks
> -- PMM

Hi Peter,

there is an offical binary for the Milkymist SoC, which should work out of the 
box. You can find it here:

  http://www.milkymist.org/updates/current/flickernoise

start it with sth like this:
 qemu-system-lm32 -M milkymist -serial stdio --kernel /tmp/flickernoise 


In case you are looking for an image which does some actual testing, i'm 
afraid there is none. But sysbus_add_memory is used in softusb, therefore it 
should be easy to see if your patch is working. That is, if mouse and keyboard 
input is working, softusb is functional.

-- 
Michael

Re: [Qemu-devel] test images for musicpal and milkymist?

[Peter Maydell]

On 14 February 2013 19:03, Michael Walle  wrote:
> there is an offical binary for the Milkymist SoC, which should work
> out of the box. You can find it here:
>
>   http://www.milkymist.org/updates/current/flickernoise
>
> start it with sth like this:
>  qemu-system-lm32 -M milkymist -serial stdio --kernel /tmp/flickernoise

> In case you are looking for an image which does some actual testing,
> i'm afraid there is none. But sysbus_add_memory is used in softusb,
> therefore it should be easy to see if your patch is working. That is,
> if mouse and keyboard input is working, softusb is functional.

That's great, thanks. I confirmed my patch doesn't break the mouse
and keyboard. The other thing I changed was the minimac2 ethernet,
but the test image seems to talk to the DHCP server OK so that
looks like it's also working fine.

-- PMM

Re: [Qemu-devel] [PATCH v2 00/10] Cleanup bitops vs host-utils

[Richard Henderson]

On 02/14/2013 01:43 AM, Peter Maydell wrote:
> I was hoping we'd be able to get rid of host-utils.h instead,
> since "Utility compute operations used by translated code" is
> now a completely irrelevant categorisation...

I suppose an 11th patch could move the code back to bitops.h,
but I much prefer the function names used in host-utils.h.


r~

Re: [Qemu-devel] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Igor Mammedov]

On Thu, 14 Feb 2013 13:13:18 +0100
Paolo Bonzini  wrote:

> Il 14/02/2013 12:18, Eduardo Habkost ha scritto:
> >> > qemu boots from disk image 3 times faster than direct kernel load.
> > That's surprising. Do you have any idea why that happens?
> 
> Because kernel load uses MMIO (from fw_cfg), while booting from disk
> uses at worst PCI DMA and at best virtio.
> 
> Paolo
I've tried Paolo's suggestion to run in TCG mode with instruction logging,
and it spends too much time reading 11K kernel from qemu compared to
kvm-unit-test kernel. I'll need debug it further to find out why.
But this patch looks unnecessary if I could fix qemu/seabios or test kernel
to reach the same load time as kvm-unit-test kernel.

-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH v4 3/3] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices

[Alex Williamson]

On Thu, 2013-02-14 at 04:41 -0600, Vijay Mohan Pandarathil wrote:
>   - Create eventfd per vfio device assigned to a guest and register an
>   event handler
> 
>   - This fd is passed to the vfio_pci driver through the SET_IRQ ioctl
> 
>   - When the device encounters an error, the eventfd is signalled
>   and the qemu eventfd handler gets invoked.
> 
>   - In the handler decide what action to take. Current action taken
>   is to stop the guest.
> 
> Signed-off-by: Vijay Mohan Pandarathil 
> ---
>  hw/vfio_pci.c  | 112 
> +
>  linux-headers/linux/vfio.h |   1 +
>  2 files changed, 113 insertions(+)
> 
> diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
> index c51ae67..05da53b 100644
> --- a/hw/vfio_pci.c
> +++ b/hw/vfio_pci.c
> @@ -38,6 +38,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/queue.h"
>  #include "qemu/range.h"
> +#include "sysemu/sysemu.h"
>  
>  /* #define DEBUG_VFIO */
>  #ifdef DEBUG_VFIO
> @@ -129,7 +130,9 @@ typedef struct VFIODevice {
>  PCIHostDeviceAddress host;
>  QLIST_ENTRY(VFIODevice) next;
>  struct VFIOGroup *group;
> +EventNotifier err_notifier;
>  bool reset_works;
> +bool pci_aer;
>  } VFIODevice;
>  
>  typedef struct VFIOGroup {
> @@ -1802,6 +1805,7 @@ static int vfio_get_device(VFIOGroup *group, const char 
> *name, VFIODevice *vdev)
>  {
>  struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
>  struct vfio_region_info reg_info = { .argsz = sizeof(reg_info) };
> +struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
>  int ret, i;
>  
>  ret = ioctl(group->fd, VFIO_GROUP_GET_DEVICE_FD, name);
> @@ -1901,6 +1905,15 @@ static int vfio_get_device(VFIOGroup *group, const 
> char *name, VFIODevice *vdev)
>  vdev->config_size = reg_info.size;
>  vdev->config_offset = reg_info.offset;
>  
> +irq_info.index = VFIO_PCI_ERR_IRQ_INDEX;
> +
> +ret = ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> +if (ret) {
> +error_report("vfio: Error getting IRQ info: %m\n");

What if we're assigning a Legacy PCI device or running on an old kernel,
neither of these warrant an error_report.

> +goto error;
> +}
> +if (irq_info.count == 1)
> +vdev->pci_aer = true;

This is where I'd actually expect some kind of warning, you're getting
something back that you don't expect (if != 1).

>  error:
>  if (ret) {
>  QLIST_REMOVE(vdev, next);
> @@ -1922,6 +1935,102 @@ static void vfio_put_device(VFIODevice *vdev)
>  }
>  }
>  
> +static void vfio_err_notifier_handler(void *opaque)
> +{
> +VFIODevice *vdev = opaque;
> +
> +if (!event_notifier_test_and_clear(&vdev->err_notifier)) {
> +return;
> +}
> +
> +/*
> + * TBD. Retrieve the error details and decide what action
> + * needs to be taken. One of the actions could be to pass
> + * the error to the guest and have the guest driver recover
> + * from the error. This requires that PCIe capabilities be
> + * exposed to the guest. For now, we just terminate the
> + * guest to contain the error.
> + */
> +
> +error_report("%s (%04x:%02x:%02x.%x)"
> +"Unrecoverable error detected...\n"
> +"Please inestigate/collect any data required and then kill the 
> quest",

s/inestigate/investigate/

This seems like a lot to ask of a user.

> +__func__, vdev->host.domain, vdev->host.bus,
> +vdev->host.slot, vdev->host.function);
> +
> +vm_stop(RUN_STATE_IO_ERROR);

Gleb, were you looking for a new stop condition or is this one ok to
re-use?

> +}
> +
> +static void vfio_register_err_notifier(VFIODevice *vdev)
> +{
> +int ret;
> +int argsz;
> +struct vfio_irq_set *irq_set;
> +int32_t *pfd;
> +
> +if (!vdev->pci_aer) {
> +return;
> +}
> +
> +if (event_notifier_init(&vdev->err_notifier, 0)) {
> +error_report("vfio: Warning: Unable to init event notifier for error 
> detection\n");
> +return;
> +}
> +
> +argsz = sizeof(*irq_set) + sizeof(*pfd);
> +
> +irq_set = g_malloc0(argsz);
> +irq_set->argsz = argsz;
> +irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
> + VFIO_IRQ_SET_ACTION_TRIGGER;
> +irq_set->index = VFIO_PCI_ERR_IRQ_INDEX;
> +irq_set->start = 0;
> +irq_set->count = 1;
> +pfd = (int32_t *)&irq_set->data;
> +
> +*pfd = event_notifier_get_fd(&vdev->err_notifier);
> +qemu_set_fd_handler(*pfd, vfio_err_notifier_handler, NULL, vdev);
> +
> +ret = ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +if (ret) {
> +error_report("vfio: Failed to set up error notification\n");
> +qemu_set_fd_handler(*pfd, NULL, NULL, vdev);
> +event_notifier_cleanup(&vdev->err_notifier);
> +}
> +g_free(irq_set);
> +}

Why does the register return void?  If it's supported and fails, isn't
that a condition where we'd want to fai

Re: [Qemu-devel] [virt-tools-list] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Richard W.M. Jones]

On Thu, Feb 14, 2013 at 05:07:48PM -0200, Eduardo Habkost wrote:
> On Thu, Feb 14, 2013 at 06:41:26PM +, Richard W.M. Jones wrote:
> > On Thu, Feb 14, 2013 at 11:41:08AM -0200, Eduardo Habkost wrote:
> > > (CCing virt-tools list in case they have any existing solutions or ideas
> > > on how to make this easier for everybody)
> > 
> > supermin?  http://people.redhat.com/~rjones/supermin/
> > 
> > It already exists on Fedora, Debian & Ubuntu, although it
> > may be known there by its former name of febootstrap.
> > 
> > Try:
> > 
> >   cd /tmp
> >   mkdir supermin.d
> >   supermin -o supermin.d --names bash
> >   supermin-helper -f ext2 supermin.d `uname -m` ./kernel ./initrd 
> > ./appliance
> 
> Here ./kernel and ./initrd are the output of supermin-helper, not input,
> right?

Yup.

> >   qemu-kvm -m 256 -kernel ./kernel -initrd ./initrd -hda ./appliance \
> > --append "init=/bin/bash root=/dev/hda"
> > 
> > (Substitute 'febootstrap' for 'supermin' and 'febootstrap-supermin-helper'
> > for 'supermin-helper', if using febootstrap 3.x)
> 
> I am not sure this is what I was looking for. The problem is:
> -kernel/-initrd are slow,

OK, reading back up the thread now, I see that DMA was suggested :-)

supermin is probably not what you need in this case.

[...]

Rich.

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

Re: [Qemu-devel] [RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM subclasses

[Igor Mammedov]

On Thu, 14 Feb 2013 13:52:59 -0200
Eduardo Habkost  wrote:

> On Thu, Feb 14, 2013 at 04:13:22PM +0100, Igor Mammedov wrote:
> > On Thu, 14 Feb 2013 09:44:59 -0200
> > Eduardo Habkost  wrote:
> [...]
> > > > +if (kvm_enabled()) {
> > > > +typename = g_strdup_printf("%s-kvm-" TYPE_X86_CPU, name);
> > > 
> > > * Could we use the "-tcg" suffix for the non-KVM mode?
> > sure, I'll add it for the next re-spin.
> > 
> > > * Can we make the code fall back to no-suffix CPU names? We need to add
> > >   the -kvm suffix (and maybe a -tcg suffix) to keep compatibility with
> > >   existing CPU models, but maybe we should deprecate the automatic
> > >   -tcg/-kvm suffixing and ask users to specify the full CPU name.
> > I'm not sure that I got you right, Have you meant something like this:
> > 
> > if (kvm) {
> > typename = name-kvm-...
> > } else {
> > typename = name-tcg-...
> > }
> > 
> > oc = object_class_by_name(typename)
> > if (!oc) {
> > oc = object_class_by_name(name)
> > }
> 
> Yes, exactly.
> 
> 
> [...]
> > > > +
> > > > +}
> > > > +
> > > > +static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
> > > > +{
> > > > +X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > > +x86_def_t *def = data;
> > > > +int i;
> > > > +static const char *versioned_models[] = { "qemu32", "qemu64",
> > > > "athlon" }; +
> > > > +memcpy(&xcc->info, def, sizeof(x86_def_t));
> > > > +xcc->info.ext_features |= CPUID_EXT_HYPERVISOR;
> > > 
> > > If this is TCG-specific now, we could make the class match the reality
> > > and mask TCG_FEATURES/TCG_EXT_FEATURES/etc here. That's what happens in
> > > practice, e.g.: "-cpu SandyBridge" in TCG mode is a completely different
> > > CPU, today.
> > well, this function is shared between TCG and KVM, I mean, it's common
> > code for both. Which asks for one more TCG class_init function for TCG
> > specific behavior.
> 
> Having both TCG-specific and KVM-specific subclasses instead of making
> the KVM class inherit from the TCG class would make sense to me, as we
> may have TCG-specific behavior in other places too.
> 
> Or we could do memcpy() again on the KVM class_init function. Or we
> could set a different void* pointer on the TCG class, with
> already-filtered x86_def_t object. There are multiple possible
> solutions.
> 
> 
> > But could it be a separate patch (i.e. fixing TCG filtering), I think
> > just moving tcg filtering might cause regression. And need to worked on
> > separately.
> 
> I'm OK with doing that in a separate patch, as it is not a bug fix or
> behavior change. But it would be nice to do that before we introduce the
> feature properties, to make the reported defaults right since the
> beginning.
It's behavior change, if we move filtering from realizefn to class_init, user
would be able to add features not visible now to guest.

ordering now:
class_init, initfn, setting defautls, custom features, realizefn(filtering)

would be ordering with static properties:
clas_init, static props defaults, global properties(custom features),
x86_cpu_initfn, realizefn

in would be scenario filtering comes before custom features, which is not
necessarily bad and may be nice to consciously add/test features before
enabling them in filter for everyone, but it's behavior change.

> [...]
> > > > +#ifdef CONFIG_KVM
> > > > +static void x86_cpu_kvm_def_class_init(ObjectClass *oc, void *data)
> > > > +{
> > > > +uint32_t eax, ebx, ecx, edx;
> > > > +X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > > +
> > > > +x86_cpu_def_class_init(oc, data);
> > > > +/* sysenter isn't supported in compatibility mode on AMD,
> > > > + * syscall isn't supported in compatibility mode on Intel.
> > > > + * Normally we advertise the actual CPU vendor, but you can
> > > > + * override this using the 'vendor' property if you want to use
> > > > + * KVM's sysenter/syscall emulation in compatibility mode and
> > > > + * when doing cross vendor migration
> > > > + */
> > > > +host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
> > > 
> > > Cool, this is exactly what I was going to suggest, to avoid depending on
> > > the "host" CPU class and KVM initialization.
> > > 
> > > I see two options when making the "vendor" property static, and I don't
> > > know which one is preferable:
> > > 
> > > One solution is the one in this patch: to call host_cpuid() here in
> > > class_init, and have a different property default depending on the host
> > > CPU.
> > I prefer this one ^^^.
> > 
> > > Another solution is to have default to vendor="host", and have
> > > instance_init understand vendor="host" as "use the host CPU vendor".
> > > This would make the property default really static (not depending on the
> > > host CPU).
> > > 
> > 
> > 
> > > I am inclined for the latter, because I am assuming that the QOM design
> > > expects class_init results to be completely static. This is not as bad
> > > as depending on KVM initialization, but it may be 

Re: [Qemu-devel] [virt-tools-list] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Eduardo Habkost]

On Thu, Feb 14, 2013 at 06:41:26PM +, Richard W.M. Jones wrote:
> On Thu, Feb 14, 2013 at 11:41:08AM -0200, Eduardo Habkost wrote:
> > (CCing virt-tools list in case they have any existing solutions or ideas
> > on how to make this easier for everybody)
> 
> supermin?  http://people.redhat.com/~rjones/supermin/
> 
> It already exists on Fedora, Debian & Ubuntu, although it
> may be known there by its former name of febootstrap.
> 
> Try:
> 
>   cd /tmp
>   mkdir supermin.d
>   supermin -o supermin.d --names bash
>   supermin-helper -f ext2 supermin.d `uname -m` ./kernel ./initrd ./appliance

Here ./kernel and ./initrd are the output of supermin-helper, not input,
right?

>   qemu-kvm -m 256 -kernel ./kernel -initrd ./initrd -hda ./appliance \
> --append "init=/bin/bash root=/dev/hda"
> 
> (Substitute 'febootstrap' for 'supermin' and 'febootstrap-supermin-helper'
> for 'supermin-helper', if using febootstrap 3.x)

I am not sure this is what I was looking for. The problem is:
-kernel/-initrd are slow, but we could have a tool that takes kernel
image + initrd image + kernel command-line as input, and would generate
a small disk image that would boot the provided kernel/initrd images
(using the provided kernel command-line) faster than when using
-kernel/-initrd.

But anyway, it looks like our problem is somewhere else. A 86KB kernel
from kvm-unit-tests boots instantly, but for some reason QEMU is taking
15 seconds to boot our 11KB kernel.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v4 2/3] VFIO-AER: Vfio-pci driver changes for supporting AER

[Alex Williamson]

On Thu, 2013-02-14 at 04:41 -0600, Vijay Mohan Pandarathil wrote:
>   - New VFIO_SET_IRQ ioctl option to pass the eventfd that is signaled 
> when
>   an error occurs in the vfio_pci_device
> 
>   - Register pci_error_handler for the vfio_pci driver
> 
>   - When the device encounters an error, the error handler registered by
>   the vfio_pci driver gets invoked by the AER infrastructure
> 
>   - In the error handler, signal the eventfd registered for the device.
> 
>   - This results in the qemu eventfd handler getting invoked and
>   appropriate action taken for the guest.
> 
> Signed-off-by: Vijay Mohan Pandarathil 
> ---
>  drivers/vfio/pci/vfio_pci.c | 44 +-
>  drivers/vfio/pci/vfio_pci_intrs.c   | 47 
> +
>  drivers/vfio/pci/vfio_pci_private.h |  1 +
>  include/uapi/linux/vfio.h   |  1 +
>  4 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index b28e66c..d70bd58 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -196,7 +196,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device 
> *vdev, int irq_type)
>  
>   return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
>   }
> - }
> + } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX)
> + if (pci_is_pcie(vdev->pdev))
> + return 1;
>  
>   return 0;
>  }
> @@ -302,6 +304,17 @@ static long vfio_pci_ioctl(void *device_data,
>   if (info.argsz < minsz || info.index >= VFIO_PCI_NUM_IRQS)
>   return -EINVAL;
>  
> + switch (info.index) {
> + case VFIO_PCI_INTX_IRQ_INDEX ... VFIO_PCI_MSIX_IRQ_INDEX:
> + break;
> + case VFIO_PCI_ERR_IRQ_INDEX:
> + if (pci_is_pcie(vdev->pdev))
> + break;
> + /* pass thru to return error */
> + default:
> + return -EINVAL;
> + }
> +
>   info.flags = VFIO_IRQ_INFO_EVENTFD;
>  
>   info.count = vfio_pci_get_irq_count(vdev, info.index);
> @@ -538,11 +551,40 @@ static void vfio_pci_remove(struct pci_dev *pdev)
>   kfree(vdev);
>  }
>  
> +static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
> +   pci_channel_state_t state)
> +{
> + struct vfio_pci_device *vpdev;
> + struct vfio_device *vdev;

These are very confusing given that everywhere else in this file vdev is
a struct vfio_pci_device.  I'd suggest "vdev" and "device".

> +
> + vdev = vfio_device_get_from_dev(&pdev->dev);
> + if (vdev == NULL)
> + return PCI_ERS_RESULT_DISCONNECT;
> +
> + vpdev = vfio_device_data(vdev);
> + if (vpdev == NULL) {
> + vfio_device_put(vdev);
> + return PCI_ERS_RESULT_DISCONNECT;
> + }
> +
> + if (vpdev->err_trigger)
> + eventfd_signal(vpdev->err_trigger, 1);
> +
> + vfio_device_put(vdev);
> +
> + return PCI_ERS_RESULT_CAN_RECOVER;
> +}
> +
> +static struct pci_error_handlers vfio_err_handlers = {
> + .error_detected = vfio_pci_aer_err_detected,
> +};
> +
>  static struct pci_driver vfio_pci_driver = {
>   .name   = "vfio-pci",
>   .id_table   = NULL, /* only dynamic ids */
>   .probe  = vfio_pci_probe,
>   .remove = vfio_pci_remove,
> + .err_handler= &vfio_err_handlers,
>  };
>  
>  static void __exit vfio_pci_cleanup(void)
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c 
> b/drivers/vfio/pci/vfio_pci_intrs.c
> index 3639371..8036f3a 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -745,6 +745,46 @@ static int vfio_pci_set_msi_trigger(struct 
> vfio_pci_device *vdev,
>   return 0;
>  }
>  
> +static int vfio_pci_set_err_trigger(struct vfio_pci_device *vdev,
> + unsigned index, unsigned start,
> + unsigned count, uint32_t flags, void *data)
> +{
> + int32_t fd = *(int32_t *)data;
> +
> + if ((index != VFIO_PCI_ERR_IRQ_INDEX) ||
> + !(flags & VFIO_IRQ_SET_DATA_TYPE_MASK))
> + return -EINVAL;
> +
> + /* DATA_NONE/DATA_BOOL enables loopback testing */
> +
> + if (flags & VFIO_IRQ_SET_DATA_NONE) {
> + if (vdev->err_trigger)
> + eventfd_signal(vdev->err_trigger, 1);
> + return 0;
> + } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
> + uint8_t trigger = *(uint8_t *)data;
> + if (trigger && vdev->err_trigger)
> + eventfd_signal(vdev->err_trigger, 1);
> + return 0;
> + }
> +
> + /* Handle SET_DATA_EVENTFD */
> +
> + if (fd == -1) {
> + if (vdev->err_trigger)
> +   

Re: [Qemu-devel] 3 new x86 instructions

[Richard Henderson]

On 02/14/2013 05:38 AM, Torbjorn Granlund wrote:
> I now updated x86-next in order to get support for adox/adcx.

Hmm.  I'm not quite sure what the state of that branch is atm.

I know I started splitting out work into other branches as will
be required by the eventual merge; perhaps I didn't put them
back together again properly in the x86-next branch.

I'll have a look and get back to you.


r~

[Qemu-devel] [PATCH] linux-user: Fix layout of usage table to account for option text

[Peter Maydell]

The linux-user usage message attempts to line up the columns in
its table by calculating the maximum width of any item in them.
However for the 'Argument' column it was only accounting for the
length of the option switch (eg "-d"), not the additional example
text (eg "item[,...]"). This currently has no adverse effects
because the widest item in the column happens to be the argumentless
"-singlestep" option, but improving the "-d" option help to read
"-d item[,...]" exceeds that limit.

Fix this by correctly calculating maxarglen as the width of the
first column text including a possible option argument, and
adjusting its uses to match.

Signed-off-by: Peter Maydell 
---
This patch doesn't affect the help output at the moment; you
can test it by changing the "options" text for the "d" entry
in arg_table[] to read "item[,...]", which is what my not-yet-posted
update of the 'default logging to stderr' patch will do.

 linux-user/main.c |   24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 3df8aa2..b28d4fd 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3319,27 +3319,35 @@ static void usage(void)
"Options and associated environment variables:\n"
"\n");
 
-maxarglen = maxenvlen = 0;
+/* Calculate column widths. We must always have at least enough space
+ * for the column header.
+ */
+maxarglen = strlen("Argument");
+maxenvlen = strlen("Env-variable");
 
 for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+int arglen = strlen(arginfo->argv);
+if (arginfo->has_arg) {
+arglen += strlen(arginfo->example) + 1;
+}
 if (strlen(arginfo->env) > maxenvlen) {
 maxenvlen = strlen(arginfo->env);
 }
-if (strlen(arginfo->argv) > maxarglen) {
-maxarglen = strlen(arginfo->argv);
+if (arglen > maxarglen) {
+maxarglen = arglen;
 }
 }
 
-printf("%-*s%-*sDescription\n", maxarglen+3, "Argument",
-maxenvlen+1, "Env-variable");
+printf("%-*s %-*s Description\n", maxarglen+1, "Argument",
+maxenvlen, "Env-variable");
 
 for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
 if (arginfo->has_arg) {
 printf("-%s %-*s %-*s %s\n", arginfo->argv,
-(int)(maxarglen-strlen(arginfo->argv)), arginfo->example,
-maxenvlen, arginfo->env, arginfo->help);
+   (int)(maxarglen - strlen(arginfo->argv) - 1),
+   arginfo->example, maxenvlen, arginfo->env, arginfo->help);
 } else {
-printf("-%-*s %-*s %s\n", maxarglen+1, arginfo->argv,
+printf("-%-*s %-*s %s\n", maxarglen, arginfo->argv,
 maxenvlen, arginfo->env,
 arginfo->help);
 }
-- 
1.7.9.5

Re: [Qemu-devel] [virt-tools-list] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Richard W.M. Jones]

On Thu, Feb 14, 2013 at 06:41:26PM +, Richard W.M. Jones wrote:
> On Thu, Feb 14, 2013 at 11:41:08AM -0200, Eduardo Habkost wrote:
> > (CCing virt-tools list in case they have any existing solutions or ideas
> > on how to make this easier for everybody)
> 
> supermin?  http://people.redhat.com/~rjones/supermin/

BTW this documentation is more comprehensive, including
explaining the supermin appliance format:

http://libguestfs.org/supermin.8.html

Rich.

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

Re: [Qemu-devel] [virt-tools-list] [PATCH] qemu:cpuid: speedup test by 3x times if grub2 is available

[Richard W.M. Jones]

On Thu, Feb 14, 2013 at 11:41:08AM -0200, Eduardo Habkost wrote:
> (CCing virt-tools list in case they have any existing solutions or ideas
> on how to make this easier for everybody)

supermin?  http://people.redhat.com/~rjones/supermin/

It already exists on Fedora, Debian & Ubuntu, although it
may be known there by its former name of febootstrap.

Try:

  cd /tmp
  mkdir supermin.d
  supermin -o supermin.d --names bash
  supermin-helper -f ext2 supermin.d `uname -m` ./kernel ./initrd ./appliance
  qemu-kvm -m 256 -kernel ./kernel -initrd ./initrd -hda ./appliance \
--append "init=/bin/bash root=/dev/hda"

(Substitute 'febootstrap' for 'supermin' and 'febootstrap-supermin-helper'
for 'supermin-helper', if using febootstrap 3.x)

Rich.

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

Re: [Qemu-devel] VFIO: Not require to make VFIO_IOMMU_MAP_DMA for MMAPED PCI bars

[Alex Williamson]

On Thu, 2013-02-14 at 08:22 +, Bhushan Bharat-R65777 wrote:
> Hi Alex Williamson,
> 
> I am able (with some hacks :)) to directly assign the e1000 PCI device
> to KVM guest using VFIO on Freescale device. One of the problem I am
> facing is about the DMA mapping in IOMMU for PCI device BARs. On
> Freescale devices, the mmaped PCI device BARs are not required to be
> mapped in IOMMU. Typically the flow of in/out transaction (from CPU)
> is: 
> 
> Incoming flow:
> 
> -| |--| |---| 
>   |-|
> CORE |<<--<-<--| IOMMU|<---<---<| 
> PCI-Controller|<--<-<<| PCI device  |
> -| |--| |---| 
>   |-|
> 
> Outgoing Flow: IOMMU is bypassed for out transactions
> 
> -| |--| |---| 
>   |-|
> CORE |>>-->|   | IOMMU|->-->| 
> PCI-Controller|>-->->>| PCI device  |
> -| |   |--|   ^ |---| 
>   |-|
>|  |
>|--|

Mapping the BAR into the IOMMU isn't for this second use case, CPU to
device is never IOMMU translated.  Instead, it's for this:

|--| |---|   |-|
| IOMMU|<---<---<| PCI-Controller|<--<-<<| PCI device  |
|--| |---|   |-|
  |
  |  |---|   |-|
  +-->>--- ->| PCI-Controller|>-->->>| PCI device  |
 |---|   |-|

It's perfectly fine to not support peer-to-peer DMA, but let's skip it
where it's not supported in case it might actually work in some cases.

> Also because of some hardware limitations on our IOMMU it is difficult
> to map these BAR regions with RAM (DDR) regions. So on Freescale
> device we want the VFIO_IOMMU_MAP_DMA ioctl to be called for RAM
> regions (DDR) only and _not_ for these mmaped ram regions of PCI
> device bars. I can understand that we need to add these mmaped PCI
> bars as RAM type in qemu memory_region_*(). So for that I tried to
> skip these regions in VFIO memory_listeners. Below changes which works
> for me. I am not sure whether this is correct approach, please
> suggest.
> 
> -
> diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
> index c51ae67..63728d8 100644
> --- a/hw/vfio_pci.c
> +++ b/hw/vfio_pci.c
> @@ -1115,9 +1115,35 @@ static int vfio_dma_map(VFIOContainer *container, 
> hwaddr iova,
>  return -errno;
>  }
>  
> -static bool vfio_listener_skipped_section(MemoryRegionSection *section)
> +static int memory_region_is_mmap_bars(VFIOContainer *container,
> +  MemoryRegionSection *section)
>  {
> -return !memory_region_is_ram(section->mr);
> +VFIOGroup *group;
> +VFIODevice *vdev;
> +int i;
> +
> +QLIST_FOREACH(group, &container->group_list, next) {

I think you want to start at the global &group_list

> +QLIST_FOREACH(vdev, &group->device_list, next) {
> +if (vdev->msix->mmap_mem.ram_addr == section->mr->ram_addr)

Note the comment in memory.h:

struct MemoryRegion {
/* All fields are private - violators will be prosecuted */
...
ram_addr_t ram_addr;

You'll need to invent some kind of memory region comparison interfaces
instead of accessing these private fields.

> +return 1;
> +for (i = 0; i < PCI_ROM_SLOT; i++) {
> +VFIOBAR *bar = &vdev->bars[i];
> +if (bar->mmap_mem.ram_addr == section->mr->ram_addr)
> +return 1;
> +}
> +}
> +}
> +
> +return 0;
> +}

It's a pretty heavy weight function, I think the memory API should help
us differentiate MMIO from RAM.

> +static bool vfio_listener_skipped_section(VFIOContainer *container,
> +  MemoryRegionSection *section)
> +{
> +if (!memory_region_is_ram(section->mr))
> +   return 1;

Need some kind of conditional here for platforms that support
peer-to-peer.  Thanks,

Alex

> +return memory_region_is_mmap_bars(container, section);
>  }
>  
>  static void vfio_listener_region_add(MemoryListener *listener,
> @@ -1129,7 +1155,7 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>  void *vaddr;
>  int ret;
>  
> -if (vfio_listener_skipped_section(section)) {
> +if (vfio_listener_skipped_section(container, section)) {
>  DPRINTF("vfio: SKIPPING region_add %"HWADDR_PRIx" - %"PRIx64"\n",
>  section->offset_within_address_space,
>  section->offset_within_address_space + section->size - 1);
> @@ -1173,7 +1199,7 @@ static vo

Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant

[Avi Kivity]

On Thu, Feb 14, 2013 at 8:12 PM, Michael S. Tsirkin  wrote:
>>
>> Is there an actual real problem that needs fixing?
>
> Yes. Guests sometimes cause device BARs to temporary overlap
> the APIC range during BAR sizing. It works fine on a physical
> system but fails on KVM since pci has same priority.
>
> See the report:
> [BUG] Guest OS hangs on boot when 64bit BAR present
>

Is PCI_COMMAND_MEMORY set while this is going on?

[Qemu-devel] RFC: handling "backend too fast" in virtio-net

[Luigi Rizzo]

virtio-style network devices (where the producer and consumer chase
each other through a shared memory block) can enter into a
bad operating regime when the consumer is too fast.

I am hitting this case right now when virtio is used on top of the
netmap/VALE backend that I posted a few weeks ago: what happens is that
the backend is so fast that the io thread keeps re-enabling notifications
every few packets.  This results, on my test machine, in a throughput of
250-300Kpps (and extremely unstable, oscillating between 200 and 600Kpps).

I'd like to get some feedback on the following trivial patch to have
the thread keep spinning for a bounded amount of time when the producer
is slower than the consumer. This gives a relatively stable throughput
between 700 and 800 Kpps (we have something similar in our paravirtualized
e1000 driver, which is slightly faster at 900-1100 Kpps).

EXISTING LOGIC: reschedule the bh when at least a burst of packets has
   been received. Otherwise enable notifications and do another
   race-prevention check.

NEW LOGIC: when the bh is scheduled the first time, establish a
   budget (currently 20) of reschedule attempts.
   Every time virtio_net_flush_tx() returns 0 packets [this could 
   be changed to 'less than a full burst'] the counter is increased.
   The bh is rescheduled until the counter reaches the budget,
   at which point we re-enable notifications as above.

I am not 100% happy with this patch because there is a "magic"
constant (the maximum number of retries) which should be really
adaptive, or perhaps we should even bound the amount of time the
bh runs, rather than the number of retries.
In practice, having the thread spin (or sleep) for 10-20us 
even without new packets is probably preferable to 
re-enable notifications and request a kick.

opinions ?

cheers
luigi

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 573c669..5389088 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -49,6 +51,7 @@ typedef struct VirtIONet
 NICState *nic;
 uint32_t tx_timeout;
 int32_t tx_burst;
+int32_t tx_retries; /* keep spinning a bit on tx */
 uint32_t has_vnet_hdr;
 size_t host_hdr_len;
 size_t guest_hdr_len;
@@ -1062,7 +1065,9 @@ static void virtio_net_tx_bh(void *opaque)

 /* If we flush a full burst of packets, assume there are
  * more coming and immediately reschedule */
-if (ret >= n->tx_burst) {
+if (ret == 0)
+   n->tx_retries++;
+if (n->tx_retries < 20) {
 qemu_bh_schedule(q->tx_bh);
 q->tx_waiting = 1;
 return;
@@ -1076,6 +1082,8 @@ static void virtio_net_tx_bh(void *opaque)
 virtio_queue_set_notification(q->tx_vq, 0);
 qemu_bh_schedule(q->tx_bh);
 q->tx_waiting = 1;
+} else {
+   n->tx_retries = 0;
 }
 }

Re: [Qemu-devel] [PATCH] qemu-log: default to stderr for logging output

[Peter Maydell]

On 13 February 2013 12:57, Markus Armbruster  wrote:
>> -   "-d options   activate log (default logfile=%s)\n"
>> -   "-D logfile   override default logfile location\n"
>> +   "-d options   activate log (default is to log to stderr)\n"
>> +   "-D logfile   write logs to 'logfile' rather than stderr\n"
>> "-p pagesize  set the host page size to 'pagesize'\n"
>> "-singlestep  always run in singlestep mode\n"
>> "-strace  log system calls\n"
>
> No need to mention the default twice.
>
> Pointing to -d help would be nice.

Yeah. I'll standardise the phrasing here bsd-user, linux-user
and system mode all say
   "-d item1[,...]enable logging of specified items\n"
   "  (use '-d help' for a list of log items)\n"
   "-D logfilewrite logs to 'logfile' (default stderr)\n"

>> @@ -861,7 +858,9 @@ int main(int argc, char **argv)
>>  }
>>
>>  /* init debug */
>> -qemu_set_log_filename(log_file);
>> +if (log_file) {
>> +qemu_set_log_filename(log_file);
>> +}
>>  if (log_mask) {
>>  int mask;
>>
>
> Doesn't qemu_set_log_filename(NULL) do the right thing?

Yes, it does. I'll drop this hunk.

> bsd-user: if there's more than one -D, all but the last get silently
> ignored.  We create that log file when logging is enabled.
>
> linux-user: we create all the log files when logging is enabled.
>
> No biggie, but consistency would be nice.  I prefer bsd-user's behavior.

I agree, but I think changing the linux-user behaviour to
match bsd-user should be a separate patch.

-- PMM

Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant

[Michael S. Tsirkin]

On Thu, Feb 14, 2013 at 07:02:15PM +0200, Avi Kivity wrote:
> On Thu, Feb 14, 2013 at 6:50 PM, Michael S. Tsirkin  wrote:
> >> > As you see, ioapic at 0xfec0 overlaps pci-hole.
> >> > ioapic is guest programmable in theory - should use _overlap?
> >> > pci-hole is not but can overlap with ioapic.
> >> > So also _overlap?
> >>
> >> It's a bug.  The ioapic is in the pci address space, not the system
> >> address space.  And yes it's overlappable.
> >
> > So you want to put it where? Under pci-hole?
> 
> No, under the pci address space.  Look at the 440fx block diagram.
> 
> > And we'll have to teach all machine types
> > creating pci-hole about it?
> 
> No.
> 
> >
> >> >
> >> > Let's imagine someone writes a guest programmable device for
> >> > ARM. Now we should update all ARM devices from regular to _overlap?
> >>
> >> It's sufficient to update the programmable device.
> >
> > Then the device can be higher priority (works for apic)
> > but not lower priority. Make priority signed?
> 
> Is there an actual real problem that needs fixing?

Yes. Guests sometimes cause device BARs to temporary overlap
the APIC range during BAR sizing. It works fine on a physical
system but fails on KVM since pci has same priority.

See the report:
[BUG] Guest OS hangs on boot when 64bit BAR present 

-- 
MST

Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

[Paolo Bonzini]

Il 14/02/2013 18:15, Michael S. Tsirkin ha scritto:
> QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
> 1.3 machine during link auto negotiation, the guest link will be set to down.
> Fix this by just disabling auto negotiation for 1.3.
> 
> Signed-off-by: Michael S. Tsirkin 

A nicer property name perhaps would be preferrable, but it's fine if
Anthony doesn't want to let the release slip.

Paolo

> ---
>  hw/e1000.c   | 25 +
>  hw/pc_piix.c |  4 
>  2 files changed, 29 insertions(+)
> 
> diff --git a/hw/e1000.c b/hw/e1000.c
> index d6fe815..263f2f4 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -131,6 +131,11 @@ typedef struct E1000State_st {
>  } eecd_state;
>  
>  QEMUTimer *autoneg_timer;
> +
> +/* Enabled compatibility for migration to/from rhel6.3.0 and older */
> +#define E1000_FLAG_QEMU_13_BIT 0
> +#define E1000_FLAG_QEMU_13 (1 << E1000_FLAG_QEMU_13_BIT)
> +uint32_t compat_flags;
>  } E1000State;
>  
>  #define  defreg(x)   x = (E1000_##x>>2)
> @@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
>  static void
>  set_phy_ctrl(E1000State *s, int index, uint16_t val)
>  {
> +/*
> + * QEMU 1.3 does not support link auto-negotiation emulation, so if we
> + * migrate during auto negotiation, after migration the link will be
> + * down.
> + */
> +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> +return;
> +}
>  if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
>  e1000_link_down(s);
>  s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
> @@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
>  {
>  E1000State *s = opaque;
>  NetClientState *nc = qemu_get_queue(s->nic);
> +
> +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> +return;
> +}
> +
>  /*
>   * If link is down and auto-negotiation is ongoing, complete
>   * auto-negotiation immediately.  This allows is to look at
> @@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int 
> version_id)
>   * to link status bit in mac_reg[STATUS].
>   * Alternatively, restart link negotiation if it was in progress. */
>  nc->link_down = (s->mac_reg[STATUS] & E1000_STATUS_LU) == 0;
> +
> +if (s->compat_flags & E1000_FLAG_QEMU_13) {
> +return 0;
> +}
> +
>  if (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN &&
>  s->phy_reg[PHY_CTRL] & MII_CR_RESTART_AUTO_NEG &&
>  !(s->phy_reg[PHY_STATUS] & MII_SR_AUTONEG_COMPLETE)) {
> @@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
>  
>  static Property e1000_properties[] = {
>  DEFINE_NIC_PROPERTIES(E1000State, conf),
> +DEFINE_PROP_BIT("x-qemu_13_compat", E1000State,
> +compat_flags, E1000_FLAG_QEMU_13, false),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index ba09714..89a3f1f 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
>  .driver   = "vmware-svga",
>  .property = "rombar",
>  .value= stringify(0),
> +},{
> +.driver   = "e1000",
> +.property = "x-qemu_13_compat",
> +.value= "on",
>  },
>  { /* end of list */ }
>  },
> 

Re: [Qemu-devel] [PATCH qom-cpu-next 0/6] QOM CPUState, part 8: CPU_COMMON continued

[Andreas Färber]

Am 01.02.2013 13:38, schrieb Andreas Färber:
> Hello,
> 
> This series moves more fields from CPU_COMMON / CPU*State to CPUState,
> allowing access from target-independent code.
> 
> The final patch in this series will help solve some issues (in particular
> avoid a dependency on CPU_COMMON TLB refactoring for now) but opens a can
> of worms: Since it is initialized in derived instance_init functions,
> functions cannot randomly be changed to operate on CPUState and be called
> from CPUState's instance_init or they will crash due to NULL env_ptr.

The "questionable" patch in this series has been acked by rth, so if
nobody objects, I'll queue it on qom-cpu-next tonight, to base further
work on. I'm not aware of any conflicting maintainer's queue so far.

Andreas

> 
> For those of you that may have been following the CPU refactorings closely,
> I have now split off part of former qom-cpu-8 branch into qom-cpu-9.
> This series thereby applies directly to qom-cpu-next,
> whereas qom-cpu-9 depends on the pending s390x pull, my m68k cleanups and
> may be changed for VMState changes cooking elsewhere to keep i386 v5 compat.
> 
> Available for testing at:
> git://github.com/afaerber/qemu-cpu.git qom-cpu-8.v1
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-8.v1
> 
> Regards,
> Andreas
> 
> Changes from previews:
> * Drop #ifdefs for user-only CPUState fields.
> * Defer interrupt-related changes to part 9.
> 
> Andreas Färber (6):
>   cpu: Move host_tid field to CPUState
>   cpu: Move running field to CPUState
>   cpu: Move exit_request field to CPUState
>   cpu: Move current_tb field to CPUState
>   cputlb: Pass CPUState to cpu_unlink_tb()
>   cpu: Add CPUArchState pointer to CPUState
> 
>  cpu-exec.c  |   21 -
>  cputlb.c|6 --
>  dump.c  |8 ++--
>  exec.c  |6 --
>  gdbstub.c   |   14 +-
>  hw/apic_common.c|2 +-
>  hw/apic_internal.h  |2 +-
>  hw/kvmvapic.c   |   13 -
>  hw/spapr_hcall.c|5 +++--
>  include/exec/cpu-defs.h |5 -
>  include/exec/exec-all.h |4 +++-
>  include/exec/gdbstub.h  |5 ++---
>  include/qom/cpu.h   |   11 +++
>  kvm-all.c   |6 +++---
>  linux-user/main.c   |   37 ++---
>  linux-user/syscall.c|4 +++-
>  qom/cpu.c   |2 ++
>  target-alpha/cpu.c  |2 ++
>  target-arm/cpu.c|2 ++
>  target-cris/cpu.c   |2 ++
>  target-i386/cpu.c   |1 +
>  target-i386/kvm.c   |4 ++--
>  target-lm32/cpu.c   |2 ++
>  target-m68k/cpu.c   |2 ++
>  target-microblaze/cpu.c |2 ++
>  target-mips/cpu.c   |2 ++
>  target-openrisc/cpu.c   |2 ++
>  target-ppc/translate_init.c |2 ++
>  target-s390x/cpu.c  |2 ++
>  target-sh4/cpu.c|2 ++
>  target-sparc/cpu.c  |2 ++
>  target-unicore32/cpu.c  |2 ++
>  target-xtensa/cpu.c |2 ++
>  translate-all.c |   36 +++-
>  translate-all.h |2 +-
>  35 Dateien geändert, 149 Zeilen hinzugefügt(+), 73 Zeilen entfernt(-)

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

Re: [Qemu-devel] Google Summer of Code 2013 ideas wiki open

[harryxiyou]

On Thu, Feb 14, 2013 at 11:15 PM, Stefan Hajnoczi  wrote:
[...]
> Hi Harry,
Hi Stefan,

> Thanks for your interest.  You can begin thinking about ideas but
> please keep in mind that we are still in the very early stages of GSoC
> preparation.
>
> Google will publish the list of accepted organizations on April 8th.
> Then there is a period of over 3 weeks to discuss your project idea
> with the organization.
>
> In the meantime, the best thing to do is to get familiar with the code
> bases and see if you can find/fix a bug.  Contributing patches is a
> great way to get noticed.
>
> There is always a chance that QEMU and/or libvirt may not be among the
> list of accepted organizations, so don't put all your eggs in one
> basket :).
>

Thanks for your suggestions.


-- 
Thanks
Harry Wei

[Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

[Michael S. Tsirkin]

QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
1.3 machine during link auto negotiation, the guest link will be set to down.
Fix this by just disabling auto negotiation for 1.3.

Signed-off-by: Michael S. Tsirkin 
---
 hw/e1000.c   | 25 +
 hw/pc_piix.c |  4 
 2 files changed, 29 insertions(+)

diff --git a/hw/e1000.c b/hw/e1000.c
index d6fe815..263f2f4 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -131,6 +131,11 @@ typedef struct E1000State_st {
 } eecd_state;
 
 QEMUTimer *autoneg_timer;
+
+/* Enabled compatibility for migration to/from rhel6.3.0 and older */
+#define E1000_FLAG_QEMU_13_BIT 0
+#define E1000_FLAG_QEMU_13 (1 << E1000_FLAG_QEMU_13_BIT)
+uint32_t compat_flags;
 } E1000State;
 
 #definedefreg(x)   x = (E1000_##x>>2)
@@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
 static void
 set_phy_ctrl(E1000State *s, int index, uint16_t val)
 {
+/*
+ * QEMU 1.3 does not support link auto-negotiation emulation, so if we
+ * migrate during auto negotiation, after migration the link will be
+ * down.
+ */
+if (s->compat_flags & E1000_FLAG_QEMU_13) {
+return;
+}
 if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) {
 e1000_link_down(s);
 s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE;
@@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
 {
 E1000State *s = opaque;
 NetClientState *nc = qemu_get_queue(s->nic);
+
+if (s->compat_flags & E1000_FLAG_QEMU_13) {
+return;
+}
+
 /*
  * If link is down and auto-negotiation is ongoing, complete
  * auto-negotiation immediately.  This allows is to look at
@@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int version_id)
  * to link status bit in mac_reg[STATUS].
  * Alternatively, restart link negotiation if it was in progress. */
 nc->link_down = (s->mac_reg[STATUS] & E1000_STATUS_LU) == 0;
+
+if (s->compat_flags & E1000_FLAG_QEMU_13) {
+return 0;
+}
+
 if (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN &&
 s->phy_reg[PHY_CTRL] & MII_CR_RESTART_AUTO_NEG &&
 !(s->phy_reg[PHY_STATUS] & MII_SR_AUTONEG_COMPLETE)) {
@@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
 
 static Property e1000_properties[] = {
 DEFINE_NIC_PROPERTIES(E1000State, conf),
+DEFINE_PROP_BIT("x-qemu_13_compat", E1000State,
+compat_flags, E1000_FLAG_QEMU_13, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index ba09714..89a3f1f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
 .driver   = "vmware-svga",
 .property = "rombar",
 .value= stringify(0),
+},{
+.driver   = "e1000",
+.property = "x-qemu_13_compat",
+.value= "on",
 },
 { /* end of list */ }
 },
-- 
MST

Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant

[Avi Kivity]

On Thu, Feb 14, 2013 at 6:50 PM, Michael S. Tsirkin  wrote:
>> > As you see, ioapic at 0xfec0 overlaps pci-hole.
>> > ioapic is guest programmable in theory - should use _overlap?
>> > pci-hole is not but can overlap with ioapic.
>> > So also _overlap?
>>
>> It's a bug.  The ioapic is in the pci address space, not the system
>> address space.  And yes it's overlappable.
>
> So you want to put it where? Under pci-hole?

No, under the pci address space.  Look at the 440fx block diagram.

> And we'll have to teach all machine types
> creating pci-hole about it?

No.

>
>> >
>> > Let's imagine someone writes a guest programmable device for
>> > ARM. Now we should update all ARM devices from regular to _overlap?
>>
>> It's sufficient to update the programmable device.
>
> Then the device can be higher priority (works for apic)
> but not lower priority. Make priority signed?

Is there an actual real problem that needs fixing?

Re: [Qemu-devel] [PATCH RFC] memory: drop _overlap variant

[Michael S. Tsirkin]

On Thu, Feb 14, 2013 at 05:07:02PM +0200, Avi Kivity wrote:
> On Thu, Feb 14, 2013 at 4:40 PM, Michael S. Tsirkin  wrote:
> > On Thu, Feb 14, 2013 at 04:14:39PM +0200, Avi Kivity wrote:
> >
> > But some parents are system created and shared by many devices so children 
> > for
> > such have no idea who their siblings are.
> >
> > Please take a look at the typical map in this mail:
> > '[BUG] Guest OS hangs on boot when 64bit BAR present'
> >
> > system overlap 0 pri 0 [0x0 - 0x7fff]
> >  kvmvapic-rom overlap 1 pri 1000 [0xca000 - 0xcd000]
> >  pc.ram overlap 0 pri 0 [0xca000 - 0xcd000]
> >  ++ pc.ram [0xca000 - 0xcd000] is added to view
> >  
> >  smram-region overlap 1 pri 1 [0xa - 0xc]
> >  pci overlap 0 pri 0 [0xa - 0xc]
> >  cirrus-lowmem-container overlap 1 pri 1 [0xa - 0xc]
> >  cirrus-low-memory overlap 0 pri 0 [0xa - 0xc]
> > ++cirrus-low-memory [0xa - 0xc] is added to view
> >  kvm-ioapic overlap 0 pri 0 [0xfec0 - 0xfec01000]
> > ++kvm-ioapic [0xfec0 - 0xfec01000] is added to view
> >  pci-hole64 overlap 0 pri 0 [0x1 - 0x4001]
> >  pci overlap 0 pri 0 [0x1 - 0x4001]
> >  pci-hole overlap 0 pri 0 [0x7d00 - 0x1]
> >  pci overlap 0 pri 0 [0x7d00 - 0x1]
> >  ivshmem-bar2-container overlap 1 pri 1 [0xfe00 - 
> > 0x1]
> >  ivshmem.bar2 overlap 0 pri 0 [0xfe00 - 0x1]
> > ++ivshmem.bar2 [0xfe00 - 0xfec0] is added to view
> > ++ivshmem.bar2  [0xfec01000 - 0x1] is added to view
> >
> > As you see, ioapic at 0xfec0 overlaps pci-hole.
> > ioapic is guest programmable in theory - should use _overlap?
> > pci-hole is not but can overlap with ioapic.
> > So also _overlap?
> 
> It's a bug.  The ioapic is in the pci address space, not the system
> address space.  And yes it's overlappable.

So you want to put it where? Under pci-hole?
And we'll have to teach all machine types
creating pci-hole about it?

> >
> > Let's imagine someone writes a guest programmable device for
> > ARM. Now we should update all ARM devices from regular to _overlap?
> 
> It's sufficient to update the programmable device.

Then the device can be higher priority (works for apic)
but not lower priority. Make priority signed?

> >> >
> >> > Non overlapping is not a common case at all.  E.g. with normal PCI
> >> > devices you have no way to know nothing overlaps - addresses are guest
> >> > programmable.
> >>
> >> Non overlapping is mostly useful for embedded platforms.
> >
> > Maybe it should have a longer name like _nonoverlap then?
> > Current API makes people assume _overlap is only for special
> > cases and default should be non overlap.
> 
> The assumption is correct.

[Qemu-devel] test images for musicpal and milkymist?

[Peter Maydell]

Hi; I'm looking for test images (and command lines) for the musicpal
and milkymist targets, because I have a patchset which makes some
changes to them [I'm trying to get rid of sysbus_add_memory()].
Does anybody have any to hand?

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH v2 03/23] qcow2: Change handle_dependency to byte granularity

[Kevin Wolf]

On Thu, Feb 14, 2013 at 03:48:51PM +0100, Stefan Hajnoczi wrote:
> On Wed, Feb 13, 2013 at 02:21:53PM +0100, Kevin Wolf wrote:
> > Signed-off-by: Kevin Wolf 
> > ---
> >  block/qcow2-cluster.c |   40 
> >  block/qcow2.h |   11 +++
> >  2 files changed, 39 insertions(+), 12 deletions(-)
> > 
> > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> > index 0e804ba..a3b2447 100644
> > --- a/block/qcow2-cluster.c
> > +++ b/block/qcow2-cluster.c
> > @@ -756,31 +756,41 @@ out:
> >   * Check if there already is an AIO write request in flight which allocates
> >   * the same cluster. In this case we need to wait until the previous
> >   * request has completed and updated the L2 table accordingly.
> > + *
> > + * Returns:
> > + *   0   if there was no dependency. *cur_bytes indicates the number of
> > + *   bytes from guest_offset that can be read before the next
> > + *   dependency must be processed (or the request is complete)
> 
> s/read/accessed/
> 
> IMO "read" is too specific, the doc comment doesn't need to contain
> knowledge of what the caller will do at guest_offset.

Right. In fact, "read" is even wrong, if anything it should be "written to".

> > + *
> > + *   -EAGAIN if we had to wait for another request, previously gathered
> > + *   information on cluster allocation may be invalid now. The 
> > caller
> > + *   must start over anyway, so consider *cur_bytes undefined.
> >   */
> >  static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
> > -unsigned int *nb_clusters)
> > +uint64_t *cur_bytes)
> >  {
> >  BDRVQcowState *s = bs->opaque;
> >  QCowL2Meta *old_alloc;
> > +uint64_t bytes = *cur_bytes;
> >  
> >  QLIST_FOREACH(old_alloc, &s->cluster_allocs, next_in_flight) {
> >  
> > -uint64_t start = guest_offset >> s->cluster_bits;
> > -uint64_t end = start + *nb_clusters;
> > -uint64_t old_start = old_alloc->offset >> s->cluster_bits;
> > -uint64_t old_end = old_start + old_alloc->nb_clusters;
> > +uint64_t start = guest_offset;
> 
> I'm not sure this is safe.  Previously the function caught write
> requests which overlap at cluster granularity, now it will allow them?

At this point in the series, old_start and old_end are still aligned to
cluster boundaries. So the cases in which an overlap is detected stay exactly
the same with this patch. This is just a more precise description of what we
really wanted to compare.

Kevin

Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently

[Luiz Capitulino]

On Thu, 14 Feb 2013 14:31:50 +0100
Markus Armbruster  wrote:

> [Some quoted material restored]
> 
> Luiz Capitulino  writes:
> 
> > On Thu, 14 Feb 2013 10:45:22 +0100
> > Markus Armbruster  wrote:
> >
> >> [Note cc: +Laszlo, +Anthony, -qemu-trivial]
> >> 
> >> Luiz Capitulino  writes:
> >> 
> >> > On Fri, 08 Feb 2013 20:34:20 +0100
> >> > Markus Armbruster  wrote:
> >> >
> >> >> > The real problem here is that the k, M, G suffixes, for example, are 
> >> >> > not
> >> >> > good to be reported by QMP. So maybe we should refactor the code in a 
> >> >> > way
> >> >> > that we separate what's done in QMP from what is done in
> >> >> > HMP/command-line.
> >> >> 
> >> >> Isn't it separated already?  parse_option_size() is used when parsing
> >> >> key=value,...  Such strings should not exist in QMP.  If they do, it's a
> >> >> design bug.
> >> >
> >> > No and no. Such strings don't exist in QMP as far as can tell (see bugs
> >> > below though), but parse_option_size() is theoretically "present" in a
> >> > possible QMP call stack:
> >> >
> >> > qemu_opts_from_dict_1()
> >> >   qemu_opt_set_err()
> >> > opt_set()
> >> >   qemu_opt_paser()
> >> > parse_option_size()
> >> >
> >> > I can't tell if this will ever happen because qemu_opts_from_dict_1()
> >> > restricts the call to qemu_opt_set_err() to certain values, but the
> >> > fact that it's not clear is an indication that a better separation is
> >> > necessary.
> >> 
> >> Permit me two detours.
> >> 
> >> One, qemu_opt_set_err() is a confusing name.  I doesn't set an error.
> >
> > It takes an Error ** object and it was introduced to avoid having
> > to convert qemu_opt_set() to take an Error ** object, as this would
> > multiply the work required to get netdev_add converted to the qapi.
> >
> > Now, I pass the bikeshed :)
> 
> I get why it's there, I just find its name confusing.
> 
> > [...]
> >> Now, to build hmp_device_add() on top of qmp_device_add(), we'd have to
> >> convert first from QemuOpts to QDict and then back to QemuOpts.  Blech.
> >> Instead, we made do_device_add() go straight to qdev_device_add().  Same
> >> for hmp_netdev_add(): it goes straight to netdev_add().
> >
> > Yes, the idea was to have netdev_add() and add frontends to hmp
> > and qmp. More on this below.
> >
> > [...]
> >> I guess weird things can happen with qemu_opts_from_qdict(), at least in
> >> theory.
> >> 
> >> For each (key, value) in the QDict, qemu_opts_from_qdict() converts
> >> value to string according to its qtype_code.  The string then gets
> >> parsed according to key's QemuOptType.  Yes, that means you can pass a
> >> QString to QEMU_OPT_SIZE option, and get the suffixes interpreted.
> >> 
> >> However, we've only used qemu_opts_from_qdict() with QemuOptsLists that
> >> have empty desc[]!  Specifically: qemu_netdev_opts and qemu_device_opts.
> >> Without desc, qemu_opt_parse() does nothing, and QemuOpts holds just
> >> string values.  Actual parsing left to the consumer.
> >> 
> >> Two consumers: qdev_device_add() and netdev_add().
> >> 
> >> netdev_add() uses QAPI wizardry to create the appropriate object from
> >> the QemuOpts.  The parsing is done by visitors.  Things get foggy for me
> >> from there on, but it looks like one of them, opts_type_size(), can
> >> parse size suffixes, which is inappropriate for QMP.  A quick test
> >> confirms that this is indeed possible:
> >> 
> >> {"execute": "netdev_add","arguments": {"type":"tap","id":"net.1",
> >> "sndbuf": "8k" }}
> >> 
> >> Sets NetdevTapOptions member sndbuf to 8192.
> >
> > Well, I've just tested this with commit 783e9b4, which is before
> > netdev_add conversion to the qapi, and the command above just works.
> >
> > Didn't check if sndbuf is actually set to 8192, but this shows that
> > we've always accepted such a command.
> 
> Plausible.  Nevertheless, we really shouldn't.

Agreed. I would be willing to break compatibility to fix the suffix
part of the problem, but there's another issue: sndbuf shouldn't be
a string in QMP, and that's unfixable.

> >> To sum up, we go from QDict delivered by the JSON parser via QemuOpts to
> >> QAPI object, with a few cock-ups along the way.  Ugh.
> >> 
> >> By the way, the JSON schema reads
> >> 
> >> { 'command': 'netdev_add',
> >>   'data': {'type': 'str', 'id': 'str', '*props': '**'},
> >>   'gen': 'no' }
> >> 
> >> I'll be hanged if I know what '**' means.
> >
> > For QMP on the wire it means that the command accepts a bunch of
> > arguments not specified in the schema.
> 
> Thanks!  Is the schema language documented anywhere?

Unfortunately not.

> > Yes, it's a dirty trick. Long story short: we decide to maintain
> > QemuOpts usage in both HMP and QMP to speed up netdev_add conversion.
> 
> I don't think '**' is as dirty as you make it sound.  It's simply a way
> to relax the rigidity of the schema.  I got no problem with that.

Problem is, I don't know how to make it better if we want to. I think
we could use it for the ol

Re: [Qemu-devel] [PATCH v2 00/10] Cleanup bitops vs host-utils

[Eric Blake]

On 02/13/2013 06:47 PM, Richard Henderson wrote:
> Version 1 merely tried to adjust bitops_flsl, here I instead eliminate
> it all from bitops.h, and standardizes on the routines from host-utils.h.
> 
> 
> r~
> 
> 
> Richard Henderson (10):
>   host-utils: Add host long specific aliases for clz, ctz, ctpop
>   host-utils: Fix coding style and add comments
>   hbitmap: Use non-bitops ctzl
>   bitops: Use non-bitops ctzl
>   memory: Use non-bitops ctzl
>   bitops: Write bitops_flsl in terms of clzl
>   target-i386: Inline bitops_flsl
>   bitops: Inline bitops_flsl
>   bitops: Replace bitops_ctol with ctzl
>   bitops: Remove routines redundant with host-utils
> 
>  include/qemu/bitops.h |  75 -
>  include/qemu/hbitmap.h|   3 +-
>  include/qemu/host-utils.h | 119 
> +++---
>  memory.c  |   4 +-
>  target-i386/topology.h|   6 +--
>  util/bitops.c |   6 +--
>  util/hbitmap.c|   3 +-
>  7 files changed, 112 insertions(+), 104 deletions(-)

Series:
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] single step not working after hit a break point

[Peter Cheung]

Hi All
I use the following code to insert a breakpoint in physical address 
0×160CPUArchState *cpu = first_cpu;
hwaddr addr;
sscanf(command + 2, "%ld", &addr);
int err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL );qemu successfully hit 
the breakpoint and stop, then i try to single-step by the following 
code:CPUArchState *cpu = first_cpu;
cpu_single_step(cpu, sstep_flags);
vm_start();Nothing happened, the EIP still stay in 0×160, but if i delete 
the breakpoint, the single step just work again. Am I missed something? 
thanks

Re: [Qemu-devel] [RFC v5] target-i386: Slim conversion to X86CPU subclasses + KVM subclasses

[Eduardo Habkost]

On Thu, Feb 14, 2013 at 04:13:22PM +0100, Igor Mammedov wrote:
> On Thu, 14 Feb 2013 09:44:59 -0200
> Eduardo Habkost  wrote:
[...]
> > > +if (kvm_enabled()) {
> > > +typename = g_strdup_printf("%s-kvm-" TYPE_X86_CPU, name);
> > 
> > * Could we use the "-tcg" suffix for the non-KVM mode?
> sure, I'll add it for the next re-spin.
> 
> > * Can we make the code fall back to no-suffix CPU names? We need to add
> >   the -kvm suffix (and maybe a -tcg suffix) to keep compatibility with
> >   existing CPU models, but maybe we should deprecate the automatic
> >   -tcg/-kvm suffixing and ask users to specify the full CPU name.
> I'm not sure that I got you right, Have you meant something like this:
> 
> if (kvm) {
> typename = name-kvm-...
> } else {
> typename = name-tcg-...
> }
> 
> oc = object_class_by_name(typename)
> if (!oc) {
> oc = object_class_by_name(name)
> }

Yes, exactly.


[...]
> > > +
> > > +}
> > > +
> > > +static void x86_cpu_def_class_init(ObjectClass *oc, void *data)
> > > +{
> > > +X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > +x86_def_t *def = data;
> > > +int i;
> > > +static const char *versioned_models[] = { "qemu32", "qemu64",
> > > "athlon" }; +
> > > +memcpy(&xcc->info, def, sizeof(x86_def_t));
> > > +xcc->info.ext_features |= CPUID_EXT_HYPERVISOR;
> > 
> > If this is TCG-specific now, we could make the class match the reality
> > and mask TCG_FEATURES/TCG_EXT_FEATURES/etc here. That's what happens in
> > practice, e.g.: "-cpu SandyBridge" in TCG mode is a completely different
> > CPU, today.
> well, this function is shared between TCG and KVM, I mean, it's common
> code for both. Which asks for one more TCG class_init function for TCG
> specific behavior.

Having both TCG-specific and KVM-specific subclasses instead of making
the KVM class inherit from the TCG class would make sense to me, as we
may have TCG-specific behavior in other places too.

Or we could do memcpy() again on the KVM class_init function. Or we
could set a different void* pointer on the TCG class, with
already-filtered x86_def_t object. There are multiple possible
solutions.


> But could it be a separate patch (i.e. fixing TCG filtering), I think
> just moving tcg filtering might cause regression. And need to worked on
> separately.

I'm OK with doing that in a separate patch, as it is not a bug fix or
behavior change. But it would be nice to do that before we introduce the
feature properties, to make the reported defaults right since the
beginning.

[...]
> > > +#ifdef CONFIG_KVM
> > > +static void x86_cpu_kvm_def_class_init(ObjectClass *oc, void *data)
> > > +{
> > > +uint32_t eax, ebx, ecx, edx;
> > > +X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > +
> > > +x86_cpu_def_class_init(oc, data);
> > > +/* sysenter isn't supported in compatibility mode on AMD,
> > > + * syscall isn't supported in compatibility mode on Intel.
> > > + * Normally we advertise the actual CPU vendor, but you can
> > > + * override this using the 'vendor' property if you want to use
> > > + * KVM's sysenter/syscall emulation in compatibility mode and
> > > + * when doing cross vendor migration
> > > + */
> > > +host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
> > 
> > Cool, this is exactly what I was going to suggest, to avoid depending on
> > the "host" CPU class and KVM initialization.
> > 
> > I see two options when making the "vendor" property static, and I don't
> > know which one is preferable:
> > 
> > One solution is the one in this patch: to call host_cpuid() here in
> > class_init, and have a different property default depending on the host
> > CPU.
> I prefer this one ^^^.
> 
> > Another solution is to have default to vendor="host", and have
> > instance_init understand vendor="host" as "use the host CPU vendor".
> > This would make the property default really static (not depending on the
> > host CPU).
> > 
> 
> 
> > I am inclined for the latter, because I am assuming that the QOM design
> > expects class_init results to be completely static. This is not as bad
> > as depending on KVM initialization, but it may be an unexpected
> > surprise, or even something not allowed by QOM.
> That's where I have to disagree :)
> 
> You might have a point with 'static' if our goal is for introspection for
> source level to work. But we have a number of issues with that:
> 
> * model_id is not static for some cpus, 'vendor' is the same just for
>   different set of classes

model_id is static for a given QEMU version, isn't it? This may be OK
(but I am _also_ not sure about this case).


> * we generate sub-classes at runtime dynamically, which makes source level
>   introspection impossible for them.

It's not source-level introspection I am looking for, but runtime
introspection that won't surprise other components in the system by
changing unexpectedly (e.g. when the host CPU changes).

> 
> I think that QOM is aiming towards run-time introspection of
> classe

Re: [Qemu-devel] [PATCH for-1.4 07/19] target-sparc: Fix debug output for DEBUG_MMU

[Andreas Färber]

Blue,

Am 27.01.2013 14:32, schrieb Andreas Färber:
> Signed-off-by: Andreas Färber 
> ---
>  target-sparc/ldst_helper.c |2 +-
>  1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-)
> 
> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index cf1bddf..7decd66 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -1850,7 +1850,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
> addr, target_ulong val,
>  DPRINTF_MMU("LSU change: 0x%" PRIx64 " -> 0x%" PRIx64 "\n",
>  oldreg, env->lsu);
>  #ifdef DEBUG_MMU
> -dump_mmu(stdout, fprintf, env1);
> +dump_mmu(stdout, fprintf, env);
>  #endif
>  tlb_flush(env, 1);
>  }

This patch was not applied to master. Was that an oversight or is
something wrong with it? (Preparing a v2 series for 1.5.)

Andreas

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