[Qemu-devel] [PATCH 3/4] qemu-ga: Introduce Windows MSI script

[Yossi Hindin]

The script enables building Windows MSI installation package on Linux with wixl 
tool.

Signed-off-by: Yossi Hindin 
---
 qga/installer/qemu-ga.wxs | 145 ++
 1 file changed, 145 insertions(+)
 create mode 100644 qga/installer/qemu-ga.wxs

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
new file mode 100644
index 000..44c844b
--- /dev/null
+++ b/qga/installer/qemu-ga.wxs
@@ -0,0 +1,145 @@
+
+http://schemas.microsoft.com/wix/2006/wi";>
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
+  
+
+NOT VersionNT64
+
+
+
+1
+
+
+  
+
+
+
+  
+
+  
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+  
+
+  
+
+  
+  
+
+  
+
+  
+
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+
+  
+  
+  NOT 
Installed
+  Installed
+  
+
+  
+
-- 
2.1.0

[Qemu-devel] [PATCH 0/4] MSI installation for Windows Guest Agent

[Yossi Hindin]

The patches' set introduce creation of MSI installer for Windows guest agent
with wixl tool.

Yossi Hindin (4):
  qemu-ga: adding vss-[un]install options
  qemu-ga: debug printouts to help troubleshoot installation
  qemu-ga: Introduce Windows MSI script
  qemu-ga: Building Windows MSI installation with configure/Makefile

 Makefile  |  17 +-
 configure |  63 
 qga/channel-win32.c   |   2 +-
 qga/commands-win32.c  |   1 +
 qga/installer/qemu-ga.wxs | 145 ++
 qga/main.c|  10 +++-
 6 files changed, 235 insertions(+), 3 deletions(-)
 create mode 100644 qga/installer/qemu-ga.wxs

-- 
2.1.0

[Qemu-devel] [PATCH 2/4] qemu-ga: debug printouts to help troubleshoot installation

[Yossi Hindin]

Debug printouts extended, helps installation troubleshooting

Signed-off-by: Yossi Hindin 
---
 qga/channel-win32.c  | 2 +-
 qga/commands-win32.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 0d5e5f5..04fa5e4 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -306,7 +306,7 @@ static gboolean ga_channel_open(GAChannel *c, 
GAChannelMethod method,
OPEN_EXISTING,
FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, 
NULL);
 if (c->handle == INVALID_HANDLE_VALUE) {
-g_critical("error opening path");
+g_critical("error opening path %s", newpath);
 return false;
 }
 
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 3ef0549..d0aaec7 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -721,6 +721,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
 }
 
 if (!vss_init(true)) {
+g_debug("vss_init failed, vss commands are going to be disabled");
 const char *list[] = {
 "guest-get-fsinfo", "guest-fsfreeze-status",
 "guest-fsfreeze-freeze", "guest-fsfreeze-thaw", NULL};
-- 
2.1.0

[Qemu-devel] [PATCH 1/4] qemu-ga: adding vss-[un]install options

[Yossi Hindin]

Existing command line options include '-s install' and '-s uninstall'.
These options install/uninstall both Windows QEMU GA service
and optional VSS COM server. The QEMU GA Windows service allows
always-on serving guest agent's QMP commands and VSS COM server
enables guest agent integration with Volume Shadow Service.

This commit introdices new options '-s vss-install' and '-s vss-uninstall',
affecting only GA VSS COM server registration. The new options are useful
for registering and unregistering the COM server during MSI installation,
upgrade and uninstallation.

Signed-off-by: Yossi Hindin 
---
 qga/main.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index 9939a2b..7e1e438 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -211,7 +211,7 @@ static void usage(const char *cmd)
 "  -V, --version print version information and exit\n"
 "  -d, --daemonize   become a daemon\n"
 #ifdef _WIN32
-"  -s, --service service commands: install, uninstall\n"
+"  -s, --service service commands: install, uninstall, vss-install, 
vss-uninstall\n"
 #endif
 "  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, 
\"?\"\n"
 "to list available RPCs)\n"
@@ -1036,6 +1036,14 @@ int main(int argc, char **argv)
 } else if (strcmp(service, "uninstall") == 0) {
 ga_uninstall_vss_provider();
 return ga_uninstall_service();
+} else if (strcmp(service, "vss-install") == 0) {
+if (ga_install_vss_provider()) {
+return EXIT_FAILURE;
+}
+return EXIT_SUCCESS;
+} else if (strcmp(service, "vss-uninstall") == 0) {
+ga_uninstall_vss_provider();
+return EXIT_SUCCESS;
 } else {
 printf("Unknown service command.\n");
 return EXIT_FAILURE;
-- 
2.1.0

[Qemu-devel] [PATCH 4/4] qemu-ga: Building Windows MSI installation with configure/Makefile

[Yossi Hindin]

New options were added to enable Windows MSI installation package
creation:

Option --enable-guest-msi, like the name suggest, enables building
Windows MSI package for QEMU guest agent; option --disable-guest-msi
disables MSI package creation; by default, no MSI package is created

Signed-off-by: Yossi Hindin 
---
 Makefile  | 17 -
 configure | 63 +++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 93af871..67b007c 100644
--- a/Makefile
+++ b/Makefile
@@ -74,7 +74,7 @@ Makefile: ;
 configure: ;
 
 .PHONY: all clean cscope distclean dvi html info install install-doc \
-   pdf recurse-all speed test dist
+   pdf recurse-all speed test dist msi
 
 $(call set-vpath, $(SRC_PATH))
 
@@ -287,10 +287,25 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
$(call LINK, $^)
 
+QEMU_GA_MSI=qemu-ga-${ARCH}.msi
+
+msi: ${QEMU_GA_MSI}
+
+ifdef QEMU_GA_MSI_WITH_VSS
+${QEMU_GA_MSI}: qga/vss-win32/qga-vss.dll qemu-ga.exe
+endif
+
+${QEMU_GA_MSI}: config-host.mak
+
+${QEMU_GA_MSI}:  qga/installer/qemu-ga.wxs
+   $(call quiet-command,QEMU_GA_VERSION="$(QEMU_GA_VERSION)" 
QEMU_GA_MANUFACTURER="$(QEMU_GA_MANUFACTURER)" 
QEMU_GA_DISTRO="$(QEMU_GA_DISTRO)" \
+   wixl -o $@ ${QEMU_GA_MSI_ARCH} ${QEMU_GA_MSI_WITH_VSS} 
${QEMU_GA_MSI_MINGW_DLL_PATH} $<, "  WIXL  $@")
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
rm -f qemu-options.def
+   rm -f *.msi
find . \( -name '*.l[oa]' -o -name '*.so' -o -name '*.dll' -o -name 
'*.mo' -o -name '*.[oda]' \) -type f -exec rm {} +
rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* 
*.pod *~ */*~
rm -f fsdev/*.pod
diff --git a/configure b/configure
index 6969f6f..0aa79bb 100755
--- a/configure
+++ b/configure
@@ -316,6 +316,7 @@ snappy=""
 bzip2=""
 guest_agent=""
 guest_agent_with_vss="no"
+guest_msi=""
 vss_win32_sdk=""
 win_sdk="no"
 want_tools="yes"
@@ -1069,6 +1070,10 @@ for opt do
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --enable-guest-msi) guest_msi="yes"
+  ;;
+  --disable-guest-msi) guest_msi="no"
+  ;;
   --with-vss-sdk) vss_win32_sdk=""
   ;;
   --with-vss-sdk=*) vss_win32_sdk="$optarg"
@@ -1407,6 +1412,8 @@ Advanced options (experts only):
   --enable-quorum  enable quorum block filter support
   --disable-numa   disable libnuma support
   --enable-numaenable libnuma support
+  --enable-guest-msi   enable building guest agent Windows MSI 
installation package
+  --disable-guest-msi  disable building guest agent Windows MSI 
installation package
 
 NOTE: The object files are built at the place where configure is launched
 EOF
@@ -3832,6 +3839,54 @@ if test "$mingw32" = "yes" -a "$guest_agent" != "no" -a 
"$guest_agent_with_vss"
 fi
 
 ##
+# Guest agent Window MSI  package
+
+if test "$guest_msi" = "yes"; then
+
+  if test "$guest_agent" != "yes"; then
+  error_exit "MSI guest agent package requires guest agent enabled"
+  fi
+
+  if test "$mingw32" != "yes"; then
+  error_exit "MSI guest agent is available only for MinGW Windows 
cross-compilation"
+  fi
+
+  if ! has wixl; then
+error_exit "wixl not found, required for building installation"
+  fi
+
+
+  if test "$guest_agent_with_vss" = "yes"; then
+QEMU_GA_MSI_WITH_VSS="-D InstallVss"
+  fi
+
+  if test "$QEMU_GA_MANUFACTURER" = ""; then
+QEMU_GA_MANUFACTURER=QEMU
+  fi
+
+  if test "$QEMU_GA_DISTRO" = ""; then
+QEMU_GA_DISTRO=Linux
+  fi
+
+  if test "$QEMU_GA_VERSION" = ""; then
+  QEMU_GA_VERSION=`cat $source_path/VERSION`
+  fi
+
+  case "$cpu" in
+  x86_64)
+QEMU_GA_MSI_ARCH="-a x64 -D Arch=64"
+;;
+  i386)
+QEMU_GA_MSI_ARCH="-D Arch=32"
+;;
+  *)
+error_exit "CPU $cpu not supported for building installation package"
+;;
+  esac
+
+fi
+
+##
 
 ##
 # check if we have fdatasync
@@ -4500,6 +4555,14 @@ if test "$mingw32" = "yes" ; then
 echo "CONFIG_QGA_VSS=y" >> $config_host_mak
 echo "WIN_SDK=\"$win_sdk\"" >> $config_host_mak
   fi
+  if test "$guest_msi" = "yes"; then
+echo "QEMU_GA_MSI_MINGW_DLL_PATH=-D Mingw_dlls=`$pkg_config 
--variable=prefix glib-2.0`/bin" >> $config_host_mak
+echo "QEMU_GA_MSI_WITH_VSS=${QEMU_GA_MSI_WITH_VSS}" >> $config_host_mak
+echo "QEMU_GA_MSI_ARCH=${QEMU_GA_MSI_ARCH}" >> $config_host_mak
+echo "QEMU_GA_MANUFACTURER=${QEMU_GA_MANUFACTURER}" >> $config_host_mak
+echo "QEMU_GA_DISTRO=${QEMU_GA_DISTRO}" >> $config_host_mak
+echo "QEMU_GA_VERSION=${QEMU_GA_VERSION}" >> $config_host_mak
+  fi
 else
   echo "CONFIG_POSIX=y" >> $config_host_mak
 fi
-- 
2.1.0

Re: [Qemu-devel] [PATCH 16/17] ipmi: Add ACPI table entries

[Michael S. Tsirkin]

On Thu, Apr 23, 2015 at 05:57:57PM -0500, miny...@acm.org wrote:
> From: Corey Minyard 
> 
> Use the new ACPI table construction tools to create an ACPI
> entry for IPMI.
> 
> Signed-off-by: Corey Minyard 

Acked-by: Michael S. Tsirkin 

> ---
>  hw/ipmi/Makefile.objs |   1 +
>  hw/ipmi/ipmi_acpi.c   | 122 
> ++
>  hw/ipmi/isa_ipmi.c|   4 ++
>  3 files changed, 127 insertions(+)
>  create mode 100644 hw/ipmi/ipmi_acpi.c
> 
> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
> index d0129cf..17ca09f 100644
> --- a/hw/ipmi/Makefile.objs
> +++ b/hw/ipmi/Makefile.objs
> @@ -5,3 +5,4 @@ common-obj-$(CONFIG_IPMI_BT) += ipmi_bt.o
>  common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_sim.o
>  common-obj-$(CONFIG_IPMI_EXTERN) += ipmi_extern.o
>  common-obj-$(call land,$(CONFIG_IPMI),$(CONFIG_SMBIOS)) += ipmi_smbios.o
> +common-obj-$(call land,$(CONFIG_IPMI),$(CONFIG_ACPI)) += ipmi_acpi.o
> diff --git a/hw/ipmi/ipmi_acpi.c b/hw/ipmi/ipmi_acpi.c
> new file mode 100644
> index 000..e014da7
> --- /dev/null
> +++ b/hw/ipmi/ipmi_acpi.c
> @@ -0,0 +1,122 @@
> +/*
> + * IPMI ACPI firmware handling
> + *
> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "ipmi.h"
> +#include "hw/acpi/aml-build.h"
> +#include "hw/acpi/acpi.h"
> +#include "hw/acpi/acpi-dev-tables.h"
> +#include 
> +
> +static Aml *aml_ipmi_crs(IPMIFwInfo *info)
> +{
> +Aml *crs = aml_resource_template();
> +uint8_t regspacing = info->register_spacing;
> +
> +if (regspacing == 1) {
> +regspacing = 0;
> +}
> +
> +switch (info->memspace) {
> +case IPMI_MEMSPACE_IO:
> +aml_append(crs, aml_io(aml_decode16, info->base_address,
> +   info->base_address + info->register_length - 
> 1,
> +   regspacing, info->register_length));
> +break;
> +case IPMI_MEMSPACE_MEM32:
> +aml_append(crs,
> +   aml_dword_memory(aml_pos_decode,
> +aml_min_fixed, aml_max_fixed,
> +aml_non_cacheable, aml_ReadWrite,
> +0x,
> +info->base_address,
> +info->base_address + info->register_length - 1,
> +regspacing, info->register_length));
> +break;
> +case IPMI_MEMSPACE_MEM64:
> +aml_append(crs,
> +   aml_qword_memory(aml_pos_decode,
> +aml_min_fixed, aml_max_fixed,
> +aml_non_cacheable, aml_ReadWrite,
> +0xULL,
> +info->base_address,
> +info->base_address + info->register_length - 1,
> +regspacing, info->register_length));
> +break;
> +case IPMI_MEMSPACE_SMBUS:
> +aml_append(crs, aml_return(aml_int(info->base_address)));
> +break;
> +}
> +
> +if (info->interrupt_number) {
> +aml_append(crs, aml_irq_no_flags(info->interrupt_number));
> +}
> +
> +return crs;
> +}
> +
> +static void
> +ipmi_encode_one_acpi(IPMIFwInfo *info, void *opaque)
> +{
> +Aml *ssdt, *scope, *dev, *method;
> +char *name;
> +int version = ((info->ipmi_spec_major_revision << 8)
> +   | (info->ipmi_spec_minor_revision << 4));
> +
> +if (!info->acpi_parent) {
> +ipmi_debug("device %s not compatible with ACPI, no parent given.",
> +   info->interface_name);
> +return;
> +}
> +
> +ssdt = init_aml_allocator();
> +
> +scope = aml_scope("%s", info->acpi_parent);
> +name = g_strdup_printf("ipmi_%s", info->interface_name);
> +
> +dev = aml_device("MI0");
> +aml_append(dev, a

Re: [Qemu-devel] [PATCH 14/17] ipmi: Add SMBIOS table entry

[Michael S. Tsirkin]

On Thu, Apr 23, 2015 at 05:57:55PM -0500, miny...@acm.org wrote:
> From: Corey Minyard 
> 
> Add an IPMI table entry to the SMBIOS.
> 
> Signed-off-by: Corey Minyard 

Acked-by: Michael S. Tsirkin 

> ---
>  default-configs/i386-softmmu.mak   |  1 +
>  default-configs/x86_64-softmmu.mak |  1 +
>  hw/ipmi/Makefile.objs  |  1 +
>  hw/ipmi/ipmi_smbios.c  | 89 
> ++
>  4 files changed, 92 insertions(+)
>  create mode 100644 hw/ipmi/ipmi_smbios.c
> 
> diff --git a/default-configs/i386-softmmu.mak 
> b/default-configs/i386-softmmu.mak
> index 2448543..15c0b70 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -20,6 +20,7 @@ CONFIG_I8254=y
>  CONFIG_PCSPK=y
>  CONFIG_PCKBD=y
>  CONFIG_FDC=y
> +CONFIG_SMBIOS=y
>  CONFIG_ACPI=y
>  CONFIG_APM=y
>  CONFIG_I8257=y
> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index 3ed8877..dac877a 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -20,6 +20,7 @@ CONFIG_I8254=y
>  CONFIG_PCSPK=y
>  CONFIG_PCKBD=y
>  CONFIG_FDC=y
> +CONFIG_SMBIOS=y
>  CONFIG_ACPI=y
>  CONFIG_APM=y
>  CONFIG_I8257=y
> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
> index abfa827..d0129cf 100644
> --- a/hw/ipmi/Makefile.objs
> +++ b/hw/ipmi/Makefile.objs
> @@ -4,3 +4,4 @@ common-obj-$(CONFIG_IPMI_KCS) += ipmi_kcs.o
>  common-obj-$(CONFIG_IPMI_BT) += ipmi_bt.o
>  common-obj-$(CONFIG_IPMI_LOCAL) += ipmi_sim.o
>  common-obj-$(CONFIG_IPMI_EXTERN) += ipmi_extern.o
> +common-obj-$(call land,$(CONFIG_IPMI),$(CONFIG_SMBIOS)) += ipmi_smbios.o
> diff --git a/hw/ipmi/ipmi_smbios.c b/hw/ipmi/ipmi_smbios.c
> new file mode 100644
> index 000..b818f5b
> --- /dev/null
> +++ b/hw/ipmi/ipmi_smbios.c
> @@ -0,0 +1,89 @@
> +/*
> + * IPMI SMBIOS firmware handling
> + *
> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "ipmi.h"
> +#include 
> +#include 
> +
> +/* SMBIOS type 38 - IPMI */
> +struct smbios_type_38 {
> +struct smbios_structure_header header;
> +uint8_t interface_type;
> +uint8_t ipmi_spec_revision;
> +uint8_t i2c_slave_address;
> +uint8_t nv_storage_device_address;
> +uint64_t base_address;
> +uint8_t base_address_modifier;
> +uint8_t interrupt_number;
> +} QEMU_PACKED;
> +
> +static void ipmi_encode_one_smbios(IPMIFwInfo *info, void *opaque)
> +{
> +struct smbios_type_38 smb38;
> +uint64_t baseaddr = info->base_address;
> +
> +smb38.header.type = 38;
> +smb38.header.length = sizeof(smb38);
> +smb38.header.handle = cpu_to_le16(0x3000);
> +smb38.interface_type = info->interface_type;
> +smb38.ipmi_spec_revision = ((info->ipmi_spec_major_revision << 4)
> +| info->ipmi_spec_minor_revision);
> +smb38.i2c_slave_address = info->i2c_slave_address;
> +smb38.nv_storage_device_address = 0;
> +
> +/* or 1 to set it to I/O space */
> +switch (info->memspace) {
> +case IPMI_MEMSPACE_IO: baseaddr |= 1; break;
> +case IPMI_MEMSPACE_MEM32: break;
> +case IPMI_MEMSPACE_MEM64: break;
> +case IPMI_MEMSPACE_SMBUS: baseaddr <<= 1; break;
> +}
> +
> +smb38.base_address = cpu_to_le64(baseaddr);
> +
> +smb38.base_address_modifier = 0;
> +if (info->irq_type == IPMI_LEVEL_IRQ) {
> +smb38.base_address_modifier |= 1;
> +}
> +switch (info->register_spacing) {
> +case 1: break;
> +case 4: smb38.base_address_modifier |= 1 << 6; break;
> +case 16: smb38.base_address_modifier |= 2 << 6; break;
> +default:
> +error_report("IPMI register spacing %d is not compatible with"
> + " SMBIOS, ignoring this entry.", 
> info->register_spacing);
> +return;
> +}
> +smb38.interr

Re: [Qemu-devel] [PATCH 02/17] ipmi: Add a PC ISA type structure

[Michael S. Tsirkin]

On Thu, Apr 23, 2015 at 05:57:43PM -0500, miny...@acm.org wrote:
> From: Corey Minyard 
> 
> This provides the base infrastructure to tie IPMI low-level
> interfaces into a PC ISA bus.
> 
> Signed-off-by: Corey Minyard 
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/ipmi/Makefile.objs  |   1 +
>  hw/ipmi/isa_ipmi.c | 144 
> +
>  include/hw/nvram/fw_cfg.h  |  11 ++-
>  5 files changed, 157 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ipmi/isa_ipmi.c
> 
> diff --git a/default-configs/i386-softmmu.mak 
> b/default-configs/i386-softmmu.mak
> index ab1a552..1c3153b 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -9,6 +9,7 @@ CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
> +CONFIG_ISA_IPMI=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index 82bafcc..6b57430 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -9,6 +9,7 @@ CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
> +CONFIG_ISA_IPMI=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
> index 65bde11..1518216 100644
> --- a/hw/ipmi/Makefile.objs
> +++ b/hw/ipmi/Makefile.objs
> @@ -1 +1,2 @@
> +common-obj-$(CONFIG_ISA_IPMI) += isa_ipmi.o
>  common-obj-$(CONFIG_IPMI) += ipmi.o
> diff --git a/hw/ipmi/isa_ipmi.c b/hw/ipmi/isa_ipmi.c
> new file mode 100644
> index 000..1c1ab8d
> --- /dev/null
> +++ b/hw/ipmi/isa_ipmi.c
> @@ -0,0 +1,144 @@
> +/*
> + * QEMU ISA IPMI emulation
> + *
> + * Copyright (c) 2012 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "hw/hw.h"
> +#include "hw/isa/isa.h"
> +#include "hw/i386/pc.h"
> +#include "qemu/timer.h"
> +#include "sysemu/char.h"
> +#include "sysemu/sysemu.h"
> +#include "ipmi.h"
> +
> +/* This is the type the user specifies on the -device command line */
> +#define TYPE_ISA_IPMI   "isa-ipmi"
> +#define ISA_IPMI(obj) OBJECT_CHECK(ISAIPMIDevice, (obj), TYPE_ISA_IPMI)
> +
> +typedef struct ISAIPMIDevice {
> +ISADevice dev;
> +char *interface;
> +uint32_t iobase;
> +int32 isairq;
> +uint8_t slave_addr;
> +CharDriverState *chr;
> +IPMIInterface *intf;
> +} ISAIPMIDevice;
> +
> +static void ipmi_isa_realizefn(DeviceState *dev, Error **errp)
> +{
> +ISADevice *isadev = ISA_DEVICE(dev);
> +ISAIPMIDevice *ipmi = ISA_IPMI(dev);
> +char typename[20];
> +Object *intfobj;
> +IPMIInterface *intf;
> +Object *bmcobj;
> +IPMIBmc *bmc;
> +
> +if (!ipmi->interface) {
> +ipmi->interface = g_strdup("kcs");
> +}
> +
> +if (ipmi->chr) {
> +bmcobj = object_new(TYPE_IPMI_BMC_EXTERN);
> +} else {
> +bmcobj = object_new(TYPE_IPMI_BMC_SIMULATOR);
> +}
> +bmc = IPMI_BMC(bmcobj);
> +bmc->chr = ipmi->chr;
> +snprintf(typename, sizeof(typename),
> + TYPE_IPMI_INTERFACE_PREFIX "%s", ipmi->interface);
> +intfobj = object_new(typename);


I wonder what Andreas thinks about this.
There are only 3 legal types, correct?
I think it would be cleaner to avoid the prefix trick,
and just do a plain
if (!ipmi->interface)) {
typename = TYPE_IPMI_INTERFACE_KCS
} else if (!strcmp(ipmi->interface, "kcs")) {
typename = TYPE_IPMI_INTERFACE_KCS
} else if 


etc



> +intf = IPMI_INTERFACE(intfobj);
> +bmc->intf = intf;
> +intf->bmc = bmc;
> +intf->io_base = ipmi->iobase;
> +intf->slave_addr = ipmi->slave_addr;
> +ipmi_interface_init(intf, errp);
> +if (*errp) 

Re: [Qemu-devel] [PATCH 02/17] ipmi: Add a PC ISA type structure

[Michael S. Tsirkin]

On Thu, Apr 23, 2015 at 05:57:43PM -0500, miny...@acm.org wrote:
> From: Corey Minyard 
> 
> This provides the base infrastructure to tie IPMI low-level
> interfaces into a PC ISA bus.
> 
> Signed-off-by: Corey Minyard 
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/ipmi/Makefile.objs  |   1 +
>  hw/ipmi/isa_ipmi.c | 144 
> +
>  include/hw/nvram/fw_cfg.h  |  11 ++-
>  5 files changed, 157 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ipmi/isa_ipmi.c
> 
> diff --git a/default-configs/i386-softmmu.mak 
> b/default-configs/i386-softmmu.mak
> index ab1a552..1c3153b 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -9,6 +9,7 @@ CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
> +CONFIG_ISA_IPMI=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index 82bafcc..6b57430 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -9,6 +9,7 @@ CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VMMOUSE=y
>  CONFIG_IPMI=y
> +CONFIG_ISA_IPMI=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
> index 65bde11..1518216 100644
> --- a/hw/ipmi/Makefile.objs
> +++ b/hw/ipmi/Makefile.objs
> @@ -1 +1,2 @@
> +common-obj-$(CONFIG_ISA_IPMI) += isa_ipmi.o
>  common-obj-$(CONFIG_IPMI) += ipmi.o
> diff --git a/hw/ipmi/isa_ipmi.c b/hw/ipmi/isa_ipmi.c
> new file mode 100644
> index 000..1c1ab8d
> --- /dev/null
> +++ b/hw/ipmi/isa_ipmi.c
> @@ -0,0 +1,144 @@
> +/*
> + * QEMU ISA IPMI emulation
> + *
> + * Copyright (c) 2012 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "hw/hw.h"
> +#include "hw/isa/isa.h"
> +#include "hw/i386/pc.h"
> +#include "qemu/timer.h"
> +#include "sysemu/char.h"
> +#include "sysemu/sysemu.h"
> +#include "ipmi.h"
> +
> +/* This is the type the user specifies on the -device command line */
> +#define TYPE_ISA_IPMI   "isa-ipmi"
> +#define ISA_IPMI(obj) OBJECT_CHECK(ISAIPMIDevice, (obj), TYPE_ISA_IPMI)
> +
> +typedef struct ISAIPMIDevice {
> +ISADevice dev;
> +char *interface;
> +uint32_t iobase;
> +int32 isairq;
> +uint8_t slave_addr;
> +CharDriverState *chr;
> +IPMIInterface *intf;
> +} ISAIPMIDevice;
> +
> +static void ipmi_isa_realizefn(DeviceState *dev, Error **errp)
> +{
> +ISADevice *isadev = ISA_DEVICE(dev);
> +ISAIPMIDevice *ipmi = ISA_IPMI(dev);
> +char typename[20];
> +Object *intfobj;
> +IPMIInterface *intf;
> +Object *bmcobj;
> +IPMIBmc *bmc;
> +
> +if (!ipmi->interface) {
> +ipmi->interface = g_strdup("kcs");
> +}
> +
> +if (ipmi->chr) {
> +bmcobj = object_new(TYPE_IPMI_BMC_EXTERN);
> +} else {
> +bmcobj = object_new(TYPE_IPMI_BMC_SIMULATOR);
> +}
> +bmc = IPMI_BMC(bmcobj);
> +bmc->chr = ipmi->chr;
> +snprintf(typename, sizeof(typename),
> + TYPE_IPMI_INTERFACE_PREFIX "%s", ipmi->interface);
> +intfobj = object_new(typename);
> +intf = IPMI_INTERFACE(intfobj);
> +bmc->intf = intf;
> +intf->bmc = bmc;
> +intf->io_base = ipmi->iobase;
> +intf->slave_addr = ipmi->slave_addr;
> +ipmi_interface_init(intf, errp);
> +if (*errp) {
> +return;
> +}
> +ipmi_bmc_init(bmc, errp);
> +if (*errp) {
> +return;
> +}
> +
> +/* These may be set by the interface. */
> +ipmi->iobase = intf->io_base;
> +ipmi->slave_addr = intf->slave_addr;
> +
> +if (ipmi->isairq > 0) {
> +isa_init_irq(isadev, &intf->irq, ipmi->isairq);
> +intf->use_irq = 1;
> +}
>

Re: [Qemu-devel] [PATCH 02/17] ipmi: Add a PC ISA type structure

[Michael S. Tsirkin]

On Sun, Apr 26, 2015 at 10:58:56AM +0200, Michael S. Tsirkin wrote:
> On Thu, Apr 23, 2015 at 05:57:43PM -0500, miny...@acm.org wrote:
> > From: Corey Minyard 
> > 
> > This provides the base infrastructure to tie IPMI low-level
> > interfaces into a PC ISA bus.
> > 
> > Signed-off-by: Corey Minyard 

BTW can you version patches please?
recent git format-patch has a -v flag, older
ones let you use --subject-prefix='PATCH vX'.

Re: [Qemu-devel] [PATCH 00/17] Update to adding an IPMI device to qemu

[Andreas Färber]

Am 24.04.2015 um 01:11 schrieb Eric Blake:
> On 04/23/2015 04:57 PM, miny...@acm.org wrote:
>> The major changes from last time are:
> 
> That says this series should probably be named v2 (git
> format-patch/send-email -v2) or later, as part of the subject line. If
> the previous version is more than a couple weeks ago, it's also nice
> (but not required) to provide a URL to the archives of the previous
> discussion.

Even worse, I don't see any update on the licensing questions previously
raised... did Intel ever get back to you?

> Also, you might want to use git format-patch --cover-letter, which helps
> form a 00/17 letter with a diffstat pre-filled; the diffstat gives
> reviewers a quick glance at what files are touched, [...]

+1.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH v3 05/24] spapr: Reorganize CPU dt generation code

[Bharata B Rao]

On Fri, Apr 24, 2015 at 12:17:27PM +0530, Bharata B Rao wrote:
> Reorganize CPU device tree generation code so that it be reused from
> hotplug path. CPU dt entries are now generated from spapr_finalize_fdt()
> instead of spapr_create_fdt_skel().

Creating CPU DT entries from spapr_finalize_fdt() instead of
spapr_create_fdt_skel() has an interesting side effect.

Before this patch, when I boot an SMP guest with the following configuration:

-smp 4 -numa node,cpus=0-1,mem=4G,nodeid=0 -numa node,cpus=2-3,mem=4G,nodeid=1

the guest CPUs come up in the following fashion:

[root@localhost ~]# cat /proc/cpuinfo
processor   : 0
cpu dt id   : 0

processor   : 1
cpu dt id   : 8

processor   : 2
cpu dt id   : 16

processor   : 3
cpu dt id   : 24

In the above /proc/cpuinfo output, only the relevant fields are retained
and the newly added field "cpu dt id" is essentially obtained by
arch/powerpc/include/asm/smp.h:get_hard_smp_processor_id() in the kernel.

[root@localhost ~]# lscpu
CPU(s):4
On-line CPU(s) list:   0-3
Thread(s) per core:1
Core(s) per socket:1
Socket(s): 4
NUMA node(s):  2
NUMA node0 CPU(s): 0,1
NUMA node1 CPU(s): 2,3

Here CPUs 0,1 are in node0 and 2,3 are in node1 as specified. The same is
reported by QEMU monitor below.

(qemu) info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 4096 MB
node 1 cpus: 2 3
node 1 size: 4096 MB

After this patch where CPU DT entries are built in spapr_finalize_fdt()
completely, CPU enumeration done by the guest kernel gets reversed since
the CPU DT nodes end up getting discovered by guest kernel in the reverse
order in arch/powerpc/kernel/setup-common.c:smp_setup_cpu_maps(). With
this the resulting guest SMP configuration looks like this:

[root@localhost ~]# cat /proc/cpuinfo 
processor   : 0
cpu dt id   : 24  <--- was 0 previously

processor   : 1
cpu dt id   : 16  <--- was 8 previously

processor   : 2
cpu dt id   : 8   <--- was 16 previously

processor   : 3
cpu dt id   : 0   <--- was 24 previously

[root@localhost ~]# lscpu
CPU(s):4
On-line CPU(s) list:   0-3
Thread(s) per core:1
Core(s) per socket:1
Socket(s): 4
NUMA node(s):  2
NUMA node0 CPU(s): 2,3 <--- node0 was supposed to have 0,1
NUMA node1 CPU(s): 0,1 <--- node1 was supposed to have 2,3

(qemu) info numa
2 nodes
node 0 cpus: 0 1
node 0 size: 4096 MB
node 1 cpus: 2 3
node 1 size: 4096 MB

This is not wrong per se because CPUs with correct DT ids ended up on
right NUMA nodes, but just that the CPU numbers assigned by guest got
reversed.

Is this acceptable or will this break some userpace ?

In both the cases, I am adding CPU DT nodes from QEMU in the same order,
but not sure why the guest kernel discovers them in different orders in
each case.

> +static void spapr_populate_cpus_dt_node(void *fdt, sPAPREnvironment *spapr)
> +{
> +CPUState *cs;
> +int cpus_offset;
> +char *nodename;
> +int smt = kvmppc_smt_threads();
> +
> +cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
> +_FDT(cpus_offset);
> +_FDT((fdt_setprop_cell(fdt, cpus_offset, "#address-cells", 0x1)));
> +_FDT((fdt_setprop_cell(fdt, cpus_offset, "#size-cells", 0x0)));
> +
> +CPU_FOREACH(cs) {
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +int index = ppc_get_vcpu_dt_id(cpu);
> +DeviceClass *dc = DEVICE_GET_CLASS(cs);
> +int offset;
> +
> +if ((index % smt) != 0) {
> +continue;
> +}
> +
> +nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
> +offset = fdt_add_subnode(fdt, cpus_offset, nodename);
> +g_free(nodename);
> +_FDT(offset);
> +spapr_populate_cpu_dt(cs, fdt, offset);
> +}

I can simply fix this by walking the CPUs in reverse order in the above
code which makes the guest kernel to discover the CPU DT nodes in the
right order.

s/CPU_FOREACH(cs)/CPU_FOREACH_REVERSE(cs) will solve this problem. Would this
be the right approach or should we just leave it to the guest kernel to
discover and enumerate CPUs in whatever order it finds the DT nodes in FDT ?

Regards,
Bharata.

Re: [Qemu-devel] Trouble with numlock and SDL

[Erik Rull]

Hi Gerd,

it seems to be a bug sitting in front of the computer :-)
I just recompiled the new SDL and didn't recompile QEMU against the new SDL 
- I recognized later that there are version dependent pieces of code in QEMU.


I will proceed my tests on this topic on other systems, I will send an 
update when the results are available.


Best regards,

Erik


Gerd Hoffmann wrote:

On Mi, 2015-04-22 at 18:20 +0200, Erik Rull wrote:

Hi all,

I'm struggling a bit with the numlock state when using SDL.
On SDL 1.2.13 I have the problem that the numlock state is inverted for QEMU -
but it is switchable.


Move focus out of sdl window -- hit numlock once -- move focus back in.
Does that synchronize things?

Is this something new?  IIRC there have been no (intentional) changes in
that area recently.


On SDL 1.2.14 and 1.2.15 I can't enable the number input in any state of the
numlock key.


No idea on that one.  Sounds like SDL not sending numlock key events at
all.


With VNC everything is fine.


Yep.  vnc has some special logic to sync guest/host numlock state (also
for capslock).  Maybe we should factor that out into reusable helper
functions.

cheers,
   Gerd

[Qemu-devel] [PATCH] ps2 keyboard:fix can't use ps2 keyboard if typing many times After leaving grub and before finishing linux kernel ps2 driver initialization

[penghao122]

Starting  a linux guest with ps2 keyboard ,if you type many times during 
leaving grub and into linux kernel ,then you can't use keyboard after linux 
initialization finished.
during grub,the work method of ps2 keyboard is like this:
first ,ps2 keyboard driver send command KBD_CCMD_KBD_ENABLE,
second,if there is a keyboard input,then  ps2 keyboard driver read data.
third ,ps2 keyboard driver send command KBD_CCMD_KBD_ENABLE,
...
this is diffrent method of linux kernel .
After leaving  grub and before finishing linux kernel ps2 driver 
initialization,if you type many times,the input data keep saving the ps2 queue 
in qemu .Before linux kernel initialize ps2 keyboard,linux call 
i8042_init->i8042_controller_check,if i8042_controller_check return fail,then 
linux kernel ps2 keyboard driver will never initialize. 
(linux kernel 2.6.32 i8042.c)
static int i8042_controller_check(void)
{
 if (i8042_flush() == I8042_BUFFER_SIZE)
  return -ENODEV;
 return 0;
}
static int i8042_flush(void)
{...
while (((str = i8042_read_status()) & I8042_STR_OBF) && (i < 
I8042_BUFFER_SIZE)) {
  udelay(50);
  data = i8042_read_data();
  i++;
  }
return i;
}
 
during calling i8042_flush it is full in qemu queue .
ps_read_data:
   s->update_irq(s->update_arg, 0);
   s->update_irq(s->update_arg, q->count != 0);
because q->count!=0, kbd_update_irq can set I8042_STR_OBF.Then i8042_flush()  
will return I8042_BUFFER_SIZE.
 
 
--- ps2.c.orig 2015-04-25 09:44:38.865777168 -0400
+++ ps2.c 2015-04-25 09:48:46.385121172 -0400
@@ -150,7 +150,12 @@
 q->count++;
 s->update_irq(s->update_arg, 1);
 }
-
+void ps2_clear_queue(void *opaque)
+{
+PS2State *s = (PS2State *)opaque;
+PS2Queue *q = &s->queue;
+q->wptr = q->rptr = q->count = 0;
+}
 /*
keycode is expressed as follow:
bit 7- 0 key pressed, 1 = key released
 
--- pckbd.c.orig 2015-04-25 09:36:59.960640241 -0400
+++ pckbd.c 2015-04-25 09:44:27.298562406 -0400
@@ -207,6 +207,8 @@
 KBDState *s = opaque;
 int val;
 val = s->status;
+if(s->write-cmd == KBD_CCMD_KBD_ENABLE)
+val &= ~KBD_STAT_OBF;
 DPRINTF("kbd: read status=0x%02x\n", val);
 return val;
 }
@@ -251,9 +253,10 @@
 else
 val = KBD_CCMD_NO_OP;
 }
-
+s->write_cmd = 0;
 switch(val) {
 case KBD_CCMD_READ_MODE:
+ps2_clear_queue(s->kbd);
 kbd_queue(s, s->mode, 0);
 break;
 case KBD_CCMD_WRITE_MODE:
@@ -284,6 +287,7 @@
 kbd_update_irq(s);
 break;
 case KBD_CCMD_KBD_ENABLE:
+s->write_cmd = KBD_CCMD_KBD_ENABLE;
 s->mode &= ~KBD_MODE_DISABLE_KBD;
 kbd_update_irq(s);
 break;

Re: [Qemu-devel] [PATCH v2 0/7] Remove more unused functions

[Thomas Huth]

Am Wed, 18 Mar 2015 12:22:35 +0100
schrieb Thomas Huth :

> 
> Here are some more patches to remove completely unused functions
> from QEMU. Please review carefully, some of the functions might
> still get usefull in the future again, so if you discover one,
> please let me know, then I'll remove it from the patch series
> again.
> 
> v2: Changed series according to review comment from v1:
> - Dropped the patch to remove portio_list_destroy() and
> portio_list_del()
> - Keep the ARI functions in the pci patch since they might get useful
> again
> 
> Thomas Huth (7):
>   migration: Remove unused functions
>   vmxnet: Remove unused function vmxnet_rx_pkt_get_num_frags()
>   pci: Remove unused function ich9_d2pbr_init()
>   monitor: Remove unused functions
>   usb: Remove unused functions
>   util: Remove unused functions
>   Remove various unused functions

Ping! Now that 2.3.0 (congrats!) has been tagged, could somebody
(Michael via qemu-trivial tree?) please pick up patches 2 to 7? As far
as I know, there haven't been any complaints about them anymore, so I
think it should be ok to include them.
(patch 1 has already been included via the migration tree, so that's
already fine)

 Cheers,
  Thomas

[Qemu-devel] [PATCH] vhost-user: Send VHOST_RESET_OWNER on vhost stop

[Luke Gorrie]

Hilarity can ensue if vhost is left enabled while a guest reboots.

Luke Gorrie (1):
  vhost-user: Send VHOST_RESET_OWNER on vhost stop

 hw/net/vhost_net.c | 7 +++
 1 file changed, 7 insertions(+)

-- 
2.1.4

[Qemu-devel] [PATCH] vhost-user: Send VHOST_RESET_OWNER on vhost stop

[Luke Gorrie]

Ensure that the vhost-user slave knows when the vrings are valid and
when they are invalid, for example during a guest reboot.

The vhost-user protocol says this of VHOST_RESET_OWNER:

  Issued when a new connection is about to be closed. The Master
  will no longer own this connection (and will usually close it).

Send this message to tell the vhost-user slave that the vhost session
has ended and that session state (e.g. vrings) is no longer valid.

Signed-off-by: Luke Gorrie 
---
 hw/net/vhost_net.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index cf23335..47f8b89 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -263,6 +263,13 @@ static void vhost_net_stop_one(struct vhost_net *net,
   &file);
 assert(r >= 0);
 }
+} else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
+const VhostOps *vhost_ops = net->dev.vhost_ops;
+int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
+  NULL);
+assert(r >= 0);
+}
 }
 if (net->nc->info->poll) {
 net->nc->info->poll(net->nc, true);
-- 
2.1.4

Re: [Qemu-devel] [PATCH 00/17] Update to adding an IPMI device to qemu

[Paolo Bonzini]

On 26/04/2015 13:39, Andreas Färber wrote:
>> That says this series should probably be named v2 (git 
>> format-patch/send-email -v2) or later, as part of the subject
>> line. If the previous version is more than a couple weeks ago,
>> it's also nice (but not required) to provide a URL to the
>> archives of the previous discussion.
> 
> Even worse, I don't see any update on the licensing questions
> previously raised... did Intel ever get back to you?

A reminder for the lazy and/or uninformed?

Paolo

Re: [Qemu-devel] [PATCH 02/17] ipmi: Add a PC ISA type structure

[Paolo Bonzini]

On 26/04/2015 11:05, Michael S. Tsirkin wrote:
>> +
>> +#define FW_CFG_IPMI_INTERFACE   0x30
>> +#define FW_CFG_IPMI_BASE_ADDR   0x31
>> +#define FW_CFG_IPMI_REG_SPACE   0x32
>> +#define FW_CFG_IPMI_REG_SPACING 0x33
>> +#define FW_CFG_IPMI_SLAVE_ADDR  0x34
>> +#define FW_CFG_IPMI_IRQ 0x35
>> +#define FW_CFG_IPMI_VERSION 0x36
>> +
>> +#define FW_CFG_MAX_ENTRY(FW_CFG_IPMI_VERSION + 1)

Also, what is this used for?  I haven't seen firmware patches, maybe
it's obsoleted by SMBIOS support in QEMU?

Paolo

[Qemu-devel] [PATCH 1/2] gtk: Fix VTE focus grabbing

[Jan Kiszka]

From: Jan Kiszka 

At least on GTK2, the VTE terminal has to be specified as target of
gtk_widget_grab_focus. Otherwise, switching from one VTE terminal to
another causes the focus to get lost.

CC: John Snow 
Signed-off-by: Jan Kiszka 
---
 ui/gtk.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 51abac9..76d9b73 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1021,15 +1021,17 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void 
*opaque)
 GtkDisplayState *s = opaque;
 VirtualConsole *vc = gd_vc_find_by_menu(s);
 GtkNotebook *nb = GTK_NOTEBOOK(s->notebook);
-GtkWidget *child;
 gint page;
 
 gtk_release_modifiers(s);
 if (vc) {
 page = gtk_notebook_page_num(nb, vc->tab_item);
 gtk_notebook_set_current_page(nb, page);
-child = gtk_notebook_get_nth_page(nb, page);
-gtk_widget_grab_focus(child);
+if (vc->type == GD_VC_VTE) {
+gtk_widget_grab_focus(vc->vte.terminal);
+} else {
+gtk_widget_grab_focus(vc->tab_item);
+}
 }
 }
 
-- 
2.1.4

[Qemu-devel] [PATCH 0/2] Usability fixes for GTK2

[Jan Kiszka]

Two small patches to fix annoying problems on GTK2. Both weren't tested
on GTK3 (no working setup at hand). The first one comes with a tiny risk
of behaving differently there. The second one may actually be required
on GTK3 as well or is at least a nop there and doesn't need the version
check.

Jan

CC: John Snow 

Jan Kiszka (2):
  gtk: Fix VTE focus grabbing
  gtk: Avoid accel key leakage into guest on console switch

 ui/gtk.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 2/2] gtk: Avoid accel key leakage into guest on console switch

[Jan Kiszka]

From: Jan Kiszka 

GTK2 sends the accel key to the guest when switching to the graphic
console via that shortcut. Resolve this by ignoring any keys until the
next key-release event. However, do not ignore keys when switching via
the menu or when on GTK3.

Signed-off-by: Jan Kiszka 
---
 ui/gtk.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 76d9b73..0ef03e7 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -230,6 +230,7 @@ struct GtkDisplayState {
 
 bool modifier_pressed[ARRAY_SIZE(modifier_keycode)];
 bool has_evdev;
+bool ignore_keys;
 };
 
 static void gd_grab_pointer(VirtualConsole *vc);
@@ -953,6 +954,11 @@ static gboolean gd_key_event(GtkWidget *widget, 
GdkEventKey *key, void *opaque)
 int qemu_keycode;
 int i;
 
+if (s->ignore_keys) {
+s->ignore_keys = (key->type == GDK_KEY_PRESS);
+return TRUE;
+}
+
 if (key->keyval == GDK_KEY_Pause) {
 qemu_input_event_send_key_qcode(vc->gfx.dcl.con, Q_KEY_CODE_PAUSE,
 key->type == GDK_KEY_PRESS);
@@ -1033,12 +1039,18 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void 
*opaque)
 gtk_widget_grab_focus(vc->tab_item);
 }
 }
+s->ignore_keys = false;
 }
 
 static void gd_accel_switch_vc(void *opaque)
 {
 VirtualConsole *vc = opaque;
+
 gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(vc->menu_item), TRUE);
+#if !GTK_CHECK_VERSION(3, 0, 0)
+/* GTK2 sends the accel key to the target console - ignore this until */
+vc->s->ignore_keys = true;
+#endif
 }
 
 static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque)
-- 
2.1.4

[Qemu-devel] [PATCH microblaze v1 2/6] mb: mmu: Delete flip_um fn prototype

[Peter Crosthwaite]

This is not implemented or used.

Signed-off-by: Peter Crosthwaite 
---
 target-microblaze/mmu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target-microblaze/mmu.h b/target-microblaze/mmu.h
index 3f74dda..3b7a998 100644
--- a/target-microblaze/mmu.h
+++ b/target-microblaze/mmu.h
@@ -82,7 +82,6 @@ struct microblaze_mmu_lookup
 } err;
 };
 
-void mmu_flip_um(CPUMBState *env, unsigned int um);
 unsigned int mmu_translate(struct microblaze_mmu *mmu,
struct microblaze_mmu_lookup *lu,
target_ulong vaddr, int rw, int mmu_idx);
-- 
1.9.1

[Qemu-devel] [PATCH microblaze v1 5/6] mb: cpu: Delete EXCP_NMI

[Peter Crosthwaite]

This define is unused. Remove.

Signed-off-by: Peter Crosthwaite 
---
 target-microblaze/cpu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index f21da2f..6522af7 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -36,7 +36,6 @@ typedef struct CPUMBState CPUMBState;
 
 #define ELF_MACHINEEM_MICROBLAZE
 
-#define EXCP_NMI1
 #define EXCP_MMU2
 #define EXCP_IRQ3
 #define EXCP_BREAK  4
-- 
1.9.1

[Qemu-devel] [PATCH microblaze v1 4/6] mb: cpu: Remove unused CC_OP enum

[Peter Crosthwaite]

This enum is not used by anything. Remove.

Signed-off-by: Peter Crosthwaite 
---
 target-microblaze/cpu.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 2c18b49..f21da2f 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -284,12 +284,6 @@ int cpu_mb_exec(CPUMBState *s);
 int cpu_mb_signal_handler(int host_signum, void *pinfo,
   void *puc);
 
-enum {
-CC_OP_DYNAMIC, /* Use env->cc_op  */
-CC_OP_FLAGS,
-CC_OP_CMP,
-};
-
 /* FIXME: MB uses variable pages down to 1K but linux only uses 4k.  */
 #define TARGET_PAGE_BITS 12
 #define MMAP_SHIFT TARGET_PAGE_BITS
-- 
1.9.1

[Qemu-devel] [PATCH microblaze v1 0/6] Microblaze Cleanups

[Peter Crosthwaite]

Hi Edgar,

This is some code cleanup of Microblaze. Mainly unused code
deletion but also code share of the defconfig (P1).

Regards,
Peter

Peter Crosthwaite (6):
  defconfigs: Piggyback microblazeel on microblaze
  mb: mmu: Delete flip_um fn prototype
  mb: cpu: Remote unused cpu_get_pc
  mb: cpu: Remove unused CC_OP enum
  mb: cpu: Delete EXCP_NMI
  mb: cpu: delete unused cpu_interrupts_enabled

 default-configs/microblazeel-softmmu.mak | 10 +-
 target-microblaze/cpu.h  | 17 -
 target-microblaze/mmu.h  |  1 -
 3 files changed, 1 insertion(+), 27 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH microblaze v1 1/6] defconfigs: Piggyback microblazeel on microblaze

[Peter Crosthwaite]

Theres no difference in defconfig. Going forward microblazeel should
superset microblaze so use an include.

Signed-off-by: Peter Crosthwaite 
---
 default-configs/microblazeel-softmmu.mak | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/default-configs/microblazeel-softmmu.mak 
b/default-configs/microblazeel-softmmu.mak
index acf22c5..2fcf442 100644
--- a/default-configs/microblazeel-softmmu.mak
+++ b/default-configs/microblazeel-softmmu.mak
@@ -1,11 +1,3 @@
 # Default configuration for microblazeel-softmmu
 
-CONFIG_PTIMER=y
-CONFIG_PFLASH_CFI01=y
-CONFIG_SERIAL=y
-CONFIG_XILINX=y
-CONFIG_XILINX_AXI=y
-CONFIG_XILINX_SPI=y
-CONFIG_XILINX_ETHLITE=y
-CONFIG_SSI=y
-CONFIG_SSI_M25P80=y
+include microblaze-softmmu.mak
-- 
1.9.1

[Qemu-devel] [PATCH microblaze v1 3/6] mb: cpu: Remote unused cpu_get_pc

[Peter Crosthwaite]

This function is not used by anything. Remove.

Signed-off-by: Peter Crosthwaite 
---
 target-microblaze/cpu.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 7d06227..2c18b49 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -333,11 +333,6 @@ static inline int cpu_interrupts_enabled(CPUMBState *env)
 
 #include "exec/cpu-all.h"
 
-static inline target_ulong cpu_get_pc(CPUMBState *env)
-{
-return env->sregs[SR_PC];
-}
-
 static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
 {
-- 
1.9.1

[Qemu-devel] [PATCH microblaze v1 6/6] mb: cpu: delete unused cpu_interrupts_enabled

[Peter Crosthwaite]

This function is unused. Remove.

Signed-off-by: Peter Crosthwaite 
---
 target-microblaze/cpu.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 6522af7..56d3403 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -319,11 +319,6 @@ static inline int cpu_mmu_index (CPUMBState *env)
 int mb_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
 int mmu_idx);
 
-static inline int cpu_interrupts_enabled(CPUMBState *env)
-{
-return env->sregs[SR_MSR] & MSR_IE;
-}
-
 #include "exec/cpu-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUMBState *env, target_ulong *pc,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v1 5/7] STM32F205: Connect the ADC device

[Peter Crosthwaite]

On Sat, Apr 25, 2015 at 7:21 PM, Alistair Francis  wrote:
> On Sun, Apr 26, 2015 at 4:32 AM, Peter Crosthwaite
>  wrote:
>> "devices"
>>
>> On Sat, Apr 25, 2015 at 1:18 AM, Alistair Francis  
>> wrote:
>>> Connect the ADC device to the STM32F205 SoC.
>>>
>>
>> "devices"
>
> Will fix both.
>
>>
>>> Signed-off-by: Alistair Francis 
>>> ---
>>>
>>>  hw/arm/stm32f205_soc.c | 22 ++
>>>  include/hw/arm/stm32f205_soc.h |  3 +++
>>>  2 files changed, 25 insertions(+)
>>>
>>> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
>>> index 63893f3..641ecbb 100644
>>> --- a/hw/arm/stm32f205_soc.c
>>> +++ b/hw/arm/stm32f205_soc.c
>>> @@ -31,9 +31,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 
>>> 0x4000, 0x4400,
>>>  0x4800, 0x4C00 };
>>>  static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 
>>> 0x40004400,
>>>  0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
>>> +static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
>>> +0x40012200 };
>>>
>>>  static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
>>>  static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
>>> +#define ADC_IRQ 18
>>>
>>>  static void stm32f205_soc_initfn(Object *obj)
>>>  {
>>> @@ -54,6 +57,12 @@ static void stm32f205_soc_initfn(Object *obj)
>>>TYPE_STM32F2XX_TIMER);
>>>  qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
>>>  }
>>> +
>>> +for (i = 0; i < STM_NUM_ADCS; i++) {
>>> +object_initialize(&s->adc[i], sizeof(s->adc[i]),
>>> +  TYPE_STM32F2XX_ADC);
>>> +qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
>>> +}
>>>  }
>>>
>>>  static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
>>> @@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState 
>>> *dev_soc, Error **errp)
>>>  sysbus_mmio_map(busdev, 0, timer_addr[i]);
>>>  sysbus_connect_irq(busdev, 0, pic[timer_irq[i]]);
>>>  }
>>> +
>>> +/* ADC 1 to 3 */
>>> +for (i = 0; i < STM_NUM_ADCS; i++) {
>>> +dev = DEVICE(&(s->adc[i]));
>>> +object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", 
>>> &err);
>>> +if (err != NULL) {
>>> +error_propagate(errp, err);
>>> +return;
>>> +}
>>> +busdev = SYS_BUS_DEVICE(dev);
>>> +sysbus_mmio_map(busdev, 0, adc_addr[i]);
>>> +sysbus_connect_irq(busdev, 0, pic[ADC_IRQ]);
>>
>> This looks inconsistent with other devs. Is it a shared IRQ?
>
> Yeah, all the ADCs use the same IRQ. I guess I could have an array
> like the others with the three values repeated, but it seemed
> unnecessary. I'm ambivalent though, so happy to change it if you think
> the consistency is better.
>

So shared IRQs are shaky, as they only work if its an edge sensitive
semantic. Is that the case (I found the datasheet confusing on this)?
But as you aren't using the IRQ yet, you could just remove the
connection completely.

Regards,
Peter

> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> +}
>>>  }
>>>
>>>  static Property stm32f205_soc_properties[] = {
>>> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
>>> index 0390eff..7d6603b 100644
>>> --- a/include/hw/arm/stm32f205_soc.h
>>> +++ b/include/hw/arm/stm32f205_soc.h
>>> @@ -28,6 +28,7 @@
>>>  #include "hw/misc/stm32f2xx_syscfg.h"
>>>  #include "hw/timer/stm32f2xx_timer.h"
>>>  #include "hw/char/stm32f2xx_usart.h"
>>> +#include "hw/misc/stm32f2xx_adc.h"
>>>
>>>  #define TYPE_STM32F205_SOC "stm32f205-soc"
>>>  #define STM32F205_SOC(obj) \
>>> @@ -35,6 +36,7 @@
>>>
>>>  #define STM_NUM_USARTS 6
>>>  #define STM_NUM_TIMERS 4
>>> +#define STM_NUM_ADCS 3
>>>
>>>  #define FLASH_BASE_ADDRESS 0x0800
>>>  #define FLASH_SIZE (1024 * 1024)
>>> @@ -52,6 +54,7 @@ typedef struct STM32F205State {
>>>  STM32F2XXSyscfgState syscfg;
>>>  STM32F2XXUsartState usart[STM_NUM_USARTS];
>>>  STM32F2XXTimerState timer[STM_NUM_TIMERS];
>>> +STM32F2XXAdcState adc[STM_NUM_ADCS];
>>>  } STM32F205State;
>>>
>>>  #endif
>>> --
>>> 2.1.4
>>>
>>>
>

[Qemu-devel] [PATCH] i440fx-test: remove ARRAY_SIZE redefinition

[Emilio G. Cota]

It's defined in osdep.h and shouldn't be redefined here.

Signed-off-by: Emilio G. Cota 
---
 tests/i440fx-test.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
index d0bc8de..33a7ecb 100644
--- a/tests/i440fx-test.c
+++ b/tests/i440fx-test.c
@@ -27,8 +27,6 @@
 
 #define BROKEN 1
 
-#define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0]))
-
 typedef struct TestData
 {
 int num_cpus;
-- 
1.9.1

[Qemu-devel] [PATCH target-arm 0/2] Unused code cleanups

[Peter Crosthwaite]

Hi Peter,

Two unused code cleanups for target-arm/cpu.h

Regards,
Peter

Peter Crosthwaite (2):
  arm: cpu.h: Remove unused typdefs
  arm: cpu.h: Delete unused cpu_pc_from_tb()

 target-arm/cpu.h | 14 --
 1 file changed, 14 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH target-arm 1/2] arm: cpu.h: Remove unused typdefs

[Peter Crosthwaite]

These CP accessor function prototypes are unused. Remove them.

Signed-off-by: Peter Crosthwaite 
---
 target-arm/cpu.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 083211c..7069103 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -93,11 +93,6 @@
 #define ARM_CPU_VIRQ 2
 #define ARM_CPU_VFIQ 3
 
-typedef void ARMWriteCPFunc(void *opaque, int cp_info,
-int srcreg, int operand, uint32_t value);
-typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,
-   int dstreg, int operand);
-
 struct arm_boot_info;
 
 #define NB_MMU_MODES 7
-- 
1.9.1

[Qemu-devel] [PATCH target-arm 2/2] arm: cpu.h: Delete unused cpu_pc_from_tb()

[Peter Crosthwaite]

No code uses the cpu_pc_from_tb() function. Delete.

Signed-off-by: Peter Crosthwaite 
---
 target-arm/cpu.h | 9 -
 1 file changed, 9 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7069103..c9c5d30 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1874,15 +1874,6 @@ static inline void cpu_get_tb_cpu_state(CPUARMState 
*env, target_ulong *pc,
 
 #include "exec/exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUARMState *env, TranslationBlock *tb)
-{
-if (ARM_TBFLAG_AARCH64_STATE(tb->flags)) {
-env->pc = tb->pc;
-} else {
-env->regs[15] = tb->pc;
-}
-}
-
 enum {
 QEMU_PSCI_CONDUIT_DISABLED = 0,
 QEMU_PSCI_CONDUIT_SMC = 1,
-- 
1.9.1

[Qemu-devel] [PATCH] configure: alphabetize tricore in target list

[Peter Crosthwaite]

tricore was out of alphabetical order in the target list. Fix.

Signed-off-by: Peter Crosthwaite 
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 6969f6f..9f0d9bb 100755
--- a/configure
+++ b/configure
@@ -5169,8 +5169,6 @@ case "$target_name" in
 TARGET_BASE_ARCH=mips
 echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak
   ;;
-  tricore)
-  ;;
   moxie)
   ;;
   or32)
@@ -5221,6 +5219,8 @@ case "$target_name" in
   s390x)
 gdb_xml_files="s390x-core64.xml s390-acr.xml s390-fpr.xml"
   ;;
+  tricore)
+  ;;
   unicore32)
   ;;
   xtensa|xtensaeb)
-- 
1.9.1

Re: [Qemu-devel] [PATCH] i440fx-test: remove ARRAY_SIZE redefinition

[Peter Crosthwaite]

On Sun, Apr 26, 2015 at 3:04 PM, Emilio G. Cota  wrote:
> It's defined in osdep.h and shouldn't be redefined here.
>
> Signed-off-by: Emilio G. Cota 


Reviewed-by: Peter Crosthwaite 

It looks like there may be another one too:

hw/audio/fmopl.c:#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

Regards,
Peter


> ---
>  tests/i440fx-test.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
> index d0bc8de..33a7ecb 100644
> --- a/tests/i440fx-test.c
> +++ b/tests/i440fx-test.c
> @@ -27,8 +27,6 @@
>
>  #define BROKEN 1
>
> -#define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0]))
> -
>  typedef struct TestData
>  {
>  int num_cpus;
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH RFC v7 5/7] qemu-iotests: s390x: fix test 049

[tu bo]

Hello Max:

Xiao Guang Chen left IBM last week, and I took over this job. thanks for 
your comments :-)


On 04/24/2015 12:47 AM, Max Reitz wrote:

On 23.04.2015 04:42, Xiao Guang Chen wrote:

From: Bo Tu 


Hm, why is Bo Tu the patch author, but doesn't have an S-o-b in the 
commit message?
I created the patch, but faild to send out it via 'git send-email". So 
XiaoGuang sent the patch with his account on his machine. Perhaps that's 
the reason.

when creating an image qemu-img enable us specifying the size of the
image using -o size=xx options. But when we specify an invalid size
such as a negtive size then different platform gives different result.

parse_option_size() function in util/qemu-option.c will be called to
parse the size, a cast was called in the function to cast the input
(saved as a double in the function) size to an unsigned int64 value,
when the input is a negtive value or exceeds the maximum of uint64, then
the result is undefined.

Language spec 6.3.1.4 Real floating and integers:
the result of this assignment/cast is undefined if the float is not
in the open interval (-1, U_MAX+1).


Thank you for pointing to the specific section. I guess there are 
always new things to discover in C...



Signed-off-by: Xiao Guang Chen 
---
  tests/qemu-iotests/049.out | 10 --
  util/qemu-option.c |  5 +
  2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index 9f93666..75d90b2 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -95,17 +95,15 @@ qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1024
  qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
-qemu-img: qcow2 doesn't support shrinking images yet
-qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not 
supported
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
+qemu-img: Parameter 'size' expects a positive number and must not 
exceeds the maximum UINT64

+qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
  qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2
-qemu-img: qcow2 doesn't support shrinking images yet
-qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not 
supported
-Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
+qemu-img: Parameter 'size' expects a positive number and must not 
exceeds the maximum UINT64

+qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte
  qemu-img: Invalid image size specified! You may use k, M, G, T, P 
or E suffixes for

diff --git a/util/qemu-option.c b/util/qemu-option.c
index fda4e5f..1c50fa4 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -179,6 +179,11 @@ void parse_option_size(const char *name, const 
char *value,

if (value != NULL) {
  sizef = strtod(value, &postfix);
+if (sizef < 0 || sizef > UINT64_MAX) {
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a 
positive "
+   "number and must not exceeds the maximum 
UINT64");


I think Markus would like to see these error macros not getting used 
anymore, so I think it should be dropped and the full string should be 
given here. I'll let him do the arguing, though. :-)


If you keep the macro, I'd propose "a non-negative number below 2^64" 
(or actually give the decimal value of UINT64_MAX, using 'a 
non-negative number not exceeding "%" PRIu64, UINT64_MAX'). Remember 
that 0 is not positive, but still a valid choice.
Good suggestion. I'll change the error message like "a non-negative 
number not exceeding "%" PRIu64, UINT64_MAX')" in v8.
If you drop the macro, I'd propose error_setg(errp, "'%s' must be a 
non-negative number below 2^64", name) or, like it is now, 
error_setg(errp, "Parameter '%s' expects a non-negative number below 
2^64", name).


Max


+return;
+}
  switch (*postfix) {
  case 'T':
  sizef *= 1024;

Re: [Qemu-devel] [PATCH RFC v7 6/7] qemu-iotests: s390x: fix test 051

[tu bo]

Hi Max:

On 04/24/2015 01:00 AM, Max Reitz wrote:

On 23.04.2015 04:42, Xiao Guang Chen wrote:
The tests for device type "ide_cd" should only be tested for the pc 
platform.

The default device id of hard disk on the s390 platform differs to that
of the x86 platform. A new variable device_id is defined and "virtio0"
set for the s390 platform. A x86 platform specific output file is also
needed.
A new filter was added to filter orphan warnings.

Reviewed-by: Max Reitz 
Reviewed-by: Michael Mueller 
Signed-off-by: Xiao Guang Chen 
---
  tests/qemu-iotests/051   |  79 ---
  tests/qemu-iotests/051.out   | 156 +-
  tests/qemu-iotests/051.pc.out| 433 
+++

  tests/qemu-iotests/common.filter |   7 +
  4 files changed, 545 insertions(+), 130 deletions(-)
  create mode 100644 tests/qemu-iotests/051.pc.out

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 0360f37..8de16a5 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -147,13 +147,19 @@ run_qemu -drive if=ide
  run_qemu -drive if=virtio
  run_qemu -drive if=scsi
  -run_qemu -drive if=none,id=disk -device ide-cd,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-cd,drive=disk

-
-run_qemu -drive if=none,id=disk -device ide-drive,drive=disk
-run_qemu -drive if=none,id=disk -device ide-hd,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-disk,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-hd,drive=disk

+case "$QEMU_DEFAULT_MACHINE" in
+pc)
+run_qemu -drive if=none,id=disk -device ide-cd,drive=disk
+run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-cd,drive=disk

+
+run_qemu -drive if=none,id=disk -device ide-drive,drive=disk
+run_qemu -drive if=none,id=disk -device ide-hd,drive=disk
+run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-disk,drive=disk
+run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-hd,drive=disk

+;;
+*)
+;;
+esac
echo
  echo === Read-only ===
@@ -167,13 +173,19 @@ run_qemu -drive 
file="$TEST_IMG",if=ide,readonly=on

  run_qemu -drive file="$TEST_IMG",if=virtio,readonly=on
  run_qemu -drive file="$TEST_IMG",if=scsi,readonly=on
  -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device ide-cd,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-cd,drive=disk

+case "$QEMU_DEFAULT_MACHINE" in
+pc)
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device ide-cd,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device lsi53c895a -device scsi-cd,drive=disk
  -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device ide-drive,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-hd,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-disk,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-hd,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device ide-drive,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device ide-hd,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device lsi53c895a -device scsi-disk,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on 
-device lsi53c895a -device scsi-hd,drive=disk

+;;
+*)
+;;
+esac
echo
  echo === Cache modes ===
@@ -182,12 +194,12 @@ echo
  # Cannot use the test image because cache=none might not work on 
the host FS

  # Use cdrom so that we won't get errors about missing media
  -run_qemu -drive media=cdrom,cache=none
-run_qemu -drive media=cdrom,cache=directsync
-run_qemu -drive media=cdrom,cache=writeback
-run_qemu -drive media=cdrom,cache=writethrough
-run_qemu -drive media=cdrom,cache=unsafe
-run_qemu -drive media=cdrom,cache=invalid_value
+run_qemu -drive if=scsi,media=cdrom,cache=none
+run_qemu -drive if=scsi,media=cdrom,cache=directsync
+run_qemu -drive if=scsi,media=cdrom,cache=writeback
+run_qemu -drive if=scsi,media=cdrom,cache=writethrough
+run_qemu -drive if=scsi,media=cdrom,cache=unsafe
+run_qemu -drive if=scsi,media=cdrom,cache=invalid_value
echo
  echo === Specifying the protocol layer ===
@@ -251,28 +263,37 @@ echo
  echo === Snapshot mode ===
  echo
  +case "$QEMU_DEFAULT_MACHINE" in
+pc)
+device_id="ide0-hd0"
+;;
+s390)
+device_id="virtio0"
+;;
+esac
+
  $QEMU_IO -c "write -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io
  -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive 
file="$TEST_IMG" -snapshot | _filter_qemu_io
-echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive 
file="$TEST_IMG",snapshot=on | _filte

Re: [Qemu-devel] [PATCH v2] sysbus: add irq_routing_notifier

[Peter Crosthwaite]

On Fri, Apr 24, 2015 at 5:39 AM, Eric Auger  wrote:
> Add a new irq_routing_notifier notifier in the SysBusDeviceClass. This
> notifier, if populated, is called after sysbus_connect_irq.
>
> This mechanism is used to setup VFIO signaling once VFIO platform
> devices get attached to their platform bus, on a machine init done
> notifier.
>
> Signed-off-by: Eric Auger 
>
> ---
>
> v1 -> v2:
> - duly put the notifier in the class and not in the device
> ---
>  hw/core/sysbus.c| 6 ++
>  include/hw/sysbus.h | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index b53c351..8553a6f 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -109,7 +109,13 @@ qemu_irq sysbus_get_connected_irq(SysBusDevice *dev, int 
> n)
>
>  void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
>  {
> +SysBusDeviceClass *sbd = SYS_BUS_DEVICE_GET_CLASS(dev);
> +
>  qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);

One of my long term goals is to try and get rid of sysbus IRQ
abstraction completely in favor of just qdev gpios. This means
features that apply to GPIOs automatically apply to IRQs and vice
versa. Can your notifier hook be pushed up to the qdev GPIO level to
make it more globally usable and avoid a new feature to sysbus IRQs?

> +
> +if (sbd->irq_routing_notifier) {
> +sbd->irq_routing_notifier(dev, irq);
> +}
>  }
>
>  /* Check whether an MMIO region exists */
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index d1f3f00..dbf3f0f 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -41,6 +41,7 @@ typedef struct SysBusDeviceClass {
>  /*< public >*/
>
>  int (*init)(SysBusDevice *dev);
> +void (*irq_routing_notifier)(SysBusDevice *dev, qemu_irq irq);

Is it better to make the name more matched to sysbus_connect_irq?
Perhaps connect_irq_notifier. But with the qdev approach this would be
connect_gpio_out_notifier or something along those lines.

Regards,
Peter

>  } SysBusDeviceClass;
>
>  struct SysBusDevice {
> --
> 1.8.3.2
>
>

Re: [Qemu-devel] [Qemu-block] [PATCH v2 3/4] qemu-iotests: Test that "stop" doesn't drain block jobs

[Fam Zheng]

On Fri, 04/24 17:43, Max Reitz wrote:
> On 03.04.2015 16:05, Fam Zheng wrote:
> >Signed-off-by: Fam Zheng 
> >---
> >  tests/qemu-iotests/129 | 86 
> > ++
> >  tests/qemu-iotests/129.out |  5 +++
> >  tests/qemu-iotests/group   |  1 +
> >  3 files changed, 92 insertions(+)
> >  create mode 100644 tests/qemu-iotests/129
> >  create mode 100644 tests/qemu-iotests/129.out
> >
> >diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
> >new file mode 100644
> >index 000..9e87e1c
> >--- /dev/null
> >+++ b/tests/qemu-iotests/129
> >@@ -0,0 +1,86 @@
> >+#!/usr/bin/env python
> >+#
> >+# Tests that "bdrv_drain_all" doesn't drain block jobs
> >+#
> >+# Copyright (C) 2015 Red Hat, Inc.
> >+#
> >+# This program is free software; you can redistribute it and/or modify
> >+# it under the terms of the GNU General Public License as published by
> >+# the Free Software Foundation; either version 2 of the License, or
> >+# (at your option) any later version.
> >+#
> >+# This program is distributed in the hope that it will be useful,
> >+# but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+# GNU General Public License for more details.
> >+#
> >+# You should have received a copy of the GNU General Public License
> >+# along with this program.  If not, see .
> >+#
> >+
> >+import os
> >+import iotests
> >+import time
> >+
> >+class TestStopWithBlockJob(iotests.QMPTestCase):
> >+test_img = os.path.join(iotests.test_dir, 'test.img')
> >+target_img = os.path.join(iotests.test_dir, 'target.img')
> >+base_img = os.path.join(iotests.test_dir, 'base.img')
> >+
> >+def setUp(self):
> >+iotests.qemu_img('create', '-f', iotests.imgfmt, self.base_img, 
> >"1G")
> >+iotests.qemu_img('create', '-f', iotests.imgfmt, self.test_img, 
> >"-b", self.base_img)
> >+iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 1M 128M', 
> >self.test_img)
> >+self.vm = iotests.VM().add_drive(self.test_img)
> >+self.vm.launch()
> >+
> >+def tearDown(self):
> >+params = {"device": "drive0",
> >+  "bps": 0,
> >+  "bps_rd": 0,
> >+  "bps_wr": 0,
> >+  "iops": 0,
> >+  "iops_rd": 0,
> >+  "iops_wr": 0,
> >+ }
> >+result = self.vm.qmp("block_set_io_throttle", conv_keys=False,
> >+ **params)
> >+self.vm.shutdown()
> >+
> >+def do_test_stop(self, cmd, **args):
> >+"""Test 'stop' while block job is running on a throttled drive.
> >+The 'stop' command shouldn't drain the job"""
> >+params = {"device": "drive0",
> >+  "bps": 1024,
> >+  "bps_rd": 0,
> >+  "bps_wr": 0,
> >+  "iops": 0,
> >+  "iops_rd": 0,
> >+  "iops_wr": 0,
> >+ }
> >+result = self.vm.qmp("block_set_io_throttle", conv_keys=False,
> >+ **params)
> >+self.assert_qmp(result, 'return', {})
> >+result = self.vm.qmp(cmd, **args)
> >+self.assert_qmp(result, 'return', {})
> >+result = self.vm.qmp("stop")
> >+self.assert_qmp(result, 'return', {})
> >+result = self.vm.qmp("query-block-jobs")
> >+self.assert_qmp(result, 'return[0]/busy', True)
> 
> I don't know why, but this assertion fails in tmpfs (for me, at least).

I also run tests in tmpfs, but I don't see the failure.  What is in "result"?
Is the block job completed?

(I assume you've applied and compiled patch 1-2. Worth mentioning because
that's exactly what this assertion is testing against.)

Fam

Re: [Qemu-devel] [PATCH v2 2/2] block: Fix NULL deference for unaligned write if qiov is NULL

[Fam Zheng]

On Fri, 04/24 13:51, Paolo Bonzini wrote:
> 
> 
> On 24/04/2015 13:00, Paolo Bonzini wrote:
> >> -qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
> >> -qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
> >> -use_local_qiov = true;
> >> +if (qiov) {
> >> +qemu_iovec_init(&local_qiov, qiov ? qiov->niov + 2 : 1);
> >> +qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
> >> +qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
> >> +use_local_qiov = true;
> >> +bytes += offset & (align - 1);
> >> +offset = offset & ~(align - 1);
> >> +} else {
> >> +memset(head_buf + (offset & (align - 1)), 0,
> >> +   align - (offset & (align - 1)));
> 
> Actually, is the byte count correct if bytes < align?  In the case of
> your testcase, you'd destroy bytes 1536..4095.

Yes, good catch!

Fam

> 
> Same for the computation of bytes, below.  It could underflow.
> 
> Perhaps a qemu-iotests testcase, using qemu-io, is also necessary.
> 
> Paolo
> 
> >> +ret = bdrv_aligned_pwritev(bs, &req, offset & ~(align - 1), 
> >> align,
> >> +   &head_qiov, 0);
> >> +if (ret < 0) {
> >> +goto fail;
> >> +}
> >> +bytes -= align - (offset & (align - 1));
> >> +offset = ROUND_UP(offset, align);
> >> +}
> >> +}

Re: [Qemu-devel] [RFC PATCH v3 05/24] spapr: Reorganize CPU dt generation code

[Bharata B Rao]

On Sun, Apr 26, 2015 at 05:17:48PM +0530, Bharata B Rao wrote:
> On Fri, Apr 24, 2015 at 12:17:27PM +0530, Bharata B Rao wrote:
> > Reorganize CPU device tree generation code so that it be reused from
> > hotplug path. CPU dt entries are now generated from spapr_finalize_fdt()
> > instead of spapr_create_fdt_skel().
> 
> Creating CPU DT entries from spapr_finalize_fdt() instead of
> spapr_create_fdt_skel() has an interesting side effect.
> 
>  
> 
> In both the cases, I am adding CPU DT nodes from QEMU in the same order,
> but not sure why the guest kernel discovers them in different orders in
> each case.

Nikunj and I tracked this down to the difference in device tree APIs that
we are using in two cases.

When CPU DT nodes are created from spapr_create_fdt_skel(), we are using
fdt_begin_node() API which does sequential write and hence CPU DT nodes
end up in the same order in which they are created.

However in my patch when I create CPU DT entries in spapr_finalize_fdt(),
I am using fdt_add_subnode() which ends up writing the CPU DT node at the
same parent offset for all the CPUs. This results in CPU DT nodes being
generated in reverse order in FDT.

> 
> > +static void spapr_populate_cpus_dt_node(void *fdt, sPAPREnvironment *spapr)
> > +{
> > +CPUState *cs;
> > +int cpus_offset;
> > +char *nodename;
> > +int smt = kvmppc_smt_threads();
> > +
> > +cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
> > +_FDT(cpus_offset);
> > +_FDT((fdt_setprop_cell(fdt, cpus_offset, "#address-cells", 0x1)));
> > +_FDT((fdt_setprop_cell(fdt, cpus_offset, "#size-cells", 0x0)));
> > +
> > +CPU_FOREACH(cs) {
> > +PowerPCCPU *cpu = POWERPC_CPU(cs);
> > +int index = ppc_get_vcpu_dt_id(cpu);
> > +DeviceClass *dc = DEVICE_GET_CLASS(cs);
> > +int offset;
> > +
> > +if ((index % smt) != 0) {
> > +continue;
> > +}
> > +
> > +nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
> > +offset = fdt_add_subnode(fdt, cpus_offset, nodename);
> > +g_free(nodename);
> > +_FDT(offset);
> > +spapr_populate_cpu_dt(cs, fdt, offset);
> > +}
> 
> I can simply fix this by walking the CPUs in reverse order in the above
> code which makes the guest kernel to discover the CPU DT nodes in the
> right order.
> 
> s/CPU_FOREACH(cs)/CPU_FOREACH_REVERSE(cs) will solve this problem. Would this
> be the right approach or should we just leave it to the guest kernel to
> discover and enumerate CPUs in whatever order it finds the DT nodes in FDT ?

So using CPU_FOREACH_REVERSE(cs) appears to be right way to handle this.

Regards,
Bharata.

[Qemu-devel] [PATCH v3 0/3] block: Fix unaligned bdrv_aio_write_zeroes

[Fam Zheng]

An unaligned zero write causes NULL deferencing in bdrv_co_do_pwritev. That
path is reachable from bdrv_co_write_zeroes and bdrv_aio_write_zeroes.

You can easily trigger through the former with qemu-io, as the test case added
by 61815d6e0aa. For bdrv_aio_write_zeroes, in common cases there's always a
format driver (which uses 512 alignment), so it would be much rarer to have
unaligned requests (only concerning top level here, when the request goes down
to bs->file, where for example the alignment is 4k, it would then be calling
bdrv_co_write_zeroes because it's in a coroutine).

fc3959e4669a1c fixed bdrv_co_write_zeroes but not bdrv_aio_write_zeroes.  The
lattern is the actually used one by device model. Revert the previous fix, do
it in bdrv_co_do_pwritev, to cover both paths.

v3: Fix the case where the unaligned request is contained within the first
block. (Paolo)
Also update iotests 033 to cover the code path with qemu-io.

v2: Split to three aligned pwritev.


Fam Zheng (3):
  Revert "block: Fix unaligned zero write"
  block: Fix NULL deference for unaligned write if qiov is NULL
  qemu-iotests: Test unaligned sub-block zero write

 block.c| 123 +
 tests/qemu-iotests/033 |  13 +
 tests/qemu-iotests/033.out |  30 +++
 3 files changed, 111 insertions(+), 55 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v3 1/3] Revert "block: Fix unaligned zero write"

[Fam Zheng]

This reverts commit fc3959e4669a1c2149b91ccb05101cfc7ae1fc05.

The core write code already handles the case, so remove this
duplication.

Signed-off-by: Fam Zheng 
---
 block.c | 45 ++---
 1 file changed, 6 insertions(+), 39 deletions(-)

diff --git a/block.c b/block.c
index f2f8ae7..0fe97de 100644
--- a/block.c
+++ b/block.c
@@ -3118,19 +3118,6 @@ out:
 return ret;
 }
 
-static inline uint64_t bdrv_get_align(BlockDriverState *bs)
-{
-/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */
-return MAX(BDRV_SECTOR_SIZE, bs->request_alignment);
-}
-
-static inline bool bdrv_req_is_aligned(BlockDriverState *bs,
-   int64_t offset, size_t bytes)
-{
-int64_t align = bdrv_get_align(bs);
-return !(offset & (align - 1) || (bytes & (align - 1)));
-}
-
 /*
  * Handle a read request in coroutine context
  */
@@ -3141,7 +3128,8 @@ static int coroutine_fn 
bdrv_co_do_preadv(BlockDriverState *bs,
 BlockDriver *drv = bs->drv;
 BdrvTrackedRequest req;
 
-uint64_t align = bdrv_get_align(bs);
+/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */
+uint64_t align = MAX(BDRV_SECTOR_SIZE, bs->request_alignment);
 uint8_t *head_buf = NULL;
 uint8_t *tail_buf = NULL;
 QEMUIOVector local_qiov;
@@ -3383,7 +3371,8 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 BdrvRequestFlags flags)
 {
 BdrvTrackedRequest req;
-uint64_t align = bdrv_get_align(bs);
+/* TODO Lift BDRV_SECTOR_SIZE restriction in BlockDriver interface */
+uint64_t align = MAX(BDRV_SECTOR_SIZE, bs->request_alignment);
 uint8_t *head_buf = NULL;
 uint8_t *tail_buf = NULL;
 QEMUIOVector local_qiov;
@@ -3482,10 +3471,6 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 bytes = ROUND_UP(bytes, align);
 }
 
-if (use_local_qiov) {
-/* Local buffer may have non-zero data. */
-flags &= ~BDRV_REQ_ZERO_WRITE;
-}
 ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
use_local_qiov ? &local_qiov : qiov,
flags);
@@ -3526,32 +3511,14 @@ int coroutine_fn bdrv_co_write_zeroes(BlockDriverState 
*bs,
   int64_t sector_num, int nb_sectors,
   BdrvRequestFlags flags)
 {
-int ret;
-
 trace_bdrv_co_write_zeroes(bs, sector_num, nb_sectors, flags);
 
 if (!(bs->open_flags & BDRV_O_UNMAP)) {
 flags &= ~BDRV_REQ_MAY_UNMAP;
 }
-if (bdrv_req_is_aligned(bs, sector_num << BDRV_SECTOR_BITS,
-nb_sectors << BDRV_SECTOR_BITS)) {
-ret = bdrv_co_do_writev(bs, sector_num, nb_sectors, NULL,
-BDRV_REQ_ZERO_WRITE | flags);
-} else {
-uint8_t *buf;
-QEMUIOVector local_qiov;
-size_t bytes = nb_sectors << BDRV_SECTOR_BITS;
 
-buf = qemu_memalign(bdrv_opt_mem_align(bs), bytes);
-memset(buf, 0, bytes);
-qemu_iovec_init(&local_qiov, 1);
-qemu_iovec_add(&local_qiov, buf, bytes);
-
-ret = bdrv_co_do_writev(bs, sector_num, nb_sectors, &local_qiov,
-BDRV_REQ_ZERO_WRITE | flags);
-qemu_vfree(buf);
-}
-return ret;
+return bdrv_co_do_writev(bs, sector_num, nb_sectors, NULL,
+ BDRV_REQ_ZERO_WRITE | flags);
 }
 
 /**
-- 
1.9.3

[Qemu-devel] [PATCH v3 3/3] qemu-iotests: Test unaligned sub-block zero write

[Fam Zheng]

Test zero write in byte range 512~1024 for 4k alignment.

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/033 | 13 +
 tests/qemu-iotests/033.out | 30 ++
 2 files changed, 43 insertions(+)

diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033
index 4008f10..a61d8ce 100755
--- a/tests/qemu-iotests/033
+++ b/tests/qemu-iotests/033
@@ -78,6 +78,19 @@ for align in 512 4k; do
echo
echo "== verifying patterns (2) =="
do_test $align "read -P 0x0 0x400 0x2" "$TEST_IMG" | _filter_qemu_io
+
+   echo
+   echo "== rewriting unaligned zeroes =="
+   do_test $align "write -P 0xb 0x0 0x1000" "$TEST_IMG" | _filter_qemu_io
+   do_test $align "write -z 0x200 0x200" "$TEST_IMG" | _filter_qemu_io
+
+   echo
+   echo "== verifying patterns (3) =="
+   do_test $align "read -P 0xb 0x0 0x200" "$TEST_IMG" | _filter_qemu_io
+   do_test $align "read -P 0x0 0x200 0x200" "$TEST_IMG" | _filter_qemu_io
+   do_test $align "read -P 0xb 0x400 0xc00" "$TEST_IMG" | _filter_qemu_io
+
+   echo
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/033.out b/tests/qemu-iotests/033.out
index 305949f..c3d18aa 100644
--- a/tests/qemu-iotests/033.out
+++ b/tests/qemu-iotests/033.out
@@ -27,6 +27,21 @@ wrote 65536/65536 bytes at offset 65536
 read 131072/131072 bytes at offset 1024
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+== rewriting unaligned zeroes ==
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying patterns (3) ==
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 3072/3072 bytes at offset 1024
+3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+
 == preparing image ==
 wrote 1024/1024 bytes at offset 512
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -52,4 +67,19 @@ wrote 65536/65536 bytes at offset 65536
 == verifying patterns (2) ==
 read 131072/131072 bytes at offset 1024
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rewriting unaligned zeroes ==
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying patterns (3) ==
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 512/512 bytes at offset 512
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 3072/3072 bytes at offset 1024
+3 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
 *** done
-- 
1.9.3

[Qemu-devel] [PATCH v3 2/3] block: Fix NULL deference for unaligned write if qiov is NULL

[Fam Zheng]

For zero write, qiov passed by callers (qemu-io "write -z" and
scsi-disk "write same") is NULL.

Commit fc3959e466 fixed bdrv_co_write_zeroes which is the common case
for this bug, but it still exists in bdrv_aio_write_zeroes. A simpler
fix would be in bdrv_co_do_pwritev which is the NULL dereference point
and covers both cases.

So don't access it in bdrv_co_do_pwritev in this case, use three aligned
writes.

Signed-off-by: Fam Zheng 
---
 block.c | 80 +++--
 1 file changed, 63 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 0fe97de..d0b9a4e 100644
--- a/block.c
+++ b/block.c
@@ -3403,6 +3403,8 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
  */
 tracked_request_begin(&req, bs, offset, bytes, true);
 
+assert(qiov || flags & BDRV_REQ_ZERO_WRITE);
+
 if (offset & (align - 1)) {
 QEMUIOVector head_qiov;
 struct iovec head_iov;
@@ -3425,13 +3427,39 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 }
 BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
 
-qemu_iovec_init(&local_qiov, qiov->niov + 2);
-qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
-qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
-use_local_qiov = true;
+if (qiov) {
+qemu_iovec_init(&local_qiov, qiov ? qiov->niov + 2 : 1);
+qemu_iovec_add(&local_qiov, head_buf, offset & (align - 1));
+qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
+use_local_qiov = true;
+bytes += offset & (align - 1);
+offset = offset & ~(align - 1);
+} else {
+uint64_t local_off = offset & (align - 1);
+uint64_t local_bytes = MIN(bytes, align - local_off);
 
-bytes += offset & (align - 1);
-offset = offset & ~(align - 1);
+memset(head_buf + local_off, 0, local_bytes);
+ret = bdrv_aligned_pwritev(bs, &req, offset & ~(align - 1), align,
+   &head_qiov, 0);
+if (ret < 0) {
+goto fail;
+}
+bytes -= local_bytes;
+offset = ROUND_UP(offset, align);
+}
+}
+
+if (!qiov) {
+uint64_t aligned_bytes = bytes & ~(align - 1);
+
+assert((offset & (align - 1)) == 0);
+ret = bdrv_aligned_pwritev(bs, &req, offset, aligned_bytes,
+   NULL, flags);
+if (ret < 0) {
+goto fail;
+}
+bytes -= aligned_bytes;
+offset += aligned_bytes;
 }
 
 if ((offset + bytes) & (align - 1)) {
@@ -3459,21 +3487,39 @@ static int coroutine_fn 
bdrv_co_do_pwritev(BlockDriverState *bs,
 }
 BLKDBG_EVENT(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
 
-if (!use_local_qiov) {
-qemu_iovec_init(&local_qiov, qiov->niov + 1);
-qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
-use_local_qiov = true;
+if (qiov) {
+if (!use_local_qiov) {
+qemu_iovec_init(&local_qiov, qiov->niov + 1);
+qemu_iovec_concat(&local_qiov, qiov, 0, qiov->size);
+use_local_qiov = true;
+}
+
+tail_bytes = (offset + bytes) & (align - 1);
+qemu_iovec_add(&local_qiov, tail_buf + tail_bytes,
+   align - tail_bytes);
+
+bytes = ROUND_UP(bytes, align);
+} else {
+assert((offset & (align - 1)) == 0);
+assert(bytes < align);
+
+memset(tail_buf, 0, bytes & (align - 1));
+ret = bdrv_aligned_pwritev(bs, &req, offset, align,
+   &tail_qiov, 0);
+if (ret < 0) {
+goto fail;
+}
+offset += align;
+bytes = 0;
 }
 
-tail_bytes = (offset + bytes) & (align - 1);
-qemu_iovec_add(&local_qiov, tail_buf + tail_bytes, align - tail_bytes);
-
-bytes = ROUND_UP(bytes, align);
 }
 
-ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
-   use_local_qiov ? &local_qiov : qiov,
-   flags);
+if (bytes) {
+ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
+   use_local_qiov ? &local_qiov : qiov,
+   flags);
+}
 
 fail:
 tracked_request_end(&req);
-- 
1.9.3

[Qemu-devel] libcacard: cac_is_cac_card?

[Michael Tokarev]

This function is declared in libcacard/cac.h

 VCardStatus cac_is_cac_card(VReader *reader);

it is referenced in a ifdef-ed out code in libcacard/vcard_emul_type.c:


VCardEmulType vcard_emul_type_select(VReader *vreader)
{
#ifdef notdef
/* since there is only one emulator no need to call this function */
if (cac_is_cac_card(vreader) == VCARD_DONE) {
return VCARD_EMUL_CAC;
}
#endif
/* return the default */
return VCARD_EMUL_CAC;
}

and it is mentioned in libcacard/libcacard.syms.

But it is not defined anywhere.  Should it be removed
from all places it is referenced at, or should it be
implemented instead?

Thanks,

/mjt